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

Honor Cache and NoCache annotations on implementations of a JAX-RS interface #39052

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

danielbobbert
Copy link
Contributor

@danielbobbert danielbobbert commented Feb 28, 2024

@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/cache labels Feb 28, 2024
@danielbobbert danielbobbert changed the title Honor Cache and NoCache annotations on CDI bean implementations of a JaxRS interface Honor Cache and NoCache annotations on implementations of a JaxRS interface Feb 28, 2024
@geoand
Copy link
Contributor

geoand commented Feb 28, 2024

Nice!

Can you please rebase the commit on top of the latest main?
Alternatively, I can do it for you

@danielbobbert
Copy link
Contributor Author

Done

@geoand
Copy link
Contributor

geoand commented Feb 28, 2024

Thanks! I'll review tomorrow

This comment has been minimized.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Looks good!

I just added a small comment.
Let's also squash the two commits into a single one

@danielbobbert danielbobbert force-pushed the main branch 3 times, most recently from 72bedbe to ecc0eff Compare February 29, 2024 08:48
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

This comment has been minimized.

@danielbobbert
Copy link
Contributor Author

Build was complaining about unordered imports. I have fixed these just now.

This comment has been minimized.

@danielbobbert
Copy link
Contributor Author

Fixed formatting in tests

This comment has been minimized.

@danielbobbert
Copy link
Contributor Author

When reordering imports, I skipped one class, introducing a compile error. Sorry! This is fixed now.

@geoand
Copy link
Contributor

geoand commented Feb 29, 2024

No problem!

This comment has been minimized.

@geoand geoand changed the title Honor Cache and NoCache annotations on implementations of a JaxRS interface Honor Cache and NoCache annotations on implementations of a JAX-RS interface Feb 29, 2024

This comment has been minimized.

@danielbobbert
Copy link
Contributor Author

I realized that some tests were failing, because I didn't handle the case where a Jax-RS interface contains a default method that is not implemented/overridden in the bean implementation. In that case, "actualMethod" could be null, leading to a NullPointerException. I have fixed the code, added another explicit test case for this scenario and rebased the squashed commit on main. Hopefully this will now pass all tests.

@geoand
Copy link
Contributor

geoand commented Mar 1, 2024

Great job!

This comment has been minimized.

@danielbobbert
Copy link
Contributor Author

These sanity checks are driving me crazy. Had to remove an unused import in a test class...
How can I run those checks locally before pushing?

@geoand
Copy link
Contributor

geoand commented Mar 1, 2024

mvn process-sources -f independent-projects/resteasy-reactive should be enough to get the formatter to do its work on your changes

@danielbobbert
Copy link
Contributor Author

No errors/warnings locally, so we should be fine now.

Copy link

quarkus-bot bot commented Mar 1, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 3c866fa.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@geoand geoand merged commit 24cd4b9 into quarkusio:main Mar 1, 2024
42 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.9 - main milestone Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/cache kind/bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@Cache annotation not honored on reactive REST resource beans with separate interface
2 participants