From 4ef9a5e7c4f0d2d8205224d998b99c8e5359d405 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Tue, 14 Mar 2023 11:10:19 +0100 Subject: [PATCH 1/9] ArC: remove superfluous CDI TCK exclusions Some of these tests were fixed in CDI TCK 4.0.8, some were fixed by the bean discovery in strict mode improvements, some were fixed by the improvements to interceptors. --- .../src/test/resources/testng.xml | 54 ------------------- 1 file changed, 54 deletions(-) diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index c27324111b129..e53ce51eeebdc 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -61,20 +61,6 @@ - - - - - - - - - - - - - - @@ -103,11 +89,6 @@ - - - - - @@ -173,13 +154,6 @@ - - - - - - - @@ -265,28 +239,11 @@ - - - - - - - - - - - - - - - - - @@ -335,11 +292,6 @@ - - - - - @@ -385,12 +337,6 @@ - - - - - - From 25eb1431c1b29acd816918fda7ef1aff8bf2c25c Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 17 Mar 2023 18:32:24 +0100 Subject: [PATCH 2/9] ArC: fix merging @AroundConstruct interceptor bindings Interceptor bindings have to be merged correctly. That is, a class-level binding of some type is ignored if a method-level binding of the same type is also present. This used to be the case for `@AroundInvoke` interceptors, but not for `@AroundConstruct` interceptors. This commit fixes that. --- .../io/quarkus/arc/processor/BeanInfo.java | 3 +- .../io/quarkus/arc/processor/Methods.java | 27 ++++-- .../src/test/resources/testng.xml | 9 -- ...thClassAndConstructorLevelBindingTest.java | 85 +++++++++++++++++++ 4 files changed, 108 insertions(+), 16 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithClassAndConstructorLevelBindingTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java index ffa2a7995a2d6..9a4c0fb9e6385 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java @@ -754,7 +754,8 @@ private Map initLifecycleInterceptors() { putLifecycleInterceptors(lifecycleInterceptors, classLevelBindings, InterceptionType.PRE_DESTROY); MethodInfo interceptedConstructor = findInterceptedConstructor(target.get().asClass()); if (beanDeployment.getAnnotation(interceptedConstructor, DotNames.NO_CLASS_INTERCEPTORS) == null) { - constructorLevelBindings.addAll(classLevelBindings); + constructorLevelBindings = Methods.mergeMethodAndClassLevelBindings(constructorLevelBindings, + classLevelBindings); } putLifecycleInterceptors(lifecycleInterceptors, constructorLevelBindings, InterceptionType.AROUND_CONSTRUCT); return lifecycleInterceptors; diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java index cff90da5a09d2..cfede7633304b 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java @@ -272,12 +272,7 @@ private static Set mergeBindings(BeanDeployment beanDeployme if (methodLevelBindings.isEmpty()) { merged = classLevelBindings; } else { - merged = new HashSet<>(methodLevelBindings); - for (AnnotationInstance binding : classLevelBindings) { - if (methodLevelBindings.stream().noneMatch(a -> binding.name().equals(a.name()))) { - merged.add(binding); - } - } + merged = mergeMethodAndClassLevelBindings(methodLevelBindings, classLevelBindings); if (Modifier.isPrivate(method.flags()) && !Annotations.containsAny(methodAnnotations, OBSERVER_PRODUCER_ANNOTATIONS)) { @@ -302,6 +297,26 @@ private static Set mergeBindings(BeanDeployment beanDeployme return merged; } + static Set mergeMethodAndClassLevelBindings(Collection methodLevelBindings, + Set classLevelBindings) { + if (methodLevelBindings.isEmpty()) { + return classLevelBindings; + } + + Set methodLevelNames = new HashSet<>(); + for (AnnotationInstance methodLevelBinding : methodLevelBindings) { + methodLevelNames.add(methodLevelBinding.name()); + } + + Set result = new HashSet<>(methodLevelBindings); + for (AnnotationInstance classLevelBinding : classLevelBindings) { + if (!methodLevelNames.contains(classLevelBinding.name())) { + result.add(classLevelBinding); + } + } + return result; + } + static class NameAndDescriptor { private final String name; private final String descriptor; diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index e53ce51eeebdc..5010bb49ad4bf 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -184,15 +184,6 @@ - - - - - - - - - diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithClassAndConstructorLevelBindingTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithClassAndConstructorLevelBindingTest.java new file mode 100644 index 0000000000000..b48bbd9dfb2b0 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithClassAndConstructorLevelBindingTest.java @@ -0,0 +1,85 @@ +package io.quarkus.arc.test.interceptors.aroundconstruct; + +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import jakarta.annotation.Priority; +import jakarta.enterprise.context.Dependent; +import jakarta.inject.Inject; +import jakarta.interceptor.AroundConstruct; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class AroundConstructWithClassAndConstructorLevelBindingTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBinding.class, MyInterceptor1.class, + MyInterceptor2.class, MyBean.class); + + @Test + public void test() { + MyBean bean = Arc.container().instance(MyBean.class).get(); + assertNotNull(bean); + assertTrue(MyBean.constructed); + assertFalse(MyInterceptor1.intercepted); + assertTrue(MyInterceptor2.intercepted); + } + + @Target({ TYPE, CONSTRUCTOR }) + @Retention(RUNTIME) + @InterceptorBinding + public @interface MyBinding { + int value(); + } + + @MyBinding(1) + @Interceptor + @Priority(1) + public static class MyInterceptor1 { + static boolean intercepted = false; + + @AroundConstruct + void intercept(InvocationContext ctx) throws Exception { + intercepted = true; + ctx.proceed(); + } + } + + @MyBinding(2) + @Interceptor + @Priority(1) + public static class MyInterceptor2 { + static boolean intercepted = false; + + @AroundConstruct + void intercept(InvocationContext ctx) throws Exception { + intercepted = true; + ctx.proceed(); + } + } + + @Dependent + @MyBinding(1) + static class MyBean { + static boolean constructed = false; + + @Inject + @MyBinding(2) + MyBean() { + constructed = true; + } + } +} From 3078ee48c01c83de8456be26ec5fab6807b87a10 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 20 Mar 2023 17:33:48 +0100 Subject: [PATCH 3/9] ArC: validate non-@Repeatable interceptor bindings for member values conflicts --- .../quarkus/arc/processor/BeanDeployment.java | 26 +++++++ .../io/quarkus/arc/processor/BeanInfo.java | 26 +++++-- .../quarkus/arc/processor/Interceptors.java | 55 +++++++++++++ .../src/test/resources/testng.xml | 10 --- ...onflictingStereotypeBindingOnBeanTest.java | 70 +++++++++++++++++ ...onflictingTransitiveBindingOnBeanTest.java | 55 +++++++++++++ ...ingTransitiveBindingOnInterceptorTest.java | 63 +++++++++++++++ ...TransitiveStereotypeBindingOnBeanTest.java | 78 +++++++++++++++++++ 8 files changed, 365 insertions(+), 18 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingStereotypeBindingOnBeanTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnBeanTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnInterceptorTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveStereotypeBindingOnBeanTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java index 7e9457dd05743..4d943a25ab3d2 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java @@ -1534,10 +1534,36 @@ private void findNamespaces(BeanInfo bean, Set namespaces) { } } + /** + * Returns the set of names of non-binding annotation members of given interceptor + * binding annotation that was registered through {@code InterceptorBindingRegistrar}. + *

+ * Does not return non-binding members of interceptor bindings that were + * discovered based on the {@code @InterceptorBinding} annotation; in such case, + * one has to manually check presence of the {@code @NonBinding} annotation on + * the annotation member declaration. + * + * @param name name of the interceptor binding annotation that was registered through + * {@code InterceptorBindingRegistrar} + * @return set of non-binding annotation members of the interceptor binding annotation + */ public Set getInterceptorNonbindingMembers(DotName name) { return interceptorNonbindingMembers.getOrDefault(name, Collections.emptySet()); } + /** + * Returns the set of names of non-binding annotation members of given qualifier + * annotation that was registered through {@code QualifierRegistrar}. + *

+ * Does not return non-binding members of interceptor bindings that were + * discovered based on the {@code @Qualifier} annotation; in such case, one has to + * manually check presence of the {@code @NonBinding} annotation on the annotation member + * declaration. + * + * @param name name of the qualifier annotation that was registered through + * {@code QualifierRegistrar} + * @return set of non-binding annotation members of the qualifier annotation + */ public Set getQualifierNonbindingMembers(DotName name) { return qualifierNonbindingMembers.getOrDefault(name, Collections.emptySet()); } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java index 9a4c0fb9e6385..0b2364fec6588 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java @@ -611,13 +611,21 @@ private Map initInterceptedMethods(List Map interceptedMethods = new HashMap<>(); Map> candidates = new HashMap<>(); + ClassInfo targetClass = target.get().asClass(); List classLevelBindings = new ArrayList<>(); - addClassLevelBindings(target.get().asClass(), classLevelBindings); + addClassLevelBindings(targetClass, classLevelBindings, Set.of()); if (!stereotypes.isEmpty()) { - for (StereotypeInfo stereotype : stereotypes) { - addClassLevelBindings(stereotype.getTarget(), classLevelBindings); + // interceptor binding declared on a bean class replaces an interceptor binding of the same type + // declared by a stereotype that is applied to the bean class + Set skip = classLevelBindings.stream() + .map(AnnotationInstance::name) + .collect(Collectors.toUnmodifiableSet()); + for (StereotypeInfo stereotype : Beans.stereotypesWithTransitive(stereotypes, + beanDeployment.getStereotypesMap())) { + addClassLevelBindings(stereotype.getTarget(), classLevelBindings, skip); } } + Interceptors.checkClassLevelInterceptorBindings(classLevelBindings, targetClass, beanDeployment); Set finalMethods = Methods.addInterceptedMethodCandidates(this, candidates, classLevelBindings, bytecodeTransformerConsumer, transformUnproxyableClasses); @@ -748,7 +756,7 @@ private Map initLifecycleInterceptors() { Map lifecycleInterceptors = new HashMap<>(); Set classLevelBindings = new HashSet<>(); Set constructorLevelBindings = new HashSet<>(); - addClassLevelBindings(target.get().asClass(), classLevelBindings); + addClassLevelBindings(target.get().asClass(), classLevelBindings, Set.of()); addConstructorLevelBindings(target.get().asClass(), constructorLevelBindings); putLifecycleInterceptors(lifecycleInterceptors, classLevelBindings, InterceptionType.POST_CONSTRUCT); putLifecycleInterceptors(lifecycleInterceptors, classLevelBindings, InterceptionType.PRE_DESTROY); @@ -774,15 +782,17 @@ private void putLifecycleInterceptors(Map li } } - private void addClassLevelBindings(ClassInfo classInfo, Collection bindings) { + // bindings whose class name is present in `skip` are ignored (this is used to ignore bindings on stereotypes + // when the original class has a binding of the same type) + private void addClassLevelBindings(ClassInfo classInfo, Collection bindings, Set skip) { beanDeployment.getAnnotations(classInfo).stream() - .filter(a -> beanDeployment.getInterceptorBinding(a.name()) != null - && bindings.stream().noneMatch(e -> e.name().equals(a.name()))) + .filter(a -> beanDeployment.getInterceptorBinding(a.name()) != null) + .filter(a -> !skip.contains(a.name())) .forEach(bindings::add); if (classInfo.superClassType() != null && !classInfo.superClassType().name().equals(DotNames.OBJECT)) { ClassInfo superClass = getClassByName(beanDeployment.getBeanArchiveIndex(), classInfo.superName()); if (superClass != null) { - addClassLevelBindings(superClass, bindings); + addClassLevelBindings(superClass, bindings, skip); } } } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java index 6671766609bcf..ab40dfbabd751 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java @@ -2,16 +2,22 @@ import static io.quarkus.arc.processor.IndexClassLookupUtils.getClassByName; +import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; import jakarta.enterprise.inject.spi.DefinitionException; import org.jboss.jandex.AnnotationInstance; +import org.jboss.jandex.AnnotationValue; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.FieldInfo; +import org.jboss.jandex.IndexView; import org.jboss.jandex.MethodInfo; import org.jboss.logging.Logger; @@ -65,6 +71,7 @@ static InterceptorInfo createInterceptor(ClassInfo interceptorClass, BeanDeploym priority = 0; } + checkClassLevelInterceptorBindings(bindings, interceptorClass, beanDeployment); checkInterceptorFieldsAndMethods(interceptorClass, beanDeployment); return new InterceptorInfo(interceptorClass, beanDeployment, @@ -73,6 +80,54 @@ static InterceptorInfo createInterceptor(ClassInfo interceptorClass, BeanDeploym Injection.forBean(interceptorClass, null, beanDeployment, transformer), priority); } + // similar logic already exists in InterceptorResolver, but it doesn't validate + static void checkClassLevelInterceptorBindings(Collection bindings, ClassInfo targetClass, + BeanDeployment beanDeployment) { + // when called from `createInterceptor` above, `bindings` already include transitive bindings, + // but when called from outside, that isn't guaranteed + Set allBindings = new HashSet<>(bindings); + for (AnnotationInstance binding : bindings) { + Set transitive = beanDeployment.getTransitiveInterceptorBindings(binding.name()); + if (transitive != null) { + allBindings.addAll(transitive); + } + } + + IndexView index = beanDeployment.getBeanArchiveIndex(); + + Map> seenBindings = new HashMap<>(); + for (AnnotationInstance binding : allBindings) { + DotName name = binding.name(); + if (beanDeployment.hasAnnotation(index.getClassByName(name), DotNames.REPEATABLE)) { + // don't validate @Repeatable interceptor bindings, repeatability is their entire point + continue; + } + + List seenValues = seenBindings.get(name); + if (seenValues != null) { + // interceptor binding of the same type already seen + // all annotation members (except nonbinding) must have equal values + ClassInfo declaration = beanDeployment.getInterceptorBinding(name); + Set nonBindingMembers = beanDeployment.getInterceptorNonbindingMembers(name); + + for (AnnotationValue value : seenValues) { + if (declaration.method(value.name()).hasDeclaredAnnotation(DotNames.NONBINDING) + || nonBindingMembers.contains(value.name())) { + continue; + } + + if (!value.equals(binding.valueWithDefault(index, value.name()))) { + throw new DefinitionException("Multiple instances of non-repeatable interceptor binding annotation " + + name + " with different member values on class " + targetClass); + } + } + } else { + // interceptor binding of that type not seen yet, just remember it + seenBindings.put(name, binding.valuesWithDefaults(index)); + } + } + } + private static void checkInterceptorFieldsAndMethods(ClassInfo interceptorClass, BeanDeployment beanDeployment) { ClassInfo aClass = interceptorClass; while (aClass != null) { diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index 5010bb49ad4bf..d8d06a466d69b 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -101,11 +101,6 @@ - - - - - @@ -144,11 +139,6 @@ - - - - - diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingStereotypeBindingOnBeanTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingStereotypeBindingOnBeanTest.java new file mode 100644 index 0000000000000..ba4316478240d --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingStereotypeBindingOnBeanTest.java @@ -0,0 +1,70 @@ +package io.quarkus.arc.test.interceptors.bindings.conflicting; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.enterprise.context.Dependent; +import jakarta.enterprise.inject.Stereotype; +import jakarta.enterprise.inject.spi.DefinitionException; +import jakarta.interceptor.InterceptorBinding; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.test.ArcTestContainer; + +public class ConflictingStereotypeBindingOnBeanTest { + @RegisterExtension + public ArcTestContainer container = ArcTestContainer.builder() + .beanClasses(MyBean.class, FooBinding.class, BarBinding.class, Stereotype1.class, Stereotype2.class) + .shouldFail() + .build(); + + @Test + public void trigger() { + Throwable error = container.getFailure(); + assertNotNull(error); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains("Multiple instances of non-repeatable interceptor binding annotation")); + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface FooBinding { + int value(); + } + + @FooBinding(10) + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface BarBinding { + } + + @FooBinding(1) + @Stereotype + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @interface Stereotype1 { + } + + @BarBinding + @Stereotype + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @interface Stereotype2 { + } + + @Stereotype1 + @Stereotype2 + @Dependent + static class MyBean { + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnBeanTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnBeanTest.java new file mode 100644 index 0000000000000..57977a227d0bf --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnBeanTest.java @@ -0,0 +1,55 @@ +package io.quarkus.arc.test.interceptors.bindings.conflicting; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.enterprise.context.Dependent; +import jakarta.enterprise.inject.spi.DefinitionException; +import jakarta.interceptor.InterceptorBinding; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.test.ArcTestContainer; + +public class ConflictingTransitiveBindingOnBeanTest { + @RegisterExtension + public ArcTestContainer container = ArcTestContainer.builder() + .beanClasses(MyBean.class, FooBinding.class, BarBinding.class) + .shouldFail() + .build(); + + @Test + public void trigger() { + Throwable error = container.getFailure(); + assertNotNull(error); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains("Multiple instances of non-repeatable interceptor binding annotation")); + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface FooBinding { + int value(); + } + + @FooBinding(10) + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface BarBinding { + } + + @FooBinding(1) + @BarBinding + @Dependent + static class MyBean { + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnInterceptorTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnInterceptorTest.java new file mode 100644 index 0000000000000..3756a7faccba6 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveBindingOnInterceptorTest.java @@ -0,0 +1,63 @@ +package io.quarkus.arc.test.interceptors.bindings.conflicting; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.annotation.Priority; +import jakarta.enterprise.inject.spi.DefinitionException; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.test.ArcTestContainer; + +public class ConflictingTransitiveBindingOnInterceptorTest { + @RegisterExtension + public ArcTestContainer container = ArcTestContainer.builder() + .beanClasses(MyInterceptor.class, FooBinding.class, BarBinding.class) + .shouldFail() + .build(); + + @Test + public void trigger() { + Throwable error = container.getFailure(); + assertNotNull(error); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains("Multiple instances of non-repeatable interceptor binding annotation")); + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface FooBinding { + int value(); + } + + @FooBinding(10) + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface BarBinding { + } + + @FooBinding(1) + @BarBinding + @Interceptor + @Priority(1) + static class MyInterceptor { + @AroundInvoke + Object intercept(InvocationContext ctx) throws Exception { + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveStereotypeBindingOnBeanTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveStereotypeBindingOnBeanTest.java new file mode 100644 index 0000000000000..52214b075f24d --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/conflicting/ConflictingTransitiveStereotypeBindingOnBeanTest.java @@ -0,0 +1,78 @@ +package io.quarkus.arc.test.interceptors.bindings.conflicting; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.enterprise.context.Dependent; +import jakarta.enterprise.inject.Stereotype; +import jakarta.enterprise.inject.spi.DefinitionException; +import jakarta.interceptor.InterceptorBinding; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.test.ArcTestContainer; + +public class ConflictingTransitiveStereotypeBindingOnBeanTest { + @RegisterExtension + public ArcTestContainer container = ArcTestContainer.builder() + .beanClasses(MyBean.class, FooBinding.class, BarBinding.class, Stereotype1.class, Stereotype2.class, + Stereotype3.class) + .shouldFail() + .build(); + + @Test + public void trigger() { + Throwable error = container.getFailure(); + assertNotNull(error); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains("Multiple instances of non-repeatable interceptor binding annotation")); + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface FooBinding { + int value(); + } + + @FooBinding(10) + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface BarBinding { + } + + @FooBinding(1) + @Stereotype + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @interface Stereotype1 { + } + + @BarBinding + @Stereotype + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @interface Stereotype2 { + } + + @Stereotype1 + @Stereotype2 + @Stereotype + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @interface Stereotype3 { + } + + @Stereotype3 + @Dependent + static class MyBean { + } +} From a0ccdea79f8392dcc469c3b36a363e42fef349df Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 21 Apr 2023 17:07:00 +0200 Subject: [PATCH 4/9] ArC: fix class-level interceptor bindings search to always look at stereotypes Previously, the class-level interceptor bindings search only looked at stereotypes when searching for interceptor bindings of business method interceptors (`@AroundInvoke`). With this commit, the class-level interceptor binding search works identically for business method interceptors and lifecycle callbacks (`@AroundConstruct`, `@PostConstruct`, `@PreDestroy`). --- .../io/quarkus/arc/processor/BeanInfo.java | 37 +++--- .../src/test/resources/testng.xml | 7 - .../InterceptorBindingOnStereotypeTest.java | 121 ++++++++++++++++++ 3 files changed, 143 insertions(+), 22 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/stereotype/InterceptorBindingOnStereotypeTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java index 0b2364fec6588..93803ee8fff08 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java @@ -613,18 +613,7 @@ private Map initInterceptedMethods(List ClassInfo targetClass = target.get().asClass(); List classLevelBindings = new ArrayList<>(); - addClassLevelBindings(targetClass, classLevelBindings, Set.of()); - if (!stereotypes.isEmpty()) { - // interceptor binding declared on a bean class replaces an interceptor binding of the same type - // declared by a stereotype that is applied to the bean class - Set skip = classLevelBindings.stream() - .map(AnnotationInstance::name) - .collect(Collectors.toUnmodifiableSet()); - for (StereotypeInfo stereotype : Beans.stereotypesWithTransitive(stereotypes, - beanDeployment.getStereotypesMap())) { - addClassLevelBindings(stereotype.getTarget(), classLevelBindings, skip); - } - } + addClassLevelBindings(targetClass, classLevelBindings); Interceptors.checkClassLevelInterceptorBindings(classLevelBindings, targetClass, beanDeployment); Set finalMethods = Methods.addInterceptedMethodCandidates(this, candidates, classLevelBindings, @@ -756,7 +745,7 @@ private Map initLifecycleInterceptors() { Map lifecycleInterceptors = new HashMap<>(); Set classLevelBindings = new HashSet<>(); Set constructorLevelBindings = new HashSet<>(); - addClassLevelBindings(target.get().asClass(), classLevelBindings, Set.of()); + addClassLevelBindings(target.get().asClass(), classLevelBindings); addConstructorLevelBindings(target.get().asClass(), constructorLevelBindings); putLifecycleInterceptors(lifecycleInterceptors, classLevelBindings, InterceptionType.POST_CONSTRUCT); putLifecycleInterceptors(lifecycleInterceptors, classLevelBindings, InterceptionType.PRE_DESTROY); @@ -782,9 +771,27 @@ private void putLifecycleInterceptors(Map li } } + private void addClassLevelBindings(ClassInfo targetClass, Collection bindings) { + List classLevelBindings = new ArrayList<>(); + doAddClassLevelBindings(targetClass, classLevelBindings, Set.of()); + bindings.addAll(classLevelBindings); + if (!stereotypes.isEmpty()) { + // interceptor binding declared on a bean class replaces an interceptor binding of the same type + // declared by a stereotype that is applied to the bean class + Set skip = new HashSet<>(); + for (AnnotationInstance classLevelBinding : classLevelBindings) { + skip.add(classLevelBinding.name()); + } + for (StereotypeInfo stereotype : Beans.stereotypesWithTransitive(stereotypes, + beanDeployment.getStereotypesMap())) { + doAddClassLevelBindings(stereotype.getTarget(), bindings, skip); + } + } + } + // bindings whose class name is present in `skip` are ignored (this is used to ignore bindings on stereotypes // when the original class has a binding of the same type) - private void addClassLevelBindings(ClassInfo classInfo, Collection bindings, Set skip) { + private void doAddClassLevelBindings(ClassInfo classInfo, Collection bindings, Set skip) { beanDeployment.getAnnotations(classInfo).stream() .filter(a -> beanDeployment.getInterceptorBinding(a.name()) != null) .filter(a -> !skip.contains(a.name())) @@ -792,7 +799,7 @@ private void addClassLevelBindings(ClassInfo classInfo, Collection - - - - - - - diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/stereotype/InterceptorBindingOnStereotypeTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/stereotype/InterceptorBindingOnStereotypeTest.java new file mode 100644 index 0000000000000..536cd0fb35da3 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/stereotype/InterceptorBindingOnStereotypeTest.java @@ -0,0 +1,121 @@ +package io.quarkus.arc.test.interceptors.bindings.stereotype; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.PreDestroy; +import jakarta.annotation.Priority; +import jakarta.enterprise.inject.Stereotype; +import jakarta.inject.Singleton; +import jakarta.interceptor.AroundConstruct; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.InstanceHandle; +import io.quarkus.arc.test.ArcTestContainer; + +public class InterceptorBindingOnStereotypeTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyStereotype.class, + MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void testInterception() { + assertEquals(0, MyInterceptor.aroundConstruct); + assertEquals(0, MyInterceptor.postConstruct); + assertEquals(0, MyInterceptor.aroundInvoke); + assertEquals(0, MyInterceptor.preDestroy); + + InstanceHandle bean = Arc.container().instance(MyBean.class); + MyBean instance = bean.get(); + + assertEquals(1, MyInterceptor.aroundConstruct); + assertEquals(1, MyInterceptor.postConstruct); + assertEquals(0, MyInterceptor.aroundInvoke); + assertEquals(0, MyInterceptor.preDestroy); + + instance.doSomething(); + + assertEquals(1, MyInterceptor.aroundConstruct); + assertEquals(1, MyInterceptor.postConstruct); + assertEquals(1, MyInterceptor.aroundInvoke); + assertEquals(0, MyInterceptor.preDestroy); + + bean.destroy(); + + assertEquals(1, MyInterceptor.aroundConstruct); + assertEquals(1, MyInterceptor.postConstruct); + assertEquals(1, MyInterceptor.aroundInvoke); + assertEquals(1, MyInterceptor.preDestroy); + } + + @Singleton + @MyStereotype + static class MyBean { + void doSomething() { + } + } + + @MyInterceptorBinding + @Stereotype + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @interface MyStereotype { + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor { + static int aroundConstruct = 0; + static int postConstruct = 0; + static int aroundInvoke = 0; + static int preDestroy = 0; + + @AroundConstruct + Object aroundConstruct(InvocationContext ctx) throws Exception { + aroundConstruct++; + return ctx.proceed(); + } + + @PostConstruct + Object postConstruct(InvocationContext ctx) throws Exception { + postConstruct++; + return ctx.proceed(); + } + + @AroundInvoke + Object aroundInvoke(InvocationContext ctx) throws Exception { + aroundInvoke++; + return ctx.proceed(); + } + + @PreDestroy + Object preDestroy(InvocationContext ctx) throws Exception { + preDestroy++; + return ctx.proceed(); + } + } +} From ef190cca0bc6cfb4bb73d013419c0670cf539fac Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 21 Apr 2023 18:14:33 +0200 Subject: [PATCH 5/9] ArC: fix searching for bindings on an interceptor declaration to also look for inherited bindings --- .../quarkus/arc/processor/Interceptors.java | 37 +++++-- .../src/test/resources/testng.xml | 6 - .../InheritedBindingOnInterceptorTest.java | 104 ++++++++++++++++++ 3 files changed, 132 insertions(+), 15 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnInterceptorTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java index ab40dfbabd751..9e81eba2ca1aa 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Interceptors.java @@ -36,17 +36,8 @@ private Interceptors() { */ static InterceptorInfo createInterceptor(ClassInfo interceptorClass, BeanDeployment beanDeployment, InjectionPointModifier transformer) { - Set bindings = new HashSet<>(); Integer priority = null; for (AnnotationInstance annotation : beanDeployment.getAnnotations(interceptorClass)) { - bindings.addAll(beanDeployment.extractInterceptorBindings(annotation)); - // can also be a transitive binding - Set transitiveInterceptorBindings = beanDeployment - .getTransitiveInterceptorBindings(annotation.name()); - if (transitiveInterceptorBindings != null) { - bindings.addAll(transitiveInterceptorBindings); - } - if (annotation.name().equals(DotNames.PRIORITY)) { priority = annotation.value().asInt(); } @@ -61,6 +52,9 @@ static InterceptorInfo createInterceptor(ClassInfo interceptorClass, BeanDeploym } } + Set bindings = new HashSet<>(); + addBindings(beanDeployment, interceptorClass, bindings, false); + if (bindings.isEmpty()) { throw new DefinitionException("Interceptor has no bindings: " + interceptorClass); } @@ -80,6 +74,31 @@ static InterceptorInfo createInterceptor(ClassInfo interceptorClass, BeanDeploym Injection.forBean(interceptorClass, null, beanDeployment, transformer), priority); } + private static void addBindings(BeanDeployment beanDeployment, ClassInfo classInfo, Collection bindings, + boolean onlyInherited) { + for (AnnotationInstance annotation : beanDeployment.getAnnotations(classInfo)) { + ClassInfo annotationClass = getClassByName(beanDeployment.getBeanArchiveIndex(), annotation.name()); + if (onlyInherited && !beanDeployment.hasAnnotation(annotationClass, DotNames.INHERITED)) { + continue; + } + + bindings.addAll(beanDeployment.extractInterceptorBindings(annotation)); + // can also be a transitive binding + Set transitiveInterceptorBindings = beanDeployment + .getTransitiveInterceptorBindings(annotation.name()); + if (transitiveInterceptorBindings != null) { + bindings.addAll(transitiveInterceptorBindings); + } + } + + if (classInfo.superName() != null && !classInfo.superName().equals(DotNames.OBJECT)) { + ClassInfo superClass = getClassByName(beanDeployment.getBeanArchiveIndex(), classInfo.superName()); + if (superClass != null) { + addBindings(beanDeployment, superClass, bindings, true); + } + } + } + // similar logic already exists in InterceptorResolver, but it doesn't validate static void checkClassLevelInterceptorBindings(Collection bindings, ClassInfo targetClass, BeanDeployment beanDeployment) { diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index af118b474ed4d..64c17e1e2d7d6 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -230,12 +230,6 @@ - - - - - - diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnInterceptorTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnInterceptorTest.java new file mode 100644 index 0000000000000..92e57e110e6b5 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bindings/inherited/InheritedBindingOnInterceptorTest.java @@ -0,0 +1,104 @@ +package io.quarkus.arc.test.interceptors.bindings.inherited; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class InheritedBindingOnInterceptorTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, FooBinding.class, BarBinding.class, + BarBindingUnused.class, FooInterceptor.class, BarInterceptor.class); + + @Test + public void testInterception() { + MyBean bean = Arc.container().instance(MyBean.class).get(); + assertNotNull(bean); + bean.doSomething(); + assertTrue(FooInterceptor.intercepted); + assertFalse(BarInterceptor.intercepted); + } + + @Singleton + @FooBinding + @BarBinding + static class MyBean { + void doSomething() { + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + @Inherited + @interface FooBinding { + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + // not @Inherited + @interface BarBinding { + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + @interface BarBindingUnused { + } + + @FooBinding + static class FooInterceptorSuperclass { + } + + @Interceptor + @Priority(1) + static class FooInterceptor extends FooInterceptorSuperclass { + static boolean intercepted = false; + + @AroundInvoke + Object intercept(InvocationContext ctx) throws Exception { + intercepted = true; + return ctx.proceed(); + } + } + + @BarBinding + static class BarInterceptorSuperclass { + } + + @BarBindingUnused // just to prevent the "Interceptor has no bindings" error, not used otherwise + @Interceptor + @Priority(1) + static class BarInterceptor extends BarInterceptorSuperclass { + static boolean intercepted = false; + + @AroundInvoke + Object intercept(InvocationContext ctx) throws Exception { + intercepted = true; + return ctx.proceed(); + } + } +} From f87cf8c364b5af89c11bfcb4574edddc2fa7d126 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 24 Apr 2023 17:01:11 +0200 Subject: [PATCH 6/9] ArC: fix interceptor method nesting for @PostConstruct/@PreDestroy interceptors Previously, `@PostConstruct`/`@PreDestroy` methods declared on the target class were not invoked from within the `@PostConstruct`/`@PreDestroy` methods declared on an interceptor class. Instead, they were invoked _after_ the interceptor chain finished, effectively creating two chains of lifecycle callback interceptors. This didn't allow an "outer" lifecycle callback method, when declared on an interceptor class, to catch and handle an exception thrown by an "inner" lifecycle callback method, when declared on the target class. This commit fixes the implementation of `@PostConstruct`/`@PreDestroy` interceptors by reifying the "inner" chain of lifecycle callback methods, declared on the target class, into a `Runnable` and passing that to the "outer" chain of lifecycle callback methods, declared on the interceptor class, to be invoked at the very end of the chain. This effectively merges the two chains into one, as prescribed by the specification. Note that the "inner" chain is reified into a `Runnable` only when an "outer" chain actually exists. It often doesn't, in which case, allocating a `Runnable` only to call it is a waste of resources. --- .../quarkus/arc/processor/BeanGenerator.java | 92 ++++++++++--- .../arc/processor/MethodDescriptors.java | 4 +- .../arc/processor/SubclassGenerator.java | 8 +- .../AroundConstructInvocationContext.java | 2 +- .../quarkus/arc/impl/InvocationContexts.java | 10 +- .../LifecycleCallbackInvocationContext.java | 21 +-- ...tConstructPreDestroyInvocationContext.java | 35 +++++ .../src/test/resources/testng.xml | 5 - .../interceptors/LifecycleInterceptor.java | 6 +- .../interceptors/complex/MyInterceptor.java | 6 +- .../AroundInvokeInterceptorNestingTest.java | 76 +++++++++++ .../PostConstructInterceptorNestingTest.java | 71 ++++++++++ ...eAndManySuperclassesWithOverridesTest.java | 121 ++++++++++++++++++ ...getClassAndOutsideAndSuperclassesTest.java | 81 ++++++++++++ ...tsideAndSuperclassesWithOverridesTest.java | 82 ++++++++++++ ...tConstructOnTargetClassAndOutsideTest.java | 66 ++++++++++ .../PreDestroyInterceptorNestingTest.java | 71 ++++++++++ ...eAndManySuperclassesWithOverridesTest.java | 121 ++++++++++++++++++ ...getClassAndOutsideAndSuperclassesTest.java | 81 ++++++++++++ ...tsideAndSuperclassesWithOverridesTest.java | 82 ++++++++++++ ...PreDestroyOnTargetClassAndOutsideTest.java | 66 ++++++++++ 21 files changed, 1056 insertions(+), 51 deletions(-) create mode 100644 independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/PostConstructPreDestroyInvocationContext.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/AroundInvokeInterceptorNestingTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructInterceptorNestingTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyInterceptorNestingTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java index 598c4394bc148..5a7b6d3ef9ce3 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java @@ -819,16 +819,38 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide if (bean.isClassBean()) { if (!bean.isInterceptor()) { + // if there's no `@PreDestroy` interceptor, we'll generate code to invoke `@PreDestroy` callbacks + // directly into the `destroy` method: + // + // public void destroy(MyBean var1, CreationalContext var2) { + // var1.myPreDestroyCallback(); + // var2.release(); + // } + BytecodeCreator preDestroyBytecode = destroy; + // PreDestroy interceptors if (!bean.getLifecycleInterceptors(InterceptionType.PRE_DESTROY).isEmpty()) { + // if there _is_ some `@PreDestroy` interceptor, however, we'll reify the chain of `@PreDestroy` + // callbacks into a `Runnable` that we pass into the interceptor chain to be called + // by the last `proceed()` call: + // + // public void destroy(MyBean var1, CreationalContext var2) { + // // this is a `Runnable` that calls `MyBean.myPreDestroyCallback()` + // MyBean_Bean$$function$$2 var3 = new MyBean_Bean$$function$$2(var1); + // ((MyBean_Subclass)var1).arc$destroy((Runnable)var3); + // var2.release(); + // } + FunctionCreator preDestroyForwarder = destroy.createFunction(Runnable.class); + preDestroyBytecode = preDestroyForwarder.getBytecode(); + destroy.invokeVirtualMethod( MethodDescriptor.ofMethod(SubclassGenerator.generatedName(bean.getProviderType().name(), baseName), - SubclassGenerator.DESTROY_METHOD_NAME, - void.class), - destroy.getMethodParam(0)); + SubclassGenerator.DESTROY_METHOD_NAME, void.class, Runnable.class), + destroy.getMethodParam(0), preDestroyForwarder.getInstance()); } // PreDestroy callbacks + // possibly wrapped into Runnable so that PreDestroy interceptors can proceed() correctly List preDestroyCallbacks = Beans.getCallbacks(bean.getTarget().get().asClass(), DotNames.PRE_DESTROY, bean.getDeployment().getBeanArchiveIndex()); @@ -839,16 +861,21 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide callback.declaringClass().name(), callback.name())); } reflectionRegistration.registerMethod(callback); - destroy.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, - destroy.loadClass(callback.declaringClass().name().toString()), - destroy.load(callback.name()), destroy.newArray(Class.class, destroy.load(0)), + preDestroyBytecode.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, + preDestroyBytecode.loadClass(callback.declaringClass().name().toString()), + preDestroyBytecode.load(callback.name()), + preDestroyBytecode.newArray(Class.class, preDestroyBytecode.load(0)), destroy.getMethodParam(0), - destroy.newArray(Object.class, destroy.load(0))); + preDestroyBytecode.newArray(Object.class, preDestroyBytecode.load(0))); } else { // instance.superCoolDestroyCallback() - destroy.invokeVirtualMethod(MethodDescriptor.of(callback), destroy.getMethodParam(0)); + preDestroyBytecode.invokeVirtualMethod(MethodDescriptor.of(callback), destroy.getMethodParam(0)); } } + if (preDestroyBytecode != destroy) { + // only if we're generating a `Runnable`, see above + preDestroyBytecode.returnVoid(); + } } // ctx.release() @@ -1750,9 +1777,35 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat SubclassGenerator.MARK_CONSTRUCTED_METHOD_NAME, void.class), instanceHandle); } + // if there's no `@PostConstruct` interceptor, we'll generate code to invoke `@PostConstruct` callbacks + // directly into the `doCreate` method: + // + // private MyBean doCreate(CreationalContext var1) { + // MyBean var2 = new MyBean(); + // var2.myPostConstructCallback(); + // return var2; + // } + BytecodeCreator postConstructsBytecode = create; + // PostConstruct lifecycle callback interceptors InterceptionInfo postConstructs = bean.getLifecycleInterceptors(InterceptionType.POST_CONSTRUCT); if (!postConstructs.isEmpty()) { + // if there _is_ some `@PostConstruct` interceptor, however, we'll reify the chain of `@PostConstruct` + // callbacks into a `Runnable` that we pass into the interceptor chain to be called + // by the last `proceed()` call: + // + // private MyBean doCreate(CreationalContext var1) { + // ... + // MyBean var7 = new MyBean(); + // // this is a `Runnable` that calls `MyBean.myPostConstructCallback()` + // MyBean_Bean$$function$$1 var11 = new MyBean_Bean$$function$$1(var7); + // ... + // InvocationContext var12 = InvocationContexts.postConstruct(var7, (List)var5, var10, (Runnable)var11); + // var12.proceed(); + // return var7; + // } + FunctionCreator postConstructForwarder = create.createFunction(Runnable.class); + postConstructsBytecode = postConstructForwarder.getBytecode(); // Interceptor bindings ResultHandle bindingsArray = create.newArray(Object.class, postConstructs.bindings.size()); @@ -1768,7 +1821,8 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat // InvocationContextImpl.postConstruct(instance,postConstructs).proceed() ResultHandle invocationContextHandle = create.invokeStaticMethod( MethodDescriptors.INVOCATION_CONTEXTS_POST_CONSTRUCT, instanceHandle, - postConstructsHandle, create.invokeStaticMethod(MethodDescriptors.SETS_OF, bindingsArray)); + postConstructsHandle, create.invokeStaticMethod(MethodDescriptors.SETS_OF, bindingsArray), + postConstructForwarder.getInstance()); TryBlock tryCatch = create.tryBlock(); CatchBlockCreator exceptionCatch = tryCatch.addCatch(Exception.class); @@ -1780,10 +1834,11 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat } // PostConstruct callbacks + // possibly wrapped into Runnable so that PostConstruct interceptors can proceed() correctly if (!bean.isInterceptor()) { List postConstructCallbacks = Beans.getCallbacks(bean.getTarget().get().asClass(), - DotNames.POST_CONSTRUCT, - bean.getDeployment().getBeanArchiveIndex()); + DotNames.POST_CONSTRUCT, bean.getDeployment().getBeanArchiveIndex()); + for (MethodInfo callback : postConstructCallbacks) { if (isReflectionFallbackNeeded(callback, targetPackage)) { if (Modifier.isPrivate(callback.flags())) { @@ -1792,15 +1847,20 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat callback.name())); } reflectionRegistration.registerMethod(callback); - create.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, - create.loadClass(callback.declaringClass().name().toString()), - create.load(callback.name()), create.newArray(Class.class, create.load(0)), instanceHandle, - create.newArray(Object.class, create.load(0))); + postConstructsBytecode.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, + postConstructsBytecode.loadClass(callback.declaringClass().name().toString()), + postConstructsBytecode.load(callback.name()), + postConstructsBytecode.newArray(Class.class, postConstructsBytecode.load(0)), instanceHandle, + postConstructsBytecode.newArray(Object.class, postConstructsBytecode.load(0))); } else { - create.invokeVirtualMethod(MethodDescriptor.of(callback), instanceHandle); + postConstructsBytecode.invokeVirtualMethod(MethodDescriptor.of(callback), instanceHandle); } } } + if (postConstructsBytecode != create) { + // only if we're generating a `Runnable`, see above + postConstructsBytecode.returnVoid(); + } create.returnValue(instanceHandle); } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java index 9d14b1dfad991..a5dc7fad43c2f 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java @@ -178,11 +178,11 @@ public final class MethodDescriptors { public static final MethodDescriptor INVOCATION_CONTEXTS_POST_CONSTRUCT = MethodDescriptor.ofMethod( InvocationContexts.class, "postConstruct", - InvocationContext.class, Object.class, List.class, Set.class); + InvocationContext.class, Object.class, List.class, Set.class, Runnable.class); public static final MethodDescriptor INVOCATION_CONTEXTS_PRE_DESTROY = MethodDescriptor.ofMethod(InvocationContexts.class, "preDestroy", - InvocationContext.class, Object.class, List.class, Set.class); + InvocationContext.class, Object.class, List.class, Set.class, Runnable.class); public static final MethodDescriptor INVOCATION_CONTEXTS_PERFORM_SUPERCLASS = MethodDescriptor.ofMethod( InvocationContexts.class, diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java index fcd318c61d779..284a76641324b 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java @@ -947,9 +947,10 @@ private void createInterceptedMethod(ClassOutput classOutput, BeanInfo bean, Met protected void createDestroy(ClassOutput classOutput, BeanInfo bean, ClassCreator subclass, FieldDescriptor preDestroysField) { if (preDestroysField != null) { - MethodCreator destroy = subclass - .getMethodCreator(MethodDescriptor.ofMethod(subclass.getClassName(), DESTROY_METHOD_NAME, void.class)); + MethodCreator destroy = subclass.getMethodCreator(MethodDescriptor.ofMethod(subclass.getClassName(), + DESTROY_METHOD_NAME, void.class, Runnable.class)); ResultHandle predestroysHandle = destroy.readInstanceField(preDestroysField, destroy.getThis()); + ResultHandle forward = destroy.getMethodParam(0); // Interceptor bindings InterceptionInfo preDestroy = bean.getLifecycleInterceptors(InterceptionType.PRE_DESTROY); @@ -972,7 +973,8 @@ protected void createDestroy(ClassOutput classOutput, BeanInfo bean, ClassCreato // InvocationContextImpl.preDestroy(this,predestroys) ResultHandle invocationContext = tryCatch.invokeStaticMethod(MethodDescriptors.INVOCATION_CONTEXTS_PRE_DESTROY, tryCatch.getThis(), predestroysHandle, - tryCatch.invokeStaticMethod(MethodDescriptors.SETS_OF, bindingsArray)); + tryCatch.invokeStaticMethod(MethodDescriptors.SETS_OF, bindingsArray), + forward); // InvocationContext.proceed() tryCatch.invokeInterfaceMethod(MethodDescriptors.INVOCATION_CONTEXT_PROCEED, invocationContext); diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java index 532f9a51cb552..d61b927ac3a18 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java @@ -7,7 +7,7 @@ import java.util.function.Supplier; /** - * An InvocationContext implementation used for AroundConstruct callbacks. + * An {@code InvocationContext} implementation used for {@code AroundConstruct} callbacks. */ class AroundConstructInvocationContext extends LifecycleCallbackInvocationContext { diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java index fa137958f4c7c..b7e0319ec6f1d 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java @@ -46,11 +46,12 @@ public static Object performTargetAroundInvoke(InvocationContext delegate, * @param target * @param chain * @param interceptorBindings + * @param forward * @return a new invocation context */ public static InvocationContext postConstruct(Object target, List chain, - Set interceptorBindings) { - return new LifecycleCallbackInvocationContext(target, null, interceptorBindings, chain); + Set interceptorBindings, Runnable forward) { + return new PostConstructPreDestroyInvocationContext(target, null, interceptorBindings, chain, forward); } /** @@ -58,11 +59,12 @@ public static InvocationContext postConstruct(Object target, List chain, - Set interceptorBindings) { - return new LifecycleCallbackInvocationContext(target, null, interceptorBindings, chain); + Set interceptorBindings, Runnable forward) { + return new PostConstructPreDestroyInvocationContext(target, null, interceptorBindings, chain, forward); } /** diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java index b84425940266e..c085df95ff973 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java @@ -7,11 +7,11 @@ /** * A simple stateful {@link jakarta.interceptor.InvocationContext} implementation used for - * {@link jakarta.annotation.PostConstruct} and {@link jakarta.annotation.PreDestroy} callbacks. + * lifecycle callback interceptors. *

* All lifecycle callback interceptors of a specific chain must be invoked on the same thread. */ -class LifecycleCallbackInvocationContext extends AbstractInvocationContext { +abstract class LifecycleCallbackInvocationContext extends AbstractInvocationContext { protected final Set bindings; protected final List chain; @@ -31,7 +31,8 @@ public Object proceed() throws Exception { // Invoke the next interceptor in the chain invokeNext(); } else { - // The invocation of proceed in the last interceptor method in the chain + // The invocation of proceed in the last interceptor method in the chain, + // need to forward to the target class interceptorChainCompleted(); } // The return value of a lifecycle callback is ignored @@ -53,19 +54,7 @@ public Set getInterceptorBindings() { return bindings; } - @Override - public Object[] getParameters() { - throw new IllegalStateException(); - } - - @Override - public void setParameters(Object[] params) { - throw new IllegalStateException(); - } - - protected void interceptorChainCompleted() throws Exception { - // No-op - } + protected abstract void interceptorChainCompleted() throws Exception; private Object invokeNext() throws Exception { try { diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/PostConstructPreDestroyInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/PostConstructPreDestroyInvocationContext.java new file mode 100644 index 0000000000000..3d757ea35c9ce --- /dev/null +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/PostConstructPreDestroyInvocationContext.java @@ -0,0 +1,35 @@ +package io.quarkus.arc.impl; + +import java.lang.annotation.Annotation; +import java.util.List; +import java.util.Set; + +/** + * An {@code InvocationContext} implementation used for {@code PostConstruct} and {@code PreDestroy} callbacks. + */ +class PostConstructPreDestroyInvocationContext extends LifecycleCallbackInvocationContext { + private final Runnable forward; + + PostConstructPreDestroyInvocationContext(Object target, Object[] parameters, + Set bindings, List chain, Runnable forward) { + super(target, parameters, bindings, chain); + this.forward = forward; + } + + @Override + protected void interceptorChainCompleted() { + if (forward != null) { + forward.run(); + } + } + + @Override + public Object[] getParameters() { + throw new IllegalStateException(); + } + + @Override + public void setParameters(Object[] params) { + throw new IllegalStateException(); + } +} diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index 64c17e1e2d7d6..e373449fceee7 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -61,11 +61,6 @@ - - - - - diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/LifecycleInterceptor.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/LifecycleInterceptor.java index 3cf327c15a577..19f77456d2d4a 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/LifecycleInterceptor.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/LifecycleInterceptor.java @@ -22,21 +22,23 @@ public class LifecycleInterceptor { static final List PRE_DESTROYS = new CopyOnWriteArrayList<>(); @PostConstruct - void simpleInit(InvocationContext ctx) { + void simpleInit(InvocationContext ctx) throws Exception { Object bindings = ctx.getContextData().get(ArcInvocationContext.KEY_INTERCEPTOR_BINDINGS); if (bindings == null) { throw new IllegalArgumentException("No bindings found"); } POST_CONSTRUCTS.add(ctx.getTarget()); + ctx.proceed(); } @PreDestroy - void simpleDestroy(InvocationContext ctx) { + void simpleDestroy(InvocationContext ctx) throws Exception { Object bindings = ctx.getContextData().get(ArcInvocationContext.KEY_INTERCEPTOR_BINDINGS); if (bindings == null) { throw new IllegalArgumentException("No bindings found"); } PRE_DESTROYS.add(ctx.getTarget()); + ctx.proceed(); } @AroundConstruct diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/complex/MyInterceptor.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/complex/MyInterceptor.java index ecf34d0aa52df..02585cc3f5b04 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/complex/MyInterceptor.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/complex/MyInterceptor.java @@ -21,13 +21,15 @@ public class MyInterceptor { public static AtomicBoolean aroundInvokeInvoked = new AtomicBoolean(false); @PreDestroy - public void preDestroy(InvocationContext ic) { + public void preDestroy(InvocationContext ic) throws Exception { preDestroyInvoked.set(true); + ic.proceed(); } @PostConstruct - public void postConstruct(InvocationContext ic) { + public void postConstruct(InvocationContext ic) throws Exception { postConstructInvoked.set(true); + ic.proceed(); } @AroundConstruct diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/AroundInvokeInterceptorNestingTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/AroundInvokeInterceptorNestingTest.java new file mode 100644 index 0000000000000..92734cdca87d1 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/AroundInvokeInterceptorNestingTest.java @@ -0,0 +1,76 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class AroundInvokeInterceptorNestingTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + MyBean bean = arc.instance(MyBean.class).get(); + assertEquals("expected-exception", bean.doSomething()); + assertEquals(List.of("MyInterceptor", "MyBean"), MyBean.invocations); + } + + @Singleton + @MyInterceptorBinding + static class MyBean { + static final List invocations = new ArrayList<>(); + + String doSomething() { + throw new IllegalStateException("should not be called"); + } + + @AroundInvoke + Object aroundInvoke(InvocationContext ctx) { + invocations.add(MyBean.class.getSimpleName()); + throw new IllegalArgumentException("intentional"); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor { + @AroundInvoke + Object aroundInvoke(InvocationContext ctx) throws Exception { + try { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } catch (IllegalArgumentException e) { + return "expected-exception"; + } + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructInterceptorNestingTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructInterceptorNestingTest.java new file mode 100644 index 0000000000000..9d2901d406e0c --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructInterceptorNestingTest.java @@ -0,0 +1,71 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PostConstructInterceptorNestingTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class).get(); + assertEquals(List.of("MyInterceptor", "MyBean", "expected-exception"), MyBean.invocations); + } + + @Singleton + @MyInterceptorBinding + static class MyBean { + static final List invocations = new ArrayList<>(); + + @PostConstruct + void postConstruct() { + invocations.add(MyBean.class.getSimpleName()); + throw new IllegalArgumentException("intentional"); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor { + @PostConstruct + void postConstruct(InvocationContext ctx) throws Exception { + try { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + ctx.proceed(); + } catch (IllegalArgumentException e) { + MyBean.invocations.add("expected-exception"); + } + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java new file mode 100644 index 0000000000000..5aa91030a8c26 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java @@ -0,0 +1,121 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PostConstructOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class); + assertEquals(List.of("Foxtrot", "MyInterceptor", "Charlie", "MyBean"), MyBean.invocations); + } + + static class Alpha { + @PostConstruct + void intercept() throws Exception { + MyBean.invocations.add("this should not be called as the method is overridden in MyBean"); + } + } + + static class Bravo extends Alpha { + @PostConstruct + void specialIntercept() { + MyBean.invocations.add("this should not be called as the method is overridden in Charlie"); + } + } + + static class Charlie extends Bravo { + @PostConstruct + void superIntercept() throws Exception { + MyBean.invocations.add(Charlie.class.getSimpleName()); + } + + @Override + void specialIntercept() { + MyBean.invocations.add("this is not an interceptor method"); + } + } + + @Singleton + @MyInterceptorBinding + static class MyBean extends Charlie { + static final List invocations = new ArrayList<>(); + + @Override + @PostConstruct + void intercept() throws Exception { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + @interface MyInterceptorBinding { + } + + static class Delta { + @PostConstruct + Object intercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add("this should not be called as the method is overridden in MyInterceptor"); + return ctx.proceed(); + } + } + + static class Echo extends Delta { + @PostConstruct + void specialIntercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add("this should not be called as the method is overridden in Foxtrot"); + } + } + + static class Foxtrot extends Echo { + @PostConstruct + Object superIntercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add(Foxtrot.class.getSimpleName()); + return ctx.proceed(); + } + + @Override + void specialIntercept(InvocationContext ctx) { + MyBean.invocations.add("this is not an interceptor method"); + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor extends Foxtrot { + @PostConstruct + Object intercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesTest.java new file mode 100644 index 0000000000000..8b524b195835f --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesTest.java @@ -0,0 +1,81 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PostConstructOnTargetClassAndOutsideAndSuperclassesTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class); + assertEquals(List.of("MyInterceptorSuperclass", "MyInterceptor", "MyBeanSuperclass", "MyBean"), MyBean.invocations); + } + + static class MyBeanSuperclass { + @PostConstruct + void superPostConstruct() { + MyBean.invocations.add(MyBeanSuperclass.class.getSimpleName()); + } + } + + @Singleton + @MyInterceptorBinding + static class MyBean extends MyBeanSuperclass { + static final List invocations = new ArrayList<>(); + + @PostConstruct + void postConstruct() { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + static class MyInterceptorSuperclass { + @PostConstruct + void superPostConstruct(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptorSuperclass.class.getSimpleName()); + ctx.proceed(); + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor extends MyInterceptorSuperclass { + @PostConstruct + Object postConstruct(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java new file mode 100644 index 0000000000000..361fb28bda5b6 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java @@ -0,0 +1,82 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PostConstructOnTargetClassAndOutsideAndSuperclassesWithOverridesTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class); + assertEquals(List.of("MyInterceptorSuperclass", "MyInterceptor", "MyBean"), MyBean.invocations); + } + + static class MyBeanSuperclass { + @PostConstruct + void postConstruct() { + MyBean.invocations.add("this should not be called as the method is overridden in MyBean"); + } + } + + @Singleton + @MyInterceptorBinding + static class MyBean extends MyBeanSuperclass { + static final List invocations = new ArrayList<>(); + + @PostConstruct + @Override + void postConstruct() { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + static class MyInterceptorSuperclass { + @PostConstruct + void superPostConstruct(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptorSuperclass.class.getSimpleName()); + ctx.proceed(); + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor extends MyInterceptorSuperclass { + @PostConstruct + Object postConstruct(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideTest.java new file mode 100644 index 0000000000000..eb5d81d6a38a7 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PostConstructOnTargetClassAndOutsideTest.java @@ -0,0 +1,66 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PostConstructOnTargetClassAndOutsideTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class); + assertEquals(List.of("MyInterceptor", "MyBean"), MyBean.invocations); + } + + @Singleton + @MyInterceptorBinding + static class MyBean { + static final List invocations = new ArrayList<>(); + + @PostConstruct + void postConstruct() { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor { + @PostConstruct + Object postConstruct(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyInterceptorNestingTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyInterceptorNestingTest.java new file mode 100644 index 0000000000000..a01def8b02324 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyInterceptorNestingTest.java @@ -0,0 +1,71 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PreDestroy; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PreDestroyInterceptorNestingTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class).destroy(); + assertEquals(List.of("MyInterceptor", "MyBean", "expected-exception"), MyBean.invocations); + } + + @Singleton + @MyInterceptorBinding + static class MyBean { + static final List invocations = new ArrayList<>(); + + @PreDestroy + void preDestroy() { + invocations.add(MyBean.class.getSimpleName()); + throw new IllegalArgumentException("intentional"); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor { + @PreDestroy + void preDestroy(InvocationContext ctx) throws Exception { + try { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + ctx.proceed(); + } catch (IllegalArgumentException e) { + MyBean.invocations.add("expected-exception"); + } + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java new file mode 100644 index 0000000000000..bb6f1e1107ad9 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest.java @@ -0,0 +1,121 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PreDestroy; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PreDestroyOnTargetClassAndOutsideAndManySuperclassesWithOverridesTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class).destroy(); + assertEquals(List.of("Foxtrot", "MyInterceptor", "Charlie", "MyBean"), MyBean.invocations); + } + + static class Alpha { + @PreDestroy + void intercept() throws Exception { + MyBean.invocations.add("this should not be called as the method is overridden in MyBean"); + } + } + + static class Bravo extends Alpha { + @PreDestroy + void specialIntercept() { + MyBean.invocations.add("this should not be called as the method is overridden in Charlie"); + } + } + + static class Charlie extends Bravo { + @PreDestroy + void superIntercept() throws Exception { + MyBean.invocations.add(Charlie.class.getSimpleName()); + } + + @Override + void specialIntercept() { + MyBean.invocations.add("this is not an interceptor method"); + } + } + + @Singleton + @MyInterceptorBinding + static class MyBean extends Charlie { + static final List invocations = new ArrayList<>(); + + @Override + @PreDestroy + void intercept() throws Exception { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + @interface MyInterceptorBinding { + } + + static class Delta { + @PreDestroy + Object intercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add("this should not be called as the method is overridden in MyInterceptor"); + return ctx.proceed(); + } + } + + static class Echo extends Delta { + @PreDestroy + void specialIntercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add("this should not be called as the method is overridden in Foxtrot"); + } + } + + static class Foxtrot extends Echo { + @PreDestroy + Object superIntercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add(Foxtrot.class.getSimpleName()); + return ctx.proceed(); + } + + @Override + void specialIntercept(InvocationContext ctx) { + MyBean.invocations.add("this is not an interceptor method"); + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor extends Foxtrot { + @PreDestroy + Object intercept(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesTest.java new file mode 100644 index 0000000000000..c95dbc00a645d --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesTest.java @@ -0,0 +1,81 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PreDestroy; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PreDestroyOnTargetClassAndOutsideAndSuperclassesTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class).destroy(); + assertEquals(List.of("MyInterceptorSuperclass", "MyInterceptor", "MyBeanSuperclass", "MyBean"), MyBean.invocations); + } + + static class MyBeanSuperclass { + @PreDestroy + void superPreDestroy() { + MyBean.invocations.add(MyBeanSuperclass.class.getSimpleName()); + } + } + + @Singleton + @MyInterceptorBinding + static class MyBean extends MyBeanSuperclass { + static final List invocations = new ArrayList<>(); + + @PreDestroy + void preDestroy() { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + static class MyInterceptorSuperclass { + @PreDestroy + void superPreDestroy(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptorSuperclass.class.getSimpleName()); + ctx.proceed(); + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor extends MyInterceptorSuperclass { + @PreDestroy + Object preDestroy(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java new file mode 100644 index 0000000000000..54bf433c895dc --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideAndSuperclassesWithOverridesTest.java @@ -0,0 +1,82 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PreDestroy; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PreDestroyOnTargetClassAndOutsideAndSuperclassesWithOverridesTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class).destroy(); + assertEquals(List.of("MyInterceptorSuperclass", "MyInterceptor", "MyBean"), MyBean.invocations); + } + + static class MyBeanSuperclass { + @PreDestroy + void preDestroy() { + MyBean.invocations.add("this should not be called as the method is overridden in MyBean"); + } + } + + @Singleton + @MyInterceptorBinding + static class MyBean extends MyBeanSuperclass { + static final List invocations = new ArrayList<>(); + + @PreDestroy + @Override + void preDestroy() { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + static class MyInterceptorSuperclass { + @PreDestroy + void superPreDestroy(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptorSuperclass.class.getSimpleName()); + ctx.proceed(); + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor extends MyInterceptorSuperclass { + @PreDestroy + Object preDestroy(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideTest.java new file mode 100644 index 0000000000000..c4fedd6617818 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/targetclass/mixed/PreDestroyOnTargetClassAndOutsideTest.java @@ -0,0 +1,66 @@ +package io.quarkus.arc.test.interceptors.targetclass.mixed; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import jakarta.annotation.PreDestroy; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; + +public class PreDestroyOnTargetClassAndOutsideTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + ArcContainer arc = Arc.container(); + arc.instance(MyBean.class).destroy(); + assertEquals(List.of("MyInterceptor", "MyBean"), MyBean.invocations); + } + + @Singleton + @MyInterceptorBinding + static class MyBean { + static final List invocations = new ArrayList<>(); + + @PreDestroy + void preDestroy() { + invocations.add(MyBean.class.getSimpleName()); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + public static class MyInterceptor { + @PreDestroy + Object preDestroy(InvocationContext ctx) throws Exception { + MyBean.invocations.add(MyInterceptor.class.getSimpleName()); + return ctx.proceed(); + } + } +} From 9ebd15f1c93a2b9dcc1e2c607401ceea3e4d1186 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Tue, 25 Apr 2023 12:00:53 +0200 Subject: [PATCH 7/9] ArC: fix bean creation in presence of @AroundConstruct interceptors `@AroundConstruct` interceptors may modify the constructor arguments using `InvocationContext.setParameters()`. ArC used to ignore this modification and call the bean constructor using the original arguments, obtained before calling the interceptors. This commit fixes that and uses the arguments array from `InvocationContext` to call the bean constructor. To do that, the forwarding function passed along the interceptor chain is no longer a `Supplier` (which used to call the bean constructor with original arguments), but a `Function` (which is passed the arguments stored in the `InvocationContext` and uses them to call the bean constructor). --- .../quarkus/arc/processor/BeanGenerator.java | 16 +++- .../arc/processor/MethodDescriptors.java | 3 +- .../AroundConstructInvocationContext.java | 12 +-- .../quarkus/arc/impl/InvocationContexts.java | 7 +- .../src/test/resources/testng.xml | 4 - ...roundConstructWithParameterChangeTest.java | 78 +++++++++++++++++++ 6 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithParameterChangeTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java index 5a7b6d3ef9ce3..ac3bac1a187c7 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java @@ -1609,10 +1609,20 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat interceptorToWrap, transientReferences, injectableCtorParams, allOtherCtorParams); // Forwarding function - // Supplier forward = () -> new SimpleBean_Subclass(ctx,lifecycleInterceptorProvider1) - FunctionCreator func = create.createFunction(Supplier.class); + // Function forward = (params) -> new SimpleBean_Subclass(params[0], ctx, lifecycleInterceptorProvider1) + FunctionCreator func = create.createFunction(Function.class); BytecodeCreator funcBytecode = func.getBytecode(); - List providerHandles = new ArrayList<>(injectableCtorParams); + List params = new ArrayList<>(); + if (!injectableCtorParams.isEmpty()) { + // `injectableCtorParams` are passed to the first interceptor in the chain + // the `Function` generated here obtains the parameter array from `InvocationContext` + // these 2 arrays have the same shape (size and element types), but not necessarily the same content + ResultHandle paramsArray = funcBytecode.checkCast(funcBytecode.getMethodParam(0), Object[].class); + for (int i = 0; i < injectableCtorParams.size(); i++) { + params.add(funcBytecode.readArrayValue(paramsArray, i)); + } + } + List providerHandles = new ArrayList<>(params); providerHandles.addAll(allOtherCtorParams); ResultHandle retHandle = newInstanceHandle(bean, beanCreator, funcBytecode, create, providerType.className(), baseName, diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java index a5dc7fad43c2f..4f5ebd821a8ed 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.Function; import java.util.function.Supplier; import jakarta.enterprise.context.spi.Context; @@ -173,7 +174,7 @@ public final class MethodDescriptors { public static final MethodDescriptor INVOCATION_CONTEXTS_AROUND_CONSTRUCT = MethodDescriptor.ofMethod( InvocationContexts.class, "aroundConstruct", - InvocationContext.class, Constructor.class, Object[].class, List.class, Supplier.class, Set.class); + InvocationContext.class, Constructor.class, Object[].class, List.class, Function.class, Set.class); public static final MethodDescriptor INVOCATION_CONTEXTS_POST_CONSTRUCT = MethodDescriptor.ofMethod( InvocationContexts.class, diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java index d61b927ac3a18..2455b8b40eaff 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java @@ -4,7 +4,7 @@ import java.lang.reflect.Constructor; import java.util.List; import java.util.Set; -import java.util.function.Supplier; +import java.util.function.Function; /** * An {@code InvocationContext} implementation used for {@code AroundConstruct} callbacks. @@ -12,17 +12,17 @@ class AroundConstructInvocationContext extends LifecycleCallbackInvocationContext { private final Constructor constructor; - private final Supplier aroundConstructForward; + private final Function forward; AroundConstructInvocationContext(Constructor constructor, Object[] parameters, Set interceptorBindings, - List chain, Supplier aroundConstructForward) { + List chain, Function forward) { super(null, parameters, interceptorBindings, chain); - this.aroundConstructForward = aroundConstructForward; + this.forward = forward; this.constructor = constructor; } - protected void interceptorChainCompleted() throws Exception { - target = aroundConstructForward.get(); + protected void interceptorChainCompleted() { + target = forward.apply(parameters); } @Override diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java index b7e0319ec6f1d..1650a9014725e 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java @@ -5,7 +5,7 @@ import java.util.List; import java.util.Set; import java.util.function.BiFunction; -import java.util.function.Supplier; +import java.util.function.Function; import jakarta.interceptor.InvocationContext; @@ -77,10 +77,9 @@ public static InvocationContext preDestroy(Object target, List constructor, Object[] parameters, List chain, - Supplier aroundConstructForward, + Function forward, Set interceptorBindings) { - return new AroundConstructInvocationContext(constructor, parameters, interceptorBindings, chain, - aroundConstructForward); + return new AroundConstructInvocationContext(constructor, parameters, interceptorBindings, chain, forward); } /** diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index e373449fceee7..ea2fbd43c7997 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -79,8 +79,6 @@ - - @@ -91,8 +89,6 @@ - - diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithParameterChangeTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithParameterChangeTest.java new file mode 100644 index 0000000000000..b036403615f3e --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructWithParameterChangeTest.java @@ -0,0 +1,78 @@ +package io.quarkus.arc.test.interceptors.aroundconstruct; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.annotation.Priority; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import jakarta.interceptor.AroundConstruct; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class AroundConstructWithParameterChangeTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(SimpleBean.class, MyDependency.class, + MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void test() { + SimpleBean simpleBean = Arc.container().instance(SimpleBean.class).get(); + assertNotNull(simpleBean); + assertNotNull(simpleBean.dependency); + assertEquals("from interceptor", simpleBean.dependency.value); + } + + @Singleton + @MyInterceptorBinding + static class SimpleBean { + final MyDependency dependency; + + @Inject + SimpleBean(MyDependency dependency) { + this.dependency = dependency; + } + } + + @Singleton + static class MyDependency { + final String value; + + MyDependency() { + this("default"); + } + + MyDependency(String value) { + this.value = value; + } + } + + @Target({ ElementType.TYPE, ElementType.CONSTRUCTOR }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor { + @AroundConstruct + void aroundConstruct(InvocationContext ctx) throws Exception { + ctx.setParameters(new Object[] { new MyDependency("from interceptor") }); + ctx.proceed(); + } + } +} From bcf76ea80a5200925675c4d37b8ffe1db0a343e0 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Tue, 25 Apr 2023 12:15:10 +0200 Subject: [PATCH 8/9] ArC: fix wrapping exceptions thrown by @AroundConstruct and @PostConstruct interceptors ArC used to always wrap exceptions thrown by `@AroundConstruct` and `@PostConstruct` interceptors into `RuntimeException`, even if these exceptions were themselves `RuntimeException`s. This commit fixes that and only wraps checked exceptions. --- .../quarkus/arc/processor/BeanGenerator.java | 4 + .../src/test/resources/testng.xml | 10 --- .../AroundConstructExceptionHandlingTest.java | 85 +++++++++++++++++++ .../PostConstructExceptionHandlingTest.java | 85 +++++++++++++++++++ 4 files changed, 174 insertions(+), 10 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructExceptionHandlingTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postconstruct/PostConstructExceptionHandlingTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java index ac3bac1a187c7..1c6d143763c31 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java @@ -1653,6 +1653,8 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat create.invokeStaticMethod(MethodDescriptors.SETS_OF, bindingsArray)); TryBlock tryCatch = create.tryBlock(); CatchBlockCreator exceptionCatch = tryCatch.addCatch(Exception.class); + exceptionCatch.ifFalse(exceptionCatch.instanceOf(exceptionCatch.getCaughtException(), RuntimeException.class)) + .falseBranch().throwException(exceptionCatch.getCaughtException()); // throw new RuntimeException(e) exceptionCatch.throwException(RuntimeException.class, "Error invoking aroundConstructs", exceptionCatch.getCaughtException()); @@ -1836,6 +1838,8 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat TryBlock tryCatch = create.tryBlock(); CatchBlockCreator exceptionCatch = tryCatch.addCatch(Exception.class); + exceptionCatch.ifFalse(exceptionCatch.instanceOf(exceptionCatch.getCaughtException(), RuntimeException.class)) + .falseBranch().throwException(exceptionCatch.getCaughtException()); // throw new RuntimeException(e) exceptionCatch.throwException(RuntimeException.class, "Error invoking postConstructs", exceptionCatch.getCaughtException()); diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index ea2fbd43c7997..9f4ccc7df05bf 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -77,21 +77,11 @@ - - - - - - - - - - diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructExceptionHandlingTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructExceptionHandlingTest.java new file mode 100644 index 0000000000000..2b1b43af8fbf8 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/aroundconstruct/AroundConstructExceptionHandlingTest.java @@ -0,0 +1,85 @@ +package io.quarkus.arc.test.interceptors.aroundconstruct; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.AroundConstruct; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class AroundConstructExceptionHandlingTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(SimpleBean.class, + MyInterceptorBinding.class, MyInterceptor1.class, MyInterceptor2.class); + + @Test + public void test() { + IllegalStateException e = assertThrows(IllegalStateException.class, () -> { + Arc.container().instance(SimpleBean.class).get(); + }); + + assertTrue(MyInterceptor1.intercepted); + assertTrue(MyInterceptor2.intercepted); + + assertNotNull(e.getCause()); + assertEquals(IllegalArgumentException.class, e.getCause().getClass()); + assertEquals("intentional", e.getCause().getMessage()); + } + + @Singleton + @MyInterceptorBinding + static class SimpleBean { + } + + @Target({ ElementType.TYPE, ElementType.CONSTRUCTOR }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor1 { + static boolean intercepted = false; + + @AroundConstruct + void aroundConstruct(InvocationContext ctx) throws Exception { + intercepted = true; + try { + ctx.proceed(); + } catch (IllegalArgumentException e) { + throw new IllegalStateException(e); + } + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(2) + static class MyInterceptor2 { + static boolean intercepted = false; + + @AroundConstruct + void aroundConstruct(InvocationContext ctx) { + intercepted = true; + throw new IllegalArgumentException("intentional"); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postconstruct/PostConstructExceptionHandlingTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postconstruct/PostConstructExceptionHandlingTest.java new file mode 100644 index 0000000000000..7d3c2b706c01d --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postconstruct/PostConstructExceptionHandlingTest.java @@ -0,0 +1,85 @@ +package io.quarkus.arc.test.interceptors.postconstruct; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class PostConstructExceptionHandlingTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(SimpleBean.class, + MyInterceptorBinding.class, MyInterceptor1.class, MyInterceptor2.class); + + @Test + public void test() { + IllegalStateException e = assertThrows(IllegalStateException.class, () -> { + Arc.container().instance(SimpleBean.class).get(); + }); + + assertTrue(MyInterceptor1.intercepted); + assertTrue(MyInterceptor2.intercepted); + + assertNotNull(e.getCause()); + assertEquals(IllegalArgumentException.class, e.getCause().getClass()); + assertEquals("intentional", e.getCause().getMessage()); + } + + @Singleton + @MyInterceptorBinding + static class SimpleBean { + } + + @Target({ ElementType.TYPE, ElementType.CONSTRUCTOR }) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor1 { + static boolean intercepted = false; + + @PostConstruct + void postConstruct(InvocationContext ctx) throws Exception { + intercepted = true; + try { + ctx.proceed(); + } catch (IllegalArgumentException e) { + throw new IllegalStateException(e); + } + } + } + + @MyInterceptorBinding + @Interceptor + @Priority(2) + static class MyInterceptor2 { + static boolean intercepted = false; + + @PostConstruct + void postConstruct(InvocationContext ctx) { + intercepted = true; + throw new IllegalArgumentException("intentional"); + } + } +} From 6b04b3c5c732e2ac00876a7e674908a3251ddae8 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Tue, 25 Apr 2023 15:57:37 +0200 Subject: [PATCH 9/9] ArC: fix generated Bean.destroy() in case a user directly passes in a client proxy ArC callers of `Bean.destroy()` never pass in a client proxy, but there may be direct callers too, even though this API is low-level. This commit unwraps the client proxy if it was passed to `destroy()` before proceeding, which avoids a `ClassCastException` in case the bean has a `@PreDestroy` interceptor (and hence the contextual instance is a generated subclass). --- .../quarkus/arc/processor/BeanGenerator.java | 11 ++- .../arc/processor/MethodDescriptors.java | 3 + .../src/test/resources/testng.xml | 1 + ...eDestroyInterceptorAndClientProxyTest.java | 80 +++++++++++++++++++ 4 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/predestroy/PreDestroyInterceptorAndClientProxyTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java index 1c6d143763c31..4a020beeeb6af 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java @@ -819,6 +819,11 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide if (bean.isClassBean()) { if (!bean.isInterceptor()) { + // in case someone calls `Bean.destroy()` directly (i.e., they use the low-level CDI API), + // they may pass us a client proxy + ResultHandle instance = destroy.invokeStaticInterfaceMethod(MethodDescriptors.CLIENT_PROXY_UNWRAP, + destroy.getMethodParam(0)); + // if there's no `@PreDestroy` interceptor, we'll generate code to invoke `@PreDestroy` callbacks // directly into the `destroy` method: // @@ -846,7 +851,7 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide destroy.invokeVirtualMethod( MethodDescriptor.ofMethod(SubclassGenerator.generatedName(bean.getProviderType().name(), baseName), SubclassGenerator.DESTROY_METHOD_NAME, void.class, Runnable.class), - destroy.getMethodParam(0), preDestroyForwarder.getInstance()); + instance, preDestroyForwarder.getInstance()); } // PreDestroy callbacks @@ -865,11 +870,11 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide preDestroyBytecode.loadClass(callback.declaringClass().name().toString()), preDestroyBytecode.load(callback.name()), preDestroyBytecode.newArray(Class.class, preDestroyBytecode.load(0)), - destroy.getMethodParam(0), + instance, preDestroyBytecode.newArray(Object.class, preDestroyBytecode.load(0))); } else { // instance.superCoolDestroyCallback() - preDestroyBytecode.invokeVirtualMethod(MethodDescriptor.of(callback), destroy.getMethodParam(0)); + preDestroyBytecode.invokeVirtualMethod(MethodDescriptor.of(callback), instance); } } if (preDestroyBytecode != destroy) { diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java index 4f5ebd821a8ed..b6f3c26ea216b 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java @@ -145,6 +145,9 @@ public final class MethodDescriptors { public static final MethodDescriptor CLIENT_PROXY_GET_CONTEXTUAL_INSTANCE = MethodDescriptor.ofMethod(ClientProxy.class, ClientProxyGenerator.GET_CONTEXTUAL_INSTANCE_METHOD_NAME, Object.class); + public static final MethodDescriptor CLIENT_PROXY_UNWRAP = MethodDescriptor.ofMethod(ClientProxy.class, + "unwrap", Object.class, Object.class); + public static final MethodDescriptor INJECTABLE_BEAN_DESTROY = MethodDescriptor.ofMethod(InjectableBean.class, "destroy", void.class, Object.class, CreationalContext.class); diff --git a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml index 9f4ccc7df05bf..eaf1958ef4364 100644 --- a/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml +++ b/independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml @@ -43,6 +43,7 @@ + diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/predestroy/PreDestroyInterceptorAndClientProxyTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/predestroy/PreDestroyInterceptorAndClientProxyTest.java new file mode 100644 index 0000000000000..87c45c0f8bd3f --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/predestroy/PreDestroyInterceptorAndClientProxyTest.java @@ -0,0 +1,80 @@ +package io.quarkus.arc.test.interceptors.predestroy; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.annotation.PreDestroy; +import jakarta.annotation.Priority; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.spi.CreationalContext; +import jakarta.enterprise.inject.spi.Bean; +import jakarta.enterprise.inject.spi.BeanManager; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ClientProxy; +import io.quarkus.arc.InstanceHandle; +import io.quarkus.arc.test.ArcTestContainer; + +public class PreDestroyInterceptorAndClientProxyTest { + @RegisterExtension + ArcTestContainer container = new ArcTestContainer(MyBean.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void bagr() { + InstanceHandle handle = Arc.container().instance(MyBean.class); + handle.destroy(); + } + + @Test + public void test() { + BeanManager beanManager = Arc.container().beanManager(); + + Bean bean = (Bean) beanManager.resolve(beanManager.getBeans(MyBean.class)); + CreationalContext ctx = beanManager.createCreationalContext(bean); + + MyBean instance = (MyBean) beanManager.getReference(bean, MyBean.class, ctx); + assertNotNull(instance); + assertInstanceOf(ClientProxy.class, instance); + + assertFalse(MyInterceptor.intercepted); + bean.destroy(instance, ctx); + assertTrue(MyInterceptor.intercepted); + } + + @ApplicationScoped + @MyInterceptorBinding + static class MyBean { + } + + @Target(ElementType.TYPE) + @Retention(RetentionPolicy.RUNTIME) + @InterceptorBinding + @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor { + static boolean intercepted = false; + + @PreDestroy + void intercept(InvocationContext ctx) throws Exception { + intercepted = true; + ctx.proceed(); + } + } +}