Convert negative status code to zero

https://codereview.appspot.com/9727043/
mahiuchun · May 30, 2013 · 02f2035 · 02f2035
1 parent 4896888
commit 02f2035
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 5 deletions.
diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
@@ -123,7 +123,7 @@ public final class HttpResponse {
     this.response = response;
     contentEncoding = response.getContentEncoding();
     int code = response.getStatusCode();
-    statusCode = code;
+    statusCode = code < 0 ? 0 : code;
     String message = response.getReasonPhrase();
     statusMessage = message;
     Logger logger = HttpTransport.LOGGER;
@@ -136,7 +136,7 @@ public final class HttpResponse {
       if (statusLine != null) {
         logbuf.append(statusLine);
       } else {
-        logbuf.append(code);
+        logbuf.append(statusCode);
         if (message != null) {
           logbuf.append(' ').append(message);
         }

diff --git a/google-http-client/src/main/java/com/google/api/client/http/LowLevelHttpResponse.java b/google-http-client/src/main/java/com/google/api/client/http/LowLevelHttpResponse.java
@@ -54,7 +54,14 @@ public abstract class LowLevelHttpResponse {
   /** Returns the response status line or {@code null} for none. */
   public abstract String getStatusLine() throws IOException;
 
-  /** Returns the response status code or {@code 0} for none. */
+  /**
+   * Returns the response status code or {@code <=0} for none.
+   *
+   * <p>
+   * Upgrade warning: in prior version 1.14 it could not return a negative number, but starting with
+   * version 1.15 it may.
+   * </p>
+   */
   public abstract int getStatusCode() throws IOException;
 
   /** Returns the HTTP reason phrase or {@code null} for none. */

diff --git a/...ttp-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java b/...ttp-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java
@@ -147,7 +147,8 @@ public int getStatusCode() {
 
   @Override
   public String getStatusLine() {
-    StringBuilder buf = new StringBuilder(statusCode);
+    StringBuilder buf = new StringBuilder();
+    buf.append(statusCode);
     if (reasonPhrase != null) {
       buf.append(reasonPhrase);
     }
@@ -256,7 +257,6 @@ public MockLowLevelHttpResponse setContentLength(long contentLength) {
    */
   public MockLowLevelHttpResponse setStatusCode(int statusCode) {
     this.statusCode = statusCode;
-    Preconditions.checkArgument(statusCode >= 0);
     return this;
   }
 

diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
@@ -98,6 +98,37 @@ public LowLevelHttpResponse execute() throws IOException {
     assertEquals(SAMPLE2, response.parseAsString());
   }
 
+  public void testStatusCode_negative_dontThrowException() throws Exception {
+    subtestStatusCode_negative(false);
+  }
+
+  public void testStatusCode_negative_throwException() throws Exception {
+    subtestStatusCode_negative(true);
+  }
+
+  private void subtestStatusCode_negative(boolean throwExceptionOnExecuteError) throws Exception {
+    HttpTransport transport = new MockHttpTransport() {
+      @Override
+      public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
+        return new MockLowLevelHttpRequest().setResponse(
+            new MockLowLevelHttpResponse().setStatusCode(-1));
+      }
+    };
+    HttpRequest request =
+        transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL);
+    request.setThrowExceptionOnExecuteError(throwExceptionOnExecuteError);
+    try {
+      // HttpResponse converts a negative status code to zero
+      HttpResponse response = request.execute();
+      assertEquals(0, response.getStatusCode());
+      assertFalse(throwExceptionOnExecuteError);
+    } catch (HttpResponseException e) {
+      // exception should be thrown only if throwExceptionOnExecuteError is true
+      assertTrue(throwExceptionOnExecuteError);
+      assertEquals(0, e.getStatusCode());
+    }
+  }
+
   public static class MyHeaders extends HttpHeaders {
 
     @Key