Merge pull request #18970 from mkouba/arc-remove-unused-interceptors

ArC - remove unused interceptors

docs/src/main/asciidoc/cdi-reference.adoc

@@ -396,23 +396,50 @@ NOTE: We don't generate a no-args constructor automatically if a bean class exte
 [[remove_unused_beans]]
 === Removing Unused Beans
 
-The container attempts to remove all unused beans during build by default.
-This optimization can be disabled by setting `quarkus.arc.remove-unused-beans` to `none` or `false`.
+The container attempts to remove all unused beans, interceptors and decorators during build by default.
+This optimization helps to minimize the amount of generated classes, thus conserving memory.
+However, Quarkus can't detect the programmatic lookup performed via the `CDI.current()` static method.
+Therefore, it is possible that a removal results in a false positive error, i.e. a bean is removed although it's actually used.
+In such cases, you'll notice a big warning in the log.
+Users and extension authors have several options <<eliminate_false_positives,how to eliminate false positives>>.
 
-An unused bean:
+The optimization can be disabled by setting `quarkus.arc.remove-unused-beans` to `none` or `false`.
+Quarkus also provides a middle ground where application beans are never removed whether or not they are unused, while the optimization proceeds normally for non application classes. 
+To use this mode, set `quarkus.arc.remove-unused-beans` to `fwk` or `framework`.
 
-* is not a built-in bean or an interceptor,
-* is not eligible for injection to any injection point,
-* is not excluded by any extension,
-* does not have a name,
-* does not declare an observer,
-* does not declare any producer which is eligible for injection to any injection point,
-* is not directly eligible for injection into any `javax.enterprise.inject.Instance` or `javax.inject.Provider` injection point
+==== What's Removed?
 
-This optimization applies to all forms of bean declarations: bean class, producer method, producer field.
+Quarkus first identifies so-called _unremovable_ beans that form the roots in the dependency tree.
+A good example is a JAX-RS resource class or a bean which declares a `@Scheduled` method.
 
-Users can instruct the container to not remove any of their specific beans (even if they satisfy all the rules specified above) by annotating them with `io.quarkus.arc.Unremovable`.
-This annotation can be placed on the types, producer methods, and producer fields.
+An _unremovable_ bean:
+
+* is excluded from removal by an extension, or
+* has a name designated via `@Named`, or
+* declares an observer method.
+
+An _unused_ bean:
+
+* is not _unremovable_, and
+* is not eligible for injection to any injection point in the dependency tree, and
+* does not declare any producer which is eligible for injection to any injection point in the dependency tree, and
+* is not eligible for injection into any `javax.enterprise.inject.Instance` or `javax.inject.Provider` injection point.
+
+Unused interceptors and decorators are not associated with any bean.
+
+[TIP]
+====
+When using the dev mode (running `./mvnw clean compile quarkus:dev`), you can see more information about which beans are being removed:
+
+1. In the console - just enable the DEBUG level in your `application.properties`, i.e. `quarkus.log.category."io.quarkus.arc.processor".level=DEBUG`
+2. In the relevant Dev UI page
+====
+
+[[eliminate_false_positives]]
+==== How To Eliminate False Positives
+
+Users can instruct the container to not remove any of their specific beans (even if they satisfy all the rules specified above) by annotating them with `@io.quarkus.arc.Unremovable`.
+This annotation can be declared on a class, a producer method or field.
 
 Since this is not always possible, there is an option to achieve the same via `application.properties`.
 The `quarkus.arc.unremovable-types` property accepts a list of string values that are used to match beans based on their name or package.
@@ -432,17 +459,7 @@ The `quarkus.arc.unremovable-types` property accepts a list of string values tha
 quarkus.arc.unremovable-types=org.acme.Foo,org.acme.*,Bar
 ----
 
-Furthermore, extensions can eliminate possible false positives by producing `UnremovableBeanBuildItem`.
-
-Finally, Quarkus provides a middle ground for the bean removal optimization where application beans are never removed whether or not they are unused,
-while the optimization proceeds normally for non application classes. To use this mode, set `quarkus.arc.remove-unused-beans` to `fwk` or `framework`.
-
-When using the dev mode (running `./mvnw clean compile quarkus:dev`), you can see more information about which beans are being removed
-by enabling additional logging via the following line in your `application.properties`.
-
-----
-quarkus.log.category."io.quarkus.arc.processor".level=DEBUG
-----
+Furthermore, extensions can eliminate false positives by producing an `UnremovableBeanBuildItem`.
 
 [[default_beans]]
 === Default Beans

extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java

@@ -18,6 +18,7 @@
 import java.util.Set;
 import java.util.function.BiFunction;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 import javax.enterprise.context.spi.Contextual;
 import javax.enterprise.inject.spi.InterceptionType;
@@ -41,6 +42,7 @@
 import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem;
 import io.quarkus.arc.deployment.InterceptorResolverBuildItem;
 import io.quarkus.arc.deployment.TransformedAnnotationsBuildItem;
+import io.quarkus.arc.deployment.UnremovableBeanBuildItem;
 import io.quarkus.arc.impl.CreationalContextImpl;
 import io.quarkus.arc.impl.InterceptedMethodMetadata;
 import io.quarkus.arc.impl.InterceptedStaticMethods;
