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

@Cache annotation not honored on reactive REST resource beans with separate interface #39002

Closed
danielbobbert opened this issue Feb 26, 2024 · 9 comments · Fixed by #39052
Closed
Labels
area/rest kind/bug Something isn't working triage/needs-feedback We are waiting for feedback.
Milestone

Comments

@danielbobbert
Copy link
Contributor

Describe the bug

The org.jboss.resteasy.reactive.Cache annotation is only honored, if it is put on a JaxRS bean or a JaxRS interface itself.
If put on a bean that implements a JaxRS interface, the annotation is ignored.

Reasoning: We have lots of services where the JaxRS interfaces are generated from OpenAPI specification. We provide beans that implement these interfaces and Quarkus. We would like to add @Cache annotations to some of the implemented methods to control generated "Cache-Control" headers. But the annotation is ignored.
We cannot put the annotation of the JaxRS interface itself, because that is generated from an OpenAPI spec.

Expected behavior

When putting a @Cache annotation on a bean method that implements a JaxRS interface, the appropriate "Cache-Control" header should be generated.

Actual behavior

@Cache annotation is only honored when put on the JaxRS interface itself.

How to Reproduce?

See attached maven project with failing unit test.

Output of uname -a or ver

Mac OS 14 (on MacBook Pro M1)

Output of java -version

openjdk version "17.0.2"

Quarkus version or git rev

3.7.4

Build tool (ie. output of mvnw --version or gradlew --version)

mvn 3.9.6

Additional information

resteasy-reactive-cache.zip

@danielbobbert danielbobbert added the kind/bug Something isn't working label Feb 26, 2024
@quarkus-bot quarkus-bot bot added area/cache env/m1 Impacts Apple M1 machines labels Feb 26, 2024
Copy link

quarkus-bot bot commented Feb 26, 2024

/cc @gastaldi (m1), @gwenneg (cache)

@gastaldi gastaldi added area/rest and removed area/cache env/m1 Impacts Apple M1 machines labels Feb 26, 2024
@gastaldi
Copy link
Contributor

/cc @geoand

@danielbobbert
Copy link
Contributor Author

The first sample project was actually missing the @Cache annotation on the implementation. I added it (but of course, the test still fails)
resteasy-reactive-cache.zip

@danielbobbert
Copy link
Contributor Author

danielbobbert commented Feb 26, 2024

Looks to me like that could be solved by taking the "actualEndpointInfo" into account in "CacheControlScanner", like it is done in "EndpointIndexer", line 719ff, to allow implementing methods to override the "blocking" and "runOnVirtualThread" info of the endpoint method.

@geoand
Copy link
Contributor

geoand commented Feb 26, 2024

Makes sense to me.

Would you like to implement this fix as you have already analyzed the code?

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Feb 26, 2024
@danielbobbert
Copy link
Contributor Author

Yes, I can try to provide a PR. Just one question:
In CacheControlScanner, the annotations are sometimes (it seems on the first attempt) loaded via

  • annotationStore.getAnnotation(), or
  • method.annotation() or
  • class.declaredAnnotation()

Can you very briefly point me to an explanation of the difference? In my tests, they seem to return the same thing, so I am unsure which one I should use and why.

@danielbobbert
Copy link
Contributor Author

danielbobbert commented Feb 26, 2024

And yet another question regarding the current implementation of CacheControlScanner: It first checks for @NoCache and then for @Cache, meaning that if the whole class is annotated with @NoCache but individual methods are annotated with @Cache, the @NoCache annotation on the class "wins" because it is checked first.
I would rather expect the scanner to prioritize method annotations (@Cache and @NoCache). And only if none of these is found on the method, the scanner should "fall back" to a "default" annotated on the class. That's also how many other JaxRS annotations (like @Consumes, @Produces, etc.) are handled.
Or am I missing something here?

@geoand
Copy link
Contributor

geoand commented Feb 27, 2024

Can you very briefly point me to an explanation of the difference? In my tests, they seem to return the same thing, so I am unsure which one I should use and why.

AnnotationStore gives you a way to find annotations of all classes.
method.annotation() gives you the annotations of a method.
class.declaredAnnotation() gives you the annotations declared on the class only

@geoand
Copy link
Contributor

geoand commented Feb 27, 2024

Or am I missing something here?

You are right. If you want you can fix this behavior, but please do it in a separate PR

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 triage/needs-feedback We are waiting for feedback.
Projects
None yet
3 participants