REST PUT request only passes fields to update

[hangll]

I'm trying to export REST APIs and the problem is that in my PUT request, client usually tries to pass only fields to update, but the generated proto will set all the other fields to its default value. I have no way to tell which fields to update.

Of course I can restrict client to always pass the full object, but is there better way to do this?

[tamalsaha]

@hangll , on your server side, you could check for fields that have values set (aka, non-default value) and then only update those. That is what we do for our service.

[hangll]

@tamalsaha that is hard. How can you tell if a bool value is updated to false or is not set?

[tamalsaha]

@hangll you could define your boolean variables in a way that settings true will require update. Or, you could use a tr-state variable.

[evolsnow]

FieldMask may be helpful, and it would be wonderful if grpc-gateway could generate the mask from REST API.

[achew22]

The semantics for PUT are that you will include the entire contents of the file. If this were going to be supported it is would have to be under the verb PATCH

[adematte]

Hello, we have our own implementation to map REST -> GRPC in nodejs and are evaluating if we could use/support this project instead. One of the things we are doing is populating the fieldMask automatically based on the json request. Using a fieldmask is the de facto GRPC approach for "patching" and I can see real value in it being handled by the api gateway and not having it being generated by the REST clients.

Is this something that would make sense for this projet core features, or is this something that should be handled via a custom middleware of some kind?

[achew22]

@adematte I think that is the way to go. An auto populated fieldMask is, I think, the way to do this. That said, I'm not sure what the best way to go about doing that would be. Do you have a magically named fieldmask? Is there a part of the spec that allows for that that I've missed?

[wora]

I worked on proto3, gRPC, and Google API Design Guide. This is a well known feature request. I would recommend gRPC Gateway implement this feature.

When you perform a partial update against a Google API, you can explicitly pass the field mask as HTTP header X-Goog-FieldMask, see https://cloud.google.com/apis/docs/system-parameters. gRPC Gateway can auto generate this header if the request method is PATCH and this header is absent, based on the presence of JSON fields. I think the semantics is very easy to understand and also easy to use.

[adematte]

@achew22 currently what we are doing is this:

• look for a PATCH http route
• look for a property of type google.protobuf.FieldMask in the request message
• autopopulate it based on the json request (we only use the body for this)

As a reminder, FieldMasks can also be used for partial responses but we don't handle those in any particular way. We just handle the partial update (patch) scenarios.

[adematte]

@wora if I am not mistaken the document you are pointing to, refers to fieldmasks used for response filtering. I don't think it would be a good idea to hijack the X-Goog-FieldMask http header when using PATCH requests don't you agree?

[wora]

Good point. We should use a separate header for it.

[adematte]

do we even need to pass this through an http header though instead of just using the json body? The only rational I see regarding the http header is that this way we do not affect the json request payload. Any other advantages to the use of a header?

[achew22]

I see a few advantages to putting it in a header

1. it will work even if the request proto doesn't have a FieldMask field embedded in it
2. what do you do if there are multiple FieldMasks?
3. It doesn't pollute the exposed API to swagger. If you add a FieldMask, now your public API has an extra field that you have to explain to people doesn't do anything because it is overwritten by a proxy server.

[wora]

@adematte I think you are correct. Google API Design Guide recommends every PATCH method has an update_mask in the request message. We can auto populate it. The request message should have at most one FieldMask field at the top level. Otherwise, the proxy should do nothing.

[dmacthedestroyer]

Hi all, I'm looking to rehydrate the PATCH debate -- is there still an appetite for this? If so, I think I can provide development effort for it. This is the current design I see:

HTTP PATCH request comes in
if the gRPC request message has exactly 1 FieldMask field:
set the request FieldMask field based on HTTP request JSON body

Areas that still need discussing:

• What to do about the generated Swagger file? Should it be modified to exclude FieldMask?

[achew22]

If you I'm at the field mask from that generated Swagger definition that I think that would be great. Feel free to send a PR if you want

…

On Thu, May 31, 2018, 17:59 Daniel McDonald ***@***.***> wrote: Hi all, I'm looking to rehydrate the PATCH debate -- is there still an appetite for this? If so, I think I can provide development effort for it. This is the current design I see: HTTP PATCH request comes in if the gRPC request message has exactly 1 FieldMask field: set the request FieldMask field based on HTTP request JSON body Areas that still need discussing: - What to do about the generated Swagger file? Should it be modified to exclude FieldMask? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#379 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACRVs_ysvEo5oxLkm2ICXl_0ZB3zXaCks5t4IP8gaJpZM4NSbe1> .

[adematte]

I can see pros and cons about hiding or keeping the fieldmask in swagger although I would lean toward removing it: having a fieldmask feels alien 👽 in the existing JSON Rest API ecosystem and looks like a technical artifact coming from using GRPC in the backend. Maybe this is something that should be configurable at some point?

[kibertoad]

@dmacthedestroyer What about GET requests with optional parameters? This could be useful for a search.

[dmacthedestroyer]

That's a good question, and one we've battled with in our organization. I'm inclined to leave that as a separate conversation so as to not make this issue too ambitious :)

How's that sound? Maybe open a seperate issue to discuss that? I think it'd require a deeper look into what gRPC recommends for List commands and abiding by that first and foremost.

[kibertoad]

@dmacthedestroyer Sure, created #698

[ivucica]

One way of handling this is probably switching to syntax proto2; and checking whether a struct field is nil before dereferencing it.

If the field is nil, then the proto field was not set, and it was not passed to grpc-gateway.

If the field is not nil, then the value was set.

I'm using this to skip updating fields that the user has not passed in the proto message and in JSON. This is good enough for me, even if using FieldMasks (see
#812) might have advantages. (I can't think of any right now, though I'm sure there are some!)