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 Reactive: Sub resource Path Params more than 2 levels back are not passed correctly #15114

Closed
bcluap opened this issue Feb 16, 2021 · 13 comments · Fixed by #15158
Closed
Assignees
Labels
area/rest kind/bug Something isn't working
Milestone

Comments

@bcluap
Copy link

bcluap commented Feb 16, 2021

Describe the bug
Sub resource path params are not bound correctly in a scenario with more than two sub resource levels.
E.g. if path /trees/{treeId}/branches/{branchId}/leaves/{leafId} is handled by a sub resource "LeavesResource" which uses @PathParam("treeId") and @PathParam("branchId") and @PathParam("leafId") then treeId is not injected while the other two are.

Expected behavior
Multiple levels should be supported. The correct behaviour is found in standard resteasy.

Actual behavior
The value is not injected causing difficult to track bugs.

To Reproduce
git clone https://github.com/bcluap/quarkus-examples.git
cd quarkus-examples/resteasy-reactive
mvn clean test

Configuration
Nothing abnormal

Screenshots
Run the reproducer to see failures. Change the pom to non-reactive mode and all tests pass

Environment (please complete the following information):
This is not environment-specific
999-SNAPSHOT and before

Additional context
NA

@bcluap bcluap added the kind/bug Something isn't working label Feb 16, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 16, 2021

/cc @FroMage, @geoand, @stuartwdouglas

@geoand
Copy link
Contributor

geoand commented Feb 17, 2021

@stuartwdouglas mind taking a look at this one since you did most of the subresource stuff?

@stuartwdouglas stuartwdouglas self-assigned this Feb 18, 2021
@stuartwdouglas
Copy link
Member

I will have a look, however this particular bit was done by someone else :-)

c3ebc9a

@geoand
Copy link
Contributor

geoand commented Feb 18, 2021

I must have erased it from my mind completely 🙈

@bcluap
Copy link
Author

bcluap commented Feb 18, 2021

Thanks @stuartwdouglas @geoand . I've confirmed it's working even with 4 levels of resources.

@FroMage
Copy link
Member

FroMage commented Feb 19, 2021

I must have erased it from my mind completely see_no_evil

Hahaha, I'm pretty sure we talked about this commit at the time and I noted it was only for a single level ;) 👻 🐛

@FroMage
Copy link
Member

FroMage commented Feb 19, 2021

#13853 (comment)

@FroMage
Copy link
Member

FroMage commented Feb 19, 2021

🤣

@FroMage
Copy link
Member

FroMage commented Feb 19, 2021

But OK, I was wrong, I said we had to use a stack and @stuartwdouglas used a chained list ;)

@geoand
Copy link
Contributor

geoand commented Feb 19, 2021

Somehow I had completely forgotten about it...

It's all a blur!

@bcluap
Copy link
Author

bcluap commented Feb 19, 2021 via email

@FroMage
Copy link
Member

FroMage commented Feb 19, 2021

Somehow I had completely forgotten about it...

TBH I don't have a clue how I could have remembered that and not what I did yesterday. I guess it's like those SMB secret blocks I still remember from childhood, there's special brain storage for random stuff.

@geoand
Copy link
Contributor

geoand commented Feb 19, 2021

I want a brain upgrade :P

@gsmet gsmet modified the milestones: 1.13 - master, 1.12.1.Final Feb 19, 2021
gsmet pushed a commit to gsmet/quarkus that referenced this issue Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants