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

Feature/patch2 rebased #812

Merged

Conversation

razamiDev
Copy link
Contributor

This PR is a cleaner version of PR #806
This was a collaborative effort by @razamiDev and @dmacthedestroyer

this addresses #379
If a binding is mapped to patch and the request message has exactly one FieldMask message in it, additional code is rendered for the gateway handler that will populate the FieldMask based on the request body.
This handles two scenarios:
The FieldMask is hidden from the REST request (as in the UpdateV2 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 PatchWithFieldMaskInBody example). For this case, a check is made as to whether the FieldMask is nil/empty prior to populating with the request body. If it's not nil, then it converts the FieldMask paths from the REST (snake_case) to gRPC (PascalCase) format. Otherwise, it acts like the previous case.
Additional comments of interest are inline on this PR.

@codecov-io
Copy link

codecov-io commented Nov 15, 2018

Codecov Report

Merging #812 into master will decrease coverage by 3.64%.
The diff coverage is 57.6%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #812      +/-   ##
==========================================
- Coverage    53.3%   49.66%   -3.65%     
==========================================
  Files          30       39       +9     
  Lines        3371     3725     +354     
==========================================
+ Hits         1797     1850      +53     
- Misses       1399     1697     +298     
- Partials      175      178       +3
Impacted Files Coverage Δ
examples/server/a_bit_of_everything.go 0% <0%> (ø)
utilities/readerfactory.go 0% <0%> (ø)
protoc-gen-grpc-gateway/gengateway/template.go 53.75% <21.42%> (-6.86%) ⬇️
examples/server/fieldmask_helper.go 92% <92%> (ø)
runtime/fieldmask.go 96.42% <96.42%> (ø)
examples/server/unannotatedecho.go 0% <0%> (ø)
examples/server/responsebody.go 0% <0%> (ø)
examples/server/echo.go 0% <0%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da4a1fc...c439fa6. Read the comment docs.

@razamiDev razamiDev mentioned this pull request Nov 15, 2018
@johanbrandhorst
Copy link
Collaborator

Hi @razamiDev, thanks for putting this together. I just had one quick idea - do you think you could put together a couple of little examples of requests this translates and compile it into a helpful page for the documentation (/docs)? I'd love for us to make great features like this more visible to the users. What do you think?

@drigz drigz mentioned this pull request Nov 19, 2018
@razamiDev
Copy link
Contributor Author

@johanbrandhorst I added some documentation. Let me know if that is what you had in mind.

@johanbrandhorst
Copy link
Collaborator

This is fantastic @razamiDev, thank you so much! I can't wait to merge this. Lets see what CI thinks :).

@johanbrandhorst
Copy link
Collaborator

Actually, just a quick thought, maybe add some cURL examples of each case as well?

@razamiDev
Copy link
Contributor Author

@johanbrandhorst good idea! I added some examples for curl :)

@johanbrandhorst
Copy link
Collaborator

Fantastic, let's merge this thing!

@johanbrandhorst johanbrandhorst merged commit d8ad87e into grpc-ecosystem:master Nov 19, 2018
@johanbrandhorst
Copy link
Collaborator

Congrats @razamiDev and @dmacthedestroyer on finally getting this in, we've all been very excited for this!

adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
* Updated Patch behavior

* Patch update with regenerated BUILD files

* Added documentation for PATCH usage

* Added some curl examples for PATCH
wolfinger referenced this pull request in gogo/grpc-example Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants