Support json names in field mask generation (#1050)

* Support json names in field mask generation
Added tests for using jsonpb with OrigName: false
Added benchmarks for FieldMaskFromRequestBody called with a message descriptor

* Add descriptor dependency to go_proto_library

docs/_docs/patch.md

@@ -10,8 +10,6 @@ There are two scenarios:
 - The FieldMask is hidden from the REST request as per the [Google API design guide](https://cloud.google.com/apis/design/standard_methods#update) (as in the first additional binding in the [UpdateV2](https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/proto/examplepb/a_bit_of_everything.proto#L366) example). In this case, the FieldMask is updated from the request body and set in the gRPC request message.
 - The FieldMask is exposed to the REST request (as in the second additional binding in the [UpdateV2](https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/proto/examplepb/a_bit_of_everything.proto#L370) example). For this case, the field mask is left untouched by the gateway.
 
-Currently this feature is not compatible with using the JSON marshaller option to use json names in the http request/response described [here](https://grpc-ecosystem.github.io/grpc-gateway/docs/customizingyourgateway.html#using-camelcase-for-json). If you want to use json names then you may with to disable this feature with the command line option `-allow_patch_feature=false`
-
 ## Example Usage
 1. Create PATCH request.
 

examples/integration/BUILD.bazel

@@ -4,6 +4,7 @@ go_test(
     name = "go_default_test",
     srcs = [
         "client_test.go",
+        "fieldmask_test.go",
         "integration_test.go",
         "main_test.go",
         "proto_error_test.go",
@@ -19,6 +20,7 @@ go_test(
         "//examples/server:go_default_library",
         "//runtime:go_default_library",
         "@com_github_golang_glog//:go_default_library",
+        "@com_github_golang_protobuf//descriptor:go_default_library_gen",
         "@com_github_golang_protobuf//jsonpb:go_default_library_gen",
         "@com_github_golang_protobuf//proto:go_default_library",
         "@go_googleapis//google/rpc:status_go_proto",

examples/integration/fieldmask_test.go

@@ -0,0 +1,163 @@
+package integration_test
+
+import (
+	"bytes"
+	"fmt"
+	"testing"
+
+	"github.com/golang/protobuf/descriptor"
+	"github.com/grpc-ecosystem/grpc-gateway/examples/proto/examplepb"
+	"github.com/grpc-ecosystem/grpc-gateway/runtime"
+	"google.golang.org/genproto/protobuf/field_mask"
+)
+
+func fieldMasksEqual(fm1, fm2 *field_mask.FieldMask) bool {
+	if fm1 == nil && fm2 == nil {
+		return true
+	}
+	if fm1 == nil || fm2 == nil {
+		return false
+	}
+	if len(fm1.GetPaths()) != len(fm2.GetPaths()) {
+		return false
+	}
+
+	paths := make(map[string]bool)
+	for _, path := range fm1.GetPaths() {
+		paths[path] = true
+	}
+	for _, path := range fm2.GetPaths() {
+		if _, ok := paths[path]; !ok {
+			return false
+		}
+	}
+
+	return true
+}
+
+func newFieldMask(paths ...string) *field_mask.FieldMask {
+	return &field_mask.FieldMask{Paths: paths}
+}
+
+func fieldMaskString(fm *field_mask.FieldMask) string {
+	if fm == nil {
+		return ""
+	}
+	return fmt.Sprintf("%v", fm.GetPaths())
+}
+
+// N.B. These tests are here rather than in the runtime package because they need
+// to import examplepb for the descriptor, which would result in a circular
+// dependency since examplepb imports runtime from the pb.gw.go files
+func TestFieldMaskFromRequestBodyWithDescriptor(t *testing.T) {
+	_, md := descriptor.ForMessage(new(examplepb.NonStandardMessage))
+	jsonInput := `{"id":"foo", "thing":{"subThing":{"sub_value":"bar"}}}`
+	expected := newFieldMask("id", "thing.subThing.sub_value")
+
+	actual, err := runtime.FieldMaskFromRequestBody(bytes.NewReader([]byte(jsonInput)), md)
+	if !fieldMasksEqual(actual, expected) {
+		t.Errorf("want %v; got %v", fieldMaskString(expected), fieldMaskString(actual))
+	}
+	if err != nil {
+		t.Errorf("err %v", err)
+	}
+}
+
+func TestFieldMaskFromRequestBodyWithJsonNames(t *testing.T) {
+	_, md := descriptor.ForMessage(new(examplepb.NonStandardMessageWithJSONNames))
+	jsonInput := `{"ID":"foo", "Thingy":{"SubThing":{"sub_Value":"bar"}}}`
+	expected := newFieldMask("id", "thing.subThing.sub_value")
+
+	actual, err := runtime.FieldMaskFromRequestBody(bytes.NewReader([]byte(jsonInput)), md)
+	if !fieldMasksEqual(actual, expected) {
+		t.Errorf("want %v; got %v", fieldMaskString(expected), fieldMaskString(actual))
+	}
+	if err != nil {
+		t.Errorf("err %v", err)
+	}
+}
+
+// avoid compiler optimising benchmark away
+var result *field_mask.FieldMask
+
+func BenchmarkABEFieldMaskFromRequestBodyWithDescriptor(b *testing.B) {
+	_, md := descriptor.ForMessage(new(examplepb.ABitOfEverything))
+	input := `{` +
+		`"single_nested":				{"name": "bar",` +
+		`                           	 "amount": 10,` +
+		`                           	 "ok": "TRUE"},` +
+		`"uuid":						"6EC2446F-7E89-4127-B3E6-5C05E6BECBA7",` +
+		`"nested": 						[{"name": "bar",` +
+		`								  "amount": 10},` +
+		`								 {"name": "baz",` +
+		`								  "amount": 20}],` +
+		`"float_value":             	1.5,` +
+		`"double_value":            	2.5,` +
+		`"int64_value":             	4294967296,` +
+		`"uint64_value":            	9223372036854775807,` +
+		`"int32_value":             	-2147483648,` +
+		`"fixed64_value":           	9223372036854775807,` +
+		`"fixed32_value":           	4294967295,` +
+		`"bool_value":              	true,` +
+		`"string_value":            	"strprefix/foo",` +
+		`"bytes_value":					"132456",` +
+		`"uint32_value":            	4294967295,` +
+		`"enum_value":     		        "ONE",` +
+		`"path_enum_value":	    	    "DEF",` +
+		`"nested_path_enum_value":  	"JKL",` +
+		`"sfixed32_value":          	2147483647,` +
+		`"sfixed64_value":          	-4611686018427387904,` +
+		`"sint32_value":            	2147483647,` +
+		`"sint64_value":            	4611686018427387903,` +
+		`"repeated_string_value": 		["a", "b", "c"],` +
+		`"oneof_value":					{"oneof_string":"x"},` +
+		`"map_value": 					{"a": "ONE",` +
+		`								 "b": "ZERO"},` +
+		`"mapped_string_value": 		{"a": "x",` +
+		`								 "b": "y"},` +
+		`"mapped_nested_value": 		{"a": {"name": "x", "amount": 1},` +
+		`								 "b": {"name": "y", "amount": 2}},` +
+		`"nonConventionalNameValue":	"camelCase",` +
+		`"timestamp_value":				"2016-05-10T10:19:13.123Z",` +
+		`"repeated_enum_value":			["ONE", "ZERO"],` +
+		`"repeated_enum_annotation":	 ["ONE", "ZERO"],` +
+		`"enum_value_annotation": 		"ONE",` +
+		`"repeated_string_annotation":	["a", "b"],` +
+		`"repeated_nested_annotation": 	[{"name": "hoge",` +
+		`								  "amount": 10},` +
+		`								 {"name": "fuga",` +
+		`								  "amount": 20}],` +
+		`"nested_annotation": 			{"name": "hoge",` +
+		`								 "amount": 10},` +
+		`"int64_override_type":			12345` +
+		`}`
+	var r *field_mask.FieldMask
+	var err error
+	for i := 0; i < b.N; i++ {
+		r, err = runtime.FieldMaskFromRequestBody(bytes.NewReader([]byte(input)), md)
+	}
+	if err != nil {
+		b.Error(err)
+	}
+	result = r
+}
+
+func BenchmarkNonStandardFieldMaskFromRequestBodyWithDescriptor(b *testing.B) {
+	_, md := descriptor.ForMessage(new(examplepb.NonStandardMessage))
+	input := `{` +
+		`"id":			"foo",` +
+		`"Num": 		2,` +
+		`"line_num": 	3,` +
+		`"langIdent":	"bar",` +
+		`"STATUS": 		"baz"` +
+		`}`
+	var r *field_mask.FieldMask
+	var err error
+	for i := 0; i < b.N; i++ {
+		r, err = runtime.FieldMaskFromRequestBody(bytes.NewReader([]byte(input)), md)
+	}
+	if err != nil {
+		b.Error(err)
+	}
+	result = r
+}

examples/integration/integration_test.go

@@ -1643,10 +1643,23 @@ func TestNonStandardNames(t *testing.T) {
 			return
 		}
 	}()
+	go func() {
+		if err := runGateway(
+			ctx,
+			":8082",
+			runtime.WithMarshalerOption(runtime.MIMEWildcard, &runtime.JSONPb{OrigName: false, EmitDefaults: true}),
+		); err != nil {
+			t.Errorf("runGateway() failed with %v; want success", err)
+			return
+		}
+	}()
 
 	if err := waitForGateway(ctx, 8081); err != nil {
 		t.Errorf("waitForGateway(ctx, 8081) failed with %v; want success", err)
 	}
+	if err := waitForGateway(ctx, 8082); err != nil {
+		t.Errorf("waitForGateway(ctx, 8082) failed with %v; want success", err)
+	}
 
 	for _, tc := range []struct {
 		name     string
@@ -1667,6 +1680,18 @@ func TestNonStandardNames(t *testing.T) {
 			// N.B. json_names have no effect if not using OrigName: false
 			`{"id":"foo","Num":"1","line_num":"42","langIdent":"English","STATUS":"good","en_GB":"1","no":"yes","thing":{"subThing":{"sub_value":"hi"}}}`,
 		},
+		{
+			"Test standard update method with OrigName: false marshaller option",
+			8082,
+			"update",
+			`{"id":"foo","Num":"1","lineNum":"42","langIdent":"English","STATUS":"good","enGB":"1","no":"yes","thing":{"subThing":{"subValue":"hi"}}}`,
+		},
+		{
+			"Test update method using json_names in message with OrigName: false marshaller option",
+			8082,
+			"update_with_json_names",
+			`{"ID":"foo","Num":"1","LineNum":"42","langIdent":"English","status":"good","En_GB":"1","yes":"no","Thingy":{"SubThing":{"sub_Value":"hi"}}}`,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			testNonStandardNames(t, tc.port, tc.method, tc.jsonBody)

examples/proto/examplepb/BUILD.bazel

@@ -51,6 +51,7 @@ go_proto_library(
         "//examples/proto/sub:go_default_library",
         "//examples/proto/sub2:go_default_library",
         "//protoc-gen-swagger/options:go_default_library",
+        "@com_github_golang_protobuf//descriptor:go_default_library_gen",  # keep
         "@go_googleapis//google/api:annotations_go_proto",
     ],
 )
@@ -63,6 +64,7 @@ go_library(
     deps = [
         "//runtime:go_default_library",
         "//utilities:go_default_library",
+        "@com_github_golang_protobuf//descriptor:go_default_library_gen",
         "@com_github_golang_protobuf//proto:go_default_library",
         "@org_golang_google_grpc//:go_default_library",
         "@org_golang_google_grpc//codes:go_default_library",