Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FieldMask as query param is not converted to snake case #4134

Open
goober opened this issue Mar 24, 2024 · 1 comment
Open

FieldMask as query param is not converted to snake case #4134

goober opened this issue Mar 24, 2024 · 1 comment

Comments

@goober
Copy link

goober commented Mar 24, 2024

🐛 FieldMask as query param is not converted to snake case

We are trying to comply to the following standard from AIP-134

There should be exactly one google.api.method_signature annotation, with a value of "{resource},update_mask"

We are interpreting it as that the update_mask should be sent as a query param to the endpoint with the resource to be updated as the body.

Since the resources that are exposed through the grpc-gateway have camelCase field names we were expecting that the update_mask query param value would be converted from camelCase to snake_case before the request hits the underlying grpc service.

It works as expected if you send in the update_mask as part of the body, as described in the documentation

However, for query parameters it will not do the conversion and the value of the update_mask will be in the same format as is was sent.

To Reproduce

  1. Setup a grpc service with the following specification behind a grpc-gateway
  2. Send a request to update the FooBar resource's display name through the following command
$ curl -XPATCH --data '{"displayName": "new displayname"}' "http://localhost:8080/api/v1alpha1/foobars/foobar?updateMask=displayName"
service FooBarService {
  rpc UpdateFooBar(UpdateFooBarRequest) returns (FooBar) {
    option (google.api.http) = {
      patch: "/v1alpha1/{foobar.name=foobars/*}"
      body: "foobar"
    };
    option (google.api.method_signature) = "foobar,update_mask";
  }
}

message UpdateFooBarRequest  {
  FooBar foobar = 1;
  google.protobuf.FieldMask update_mask = 2;
}

message FooBar {
  string name = 1;
  string display_name = 2;
}

Expected behavior

The update_mask field should have the value display_name in snake_case when it hits the grpc service method.

Actual Behavior

The update_mask field has the value displayName in camelCase

@johanbrandhorst
Copy link
Collaborator

Hi, thanks for your issue. I don't know that this is something we can natively support in the grpc-gateway. The intended use of updates in the grpc-gateway is to rely on the auto-population of the update mask based on the contents of the body. You can turn off this behavior and set the update mask explicitly, but is it really a better experience? I've never before heard of the update_mask being supplied in the query.

In any case, if you want to support this, you'll probably have to write your own query parameter parser and pass it into the runtime initialization with https://pkg.go.dev/github.com/grpc-ecosystem/grpc-gateway/v2/runtime#SetQueryParameterParser. The default query parameter parser attempts to match request fields both based on the JSON name and the protobuf field name, but makes no attempt to parse the update mask and perform a translation. Parsing the update mask query parameter field could break some users, I think?

I hope that makes sense, let me know if you have any other questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants