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

Support multiple query parameters with the same name #452

Closed
csmarchbanks opened this issue May 24, 2024 · 4 comments · Fixed by #498
Closed

Support multiple query parameters with the same name #452

csmarchbanks opened this issue May 24, 2024 · 4 comments · Fixed by #498
Labels
enhancement New feature or request

Comments

@csmarchbanks
Copy link
Contributor

I am looking at migrating to use huma in a project but one thing that is blocking me is support for requests such as /path?foo=bar&foo=baz. This is especially useful when a value may include a comma which would cause the current comma based approach to fail.

I am happy to implement this feature, and made a start, however I am not sure the best way to get the multiple values out of huma.Context. I could add a new method such as QueryMultiple(name string) []string to the interface, but would that be a breaking change for anyone implementing their own adapter? Happy to hear any suggestions. Worst case I suppose we could cast to a second interface and throw a runtime error?

@danielgtaylor
Copy link
Owner

danielgtaylor commented Jun 3, 2024

@csmarchbanks I'm open to trying to support this. I didn't originally implement it because of performance reasons, i.e. you can avoid all dynamic allocations in the request handling hot path by only having one value per query param name. I think returning []string will require allocations, but I'm open to ideas. The benchmarks in adapters/humachi can be helpful to see how your changes impact memory and garbage collection.

Edit: another option is to support escaping of commas.

@danielgtaylor danielgtaylor added the enhancement New feature or request label Jun 3, 2024
@turtledev1
Copy link

turtledev1 commented Jun 6, 2024

I'm trying to support the same thing. Currently it is supported by OpenAPI with the parameters "explode: true" and "style: form" (https://spec.openapis.org/oas/v3.1.0#parameter-object). I tried to add these annotations but the resulting json always have the explode property to false, and was confused until I stumbled on this thread.

EDIT: Found the exact line

huma/huma.go

Line 140 in 158b076

// If `in` is `query` then `explode` defaults to true. Parsing is *much*

@ARolek
Copy link

ARolek commented Jun 12, 2024

I'm also encountering this block. I'm happy to help with code review / testing / etc.

@csmarchbanks
Copy link
Contributor Author

I finally had some time to implement this and have a PR up at #498, let me know if you run into any issues with it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants