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

feat: support url form encoded marshaler and response in JSON #3711

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jhowliu
Copy link

@jhowliu jhowliu commented Nov 2, 2023

Fixed #7. Support request in x-www-form-urlencoded and response in JSON format

Summary by CodeRabbit

New Features:

  • Added a new file runtime/marshal_urlencode.go with a UrlEncodeMarshal struct that implements the Marshaler interface. This allows for marshaling the response in JSON format using the JSONPb struct and populating query parameters of the proto message.
  • Updated the runtime/marshaler_registry.go file to include support for the form URL-encoded MIME type, expanding the MIME type registry.

Bug Fixes:

  • None.

Documentation:

  • None.

Refactor:

  • None.

Style:

  • None.

Tests:

  • None.

Chores:

  • None.

Revert:

  • None.

Copy link

coderabbitai bot commented Nov 2, 2023

Walkthrough

The new runtime file introduces a UrlEncodeMarshal struct that implements the Marshaler interface. This struct provides methods for content type identification, JSON marshaling, and decoding of request bodies. It enhances the system's ability to handle and process JSON and form data, thereby improving the overall data management and communication within the application.

Changes

File Summary
runtime/marshal_urlencode.go Added a new file runtime with a UrlEncodeMarshal struct that implements the Marshaler interface. The UrlEncodeMarshal struct has methods for ContentType, Marshal, and NewDecoder. The ContentType method returns the content type of the response as "application/json". The Marshal method marshals the response in JSON format using the JSONPb struct. The NewDecoder method reads the request body, parses it as form data, and populates the query parameters of the proto message using the PopulateQueryParameters function.
runtime/marshaler_registry.go Summary:
- The constant FormURLEncoded with the value "application/x-www-form-urlencoded" is added.
- The defaultFormMarshaler variable is added and assigned to an instance of UrlEncodeMarshal.
- The marshalerRegistry map is updated to include FormURLEncoded as a key with the value of defaultFormMarshaler.

Poem

🍂 As the leaves fall this November, 🍁

Our codebase sees a new member. 📁

With UrlEncodeMarshal in sight, 🌟

Our data handling takes flight. 🚀

JSON and form data, hand in hand, 🤝

Make our application grand. 🏰

So let's celebrate this day, 🎉

For our code has found a better way! 💻🌈

Validation with GitHub issue(Beta)

Assessment of the PR changes against GitHub Issue

Aspect Aligns with the Linked Issue Explanation
Addition of UrlEncodeMarshal struct The new struct supports URL encoding, which aligns with the issue's requirement to add support for application/x-www-form-urlencoded encoding.
ContentType method in UrlEncodeMarshal struct The method returns "application/json" which contradicts the issue's requirement for application/x-www-form-urlencoded encoding.
Marshal method in UrlEncodeMarshal struct The method marshals the response in JSON format, which contradicts the issue's requirement for application/x-www-form-urlencoded encoding.
NewDecoder method in UrlEncodeMarshal struct The method reads the request body, parses it as form data, and populates the query parameters, which aligns with the issue's requirement for parameter deserialization.
Addition of FormURLEncoded and defaultFormMarshaler in marshalerRegistry map The addition of these elements expands the MIME type registry to include support for the form URL-encoded MIME type, aligning with the issue's requirement.
 </details>
 <!-- This is an auto-generated comment: raw summary by coderabbit.ai -->

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between b3213e8 and 97ee60f.
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 UrlEncodeMarshal struct is defined correctly and implements the Marshaler interface.

  • 18-20: The ContentType method correctly returns the content type of the response as "application/json".

  • 22-26: The Marshal method correctly uses the JSONPb struct to marshal the response into JSON format.

  • 28-56: The NewDecoder method correctly reads the request body, parses it as form data, and populates the query parameters of the proto message using the PopulateQueryParameters function. Error handling is also done correctly.

@jhowliu jhowliu closed this Nov 2, 2023
@jhowliu jhowliu reopened this Nov 2, 2023
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 97ee60f and eb16edd.
Files selected for processing (1)
  • runtime/marshaler_registry.go (3 hunks)
Files skipped from review due to trivial changes (1)
  • runtime/marshaler_registry.go

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between eb16edd and 1af1ef1.
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 MIMEFormURLEncoded is correctly defined.

  • 34-34: The defaultFormMarshaler is correctly assigned to an instance of UrlEncodeMarshal{}.

  • 102-102: The MIMEFormURLEncoded MIME type is correctly mapped to the defaultFormMarshaler in the mimeMap.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use io.ReadAll

Comment on lines +48 to +54
err = PopulateQueryParameters(msg, values, filter)

if err != nil {
return err
}

return nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're not mutating the error before returning, we can just

Suggested change
err = PopulateQueryParameters(msg, values, filter)
if err != nil {
return err
}
return nil
return PopulateQueryParameters(msg, values, filter)

@johanbrandhorst
Copy link
Collaborator

@mickael-kerjean if you want to contribute tests to this PR you may need to get access to @jhowliu's fork.

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

Successfully merging this pull request may close these issues.

Encoding with application/x-www-form-urlencoded
2 participants