feat(gax): allow non-json HttpContent in HttpRequestFormatter and HttpRequestRunnable

Author: whowes

sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpRequestFormatter.java

@@ -29,7 +29,11 @@
  */
 package com.google.api.gax.httpjson;
 
+import com.google.api.client.http.ByteArrayContent;
+import com.google.api.client.http.EmptyContent;
+import com.google.api.client.http.HttpContent;
 import com.google.api.pathtemplate.PathTemplate;
+import com.google.common.base.Strings;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -56,4 +60,16 @@ public interface HttpRequestFormatter<MessageFormatT> {
   default List<PathTemplate> getAdditionalPathTemplates() {
     return Collections.emptyList();
   }
+
+  /**
+   * Return {@link HttpContent} representing the request body. Defaults to converting {@link
+   * #getRequestBody(Object)} to JSON, or {@link EmptyContent} if the body is empty.
+   */
+  default HttpContent getHttpContent(MessageFormatT apiMessage) {
+    String requestBody = getRequestBody(apiMessage);
+    if (!Strings.isNullOrEmpty(requestBody)) {
+      return ByteArrayContent.fromString("application/json; charset=utf-8", requestBody);
+    }
+    return new EmptyContent();
+  }
 }

sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpRequestRunnable.java

@@ -29,26 +29,21 @@
  */
 package com.google.api.gax.httpjson;
 
-import com.google.api.client.http.EmptyContent;
 import com.google.api.client.http.GenericUrl;
 import com.google.api.client.http.HttpContent;
-import com.google.api.client.http.HttpMediaType;
 import com.google.api.client.http.HttpMethods;
 import com.google.api.client.http.HttpRequest;
 import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.http.HttpResponse;
 import com.google.api.client.http.HttpResponseException;
 import com.google.api.client.http.HttpTransport;
-import com.google.api.client.http.json.JsonHttpContent;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonObjectParser;
 import com.google.api.client.json.gson.GsonFactory;
-import com.google.api.client.util.GenericData;
 import com.google.api.gax.tracing.ApiTracer;
 import com.google.auth.Credentials;
 import com.google.auth.http.HttpCredentialsAdapter;
 import com.google.auto.value.AutoValue;
