incorrect field mask generated for deeply nested struct

[houz42]

🐛 Bug Report

if an input json is deeply nested, the generated field mask may be incorrect.

input: {"foo": {"bar": {"baz": {"a": 1, "b": 2}}}}
expected field mask: ["foo.bar.baz.a", "foo.bar.baz.b"]
got: ["foo.bar.baz.a", "foo.bar.baz.a"]

To Reproduce

add another test case in runtime/fieldmask_test.go.TestFieldMaskFromRequestBody before line 58:
{name: "deep", input: {"foo": {"bar": {"baz": {"a": 1, "b": 2}}}}, expected: newFieldMask("foo.bar.baz.a", "foo.bar.baz.b")},

it fails

Expected behavior

two different paths in the field mask

Actual Behavior

two same paths

Your Environment

mac OSX
go 1.14.1
(Environment name, version and operating system.)

[johanbrandhorst]

Can you please try this against v2? The fieldmask handling has been changed in v2.

[houz42]

Can you please try this against v2? The fieldmask handling has been changed in v2.

Thanks for your reply. I checked out to v2, but did not make the test run. As in v2, I have to pass a proto.Message object, but the example one does not have a very deeply nested field, and I have not set a local development environment to generate a new one. I will try it later

[houz42]

@johanbrandhorst It still fails on v2.0.0-beta.3

To reproduce:

1. add definitions to the end of runtime/internal/examplepb/example.proto:

message NestedOuter {
  NestedOne one = 1;
};

message NestedOne {
  NestedTwo two = 1;
};

message NestedTwo {
  NestedThree three = 1;
};

message NestedThree {
  bool a = 1;
  bool b = 2;
};


message UpdateNestedRequest {
  NestedOuter nested = 1;
  google.protobuf.FieldMask update_mask = 2;
};

service NestedService {
  rpc Update(UpdateNestedRequest) returns(NestedOuter);
};

2. add test case in runtime/fieldmask_test.go before line 27:

		{
			name:     "deeply_nested",
			msg:      &examplepb.NestedOuter{},
			input:    `{"one":{"two":{"three":{"a": true, "b": false}}}}`,
			expected: newFieldMask("one.two.three.a", "one.two.three.b"),
		},

test output:

Running tool: /usr/local/bin/go test -timeout 300s github.com/grpc-ecosystem/grpc-gateway/v2/runtime -run ^TestFieldMaskFromRequestBody$ -v

=== RUN   TestFieldMaskFromRequestBody
=== RUN   TestFieldMaskFromRequestBody/deeply_nested
    TestFieldMaskFromRequestBody/deeply_nested: fieldmask_test.go:159: field masks differed:
          (*field_mask.FieldMask)(Inverse(protocmp.Transform, protocmp.Message{
          	"@type": s"google.protobuf.FieldMask",
          	"paths": []string{
        - 		"one.two.three.a",
        + 		"one.two.three.b",
          		"one.two.three.b",
          	},
          }))
...

[johanbrandhorst]

Interesting! Thanks for the bug report. Would you be interested in contributing a fix for this? You've already got a test there which is a great start. The logic is in https://github.com/grpc-ecosystem/grpc-gateway/blob/v2/runtime/fieldmask.go#L24.

[houz42]

@johanbrandhorst I can try it, thanks.