fix(gengapic): remove encoded quotes in query params (#1364)

Removes all quotation marks from protojson encoded well known types that are encoded as strings. Lazily use strings.ReplaceAll to make it easier when the WKT doesn't necessarily have quotations included. Showcase integration still works (some other languages do not encode quotations), and I tested locally with another API's samples. Fixes #1363
googleapis · Jun 20, 2023 · 5d62c34 · 5d62c34
1 parent eb5dcf0
commit 5d62c34
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 16 deletions.
diff --git a/internal/gengapic/genrest.go b/internal/gengapic/genrest.go
@@ -419,7 +419,12 @@ func (g *generator) generateQueryString(m *descriptor.MethodDescriptorProto) {
 				b.WriteString("  return nil, err\n")
 			}
 			b.WriteString("}\n")
-			b.WriteString(fmt.Sprintf("params.Add(%q, string(%s))", key, field.GetJsonName()))
+			// Only some of the well known types will be encoded as strings, remove the wrapping quotations for those.
+			if strContains(wellKnownStringTypes, field.GetTypeName()) {
+				b.WriteString(fmt.Sprintf("params.Add(%q, string(%s[1:len(%[2]s)-1]))", key, field.GetJsonName()))
+			} else {
+				b.WriteString(fmt.Sprintf("params.Add(%q, string(%s))", key, field.GetJsonName()))
+			}
 			paramAdd = b.String()
 		} else {
 			paramAdd = fmt.Sprintf("params.Add(%q, fmt.Sprintf(%q, req%s))", key, "%v", accessor)

diff --git a/internal/gengapic/genrest_test.go b/internal/gengapic/genrest_test.go
@@ -506,9 +506,16 @@ func TestGenRestMethod(t *testing.T) {
 		Label:    descriptor.FieldDescriptorProto_LABEL_REPEATED.Enum(),
 	}
 
+	numericWrapperField := &descriptor.FieldDescriptorProto{
+		Name:     proto.String("numeric_wrapper"),
+		Type:     descriptor.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
+		JsonName: proto.String("numericWrapper"),
+		TypeName: proto.String(".google.protobuf.DoubleValue"),
+	}
+
 	updateReq := &descriptor.DescriptorProto{
 		Name:  proto.String("UpdateRequest"),
-		Field: []*descriptor.FieldDescriptorProto{fooField, maskField, repeatedPrimField},
+		Field: []*descriptor.FieldDescriptorProto{fooField, maskField, repeatedPrimField, numericWrapperField},
 	}
 	updateReqFqn := fmt.Sprintf(".%s.UpdateRequest", pkg)
 
@@ -704,19 +711,20 @@ func TestGenRestMethod(t *testing.T) {
 				updateReq:    f,
 			},
 			ParentElement: map[pbinfo.ProtoType]pbinfo.ProtoType{
-				opRPC:           s,
-				emptyRPC:        s,
-				unaryRPC:        s,
-				pagingRPC:       s,
-				serverStreamRPC: s,
-				clientStreamRPC: s,
-				lroRPC:          s,
-				httpBodyRPC:     s,
-				updateRPC:       s,
-				nameField:       op,
-				sizeField:       foo,
-				otherField:      foo,
-				maskField:       updateReq,
+				opRPC:               s,
+				emptyRPC:            s,
+				unaryRPC:            s,
+				pagingRPC:           s,
+				serverStreamRPC:     s,
+				clientStreamRPC:     s,
+				lroRPC:              s,
+				httpBodyRPC:         s,
+				updateRPC:           s,
+				nameField:           op,
+				sizeField:           foo,
+				otherField:          foo,
+				maskField:           updateReq,
+				numericWrapperField: updateReq,
 			},
 			Type: map[string]pbinfo.ProtoType{
 				opfqn:          op,

diff --git a/internal/gengapic/testdata/rest_UpdateRPC.want b/internal/gengapic/testdata/rest_UpdateRPC.want
@@ -14,6 +14,13 @@ func (c *fooRESTClient) UpdateRPC(ctx context.Context, req *foopb.UpdateRequest,
 
 	params := url.Values{}
 	params.Add("$alt", "json;enum-encoding=int")
+	if req.GetNumericWrapper() != nil {
+		numericWrapper, err := protojson.Marshal(req.GetNumericWrapper())
+		if err != nil {
+		  return nil, err
+		}
+		params.Add("numericWrapper", string(numericWrapper))
+	}
 	if items := req.GetPrimitives(); len(items) > 0 {
 		for _, item := range items {
 		  params.Add("primitives", fmt.Sprintf("%v", item))
@@ -24,7 +31,7 @@ func (c *fooRESTClient) UpdateRPC(ctx context.Context, req *foopb.UpdateRequest,
 		if err != nil {
 		  return nil, err
 		}
-		params.Add("updateMask", string(updateMask))
+		params.Add("updateMask", string(updateMask[1:len(updateMask)-1]))
 	}
 
 	baseUrl.RawQuery = params.Encode()

diff --git a/internal/gengapic/well_known_types.go b/internal/gengapic/well_known_types.go
@@ -44,6 +44,16 @@ var wellKnownTypeNames = []string{
 	".google.protobuf.ListValue",
 }
 
+// TODO: Look into if we need to support ListValue/Value fields with
+// StringValue or BytesValue values.
+var wellKnownStringTypes = []string{
+	".google.protobuf.FieldMask",
+	".google.protobuf.Timestamp",
+	".google.protobuf.Duration",
+	".google.protobuf.StringValue",
+	".google.protobuf.BytesValue",
+}
+
 var wellKnownTypeFiles = []*descriptor.FileDescriptorProto{
 	protodesc.ToFileDescriptorProto(emptypb.File_google_protobuf_empty_proto),
 	protodesc.ToFileDescriptorProto(anypb.File_google_protobuf_any_proto),