From 4524fadbe688d96a7c5e715ab88b542b4bb3891a Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 13 Sep 2022 14:15:22 -0400 Subject: [PATCH] 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 --- .../api/gax/httpjson/ProtoRestSerializer.java | 48 ++++++++-- .../gax/httpjson/ProtoRestSerializerTest.java | 96 ++++++++++++++++++- 2 files changed, 133 insertions(+), 11 deletions(-) diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java index 298a4ce4f..0c5158f76 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoRestSerializer.java @@ -31,6 +31,8 @@ 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; @@ -38,6 +40,7 @@ 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 { + private final TypeRegistry registry; private ProtoRestSerializer(TypeRegistry registry) { @@ -75,7 +79,7 @@ static ProtoRestSerializer 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 fields, String fieldName, Object fi fields.put(fieldName, String.valueOf(fieldValue)); } + private void putDecomposedMessageQueryParam( + Map> 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 fields, String fieldName, Object fi * @param fieldValue a field value */ public void putQueryParam(Map> fields, String fieldName, Object fieldValue) { - ImmutableList.Builder paramValueList = ImmutableList.builder(); - if (fieldValue instanceof List) { - for (Object fieldValueItem : (List) fieldValue) { - paramValueList.add(String.valueOf(fieldValueItem)); + List currentParamValueList = new ArrayList<>(); + List toProcess = + fieldValue instanceof List ? (List) 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 accumulativeParamValueList = fields.getOrDefault(fieldName, new ArrayList<>()); + accumulativeParamValueList.addAll(currentParamValueList); + fields.put(fieldName, accumulativeParamValueList); } /** diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/ProtoRestSerializerTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/ProtoRestSerializerTest.java index 0256f6726..3ff9ceaf6 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/ProtoRestSerializerTest.java +++ b/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> 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> fields = new HashMap<>(); + requestSerializer.putQueryParam(fields, "object", field); + + Map> 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> 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> expectedFields = new HashMap<>(); + expectedFields.put("retry_info.retryDelay", Arrays.asList("1.000000001s")); + + Truth.assertThat(fields).isEqualTo(expectedFields); + } + + @Test + public void putQueryParamComplexObjectTimestamp() { + Map> 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> 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> fields = new HashMap<>(); + requestSerializer.putQueryParam(fields, "value", value); + Map> 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);