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: Add struct tag query for binding query parameters in echo framework #1478

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ilick888
Copy link

@ilick888 ilick888 commented Feb 27, 2024

related: #1009

in the Echo framework, query binding is performed using the struct tag query, as detailed in Echo's documentation on struct tag binding (https://echo.labstack.com/guide/binding/#struct-tag-binding). If you are using oapi-codegen to generate an Echo server, there isn't an issue because the BindQueryParameter method handles the binding. But if oapi-codegen is only used to generate models, you'll encounter a problem since Echo won't bind query parameters to a field that lacks the query struct tag.

Therefore, I suggest adding the query struct tag. The modified code would look like this:

type FindPetsParams struct {
    Tags *[]string `form:"tags,omitempty" json:"tags,omitempty" query:"tags,omitempty"`

    Limit *int32 `form:"limit,omitempty" json:"limit,omitempty" query:"limit,omitempty"`
}

@ilick888 ilick888 changed the title Add struct tag query for binding query parameters in echo framework feat: Add struct tag query for binding query parameters in echo framework Feb 27, 2024
@jamietanna
Copy link
Collaborator

Hey @ilick888 thanks very much for this, and appreciate you helping close off #1009.

Would you be able to add this as an opt-in flag? I think it's probably best to keep it so folks intentionally opt-in to it, given it's going to change a lot of generated code, mostly unnecessarily.

Or alternatively, we make it so it only applies when just generating types?

Thoughts?

Copy link
Collaborator

@jamietanna jamietanna left a comment

Choose a reason for hiding this comment

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

See comment 😄

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.

None yet

2 participants