8000 add support for path params in middle of the path by dvjn · Pull Request #3713 · goadesign/goa · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

add support for path params in middle of the path #3713

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: v3
Choose a base branch
from
Open

add support for path params in middle of the path #3713

wants to merge 1 commit into from

Conversation

dvjn
Copy link
@dvjn dvjn commented May 24, 2025

Added support for path params in the middles of the paths as well

Earlier /user/@{username} didn't register username as a path param.
Similarly /{month}-{day}-{year} from chi's readme examples would only detect {month} as a param and nothing else.

After this change, all these path params should be detected and should work .

@dvjn
Copy link
Author
dvjn commented May 24, 2025

I have tested these changes locally in a working project and am waiting for a maintainers review before adding specific test cases for this feature.

Please let me know if anything else is needed for this PR. Thanks.

@raphael
Copy link
Member
raphael commented May 25, 2025

Thanks for this PR! This is a great enhancement that brings Goa's path parameter detection in line with Chi router. The examples you mentioned (/user/@{username} and /{month}-{day}-{year}) are useful patterns. The core change seems simple, since it still requires valid parameter names [a-zA-Z0-9_]+, it won't accidentally match anything weird. And the OpenAPI generation should work fine since the surrounding characters (like @ or -) stay in place.

To make sure everything works end-to-end, could you add a few tests? I'm thinking http/mux_test.go would be a good place to add new mux tests. We should also add a validation test to make sure routes like /{year}-{month}-{day} properly validate that all three parameters exist in the method payload - you can extend the existing TestHTTPRouteValidation in expr/http_endpoint_test.go for this. Maybe add an OpenAPI test case with one of the new patterns to verify the generated spec looks right - adding a new DSL function to http/codegen/testdata/openapi_dsls.go should automatically generate the test cases.

That should cover it! The change itself is solid - just want to make sure the integration points work as expected.
Thanks for the contribution! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dvjn @raphael
0