feat: support url form encoded marshaler and response in JSON #3711
Conversation
WalkthroughThe new Changes
Poem
Validation with GitHub issue(Beta)Assessment of the PR changes against GitHub Issue
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- runtime/marshal_urlencode.go (1 hunks)
Additional comments: 5
runtime/marshal_urlencode.go (5)
1-11: The import section looks clean and organized. All necessary packages for this file are imported correctly.
13-15: The
UrlEncodeMarshalstruct is defined correctly and implements theMarshalerinterface.18-20: The
ContentTypemethod correctly returns the content type of the response as "application/json".22-26: The
Marshalmethod correctly uses theJSONPbstruct to marshal the response into JSON format.28-56: The
NewDecodermethod correctly reads the request body, parses it as form data, and populates the query parameters of the proto message using thePopulateQueryParametersfunction. Error handling is also done correctly.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- runtime/marshaler_registry.go (3 hunks)
Additional comments: 3
runtime/marshaler_registry.go (3)
16-16: The new MIME type constant
MIMEFormURLEncodedis correctly defined.34-34: The
defaultFormMarshaleris correctly assigned to an instance ofUrlEncodeMarshal{}.102-102: The
MIMEFormURLEncodedMIME type is correctly mapped to thedefaultFormMarshalerin themimeMap.
There was a problem hiding this comment.
Hey, thanks for this contribution! We'll definitely want to add some tests, perhaps you could look at adding an integration test in https://github.com/grpc-ecosystem/grpc-gateway/blob/main/examples/internal/integration/integration_test.go? There are some example tests in that you could take inspiration from. Presumably, you should be able to add a test against the ABitOfEverything endpoint with the query parameters in the body of the request. Let me know if you need more help!
| return fmt.Errorf("not proto message") | ||
| } | ||
|
|
||
| formData, err := ioutil.ReadAll(r) |
| err = PopulateQueryParameters(msg, values, filter) | ||
|
|
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
Since we're not mutating the error before returning, we can just
| err = PopulateQueryParameters(msg, values, filter) | |
| if err != nil { | |
| return err | |
| } | |
| return nil | |
| return PopulateQueryParameters(msg, values, filter) |
|
@mickael-kerjean if you want to contribute tests to this PR you may need to get access to @jhowliu's fork. |
Fixed #7. Support request in x-www-form-urlencoded and response in JSON format
Summary by CodeRabbit
New Features:
runtime/marshal_urlencode.gowith aUrlEncodeMarshalstruct that implements theMarshalerinterface. This allows for marshaling the response in JSON format using theJSONPbstruct and populating query parameters of the proto message.runtime/marshaler_registry.gofile to include support for the form URL-encoded MIME type, expanding the MIME type registry.Bug Fixes:
Documentation:
Refactor:
Style:
Tests:
Chores:
Revert: