v1: Generated field mask is incorrect when field is of type map<string, string>

[catper]

🐛 Bug Report

A message containing a map field that is read/write

map<string, string> data = 1

will cause an incorrect field mask to be generated when attempting to execute an update.

To Reproduce

1. Define a message with a mutable field of type map:

message Resource {
    string id = 1;
    map<string, string> metadata = 2;
}

2. Create a service using said message and define an update method bound to PATCH:

syntax = "proto3";

service ResourceService {
    // stuff to create resource here

    rpc UpdateResource (UpdateResourceRequest) returns (Resource) {
        option (google.api.http) = {
            patch: "/v1/resources/{resource.id}"
            body: "resource"
        };
    }

  message UpdateResourceRequest {
      string request_id = 1;
      Resource resource = 2;
      google.protobuf.FieldMask update_mask = 3;
  }
}

3. Implement service and generate gateway per instructions.

4. Issue an update request on an existing resource:

curl -v X PATCH http://<host>:<path>/v1/resources/<resource_id>?request_id=asdf -d '{"metadata": { "key": "value"}}'

A call to FieldMaskUtil.isValid on the generated proto will return false.

Expected behavior

The correct mapping of update_mask is

update_mask {
  paths: "metadata"
}

Actual Behavior

The generated mapping of update_mask is

update_mask {
  paths: "metadata.key"
}

It seems as though the field mask generation assumes that the metadata field always is a nested proto but as demonstrated this doesn't have to be the case.

Your Environment

Running grpc gateway v. 1.14.3 but as far as I can see this behaviour exists in all newer versions too as fieldmask.go hasn't changed since last september.

[johanbrandhorst]

Hi, can you test this against v2? The field map handling has been rewritten for v2.

[catper]

i'm afraid upgrading to v2 is causing all sorts of problems when trying to build and run the service:

test/v1/resource_service.pb.gw.go:81:2: cannot use msg (type *Resource) as type protoreflect.ProtoMessage in return argument:
        *Resource does not implement protoreflect.ProtoMessage (missing ProtoReflect method)
test/v1/resource_service.pb.gw.go:98:2: cannot use msg (type *Resource) as type protoreflect.ProtoMessage in return argument:
        *Resource does not implement protoreflect.ProtoMessage (missing ProtoReflect method)
test/v1/resource_service.pb.gw.go:118:78: cannot use protoReq.Resource (type *Resource) as type protoreflect.ProtoMessage in argument to runtime.FieldMaskFromRequestBody:
        *Resource does not implement protoreflect.ProtoMessage (missing ProtoReflect method)
test/v1/resource_service.pb.gw.go:137:38: cannot use &protoReq (type *UpdateResourceRequest) as type protoreflect.ProtoMessage in argument to runtime.PopulateFieldFromPath:
        *UpdateResourceRequest does not implement protoreflect.ProtoMessage (missing ProtoReflect method)
test/v1/resource_service.pb.gw.go:146:44: cannot use &protoReq (type *UpdateResourceRequest) as type protoreflect.ProtoMessage in argument to runtime.PopulateQueryParameters:
        *UpdateResourceRequest does not implement protoreflect.ProtoMessage (missing ProtoReflect method)
test/v1/resource_service.pb.gw.go:151:2: cannot use msg (type *Resource) as type protoreflect.ProtoMessage in return argument:
        *Resource does not implement protoreflect.ProtoMessage (missing ProtoReflect method)

not sure if i did anything strange but i don't think so, i just followed the instructions here

[johanbrandhorst]

Sorry, you may need to follow the v2 migration guide: https://github.com/grpc-ecosystem/grpc-gateway/blob/v2/docs/_docs/v2-migration.md. It's not live on the docs page yet because v2 isn't stable yet. Chiefly, you'll need to upgrade your protoc-gen-go to v1.4.0+ to fix the errors you're experiencing.

[catper]

yep, seems i corrupted my dependencies somewhere along the way. redoing the whole thing and starting fresh i can compile and run without issues. as far as i can tell the fieldmasks now look correct so that's great.

any idea when there will be a stable release of v2? i really rather need the fieldmask fix now but don't want to upgrade without any idea of how stable things are. are there any known major problems with v2?

[johanbrandhorst]

v2 is as good as ready to go, there are no major changes planned and no outstanding bugs (see #1223). The only outstanding thing is an external dependency, we're waiting for grpc-go to tag a release of protoc-gen-go-grpc. Please use v2 with confidence and report any issues you find :). Glad to hear it's doing the right thing! I think we will leave this bug open since it still affects v1.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.