From 58a5197d48b8ac071aef7d0ed79bd124de189369 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Fri, 27 Jan 2023 09:40:47 +0100 Subject: [PATCH] ArC - use reflection fallback for PreDestroy callbacks if needed - resolves #30636 --- .../quarkus/arc/processor/BeanGenerator.java | 21 ++++++---- .../postConstruct/inherited/OriginalBean.java | 21 ---------- ...gePrivatePostConstructInheritanceTest.java | 29 ------------- .../inherited/OriginalBean.java | 30 +++++++++++++ ...PackagePrivateCallbackInheritanceTest.java | 42 +++++++++++++++++++ .../inherited/subpackage/AlternativeBean.java | 4 +- 6 files changed, 87 insertions(+), 60 deletions(-) delete mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/OriginalBean.java delete mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivatePostConstructInheritanceTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/OriginalBean.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/PackagePrivateCallbackInheritanceTest.java rename independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/{interceptors/postConstruct => lifecyclecallbacks}/inherited/subpackage/AlternativeBean.java (67%) 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 108e9dc8ea88f..828fb73107911 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 @@ -279,7 +279,8 @@ Collection generateSyntheticBean(BeanInfo bean) { implementGetIdentifier(bean, beanCreator); implementSupplierGet(beanCreator); if (bean.hasDestroyLogic()) { - implementDestroy(bean, beanCreator, providerType, Collections.emptyMap(), isApplicationClass, baseName); + implementDestroy(bean, beanCreator, providerType, Collections.emptyMap(), isApplicationClass, baseName, + targetPackage); } implementCreate(classOutput, beanCreator, bean, providerType, baseName, Collections.emptyMap(), Collections.emptyMap(), @@ -363,7 +364,7 @@ Collection generateClassBean(BeanInfo bean, ClassInfo beanClass) { implementSupplierGet(beanCreator); if (bean.hasDestroyLogic()) { implementDestroy(bean, beanCreator, providerType, injectionPointToProviderSupplierField, isApplicationClass, - baseName); + baseName, targetPackage); } implementCreate(classOutput, beanCreator, bean, providerType, baseName, injectionPointToProviderSupplierField, @@ -448,7 +449,8 @@ Collection generateProducerMethodBean(BeanInfo bean, MethodInfo produc implementGetIdentifier(bean, beanCreator); implementSupplierGet(beanCreator); if (bean.hasDestroyLogic()) { - implementDestroy(bean, beanCreator, providerType, injectionPointToProviderField, isApplicationClass, baseName); + implementDestroy(bean, beanCreator, providerType, injectionPointToProviderField, isApplicationClass, baseName, + targetPackage); } implementCreate(classOutput, beanCreator, bean, providerType, baseName, injectionPointToProviderField, @@ -529,7 +531,7 @@ Collection generateProducerFieldBean(BeanInfo bean, FieldInfo producer implementGetIdentifier(bean, beanCreator); implementSupplierGet(beanCreator); if (bean.hasDestroyLogic()) { - implementDestroy(bean, beanCreator, providerType, null, isApplicationClass, baseName); + implementDestroy(bean, beanCreator, providerType, null, isApplicationClass, baseName, targetPackage); } implementCreate(classOutput, beanCreator, bean, providerType, baseName, Collections.emptyMap(), Collections.emptyMap(), @@ -795,7 +797,8 @@ protected MethodCreator initConstructor(ClassOutput classOutput, ClassCreator be } protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, ProviderType providerType, - Map injectionPointToProviderField, boolean isApplicationClass, String baseName) { + Map injectionPointToProviderField, boolean isApplicationClass, String baseName, + String targetPackage) { MethodCreator destroy = beanCreator .getMethodCreator("destroy", void.class, providerType.descriptorName(), CreationalContext.class) @@ -817,9 +820,11 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide DotNames.PRE_DESTROY, bean.getDeployment().getBeanArchiveIndex()); for (MethodInfo callback : preDestroyCallbacks) { - if (Modifier.isPrivate(callback.flags())) { - privateMembers.add(isApplicationClass, String.format("@PreDestroy callback %s#%s()", - callback.declaringClass().name(), callback.name())); + if (isReflectionFallbackNeeded(callback, targetPackage)) { + if (Modifier.isPrivate(callback.flags())) { + privateMembers.add(isApplicationClass, String.format("@PreDestroy callback %s#%s()", + callback.declaringClass().name(), callback.name())); + } reflectionRegistration.registerMethod(callback); destroy.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, destroy.loadClass(callback.declaringClass().name().toString()), diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/OriginalBean.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/OriginalBean.java deleted file mode 100644 index c3ed4d97a72fd..0000000000000 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/OriginalBean.java +++ /dev/null @@ -1,21 +0,0 @@ -package io.quarkus.arc.test.interceptors.postConstruct.inherited; - -import javax.annotation.PostConstruct; -import javax.enterprise.context.ApplicationScoped; - -@ApplicationScoped -public class OriginalBean { - - public static int TIMES_INVOKED = 0; - - // package private, not visible from AlternativeBean without reflection access - @PostConstruct - void postConstruct() { - TIMES_INVOKED++; - } - - public String ping() { - return OriginalBean.class.getSimpleName(); - } - -} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivatePostConstructInheritanceTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivatePostConstructInheritanceTest.java deleted file mode 100644 index 2b7b98840924f..0000000000000 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivatePostConstructInheritanceTest.java +++ /dev/null @@ -1,29 +0,0 @@ -package io.quarkus.arc.test.interceptors.postConstruct.inherited; - -import org.junit.jupiter.api.Assertions; -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.InstanceHandle; -import io.quarkus.arc.test.ArcTestContainer; -import io.quarkus.arc.test.interceptors.postConstruct.inherited.subpackage.AlternativeBean; - -public class PackagePrivatePostConstructInheritanceTest { - - @RegisterExtension - public ArcTestContainer container = new ArcTestContainer(AlternativeBean.class, OriginalBean.class); - - @Test - public void testObserverCanBeInvoked() { - ArcContainer container = Arc.container(); - InstanceHandle origBeanInstance = container.instance(OriginalBean.class); - InstanceHandle alternativeBeanInstance = container.instance(AlternativeBean.class); - Assertions.assertTrue(origBeanInstance.isAvailable()); - Assertions.assertTrue(alternativeBeanInstance.isAvailable()); - Assertions.assertTrue(origBeanInstance.get().ping().equals(alternativeBeanInstance.get().ping())); - // post construct should be invoked via during creation of AlternativeBean even though it's pack-private - Assertions.assertEquals(1, OriginalBean.TIMES_INVOKED); - } -} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/OriginalBean.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/OriginalBean.java new file mode 100644 index 0000000000000..38341d345c33c --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/OriginalBean.java @@ -0,0 +1,30 @@ +package io.quarkus.arc.test.lifecyclecallbacks.inherited; + +import java.util.concurrent.atomic.AtomicBoolean; + +import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; +import javax.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class OriginalBean { + + static final AtomicBoolean POST_CONSTRUCT = new AtomicBoolean(); + static final AtomicBoolean PRE_DESTROY = new AtomicBoolean(); + + // package private, not visible from AlternativeBean without reflection access + @PostConstruct + void postConstruct() { + POST_CONSTRUCT.set(true); + } + + @PreDestroy + void preDestroy() { + PRE_DESTROY.set(true); + } + + public String ping() { + return OriginalBean.class.getSimpleName(); + } + +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/PackagePrivateCallbackInheritanceTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/PackagePrivateCallbackInheritanceTest.java new file mode 100644 index 0000000000000..c8c54c1025a19 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/PackagePrivateCallbackInheritanceTest.java @@ -0,0 +1,42 @@ +package io.quarkus.arc.test.lifecyclecallbacks.inherited; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +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.InstanceHandle; +import io.quarkus.arc.test.ArcTestContainer; +import io.quarkus.arc.test.lifecyclecallbacks.inherited.subpackage.AlternativeBean; + +public class PackagePrivateCallbackInheritanceTest { + + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(AlternativeBean.class, OriginalBean.class); + + @Test + public void testCallbacks() { + ArcContainer container = Arc.container(); + InstanceHandle origBeanInstance = container.instance(OriginalBean.class); + InstanceHandle alternativeBeanInstance = container.instance(AlternativeBean.class); + + assertTrue(origBeanInstance.isAvailable()); + assertTrue(alternativeBeanInstance.isAvailable()); + + // AlternativeBean overrides the OriginalBean + assertEquals(origBeanInstance.getBean(), alternativeBeanInstance.getBean()); + assertEquals(AlternativeBean.class.getSimpleName(), alternativeBeanInstance.get().ping()); + assertTrue(origBeanInstance.get().ping().equals(alternativeBeanInstance.get().ping())); + + // post construct should be invoked via during creation of AlternativeBean even though it's pack-private + assertTrue(OriginalBean.POST_CONSTRUCT.get()); + + alternativeBeanInstance.destroy(); + + // pre destroy should be invoked even though it's package-private and AlternativeBean lives in a different package + assertTrue(OriginalBean.PRE_DESTROY.get()); + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/subpackage/AlternativeBean.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/subpackage/AlternativeBean.java similarity index 67% rename from independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/subpackage/AlternativeBean.java rename to independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/subpackage/AlternativeBean.java index 3c6403ade99d7..2829a93657c2f 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/subpackage/AlternativeBean.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lifecyclecallbacks/inherited/subpackage/AlternativeBean.java @@ -1,10 +1,10 @@ -package io.quarkus.arc.test.interceptors.postConstruct.inherited.subpackage; +package io.quarkus.arc.test.lifecyclecallbacks.inherited.subpackage; import javax.annotation.Priority; import javax.enterprise.context.ApplicationScoped; import javax.enterprise.inject.Alternative; -import io.quarkus.arc.test.interceptors.postConstruct.inherited.OriginalBean; +import io.quarkus.arc.test.lifecyclecallbacks.inherited.OriginalBean; @Alternative @ApplicationScoped