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

RESTEasy standalone - handle @ApplicationPath value correctly #4814

Merged
merged 1 commit into from Oct 25, 2019

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Oct 23, 2019

IMPL NOTE: We register the delegating handler in the default route (matches HttpBuildTimeConfig.path) for a JAX-RS root resource path of a default value /. However, for any other value we do register the handler in a special route that matches the root resource path. In fact, we register two routes that share the same handler. For example, for the JAX-RS root resource path /foo we register two routes, first matches /foo (exact match) and the second one matches /foo/*. And the reason is that otherwise the resources would be served from both the root and the app path - see the issue description for an example.

@mkouba mkouba requested review from patriot1burke, cescoffier and stuartwdouglas and removed request for patriot1burke and cescoffier October 23, 2019 17:48
@mkouba mkouba added this to the 0.27.0 milestone Oct 23, 2019
Copy link
Contributor

@patriot1burke patriot1burke left a comment

Choose a reason for hiding this comment

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

Should write a regression test for this. I don't see one. I imagine we might want to rework route processing int he future and there's no test for this case.

@stuartwdouglas
Copy link
Member

LGTM except the lack of tests

@mkouba
Copy link
Contributor Author

mkouba commented Oct 24, 2019

Good point with the tests. I'll add some.

- application path value does not have to start with a slash
- resource should not be served from both the root and the app path
- resolves quarkusio#4787
@mkouba
Copy link
Contributor Author

mkouba commented Oct 24, 2019

Tests added...

@mkouba
Copy link
Contributor Author

mkouba commented Oct 24, 2019

@patriot1burke Pls could you re-review this PR again?

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

We have a proper test now, let's get it in the tree early.

@gsmet gsmet merged commit c170cd8 into quarkusio:master Oct 25, 2019
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.

RESTEasy standalone - @ApplicationPath not honored if it does not start with a slash
4 participants