Adding suppressUserAgentSuffix

Created to fix http://code.google.com/p/google-http-java-client/issues/detail?id=53 : Allow customizing user agent without USER_AGENT_SUFFIX.

[yanivi, 2012-07-30 17:30:11]

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:183:
* Whether to not suffix {@link #USER_AGENT_SUFFIX} to the User-Agent header
({@code false} by
"Whether to not add the suffix..."

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:187:
* If a User-Agent is not specified in {@link HttpHeaders} then {@link
#USER_AGENT_SUFFIX} is set
Design Issue: Seems like an unintuitive behavior.  If the developer chooses to
call setSuppressUserAgentSuffix(true) then it seems we should try to honor that
request as much as possible.  What do you think?

[rmistry, 2012-07-30 18:50:45]

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:183:
* Whether to not suffix {@link #USER_AGENT_SUFFIX} to the User-Agent header
({@code false} by
On 2012/07/30 17:30:11, yanivi wrote:
> "Whether to not add the suffix..."

Done.

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:187:
* If a User-Agent is not specified in {@link HttpHeaders} then {@link
#USER_AGENT_SUFFIX} is set
On 2012/07/30 17:30:11, yanivi wrote:
> Design Issue: Seems like an unintuitive behavior.  If the developer chooses to
> call setSuppressUserAgentSuffix(true) then it seems we should try to honor
that
> request as much as possible.  What do you think?

Yes that was the other design option. My thinking was that nobody would want a
null user-agent suffix (maybe I am wrong) and in that case we are providing a
default user-agent not really adding a suffix.
I can leave the null user-agent if you prefer that design.

[yanivi, 2012-07-30 19:48:49]

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:187:
* If a User-Agent is not specified in {@link HttpHeaders} then {@link
#USER_AGENT_SUFFIX} is set
On 2012/07/30 18:50:45, rmistry wrote:
> On 2012/07/30 17:30:11, yanivi wrote:
> > Design Issue: Seems like an unintuitive behavior.  If the developer chooses
to
> > call setSuppressUserAgentSuffix(true) then it seems we should try to honor
> that
> > request as much as possible.  What do you think?
> 
> Yes that was the other design option. My thinking was that nobody would want a
> null user-agent suffix (maybe I am wrong) and in that case we are providing a
> default user-agent not really adding a suffix.
> I can leave the null user-agent if you prefer that design.

Yeah, I think we should leave the user-agent null.  Otherwise, someone is going
to ask for it and we won't be able to do it in a backwards-compatible way.

[rmistry, 2012-07-31 15:19:59]

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/2001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:187:
* If a User-Agent is not specified in {@link HttpHeaders} then {@link
#USER_AGENT_SUFFIX} is set
On 2012/07/30 19:48:49, yanivi wrote:
> On 2012/07/30 18:50:45, rmistry wrote:
> > On 2012/07/30 17:30:11, yanivi wrote:
> > > Design Issue: Seems like an unintuitive behavior.  If the developer
chooses
> to
> > > call setSuppressUserAgentSuffix(true) then it seems we should try to honor
> > that
> > > request as much as possible.  What do you think?
> > 
> > Yes that was the other design option. My thinking was that nobody would want
a
> > null user-agent suffix (maybe I am wrong) and in that case we are providing
a
> > default user-agent not really adding a suffix.
> > I can leave the null user-agent if you prefer that design.
> 
> Yeah, I think we should leave the user-agent null.  Otherwise, someone is
going
> to ask for it and we won't be able to do it in a backwards-compatible way.

Done.

[yanivi, 2012-07-31 16:48:22]

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:867:
headers.setUserAgent(originalUserAgent);
don't call setUserAgent since it already has that value :)

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:869:
headers.setUserAgent(
[optional] here you are unnecesarily changing code style without changing
functionality.  only thing you actually need is to wrap the original code with
if (!suppressUserAgentSuffix)

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:876:
headers.setUserAgent(originalUserAgent);
if (!suppressUserAgentSuffix)

[rmistry, 2012-07-31 17:29:02]

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:867:
headers.setUserAgent(originalUserAgent);
On 2012/07/31 16:48:23, yanivi wrote:
> don't call setUserAgent since it already has that value :)

Done.

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:869:
headers.setUserAgent(
On 2012/07/31 16:48:23, yanivi wrote:
> [optional] here you are unnecesarily changing code style without changing
> functionality.  only thing you actually need is to wrap the original code with
> if (!suppressUserAgentSuffix)

True, Done.

http://codereview.appspot.com/6448055/diff/9002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:876:
headers.setUserAgent(originalUserAgent);
On 2012/07/31 16:48:23, yanivi wrote:
> if (!suppressUserAgentSuffix)

Done.

[yanivi, 2012-07-31 17:54:28]

LGTM

http://codereview.appspot.com/6448055/diff/5002/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/5002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:876:
// set the original user agent back to the headers so that retries do not keep
appending to
[optional] remove "to the headers" so it fits on 1 line

[rmistry, 2012-08-07 14:04:03]

http://codereview.appspot.com/6448055/diff/5002/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

http://codereview.appspot.com/6448055/diff/5002/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:876:
// set the original user agent back to the headers so that retries do not keep
appending to
On 2012/07/31 17:54:28, yanivi wrote:
> [optional] remove "to the headers" so it fits on 1 line

Done.