@@ -85,7 +87,8 @@ public class InterceptedStaticMethodsProcessor {
     @BuildStep
     void collectInterceptedStaticMethods(BeanArchiveIndexBuildItem beanArchiveIndex,
             BuildProducer<InterceptedStaticMethodBuildItem> interceptedStaticMethods,
-            InterceptorResolverBuildItem interceptorResolver, TransformedAnnotationsBuildItem transformedAnnotations) {
+            InterceptorResolverBuildItem interceptorResolver, TransformedAnnotationsBuildItem transformedAnnotations,
+            BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
 
         // In this step we collect all intercepted static methods, ie. static methods annotated with interceptor bindings  
         Set<DotName> interceptorBindings = interceptorResolver.getInterceptorBindings();
@@ -127,6 +130,8 @@ void collectInterceptedStaticMethods(BeanArchiveIndexBuildItem beanArchiveIndex,
                     if (!interceptors.isEmpty()) {
                         LOGGER.debugf("Intercepted static method found on %s: %s", method.declaringClass().name(),
                                 method);
+                        unremovableBeans.produce(UnremovableBeanBuildItem.beanClassNames(interceptors.stream()
+                                .map(InterceptorInfo::getBeanClass).map(Object::toString).collect(Collectors.toSet())));
                         interceptedStaticMethods.produce(
                                 new InterceptedStaticMethodBuildItem(method, methodLevelBindings, interceptors));
                     }

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

@@ -1,6 +1,8 @@
 package io.quarkus.cache.deployment;
 
 import static io.quarkus.cache.deployment.CacheDeploymentConstants.CACHE_INVALIDATE;
+import static io.quarkus.cache.deployment.CacheDeploymentConstants.CACHE_INVALIDATE_ALL;
+import static io.quarkus.cache.deployment.CacheDeploymentConstants.CACHE_INVALIDATE_ALL_LIST;
 import static io.quarkus.cache.deployment.CacheDeploymentConstants.CACHE_INVALIDATE_LIST;
 import static io.quarkus.cache.deployment.CacheDeploymentConstants.CACHE_KEY;
 import static io.quarkus.cache.deployment.CacheDeploymentConstants.CACHE_NAME;
@@ -33,13 +35,17 @@
 import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
 import io.quarkus.arc.deployment.AutoInjectAnnotationBuildItem;
 import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
+import io.quarkus.arc.deployment.UnremovableBeanBuildItem;
 import io.quarkus.arc.deployment.ValidationPhaseBuildItem.ValidationErrorBuildItem;
 import io.quarkus.cache.CacheManager;
 import io.quarkus.cache.deployment.exception.ClassTargetException;
 import io.quarkus.cache.deployment.exception.PrivateMethodTargetException;
 import io.quarkus.cache.deployment.exception.UnknownCacheNameException;
 import io.quarkus.cache.deployment.exception.UnsupportedRepeatedAnnotationException;
 import io.quarkus.cache.deployment.exception.VoidReturnTypeTargetException;
+import io.quarkus.cache.runtime.CacheInvalidateAllInterceptor;
+import io.quarkus.cache.runtime.CacheInvalidateInterceptor;
+import io.quarkus.cache.runtime.CacheResultInterceptor;
 import io.quarkus.cache.runtime.caffeine.CaffeineCacheBuildRecorder;
 import io.quarkus.cache.runtime.caffeine.CaffeineCacheInfo;
 import io.quarkus.cache.runtime.noop.NoOpCacheBuildRecorder;
@@ -209,14 +215,32 @@ SyntheticBeanBuildItem configureCacheManagerSyntheticBean(CacheNamesBuildItem ca
     }
 
     @BuildStep
-    List<BytecodeTransformerBuildItem> enhanceRestClientMethods(CombinedIndexBuildItem combinedIndex) {
+    List<BytecodeTransformerBuildItem> enhanceRestClientMethods(CombinedIndexBuildItem combinedIndex,
+            BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
         List<BytecodeTransformerBuildItem> bytecodeTransformers = new ArrayList<>();
+        boolean cacheInvalidate = false;
+        boolean cacheResult = false;
+        boolean cacheInvalidateAll = false;
+
         for (AnnotationInstance registerRestClientAnnotation : combinedIndex.getIndex().getAnnotations(REGISTER_REST_CLIENT)) {
             if (registerRestClientAnnotation.target().kind() == Kind.CLASS) {
                 ClassInfo classInfo = registerRestClientAnnotation.target().asClass();
                 for (MethodInfo methodInfo : classInfo.methods()) {
-                    if (methodInfo.hasAnnotation(CACHE_INVALIDATE) || methodInfo.hasAnnotation(CACHE_INVALIDATE_LIST)
-                            || methodInfo.hasAnnotation(CACHE_RESULT)) {
+                    boolean transform = false;
+
+                    if (methodInfo.hasAnnotation(CACHE_INVALIDATE) || methodInfo.hasAnnotation(CACHE_INVALIDATE_LIST)) {
+                        transform = true;
+                        cacheInvalidate = true;
+                    }
+                    if (methodInfo.hasAnnotation(CACHE_RESULT)) {
+                        transform = true;
+                        cacheResult = true;
+                    }
+                    if (methodInfo.hasAnnotation(CACHE_INVALIDATE_ALL) || methodInfo.hasAnnotation(CACHE_INVALIDATE_ALL_LIST)) {
+                        cacheInvalidateAll = true;
+                    }
+
+                    if (transform) {
                         short[] cacheKeyParameterPositions = getCacheKeyParameterPositions(methodInfo);
                         /*
                          * The bytecode transformation is always performed even if `cacheKeyParameterPositions` is empty because
@@ -228,6 +252,18 @@ List<BytecodeTransformerBuildItem> enhanceRestClientMethods(CombinedIndexBuildIt
                 }
             }
         }
+
+        // Interceptors need to be registered as unremovable due to the rest-client integration - interceptors 
+        // are currently resolved dynamically at runtime because per the spec interceptor bindings cannot be declared on interfaces
+        if (cacheResult) {
+            unremovableBeans.produce(UnremovableBeanBuildItem.beanClassNames(CacheResultInterceptor.class.getName()));
+        }
+        if (cacheInvalidate) {
+            unremovableBeans.produce(UnremovableBeanBuildItem.beanClassNames(CacheInvalidateInterceptor.class.getName()));
+        }
+        if (cacheInvalidateAll) {
+            unremovableBeans.produce(UnremovableBeanBuildItem.beanClassNames(CacheInvalidateAllInterceptor.class.getName()));
+        }
         return bytecodeTransformers;
     }
 

extensions/mailer/deployment/src/main/java/io/quarkus/mailer/deployment/MailerProcessor.java

@@ -36,17 +36,14 @@ public class MailerProcessor {
     private static final DotName MAIL_TEMPLATE = DotName.createSimple(MailTemplate.class.getName());
 
     @BuildStep
-    void unremoveableBeans(BuildProducer<AdditionalBeanBuildItem> additionalBeans) {
-        additionalBeans.produce(AdditionalBeanBuildItem.unremovableOf(MailClientProducer.class));
-        additionalBeans.produce(AdditionalBeanBuildItem.unremovableOf(MailerSupportProducer.class));
-    }
-
-    @BuildStep
-    AdditionalBeanBuildItem registerMailers() {
-        return AdditionalBeanBuildItem.builder()
+    void registerBeans(BuildProducer<AdditionalBeanBuildItem> beans) {
+        beans.produce(AdditionalBeanBuildItem.builder().setUnremovable()
                 .addBeanClasses(MutinyMailerImpl.class, BlockingMailerImpl.class,
-                        MockMailboxImpl.class, MailTemplateProducer.class)
-                .build();
+                        MailClientProducer.class, MailerSupportProducer.class)
+                .build());
+        beans.produce(AdditionalBeanBuildItem.builder()
+                .addBeanClasses(MockMailboxImpl.class, MailTemplateProducer.class)
+                .build());
     }
 
     @BuildStep

extensions/mailer/deployment/src/test/java/io/quarkus/mailer/MailTemplateValidationTest.java

@@ -11,6 +11,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
+import io.quarkus.arc.Unremovable;
 import io.quarkus.test.QuarkusUnitTest;
 import io.smallrye.mutiny.Uni;
 
@@ -31,6 +32,7 @@ public void testValidationFailed() {
         Assertions.fail();
     }
 
+    @Unremovable // Injection points from removed beans are not validated
     @Singleton
     static class MailTemplates {
 

extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java

@@ -6,6 +6,7 @@
 
 import javax.annotation.Priority;
 import javax.interceptor.Interceptor;
+import javax.transaction.TransactionManager;
 import javax.transaction.TransactionScoped;
 
 import com.arjuna.ats.arjuna.common.ObjectStoreEnvironmentBean;
@@ -143,9 +144,11 @@ public CustomScopeBuildItem registerScope() {
     }
 
     @BuildStep
-    UnremovableBeanBuildItem unremovableBean() {
+    void unremovableBean(BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
         // LifecycleManager comes from smallrye-context-propagation-jta and is only used via programmatic lookup in JtaContextProvider
-        return UnremovableBeanBuildItem.beanClassNames(JtaContextProvider.LifecycleManager.class.getName());
+        unremovableBeans.produce(UnremovableBeanBuildItem.beanClassNames(JtaContextProvider.LifecycleManager.class.getName()));
+        // The tx manager is obtained via CDI.current().select(TransactionManager.class) in the JtaContextProvider
+        unremovableBeans.produce(UnremovableBeanBuildItem.beanTypes(TransactionManager.class));
     }
 
 }

extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java

@@ -78,7 +78,7 @@ public class SmallRyeFaultToleranceProcessor {
 
     @BuildStep
     public void build(BuildProducer<AnnotationsTransformerBuildItem> annotationsTransformer,
-            BuildProducer<FeatureBuildItem> feature, BuildProducer<AdditionalBeanBuildItem> additionalBean,
+            BuildProducer<FeatureBuildItem> feature, BuildProducer<AdditionalBeanBuildItem> beans,
             BuildProducer<ServiceProviderBuildItem> serviceProvider,
             BuildProducer<BeanDefiningAnnotationBuildItem> additionalBda,
             Optional<MetricsCapabilityBuildItem> metricsCapability,
@@ -110,7 +110,7 @@ public void build(BuildProducer<AnnotationsTransformerBuildItem> annotationsTran
                 reflectiveClass.produce(new ReflectiveClassBuildItem(true, false, fallbackHandler));
                 fallbackHandlersBeans.addBeanClass(fallbackHandler);
             }
-            additionalBean.produce(fallbackHandlersBeans.build());
+            beans.produce(fallbackHandlersBeans.build());
         }
         // Add reflective access to fallback methods
         for (AnnotationInstance annotation : index.getAnnotations(DotNames.FALLBACK)) {
@@ -182,18 +182,25 @@ public void transform(TransformationContext context) {
         for (DotName ftAnnotation : DotNames.FT_ANNOTATIONS) {
             builder.addBeanClass(ftAnnotation.toString());
         }
-        builder.addBeanClasses(FaultToleranceInterceptor.class,
-                ExecutorHolder.class,
-                StrategyCache.class,
-                QuarkusFaultToleranceOperationProvider.class,
-                QuarkusFallbackHandlerProvider.class,
-                QuarkusExistingCircuitBreakerNames.class,
-                QuarkusAsyncExecutorProvider.class,
-                MetricsProvider.class,
-                CircuitBreakerMaintenanceImpl.class,
-                RequestContextIntegration.class,
-                SpecCompatibility.class);
-        additionalBean.produce(builder.build());
+        builder
+                .addBeanClasses(
+                        ExecutorHolder.class,
+                        StrategyCache.class,
+                        QuarkusFallbackHandlerProvider.class,
+                        QuarkusAsyncExecutorProvider.class,
+                        MetricsProvider.class,
+                        CircuitBreakerMaintenanceImpl.class,
+                        RequestContextIntegration.class,
+                        SpecCompatibility.class);
+        beans.produce(builder.build());
+
+        // TODO FT should be smart enough and only initialize the stuff in the recorder if it's really needed
+        // The FaultToleranceInterceptor needs to be registered as unremovable due to the rest-client integration - interceptors 
+        // are currently resolved dynamically at runtime because per the spec interceptor bindings cannot be declared on interfaces
+        beans.produce(AdditionalBeanBuildItem.builder().setUnremovable()
+                .addBeanClasses(FaultToleranceInterceptor.class, QuarkusFaultToleranceOperationProvider.class,
+                        QuarkusExistingCircuitBreakerNames.class)
+                .build());
 
         if (!metricsCapability.isPresent()) {
             //disable fault tolerance metrics with the MP sys props

extensions/smallrye-graphql/deployment/src/main/java/io/quarkus/smallrye/graphql/deployment/SmallRyeGraphQLProcessor.java

@@ -48,7 +48,6 @@
 import io.quarkus.deployment.util.WebJarUtil;
 import io.quarkus.runtime.LaunchMode;
 import io.quarkus.runtime.RuntimeValue;
-import io.quarkus.runtime.metrics.MetricsFactory;
 import io.quarkus.smallrye.graphql.runtime.SmallRyeGraphQLRecorder;
 import io.quarkus.smallrye.graphql.runtime.SmallRyeGraphQLRuntimeConfig;
 import io.quarkus.vertx.http.deployment.BodyHandlerBuildItem;
@@ -373,8 +372,7 @@ private Set<String> getAllReferenceClasses(Reference reference) {
     void activateMetrics(Capabilities capabilities,
             Optional<MetricsCapabilityBuildItem> metricsCapability,
             SmallRyeGraphQLConfig graphQLConfig,
-            BuildProducer<SystemPropertyBuildItem> systemProperties,
-            BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
+            BuildProducer<SystemPropertyBuildItem> systemProperties) {
 
         boolean activate = shouldActivateService(graphQLConfig.metricsEnabled,
                 metricsCapability.isPresent(),
@@ -383,9 +381,6 @@ void activateMetrics(Capabilities capabilities,
                 "quarkus.smallrye-graphql.metrics.enabled",
                 false);
         if (activate) {
-            if (metricsCapability.isPresent() && metricsCapability.get().metricsSupported(MetricsFactory.MP_METRICS)) {
-                unremovableBeans.produce(UnremovableBeanBuildItem.beanClassNames("io.smallrye.metrics.MetricRegistries"));
-            }
             systemProperties.produce(new SystemPropertyBuildItem(ConfigKey.ENABLE_METRICS, TRUE));
         } else {
             systemProperties.produce(new SystemPropertyBuildItem(ConfigKey.ENABLE_METRICS, FALSE));
@@ -395,7 +390,8 @@ void activateMetrics(Capabilities capabilities,
     @BuildStep
     void activateTracing(Capabilities capabilities,
             SmallRyeGraphQLConfig graphQLConfig,
-            BuildProducer<SystemPropertyBuildItem> systemProperties) {
+            BuildProducer<SystemPropertyBuildItem> systemProperties,
+            BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
 
         boolean activate = shouldActivateService(graphQLConfig.tracingEnabled,
                 capabilities.isPresent(Capability.OPENTRACING),