Skip to content

Commit

Permalink
Cache: fix CacheInterceptor in case of non-ArC interceptor bindings
Browse files Browse the repository at this point in the history
RESTEasy Classic's MP RestClient implementation produces annotations
at runtime, so they are not created by ArC and therefore don't extend
`AbstractAnnotationLiteral`. At the same time, that implementation
produces an `ArcInvocationContext` and puts interceptor bindings
into its context map under the ArC key.

Some places may expect that an `ArcInvocationContext` would always
contain ArC-created `AbstractAnnotationLiteral` instances, but alas,
per the description above, that is not the case.

There are multiple options for fixing that collision. My preferred one
would be to get rid of `AbstractAnnotationLiteral` and treat all
annotations uniformly. That unfortunately has negative performance
implications on the `CacheInterceptor`, so is not an option yet [1].

This commit chooses another path: it modifies the only place in Quarkus
that actually depends on `AbstractAnnotationLiteral` to check whether
the `Set<AbstractAnnotationLiteral>` actually contains instances of
`AbstractAnnotationLiteral`. I hope that before more places in Quarkus
start depending on `AbstractAnnotationLiteral`, we can get rid of it.

This commit only checks the first annotation in the set, because
if the bindings come from RESTEasy Classic, then none of them are
instances of `AbstractAnnotationLiteral`, and if they come from ArC,
then all of them are instances of `AbstractAnnotationLiteral`.

[1] The performance issue (JDK-8180450) is fixed in JDK 23 and has
    not been backported to any LTS release as of this writing.
  • Loading branch information
Ladicek committed Oct 11, 2024
1 parent aea9c27 commit 6a4fbd6
Showing 1 changed file with 4 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ public CacheInterceptionContext<T> get() {
private <T> Optional<CacheInterceptionContext<T>> getArcCacheInterceptionContext(
InvocationContext invocationContext, Class<T> interceptorBindingClass) {
Set<AbstractAnnotationLiteral> bindings = InterceptorBindings.getInterceptorBindingLiterals(invocationContext);
if (bindings == null) {
LOGGER.trace("Interceptor bindings not found in ArC");
// This should only happen when the interception is not managed by Arc.
if (bindings == null || bindings.isEmpty() || !(bindings.iterator().next() instanceof AbstractAnnotationLiteral)) {
// this should only happen when the interception is not managed by ArC
// a non-`AbstractAnnotationLiteral` can come from RESTEasy Classic's `QuarkusInvocationContextImpl`
LOGGER.trace("Interceptor bindings not found in ArC or not created by ArC");
return Optional.empty();
}
List<T> interceptorBindings = new ArrayList<>();
Expand Down

0 comments on commit 6a4fbd6

Please sign in to comment.