Cache: only dependent CacheKeyGenerator beans are destroyed after use

- docs - clarify that non-CDI key generators are instantiated per key
computation
- fixes quarkusio#38394

(cherry picked from commit b10b991)

docs/src/main/asciidoc/cache.adoc

@@ -390,9 +390,13 @@ public class CachedService {
 You may want to include more than the arguments of a method into a cache key.
 This can be done by implementing the `io.quarkus.cache.CacheKeyGenerator` interface and declaring that implementation in the `keyGenerator` field of a `@CacheResult` or `@CacheInvalidate` annotation.
 
-If a CDI scope is declared on a key generator class and if that class has a default qualifier (no qualifier annotation or `@jakarta.enterprise.inject.Default`), then the key generator will be injected as a CDI bean during the cache key computation.
-Otherwise, the key generator will be instantiated using its default constructor.
-All CDI scopes supported by Quarkus can be used on a key generator.
+The class must either represent a CDI bean or declare a public no-args constructor.
+If it represents a CDI bean, then the key generator will be injected during the cache key computation.
+Otherwise, a new instance of the key generator will be created using its default constructor for each cache key computation.
+
+In case of CDI, there must be exactly one bean that has the class in its set of bean types, otherwise the build fails. 
+The context associated with the scope of the bean must be active when the `CacheKeyGenerator#generate()` method is invoked.
+If the scope is `@Dependent` then the bean instance is destroyed when the `CacheKeyGenerator#generate()` method completes.
 
 The following key generator will be injected as a CDI bean:
 

extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/CacheProcessor.java

@@ -180,7 +180,7 @@ void validateCacheAnnotationsAndProduceCacheNames(CombinedIndexBuildItem combine
         cacheNames.produce(new CacheNamesBuildItem(names));
 
         if (!keyGenerators.isEmpty()) {
-            throwables.addAll(validateKeyGeneratorsDefaultConstructor(combinedIndex, beanDiscoveryFinished, keyGenerators));
+            throwables.addAll(validateKeyGenerators(combinedIndex, beanDiscoveryFinished, keyGenerators));
         }
 
         validationErrors.produce(new ValidationErrorBuildItem(throwables.toArray(new Throwable[0])));
@@ -226,21 +226,25 @@ private Optional<DotName> findCacheKeyGenerator(AnnotationInstance binding, Anno
         return Optional.empty();
     }
 
-    // Key generators must have a default constructor if they are not managed by Arc.
-    private List<Throwable> validateKeyGeneratorsDefaultConstructor(CombinedIndexBuildItem combinedIndex,
+    private List<Throwable> validateKeyGenerators(CombinedIndexBuildItem combinedIndex,
             BeanDiscoveryFinishedBuildItem beanDiscoveryFinished, Set<DotName> keyGenerators) {
-        List<DotName> managedBeans = beanDiscoveryFinished.getBeans()
-                .stream()
-                .filter(BeanInfo::isClassBean)
-                .map(BeanInfo::getBeanClass)
-                .collect(toList());
         List<Throwable> throwables = new ArrayList<>();
         for (DotName keyGenClassName : keyGenerators) {
-            if (!managedBeans.contains(keyGenClassName)) {
+            List<BeanInfo> beans = beanDiscoveryFinished.beanStream().withBeanType(keyGenClassName).collect();
+            if (beans.isEmpty()) {
+                // Key generators must have a default constructor if they are not CDI beans
                 ClassInfo keyGenClassInfo = combinedIndex.getIndex().getClassByName(keyGenClassName);
-                if (!keyGenClassInfo.hasNoArgsConstructor()) {
+                if (keyGenClassInfo == null) {
+                    throwables.add(new IllegalStateException(
+                            "Unable to find the key generator class in the index:" + keyGenClassName));
+                } else if (!keyGenClassInfo.hasNoArgsConstructor()) {
                     throwables.add(new KeyGeneratorConstructorException(keyGenClassInfo));
                 }
+            } else if (beans.size() > 1) {
+                String message = String.format(
+                        "There must be exactly one bean that matches the key generator class: \"%s\"\n\t- beans: %s",
+                        keyGenClassName, beans);
+                throwables.add(new IllegalStateException(message));
             }
         }
         return throwables;

extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/CacheKeyGeneratorTest.java

@@ -1,12 +1,14 @@
 package io.quarkus.cache.test.runtime;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertSame;
 
 import java.lang.reflect.Method;
 import java.math.BigInteger;
 import java.security.SecureRandom;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import jakarta.annotation.PostConstruct;
 import jakarta.annotation.PreDestroy;
@@ -54,6 +56,7 @@ public void testAllCacheKeyGeneratorKinds() {
         BigInteger value4 = cachedService.cachedMethod2();
         assertSame(value3, value4);
 
+        // Invalidate "cauliflower" and "CompositeCacheKey("asparagus", OBJECT)" entries
         cachedService.invalidate1(CAULIFLOWER, OBJECT);
 
         String value5 = cachedService.cachedMethod1(OBJECT, /* Not used */ null);
@@ -73,6 +76,10 @@ public void testAllCacheKeyGeneratorKinds() {
 
         Object value9 = cachedService.cachedMethod3(/* Not used */ null, /* Not used */ null);
         assertNotSame(value8, value9);
+
+        assertFalse(SingletonKeyGen.DESTROYED.get());
+        assertFalse(ApplicationScopedKeyGen.DESTROYED.get());
+        assertFalse(RequestScopedKeyGen.DESTROYED.get());
     }
 
     @ApplicationScoped
@@ -114,34 +121,55 @@ public void invalidate2(/* Key element */ String param0, /* Not used */ Long par
     @Singleton
     public static class SingletonKeyGen implements CacheKeyGenerator {
 
+        static final AtomicBoolean DESTROYED = new AtomicBoolean();
+
         @ConfigProperty(name = "cache-key-element")
         String cacheKeyElement;
 
         @Override
         public Object generate(Method method, Object... methodParams) {
             return new CompositeCacheKey(cacheKeyElement, methodParams[0]);
         }
+
+        @PreDestroy
+        void preDestroy() {
+            DESTROYED.set(true);
+        }
     }
 
     @ApplicationScoped
     public static class ApplicationScopedKeyGen implements CacheKeyGenerator {
 
+        static final AtomicBoolean DESTROYED = new AtomicBoolean();
+
         @Inject
         CachedService cachedService;
 
         @Override
         public Object generate(Method method, Object... methodParams) {
             return new CompositeCacheKey(method.getName(), cachedService.getCauliflower());
         }
+
+        @PreDestroy
+        void preDestroy() {
+            DESTROYED.set(true);
+        }
     }
 
     @RequestScoped
     public static class RequestScopedKeyGen implements CacheKeyGenerator {
 
+        static final AtomicBoolean DESTROYED = new AtomicBoolean();
+
         @Override
         public Object generate(Method method, Object... methodParams) {
             return new CompositeCacheKey(ASPARAGUS, methodParams[1]);
         }
+
+        @PreDestroy
+        void preDestroy() {
+            DESTROYED.set(true);
+        }
     }
 
     @Dependent

extensions/cache/runtime/src/main/java/io/quarkus/cache/CacheKeyGenerator.java

@@ -2,10 +2,16 @@
 
 import java.lang.reflect.Method;
 
+import jakarta.enterprise.context.Dependent;
+
 /**
  * Implement this interface to generate a cache key based on the cached method, its parameters or any data available from within
- * the generator. The implementation is injected as a CDI bean if possible or is instantiated using the default constructor
- * otherwise.
+ * the generator.
+ * <p>
+ * The class must either represent a CDI bean or declare a public no-args constructor. In case of CDI, there must be exactly one
+ * bean that has the class in its set of bean types, otherwise the build fails. The context associated with the scope
+ * of the bean must be active when the {@link #generate(Method, Object...)} method is invoked. If the scope is {@link Dependent}
+ * then the bean instance is destroyed when the {@link #generate(Method, Object...)} method completes.
  */
 public interface CacheKeyGenerator {
 

extensions/cache/runtime/src/main/java/io/quarkus/cache/runtime/CacheInterceptor.java

@@ -11,7 +11,11 @@
 import java.util.concurrent.CompletionStage;
 import java.util.function.Supplier;
 
+import jakarta.enterprise.context.Dependent;
+import jakarta.enterprise.inject.Any;
 import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Instance.Handle;
+import jakarta.enterprise.inject.spi.Bean;
 import jakarta.inject.Inject;
 import jakarta.interceptor.Interceptor.Priority;
 import jakarta.interceptor.InvocationContext;
@@ -40,6 +44,7 @@ public abstract class CacheInterceptor {
     CacheManager cacheManager;
 
     @Inject
+    @Any // this means that qualifiers defined on a CacheKeyGenerator are effectively ignored
     Instance<CacheKeyGenerator> keyGenerator;
 
     /*
@@ -119,11 +124,6 @@ private <T> CacheInterceptionContext<T> getNonArcCacheInterceptionContext(
         return new CacheInterceptionContext<>(interceptorBindings, cacheKeyParameterPositions);
     }
 
-    @SuppressWarnings("unchecked")
-    private <T extends Annotation> T cast(Annotation annotation, Class<T> interceptorBindingClass) {
-        return (T) annotation;
-    }
-
     protected Object getCacheKey(Cache cache, Class<? extends CacheKeyGenerator> keyGeneratorClass,
             List<Short> cacheKeyParameterPositions, Method method, Object[] methodParameterValues) {
         if (keyGeneratorClass != UndefinedCacheKeyGenerator.class) {
@@ -158,11 +158,15 @@ private <T extends CacheKeyGenerator> Object generateKey(Class<T> keyGeneratorCl
         Instance<T> keyGenInstance = keyGenerator.select(keyGeneratorClass);
         if (keyGenInstance.isResolvable()) {
             LOGGER.tracef("Using cache key generator bean from Arc [class=%s]", keyGeneratorClass.getName());
-            T keyGen = keyGenInstance.get();
+            Handle<T> keyGen = keyGenInstance.getHandle();
             try {
-                return keyGen.generate(method, methodParameterValues);
+                return keyGen.get().generate(method, methodParameterValues);
             } finally {
-                keyGenerator.destroy(keyGen);
+                Bean<T> bean = keyGen.getBean();
+                if (bean != null && Dependent.class.equals(bean.getScope())) {
+                    // Destroy @Dependent beans afterwards
+                    keyGen.destroy();
+                }
             }
         } else {
             try {