UrlEncodedContent.getContent and don't allow null data

http://codereview.appspot.com/5427043/

Author: Yaniv Inbar

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

@@ -14,6 +14,8 @@
 
 package com.google.api.client.http;
 
+import com.google.common.base.Preconditions;
+
 import java.io.IOException;
 
 /**
@@ -34,8 +36,8 @@ public final class BasicAuthentication implements HttpRequestInitializer {
   private final String password;
 
   public BasicAuthentication(String username, String password) {
-    this.username = username;
-    this.password = password;
+    this.username = Preconditions.checkNotNull(username);
+    this.password = Preconditions.checkNotNull(password);
   }
 
   /**

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

@@ -19,6 +19,7 @@
 import com.google.api.client.util.GenericData;
 import com.google.api.client.util.Key;
 import com.google.api.client.util.Strings;
+import com.google.common.base.Preconditions;
 
 import java.util.HashMap;
 
@@ -583,8 +584,9 @@ public final void setAuthenticate(String authenticate) {
    * @since 1.2
    */
   public final void setBasicAuthentication(String username, String password) {
-    String encoded =
-        Strings.fromBytesUtf8(Base64.encode(Strings.toBytesUtf8(username + ":" + password)));
+    String userPass =
+        Preconditions.checkNotNull(username) + ":" + Preconditions.checkNotNull(password);
+    String encoded = Strings.fromBytesUtf8(Base64.encode(Strings.toBytesUtf8(userPass)));
     authorization = "Basic " + encoded;
   }
 

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

@@ -25,7 +25,9 @@
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.util.HashMap;
 import java.util.Map;
+import java.util.logging.Logger;
 
 /**
  * Implements support for HTTP form content encoding serialization of type {@code
@@ -53,19 +55,39 @@ static void setContent(HttpRequest request, Object item) {
  */
 public class UrlEncodedContent extends AbstractHttpContent {
 
-  /** Content type. Default value is {@link UrlEncodedParser#CONTENT_TYPE}. */
+  private static final Logger LOGGER = Logger.getLogger(UrlEncodedContent.class.getName());
+
+  /**
+   * Content type or {@code null} for none.
+   *
+   * <p>
+   * Default value is {@link UrlEncodedParser#CONTENT_TYPE}.
+   * </p>
+   */
   private String contentType = UrlEncodedParser.CONTENT_TYPE;
 
-  /** Key name/value data or {@code null} for none. */
+  /** Key name/value data. */
   private Object data;
 
   /**
-   * @param data key name/value data or {@code null} for none
+   * <p>
+   * Upgrade warning: prior to version 1.7, the {@code data} parameter could be {@code null} but now
+   * {@code null} is not allowed and instead a new instance of an implementation of {@link Map} will
+   * be created. In version 1.8 a {@link NullPointerException} will be thrown instead.
+   * </p>
+   * 
+   * @param data key name/value data
    */
   public UrlEncodedContent(Object data) {
-    this.data = data;
+    setData(data);
   }
 
+  /**
+   * {@inheritDoc}
+   * <p>
+   * Upgrade warning: overriding this method is no longer supported, and will be made final in 1.8.
+   * </p>
+   */
   public String getType() {
     return contentType;
   }
@@ -94,7 +116,11 @@ public void writeTo(OutputStream out) throws IOException {
    * Sets the content type or {@code null} for none.
    *
    * <p>
-   * Defaults to {@link UrlEncodedParser#CONTENT_TYPE}.
+   * Default value is {@link UrlEncodedParser#CONTENT_TYPE}.
+   * </p>
+   * <p>
+   * Overriding is only supported for the purpose of calling the super implementation and changing
+   * the return type, but nothing else.
    * </p>
    *
    * @since 1.5
@@ -107,22 +133,63 @@ public UrlEncodedContent setType(String type) {
   /**
    * Returns the key name/value data or {@code null} for none.
    *
+   * <p>
+   * Upgrade warning: prior to version 1.7 this could return {@code null} but now it always returns
+   * a {@code non-null} value. Also overriding this method is no longer supported, and will be made
+   * final in 1.8.
+   * </p>
+   *
    * @since 1.5
    */
   public Object getData() {
     return data;
   }
 
   /**
-   * Sets the key name/value data or {@code null} for none.
+   * Sets the key name/value data.
+   *
+   * <p>
+   * Overriding is only supported for the purpose of calling the super implementation and changing
+   * the return type, but nothing else.
+   * </p>
+   * <p>
+   * Upgrade warning: prior to version 1.7, the {@code data} parameter could be {@code null} but now
+   * {@code null} is not allowed and instead a new instance of an implementation of {@link Map} will
+   * be created. In version 1.8 a {@link NullPointerException} will be thrown instead.
+   * </p>
    *
    * @since 1.5
    */
   public UrlEncodedContent setData(Object data) {
+    if (data == null) {
+      LOGGER.warning("UrlEncodedContent.setData(null) no longer supported");
+      data = new HashMap<String, Object>();
+    }
     this.data = data;
     return this;
   }
 
+  /**
+   * Returns the URL-encoded content of the given HTTP request, or if none return and set as content
+   * a new instance of {@link UrlEncodedContent} (whose {@link #getData()} is an implementation of
+   * {@link Map}).
+   *
+   * @param request HTTP request
+   * @return URL-encoded content
+   * @throws ClassCastException if the HTTP request has a content defined that is not
+   *         {@link UrlEncodedContent}
+   * @since 1.7
+   */
+  public static UrlEncodedContent getContent(HttpRequest request) {
+    HttpContent content = request.getContent();
+    if (content != null) {
+      return (UrlEncodedContent) content;
+    }
+    UrlEncodedContent result = new UrlEncodedContent(new HashMap<String, Object>());
+    request.setContent(result);
+    return result;
+  }
+
   private static boolean appendParam(boolean first, Writer writer, String name, Object value)
       throws IOException {
     // ignore nulls

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

@@ -14,13 +14,17 @@
 
 package com.google.api.client.http;
 
+import com.google.api.client.testing.http.HttpTesting;
+import com.google.api.client.testing.http.MockHttpTransport;
 import com.google.api.client.util.ArrayMap;
 
 import junit.framework.TestCase;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
 
 /**
  * Tests {@link UrlEncodedContent}.
@@ -51,4 +55,22 @@ static String toString(Object data) throws IOException {
     content.writeTo(out);
     return out.toString();
   }
+
+  public void testGetContent() throws IOException {
+    HttpRequest request =
+        new MockHttpTransport().createRequestFactory().buildGetRequest(
+            HttpTesting.SIMPLE_GENERIC_URL);
+    UrlEncodedContent content = UrlEncodedContent.getContent(request);
+    assertNotNull(content);
+    assertTrue(content.getData() instanceof Map);
+    assertEquals(content, UrlEncodedContent.getContent(request));
+  }
+
+  public void testGetData() {
+    UrlEncodedContent content = new UrlEncodedContent(null);
+    assertTrue(content.getData() instanceof Map);
+    Map<String, Object> map = new HashMap<String, Object>();
+    content = new UrlEncodedContent(map);
+    assertEquals(map, content.getData());
+  }
 }