From a94ad694b7deaace812c775b06110ce64d11ed17 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 | 23 ++++++---- .../postConstruct/inherited/OriginalBean.java | 13 +++++- ...PackagePrivateCallbackInheritanceTest.java | 42 +++++++++++++++++++ ...gePrivatePostConstructInheritanceTest.java | 29 ------------- 4 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivateCallbackInheritanceTest.java delete mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivatePostConstructInheritanceTest.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 108e9dc8ea88f..c720b9fe49ca2 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(), @@ -617,7 +619,7 @@ protected void createConstructor(ClassOutput classOutput, ClassCreator beanCreat initConstructor(classOutput, beanCreator, bean, injectionPointToProviderField, interceptorToProviderField, decoratorToProviderSupplierField, annotationLiterals, reflectionRegistration) - .returnValue(null); + .returnValue(null); } protected MethodCreator initConstructor(ClassOutput classOutput, ClassCreator beanCreator, BeanInfo bean, @@ -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 index c3ed4d97a72fd..8feed0df65b2c 100644 --- 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 @@ -1,17 +1,26 @@ package io.quarkus.arc.test.interceptors.postConstruct.inherited; +import java.util.concurrent.atomic.AtomicBoolean; + import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; import javax.enterprise.context.ApplicationScoped; @ApplicationScoped public class OriginalBean { - public static int TIMES_INVOKED = 0; + 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() { - TIMES_INVOKED++; + POST_CONSTRUCT.set(true); + } + + @PreDestroy + void preDestroy() { + PRE_DESTROY.set(true); } public String ping() { diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivateCallbackInheritanceTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivateCallbackInheritanceTest.java new file mode 100644 index 0000000000000..cd3d296f9d7b3 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/postConstruct/inherited/PackagePrivateCallbackInheritanceTest.java @@ -0,0 +1,42 @@ +package io.quarkus.arc.test.interceptors.postConstruct.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.interceptors.postConstruct.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/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); - } -}