NetHttpResponse.getStatusCode() should return 0 not -1

http://codereview.appspot.com/5728066/

Author: Yaniv Inbar

google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java

@@ -318,7 +318,7 @@ public boolean isSuccessStatusCode() {
   }
 
   /**
-   * Returns the HTTP status code.
+   * Returns the HTTP status code or {@code 0} for none.
    *
    * @since 1.5
    */

google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java

@@ -83,7 +83,7 @@ public boolean isSuccessStatusCode() {
   }
 
   /**
-   * Returns the HTTP status code.
+   * Returns the HTTP status code or {@code 0} for none.
    *
    * @since 1.7
    */
@@ -135,10 +135,16 @@ private static String computeMessageWithContent(HttpResponse response) {
    */
   public static StringBuilder computeMessageBuffer(HttpResponse response) {
     StringBuilder builder = new StringBuilder();
-    builder.append(response.getStatusCode());
+    int statusCode = response.getStatusCode();
+    if (statusCode != 0) {
+      builder.append(statusCode);
+    }
     String statusMessage = response.getStatusMessage();
     if (statusMessage != null) {
-      builder.append(' ').append(statusMessage);
+      if (statusCode != 0) {
+        builder.append(' ');
+      }
+      builder.append(statusMessage);
     }
     return builder;
   }

google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpResponse.java

@@ -34,7 +34,8 @@ final class NetHttpResponse extends LowLevelHttpResponse {
 
   NetHttpResponse(HttpURLConnection connection) throws IOException {
     this.connection = connection;
-    responseCode = connection.getResponseCode();
+    int responseCode = connection.getResponseCode();
+    this.responseCode = responseCode == -1 ? 0 : responseCode;
     responseMessage = connection.getResponseMessage();
     List<String> headerNames = this.headerNames;
     List<String> headerValues = this.headerValues;

google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java

@@ -50,10 +50,10 @@ public class MockLowLevelHttpResponse extends LowLevelHttpResponse {
   private String reasonPhrase;
 
   /** List of header names of HTTP response (empty array list by default). */
-  private ArrayList<String> headerNames = new ArrayList<String>();
+  private List<String> headerNames = new ArrayList<String>();
 
   /** List of header values of HTTP response (empty array list by default). */
-  private ArrayList<String> headerValues = new ArrayList<String>();
+  private List<String> headerValues = new ArrayList<String>();
 
   /** Content encoding or {@code null} for none. */
   private String contentEncoding;
@@ -154,8 +154,7 @@ public final List<String> getHeaderNames() {
    *
    * @since 1.5
    */
-  public MockLowLevelHttpResponse setHeaderNames(ArrayList<String> headerNames) {
-    // TODO(yanivi): change parameter to a List in 1.6
+  public MockLowLevelHttpResponse setHeaderNames(List<String> headerNames) {
     this.headerNames = Preconditions.checkNotNull(headerNames);
     return this;
   }
@@ -178,8 +177,7 @@ public final List<String> getHeaderValues() {
    *
    * @since 1.5
    */
-  public MockLowLevelHttpResponse setHeaderValues(ArrayList<String> headerValues) {
-    // TODO(yanivi): change parameter to a List in 1.6
+  public MockLowLevelHttpResponse setHeaderValues(List<String> headerValues) {
     this.headerValues = Preconditions.checkNotNull(headerValues);
     return this;
   }

google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java

@@ -54,6 +54,55 @@ public void testConstructorWithMessage() throws IOException {
     assertEquals("foo", e.getMessage());
   }
 
+  public void testConstructor_noStatusCode() throws IOException {
+    HttpTransport transport = new MockHttpTransport() {
+      @Override
+      public LowLevelHttpRequest buildGetRequest(String url) throws IOException {
+        return new MockLowLevelHttpRequest() {
+          @Override
+          public LowLevelHttpResponse execute() throws IOException {
+            MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
+            result.setStatusCode(0);
+            return result;
+          }
+        };
+      }
+    };
+    HttpRequest request =
+        transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL);
+    try {
+      request.execute();
+      fail();
+    } catch (HttpResponseException e) {
+      assertEquals("", e.getMessage());
+    }
+  }
+
+  public void testConstructor_messageButNoStatusCode() throws IOException {
+    HttpTransport transport = new MockHttpTransport() {
+      @Override
+      public LowLevelHttpRequest buildGetRequest(String url) throws IOException {
+        return new MockLowLevelHttpRequest() {
+          @Override
+          public LowLevelHttpResponse execute() throws IOException {
+            MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
+            result.setStatusCode(0);
+            result.setReasonPhrase("Foo");
+            return result;
+          }
+        };
+      }
+    };
+    HttpRequest request =
+        transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL);
+    try {
+      request.execute();
+      fail();
+    } catch (HttpResponseException e) {
+      assertEquals("Foo", e.getMessage());
+    }
+  }
+
   @SuppressWarnings("deprecation")
   public void testComputeMessage() throws IOException {
     HttpTransport transport = new MockHttpTransport() {
@@ -97,8 +146,8 @@ public LowLevelHttpResponse execute() throws IOException {
       request.execute();
       fail();
     } catch (HttpResponseException e) {
-      assertEquals("404 Not Found" + Strings.LINE_SEPARATOR + "Unable to find resource",
-          e.getMessage());
+      assertEquals(
+          "404 Not Found" + Strings.LINE_SEPARATOR + "Unable to find resource", e.getMessage());
     }
   }
 

google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpResponseTest.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2012 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.google.api.client.http.javanet;
+
+import junit.framework.TestCase;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+
+/**
+ * Tests {@link NetHttpResponse}.
+ *
+ * @author Yaniv Inbar
+ */
+public class NetHttpResponseTest extends TestCase {
+
+  static class MockHttpURLConnection extends HttpURLConnection {
+
+    private final int responseCode;
+
+    protected MockHttpURLConnection(int responseCode) {
+      super(null);
+      this.responseCode = responseCode;
+    }
+
+    @Override
+    public void disconnect() {
+    }
+
+    @Override
+    public boolean usingProxy() {
+      return false;
+    }
+
+    @Override
+    public void connect() throws IOException {
+    }
+
+    @Override
+    public int getResponseCode() throws IOException {
+      return responseCode;
+    }
+
+  }
+
+  public void testGetStatusCode() throws IOException {
+    assertEquals(0, new NetHttpResponse(new MockHttpURLConnection(-1)).getStatusCode());
+    assertEquals(200, new NetHttpResponse(new MockHttpURLConnection(200)).getStatusCode());
+  }
+}

google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpTransportTest.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2012 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.google.api.client.http.javanet;
+
+import junit.framework.TestCase;
+
+/**
+ * Tests {@link NetHttpTransport}.
+ *
+ * @author Yaniv Inbar
+ */
+public class NetHttpTransportTest extends TestCase {
+
+  public void testSupportsHead() {
+    NetHttpTransport transport = new NetHttpTransport();
+    assertTrue(transport.supportsHead());
+  }
+}