fix(httpjson): handle message derived query params (#1784)

* fix(httpjson): handle message derived query params

Fixes #1783
Some message derived types such as Duration, FieldMask or Int32Value
would not be correctly handled by String.valueOf(). Instead, the
toJson() method is used to make it compliant with the protobuf languague
guide

* fix(protoparser): decompose messages in query prms

Some message type objects will be passed as query params. These may have
nested properties that will now be generated as ?&foo.bar=1&foo.baz=2

* test(serializer): add test for complex msg obj

* fix(format): format ProtoRestSerializer files

* fix(queryparam): use json approach to process msgs

also fixed best practice issues pointed out in last commit's PR

* fix(queryparam): use numeric value for root enums

enums passed as root object to putQueryParam will now be serialized as
their numeric value

* chore: Refactoring the fix.

* test(queryparam): atomized tests

Also added tests for serializing objects that contain Any typed
messages. Note that the type registry must have the tested types
beforehand, so they were added in the test class setup

* test(queryparam): test objects w/ well-known types

Co-authored-by: Blake Li <[email protected]>

gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java

@@ -31,13 +31,16 @@
 
 import com.google.api.core.BetaApi;
 import com.google.common.collect.ImmutableList;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonParser;
 import com.google.protobuf.InvalidProtocolBufferException;
 import com.google.protobuf.Message;
 import com.google.protobuf.TypeRegistry;
 import com.google.protobuf.util.JsonFormat;
 import com.google.protobuf.util.JsonFormat.Printer;
 import java.io.IOException;
 import java.io.Reader;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -49,6 +52,7 @@
  */
 @BetaApi
 public class ProtoRestSerializer<RequestT extends Message> {
+
   private final TypeRegistry registry;
 
   private ProtoRestSerializer(TypeRegistry registry) {
@@ -75,7 +79,7 @@ static <RequestT extends Message> ProtoRestSerializer<RequestT> create(TypeRegis
    * @throws InvalidProtocolBufferException if failed to serialize the protobuf message to JSON
    *     format
    */
-  String toJson(RequestT message, boolean numericEnum) {
+  String toJson(Message message, boolean numericEnum) {
     try {
       Printer printer = JsonFormat.printer().usingTypeRegistry(registry);
       if (numericEnum) {
@@ -118,6 +122,23 @@ public void putPathParam(Map<String, String> fields, String fieldName, Object fi
     fields.put(fieldName, String.valueOf(fieldValue));
   }
 
+  private void putDecomposedMessageQueryParam(
+      Map<String, List<String>> fields, String fieldName, JsonElement parsed) {
+    if (parsed.isJsonPrimitive() || parsed.isJsonNull()) {
+      putQueryParam(fields, fieldName, parsed.getAsString());
+    } else if (parsed.isJsonArray()) {
+      for (JsonElement element : parsed.getAsJsonArray()) {
+        putDecomposedMessageQueryParam(fields, fieldName, element);
+      }
+    } else {
+      // it is a json object
+      for (String key : parsed.getAsJsonObject().keySet()) {
+        putDecomposedMessageQueryParam(
+            fields, String.format("%s.%s", fieldName, key), parsed.getAsJsonObject().get(key));
+      }
+    }
+  }
+
   /**
    * Puts a message field in {@code fields} map which will be used to populate query parameters of a
    * request.
@@ -127,16 +148,25 @@ public void putPathParam(Map<String, String> fields, String fieldName, Object fi
    * @param fieldValue a field value
    */
   public void putQueryParam(Map<String, List<String>> fields, String fieldName, Object fieldValue) {
-    ImmutableList.Builder<String> paramValueList = ImmutableList.builder();
-    if (fieldValue instanceof List<?>) {
-      for (Object fieldValueItem : (List<?>) fieldValue) {
-        paramValueList.add(String.valueOf(fieldValueItem));
+    List<String> currentParamValueList = new ArrayList<>();
+    List<Object> toProcess =
+        fieldValue instanceof List<?> ? (List<Object>) fieldValue : ImmutableList.of(fieldValue);
+    for (Object fieldValueItem : toProcess) {
+      if (fieldValueItem instanceof Message) {
+        String json = toJson(((Message) fieldValueItem), true);
+        JsonElement parsed = JsonParser.parseString(json);
+        putDecomposedMessageQueryParam(fields, fieldName, parsed);
+      } else {
+        currentParamValueList.add(String.valueOf(fieldValueItem));
       }
-    } else {
-      paramValueList.add(String.valueOf(fieldValue));
     }
-
-    fields.put(fieldName, paramValueList.build());
+    if (currentParamValueList.isEmpty()) {
+      // We try to avoid putting non-leaf level fields to the query params
+      return;
+    }
+    List<String> accumulativeParamValueList = fields.getOrDefault(fieldName, new ArrayList<>());
+    accumulativeParamValueList.addAll(currentParamValueList);
+    fields.put(fieldName, accumulativeParamValueList);
   }
 
   /**

gax-httpjson/src/test/java/com/google/api/gax/httpjson/ProtoRestSerializerTest.java

@@ -31,9 +31,17 @@
 package com.google.api.gax.httpjson;
 
 import com.google.common.truth.Truth;
+import com.google.protobuf.Duration;
 import com.google.protobuf.Field;
 import com.google.protobuf.Field.Cardinality;
+import com.google.protobuf.FieldMask;
+import com.google.protobuf.FloatValue;
+import com.google.protobuf.Int32Value;
 import com.google.protobuf.Option;
+import com.google.protobuf.Timestamp;
+import com.google.protobuf.TypeRegistry;
+import com.google.rpc.RetryInfo;
+import com.google.type.Interval;
 import java.io.IOException;
 import java.io.StringReader;
 import java.util.Arrays;
@@ -53,7 +61,14 @@ public class ProtoRestSerializerTest {
 
   @Before
   public void setUp() {
-    requestSerializer = ProtoRestSerializer.create();
+    // tests with Any type messages require corresponding descriptors in the type registry
+    requestSerializer =
+        ProtoRestSerializer.create(
+            TypeRegistry.newBuilder()
+                .add(FieldMask.getDescriptor())
+                .add(Duration.getDescriptor())
+                .build());
+
     field =
         Field.newBuilder()
             .setNumber(2)
@@ -163,7 +178,7 @@ public void putPathParam() {
   }
 
   @Test
-  public void putQueryParam() {
+  public void putQueryParamPrimitive() {
     Map<String, List<String>> fields = new HashMap<>();
     requestSerializer.putQueryParam(fields, "optName1", 1);
     requestSerializer.putQueryParam(fields, "optName2", 0);
@@ -181,6 +196,83 @@ public void putQueryParam() {
     Truth.assertThat(fields).isEqualTo(expectedFields);
   }
 
+  @Test
+  public void putQueryParamComplexObject() {
+    Map<String, List<String>> fields = new HashMap<>();
+    requestSerializer.putQueryParam(fields, "object", field);
+
+    Map<String, List<String>> expectedFields = new HashMap<>();
+    expectedFields.put("object.cardinality", Arrays.asList("1"));
+    expectedFields.put("object.name", Arrays.asList("field_name1"));
+    expectedFields.put("object.number", Arrays.asList("2"));
+    expectedFields.put("object.options.name", Arrays.asList("opt_name1", "opt_name2"));
+
+    Truth.assertThat(fields).isEqualTo(expectedFields);
+  }
+
+  @Test
+  public void putQueryParamComplexObjectDuration() {
+    Map<String, List<String>> fields = new HashMap<>();
+    Duration duration = Duration.newBuilder().setSeconds(1).setNanos(1).build();
+    RetryInfo input = RetryInfo.newBuilder().setRetryDelay(duration).build();
+    requestSerializer.putQueryParam(fields, "retry_info", input);
+
+    Map<String, List<String>> expectedFields = new HashMap<>();
+    expectedFields.put("retry_info.retryDelay", Arrays.asList("1.000000001s"));
+
+    Truth.assertThat(fields).isEqualTo(expectedFields);
+  }
+
+  @Test
+  public void putQueryParamComplexObjectTimestamp() {
+    Map<String, List<String>> fields = new HashMap<>();
+    Timestamp start = Timestamp.newBuilder().setSeconds(1).setNanos(1).build();
+    Timestamp end = Timestamp.newBuilder().setSeconds(2).setNanos(2).build();
+    Interval input = Interval.newBuilder().setStartTime(start).setEndTime(end).build();
+
+    requestSerializer.putQueryParam(fields, "object", input);
+
+    Map<String, List<String>> expectedFields = new HashMap<>();
+    expectedFields.put("object.startTime", Arrays.asList("1970-01-01T00:00:01.000000001Z"));
+    expectedFields.put("object.endTime", Arrays.asList("1970-01-01T00:00:02.000000002Z"));
+
+    Truth.assertThat(fields).isEqualTo(expectedFields);
+  }
+
+  @Test
+  public void putQueryParamDuration() {
+    queryParamHelper(Duration.newBuilder().setSeconds(1).setNanos(1).build(), "1.000000001s");
+  }
+
+  @Test
+  public void putQueryParamTimestamp() {
+    queryParamHelper(
+        Timestamp.newBuilder().setSeconds(1).setNanos(1).build(), "1970-01-01T00:00:01.000000001Z");
+  }
+
+  @Test
+  public void putQueryParamFieldMask() {
+    queryParamHelper(FieldMask.newBuilder().addPaths("a.b").addPaths("c.d").build(), "a.b,c.d");
+  }
+
+  @Test
+  public void putQueryParamInt32Value() {
+    queryParamHelper(Int32Value.of(1), "1");
+  }
+
+  @Test
+  public void putQueryParamFloatValue() {
+    queryParamHelper(FloatValue.of(1.1f), "1.1");
+  }
+
+  private void queryParamHelper(Object value, String expected) {
+    Map<String, List<String>> fields = new HashMap<>();
+    requestSerializer.putQueryParam(fields, "value", value);
+    Map<String, List<String>> expectedFields = new HashMap<>();
+    expectedFields.put("value", Arrays.asList(expected));
+    Truth.assertThat(fields).isEqualTo(expectedFields);
+  }
+
   @Test
   public void toBody() {
     String body = requestSerializer.toBody("bodyField1", field, false);