-import com.google.common.base.Strings;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -154,8 +149,6 @@ public void run() {
   }
 
   HttpRequest createHttpRequest() throws IOException {
-    GenericData tokenRequest = new GenericData();
-
     HttpRequestFormatter<RequestT> requestFormatter = methodDescriptor.getRequestFormatter();
 
     HttpRequestFactory requestFactory;
@@ -166,20 +159,8 @@ HttpRequest createHttpRequest() throws IOException {
       requestFactory = httpTransport.createRequestFactory();
     }
 
-    JsonFactory jsonFactory = GsonFactory.getDefaultInstance();
     // Create HTTP request body.
-    String requestBody = requestFormatter.getRequestBody(request);
-    HttpContent jsonHttpContent;
-    if (!Strings.isNullOrEmpty(requestBody)) {
-      jsonFactory.createJsonParser(requestBody).parse(tokenRequest);
-      jsonHttpContent =
-          new JsonHttpContent(jsonFactory, tokenRequest)
-              .setMediaType((new HttpMediaType("application/json; charset=utf-8")));
-    } else {
-      // Force underlying HTTP lib to set Content-Length header to avoid 411s.
-      // See EmptyContent.java.
-      jsonHttpContent = new EmptyContent();
-    }
+    HttpContent httpContent = requestFormatter.getHttpContent(request);
 
     // Populate URL path and query parameters.
     String normalizedEndpoint = normalizeEndpoint(endpoint);
@@ -196,20 +177,21 @@ HttpRequest createHttpRequest() throws IOException {
       tracer.requestUrlResolved(url.build());
     }
 
-    HttpRequest httpRequest = buildRequest(requestFactory, url, jsonHttpContent);
+    HttpRequest httpRequest = buildRequest(requestFactory, url, httpContent);
 
     for (Map.Entry<String, Object> entry : headers.getHeaders().entrySet()) {
       HttpHeadersUtils.setHeader(
           httpRequest.getHeaders(), entry.getKey(), (String) entry.getValue());
     }
 
+    JsonFactory jsonFactory = GsonFactory.getDefaultInstance();
     httpRequest.setParser(new JsonObjectParser(jsonFactory));
 
     return httpRequest;
   }
 
   private HttpRequest buildRequest(
-      HttpRequestFactory requestFactory, GenericUrl url, HttpContent jsonHttpContent)
+      HttpRequestFactory requestFactory, GenericUrl url, HttpContent httpContent)
       throws IOException {
     // A workaround to support PATCH request. This assumes support of "X-HTTP-Method-Override"
     // header on the server side, which GCP services usually do.
@@ -235,7 +217,7 @@ private HttpRequest buildRequest(
     if (HttpMethods.PATCH.equals(actualHttpMethod)) {
       actualHttpMethod = HttpMethods.POST;
     }
-    HttpRequest httpRequest = requestFactory.buildRequest(actualHttpMethod, url, jsonHttpContent);
+    HttpRequest httpRequest = requestFactory.buildRequest(actualHttpMethod, url, httpContent);
     if (originalHttpMethod != null && !originalHttpMethod.equals(actualHttpMethod)) {
       HttpHeadersUtils.setHeader(
           httpRequest.getHeaders(), "X-HTTP-Method-Override", originalHttpMethod);

sdk-platform-java/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpRequestRunnableTest.java

@@ -31,19 +31,24 @@
 
 import static org.mockito.Mockito.mock;
 
+import com.google.api.client.http.ByteArrayContent;
 import com.google.api.client.http.EmptyContent;
+import com.google.api.client.http.HttpContent;
 import com.google.api.client.http.HttpRequest;
 import com.google.api.client.testing.http.MockHttpTransport;
 import com.google.api.gax.tracing.ApiTracer;
+import com.google.api.pathtemplate.PathTemplate;
 import com.google.common.truth.Truth;
 import com.google.longrunning.ListOperationsRequest;
+import com.google.protobuf.ByteString;
 import com.google.protobuf.Empty;
 import com.google.protobuf.Field;
 import com.google.protobuf.util.JsonFormat;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -326,4 +331,62 @@ void testUpdateRunnableTimeout_shouldUpdate() throws IOException {
     Truth.assertThat(httpRequest.getReadTimeout()).isEqualTo(30000L);
     Truth.assertThat(httpRequest.getConnectTimeout()).isEqualTo(30000L);
   }
+
+  @Test
+  void testNonJsonHttpContent() throws IOException {
+    ByteString rawPayload = ByteString.copyFromUtf8("binary \0 raw \1 payload");
+    HttpRequestFormatter<Field> binaryRequestFormatter =
+        new HttpRequestFormatter<Field>() {
+          @Override
+          public Map<String, List<String>> getQueryParamNames(Field apiMessage) {
+            return Collections.emptyMap();
+          }
+
+          @Override
+          public String getRequestBody(Field apiMessage) {
+            return "";
+          }
+
+          @Override
+          public HttpContent getHttpContent(Field apiMessage) {
+            return new ByteArrayContent("application/octet-stream", rawPayload.toByteArray());
+          }
+
+          @Override
+          public String getPath(Field apiMessage) {
+            return "/upload";
+          }
+
+          @Override
+          public PathTemplate getPathTemplate() {
+            return PathTemplate.create("{+path}");
+          }
+        };
+
+    ApiMethodDescriptor<Field, Empty> methodDescriptor =
+        ApiMethodDescriptor.<Field, Empty>newBuilder()
+            .setFullMethodName("upload.binary")
+            .setHttpMethod("POST")
+            .setRequestFormatter(binaryRequestFormatter)
+            .setResponseParser(responseParser)
+            .build();
+
+    HttpRequestRunnable<Field, Empty> httpRequestRunnable =
+        new HttpRequestRunnable<>(
+            requestMessage,
+            methodDescriptor,
+            ENDPOINT,
+            HttpJsonCallOptions.newBuilder().build(),
+            new MockHttpTransport(),
+            HttpJsonMetadata.newBuilder().build(),
+            (result) -> {});
+
+    HttpRequest httpRequest = httpRequestRunnable.createHttpRequest();
+    Truth.assertThat(httpRequest.getContent()).isInstanceOf(ByteArrayContent.class);
+    Truth.assertThat(httpRequest.getContent().getType()).isEqualTo("application/octet-stream");
+    try (ByteArrayOutputStream out = new ByteArrayOutputStream()) {
+      httpRequest.getContent().writeTo(out);
+      Truth.assertThat(out.toByteArray()).isEqualTo(rawPayload.toByteArray());
+    }
+  }
 }