[1.15 and default] Convert negative status code to zero

[1.15 and default] Convert negative status code to zero

[yanivi, 2013-05-24 14:03:36]

Please change JavaDoc of LowLevelHttpResponse.getStatusCode() to specify that 
<= 0 is for none (not just 0).

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:108:
MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
[optional] return new MockLowLevelHttpRequest().setResponse(new
MockLowLevelHttpResponse.setStatusCode(-1));

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:117:
request.setThrowExceptionOnExecuteError(false);
please add a similar test without calling setThrowExceptionOnExecuteError(false)
and expect that an HttpResponseException is thrown and check that
HttpResponseException.getStatusCode() is 0.

[peleyal, 2013-05-24 15:23:58]

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:108:
MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
On 2013/05/24 14:03:36, yanivi wrote:
> [optional] return new MockLowLevelHttpRequest().setResponse(new
> MockLowLevelHttpResponse.setStatusCode(-1));

Done.

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:117:
request.setThrowExceptionOnExecuteError(false);
On 2013/05/24 14:03:36, yanivi wrote:
> please add a similar test without calling
setThrowExceptionOnExecuteError(false)
> and expect that an HttpResponseException is thrown and check that
> HttpResponseException.getStatusCode() is 0.

Done.

[yanivi, 2013-05-24 17:09:17]

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:108:
MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
On 2013/05/24 15:23:58, peleyal wrote:
> On 2013/05/24 14:03:36, yanivi wrote:
> > [optional] return new MockLowLevelHttpRequest().setResponse(new
> > MockLowLevelHttpResponse.setStatusCode(-1));
> 
> Done.

[optional] did you also want to try new
MockLowLevelHttpRequest().setResponse(...) ?  so it would look something like
this:

    HttpTransport transport = new MockHttpTransport() {
      @Override
      public LowLevelHttpRequest buildRequest(String method, String url) throws
IOException {
        return new MockLowLevelHttpRequest().setResponse(
            new MockLowLevelHttpResponse().setStatusCode(-1));
      }

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java
(right):

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:144:
public int getStatusCode() {
Please change JavaDoc of LowLevelHttpResponse.getStatusCode() to specify that 
<= 0 is for none (not just 0).

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/test/...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/test/...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:127:
assertEquals(0, response.getStatusCode());
assertFalse(throwExceptionOnExecuteError)

[peleyal, 2013-05-24 18:46:17]

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

https://codereview.appspot.com/9727043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:108:
MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
On 2013/05/24 17:09:17, yanivi wrote:
> On 2013/05/24 15:23:58, peleyal wrote:
> > On 2013/05/24 14:03:36, yanivi wrote:
> > > [optional] return new MockLowLevelHttpRequest().setResponse(new
> > > MockLowLevelHttpResponse.setStatusCode(-1));
> > 
> > Done.
> 
> [optional] did you also want to try new
> MockLowLevelHttpRequest().setResponse(...) ?  so it would look something like
> this:
> 
>     HttpTransport transport = new MockHttpTransport() {
>       @Override
>       public LowLevelHttpRequest buildRequest(String method, String url)
throws
> IOException {
>         return new MockLowLevelHttpRequest().setResponse(
>             new MockLowLevelHttpResponse().setStatusCode(-1));
>       }

Done.

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java
(right):

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:144:
public int getStatusCode() {
On 2013/05/24 17:09:17, yanivi wrote:
> Please change JavaDoc of LowLevelHttpResponse.getStatusCode() to specify that 
> <= 0 is for none (not just 0).

Done.

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/test/...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

https://codereview.appspot.com/9727043/diff/4002/google-http-client/src/test/...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:127:
assertEquals(0, response.getStatusCode());
On 2013/05/24 17:09:17, yanivi wrote:
> assertFalse(throwExceptionOnExecuteError)

Done.

[yanivi, 2013-05-30 18:14:09]

LGTM

https://codereview.appspot.com/9727043/diff/18001/google-http-client/src/main...
File
google-http-client/src/main/java/com/google/api/client/http/LowLevelHttpResponse.java
(right):

https://codereview.appspot.com/9727043/diff/18001/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/LowLevelHttpResponse.java:57:
/** Returns the response status code or {@code <=0} for none. */
Add an upgrade warning that in prior version 1.15 it could not return a negative
number, but now it may.  It is technically a breaking change, but I suspect it
won't break anything in practice since the only expected caller is HttpResponse.

[yanivi, 2013-05-30 18:30:20]

LGTM