Added a .buildPathAndQuery() method

Req. for push

[yanivi, 2012-05-03 20:22:40]

Very important: needs unit tests.

Also, this should either go into 1.9, or put it into 1.10, but wait until 1.9 is
branched off before submitting into the default branch.  That should hopefully
be next week.

Also: please send this one to Ravi for review.  I want to make sure you get a
chance to work closely with Ravi also, since he will likely be one of the people
to write feedback for you at the end of your internship.

[mlinder, 2012-05-04 13:40:24]

Added the unit tests + moved the change to 1.9

Ravi, could you have a look at this very small CL?

[rmistry, 2012-05-04 13:45:15]

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/test/j...
File
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java
(right):

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/test/j...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:339:
GenericUrl url = new
GenericUrl("http://www.google.com/m8/feeds/contacts/someone%2Fis%2F@gmail.com/full/");
Line length > 100

[mlinder, 2012-05-04 14:10:33]

Fixed :)

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/test/j...
File
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java
(right):

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/test/j...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:339:
GenericUrl url = new
GenericUrl("http://www.google.com/m8/feeds/contacts/someone%2Fis%2F@gmail.com/full/");
On 2012/05/04 13:45:15, rmistry wrote:
> Line length > 100

Done.

[rmistry, 2012-05-04 14:20:07]

LGTM

http://codereview.appspot.com/6184048/diff/9001/google-http-client/src/test/j...
File
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java
(right):

http://codereview.appspot.com/6184048/diff/9001/google-http-client/src/test/j...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:344:
private static final String BASE_URL = "http://google.com";
[optional] change to http://www.google.com and replace occurences of
http://www.google.com in this file to BASE_URL.

http://codereview.appspot.com/6184048/diff/9001/google-http-client/src/test/j...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:348:
GenericUrl url = new GenericUrl(BASE_URL+FULL_PATH);
space around '+' -> (BASE_URL + FULL_PATH)

[yanivi, 2012-05-04 14:21:34]

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
File google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java
(right):

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:272:
* Constructs the right-side portion of the URL beginning at the rooted path.
remove "right-side"

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:272:
* Constructs the right-side portion of the URL beginning at the rooted path.
give a concrete example as we do in toPathParts

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:274:
* @return path with query parameters (without leading '/')
"without" -> "with"
although only if the path is non-empty

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:277:
public final String buildPathAndQuery() {
It also returns the fragment.  I wish we had a better name for this.

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:277:
public final String buildPathAndQuery() {
Is this a bit too much of a one-off?  Would we be better served with a more
general-purpose buildAuthority(), buildPath(), buildQuery(), and
buildFragment()?  To be honest, for our immediate use case, all we actually need
is buildQuery().

[mlinder, 2012-05-04 15:16:07]

s/PathAndQuery/RelativeUrl/ and added buildAuthority()

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
File google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java
(right):

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:272:
* Constructs the right-side portion of the URL beginning at the rooted path.
On 2012/05/04 14:21:34, yanivi wrote:
> remove "right-side"

Done.

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:272:
* Constructs the right-side portion of the URL beginning at the rooted path.
On 2012/05/04 14:21:34, yanivi wrote:
> give a concrete example as we do in toPathParts

Done.

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:274:
* @return path with query parameters (without leading '/')
On 2012/05/04 14:21:34, yanivi wrote:
> "without" -> "with"
> although only if the path is non-empty

Done.

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:277:
public final String buildPathAndQuery() {
On 2012/05/04 14:21:34, yanivi wrote:
> It also returns the fragment.  I wish we had a better name for this.

Changed it to buildRelativeUrl()

http://codereview.appspot.com/6184048/diff/5001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:277:
public final String buildPathAndQuery() {
I think single build() methods for every single part of the URL are overkill and
will only clutter the library. I've added a buildAuthority() counterpart now as
well, so "fullUrl = buildAuthority() + buildRelativeUrl()".

On 2012/05/04 14:21:34, yanivi wrote:
> Is this a bit too much of a one-off?  Would we be better served with a more
> general-purpose buildAuthority(), buildPath(), buildQuery(), and
> buildFragment()?  To be honest, for our immediate use case, all we actually
need
> is buildQuery().

[yanivi, 2012-05-04 18:31:30]

LGTM

[mlinder, 2012-05-04 19:53:54]

Submitted in b90b7a53484e