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.

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

@Path lookup must also consider interfaces #30982

Closed
wants to merge 1 commit into from
Closed

@Path lookup must also consider interfaces #30982

wants to merge 1 commit into from

Conversation

lostiniceland
Copy link
Contributor

@lostiniceland lostiniceland commented Feb 8, 2023 •
edited by geoand
Loading

Fixed @path annotation-lookup

Include interfaces in lookup to resolve @path annotation in order to produce correct path-template used for metrics.

Fixes #30843

@lostiniceland lostiniceland changed the title Fixed @Path annotation-lookup fixes #30843 Fixed @Path annotation-lookup Feb 8, 2023
@lostiniceland lostiniceland changed the title fixes #30843 Fixed @Path annotation-lookup fixes #30843 Feb 8, 2023
@geoand
Copy link
Contributor

geoand commented Feb 8, 2023

Please update the title with a small description of what the change actually does.

Thanks

@lostiniceland lostiniceland changed the title fixes #30843 fixes #30843 @Path lookup must also consider interfaces Feb 8, 2023
@lostiniceland lostiniceland changed the title fixes #30843 @Path lookup must also consider interfaces @Path lookup must also consider interfaces Feb 8, 2023
lostiniceland
lostiniceland commented Feb 8, 2023
View reviewed changes
...asy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java Outdated
methodInfo.parameterTypes().toArray(new Type[] {})))
.filter(Objects::nonNull)
.findFirst()
.map(resolvedMethodInfo -> resolvedMethodInfo.annotation(REST_PATH));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might still yield a null value. Will update later.

@quarkus-bot

This comment has been minimized.

Sign in to view
@lostiniceland
Copy link
Contributor Author

The failing Quickstart compilation seems unrelated to this PR.

@lostiniceland
Copy link
Contributor Author

lostiniceland commented Feb 10, 2023 •
edited
Loading

@geoand I've rebased on the latest 2.16 which had a fix for the quickstart compilation issue.

@lostiniceland
fixes quarkusio#30843
a80be2f 
Fixed @path annotation-lookup

Include interfaces in lookup to resolve @path annotation in order to produce correct path-template used for metrics.

Replaced deprecated method calls with recommended one
@geoand
Copy link
Contributor

geoand commented Feb 10, 2023

@lostiniceland please use main as the base branch, not 2.16

@lostiniceland
Copy link
Contributor Author

Isn't main already going for 3.x? We need a fix for 2.16 because before go-live we wont have time to upgrade to 3.

@geoand
Copy link
Contributor

geoand commented Feb 10, 2023

Yes, main is for Quarkus 3, but all changes go into that branch and we consider backporting bug fixes to 2.16, we don't do changes to the 2.16 branch.

@lostiniceland
Copy link
Contributor Author

Then I close this PR and recreate it for main. Rebasing yields some results which I am no confident with (probably due to the backports)

@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Feb 10, 2023
@geoand
Copy link
Contributor

geoand commented Feb 10, 2023

No problem

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
triage/invalid This doesn't seem right
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lostiniceland @geoand