From 7df5c21371b14b7945f1e2c2a5bd4639f1b43be2 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 23 Jan 2023 15:47:56 +0100 Subject: [PATCH 1/5] ArC: fix obtaining priority of a producer-based alternative from class-level @Priority --- .../java/io/quarkus/arc/processor/Beans.java | 4 +- ...lternativeProducerPriorityOnClassTest.java | 60 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/alternatives/priority/AlternativeProducerPriorityOnClassTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Beans.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Beans.java index 5a02c4824185b..b22eaeaa4e83b 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Beans.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Beans.java @@ -172,7 +172,7 @@ static BeanInfo createProducerMethod(Set beanTypes, MethodInfo producerMet if (isAlternative) { if (priority == null) { - priority = declaringBean.getAlternativePriority(); + priority = declaringBean.getPriority(); } priority = initAlternativePriority(producerMethod, priority, stereotypes, beanDeployment); if (priority == null) { @@ -287,7 +287,7 @@ static BeanInfo createProducerField(FieldInfo producerField, BeanInfo declaringB if (isAlternative) { if (priority == null) { - priority = declaringBean.getAlternativePriority(); + priority = declaringBean.getPriority(); } priority = initAlternativePriority(producerField, priority, stereotypes, beanDeployment); // after all attempts, priority is still null diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/alternatives/priority/AlternativeProducerPriorityOnClassTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/alternatives/priority/AlternativeProducerPriorityOnClassTest.java new file mode 100644 index 0000000000000..298d1a508aa2a --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/alternatives/priority/AlternativeProducerPriorityOnClassTest.java @@ -0,0 +1,60 @@ +package io.quarkus.arc.test.alternatives.priority; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import javax.annotation.Priority; +import javax.enterprise.context.Dependent; +import javax.enterprise.inject.Alternative; +import javax.enterprise.inject.Produces; +import javax.inject.Inject; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; +import io.quarkus.arc.test.MyQualifier; + +public class AlternativeProducerPriorityOnClassTest { + @RegisterExtension + ArcTestContainer testContainer = new ArcTestContainer(Producer.class, Consumer.class); + + @Test + public void testAlternativePriorityResolution() { + Consumer bean = Arc.container().instance(Consumer.class).get(); + assertNotNull(bean.foo); + assertNotNull(bean.bar); + } + + static class Foo { + } + + @MyQualifier + static class Bar { + } + + @Dependent + @Priority(1000) + static class Producer { + @Produces + @Alternative + Foo produceFoo = new Foo(); + + @Produces + @Alternative + @MyQualifier + Bar produceBar() { + return new Bar(); + } + } + + @Dependent + static class Consumer { + @Inject + Foo foo; + + @Inject + @MyQualifier + Bar bar; + } +} From 605f5b0ac3217c808e714be6c8e85045f88561cb Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Wed, 25 Jan 2023 14:08:36 +0100 Subject: [PATCH 2/5] ArC: fix generating Bean.create() to wrap checked exceptions in CreationException --- .../quarkus/arc/processor/BeanGenerator.java | 37 +++++--- .../java/io/quarkus/arc/impl/Reflections.java | 14 ++- .../test/bean/create/BeanCreateErrorTest.java | 86 +++++++++++++++++++ 3 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/create/BeanCreateErrorTest.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 828fb73107911..279f4b60c6687 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 @@ -940,22 +940,23 @@ protected void implementCreate(ClassOutput classOutput, ClassCreator beanCreator Map decoratorToProviderSupplierField, String targetPackage, boolean isApplicationClass) { - MethodCreator create = beanCreator.getMethodCreator("create", providerType.descriptorName(), CreationalContext.class) - .setModifiers(ACC_PUBLIC); + MethodCreator doCreate = beanCreator + .getMethodCreator("doCreate", providerType.descriptorName(), CreationalContext.class) + .setModifiers(ACC_PRIVATE); if (bean.isClassBean()) { implementCreateForClassBean(classOutput, beanCreator, bean, providerType, baseName, injectionPointToProviderSupplierField, interceptorToProviderSupplierField, decoratorToProviderSupplierField, reflectionRegistration, - targetPackage, isApplicationClass, create); + targetPackage, isApplicationClass, doCreate); } else if (bean.isProducerMethod()) { implementCreateForProducerMethod(classOutput, beanCreator, bean, providerType, baseName, injectionPointToProviderSupplierField, reflectionRegistration, - targetPackage, isApplicationClass, create); + targetPackage, isApplicationClass, doCreate); } else if (bean.isProducerField()) { implementCreateForProducerField(classOutput, beanCreator, bean, providerType, baseName, injectionPointToProviderSupplierField, reflectionRegistration, - targetPackage, isApplicationClass, create); + targetPackage, isApplicationClass, doCreate); } else if (bean.isSynthetic()) { if (bean.getScope().isNormal()) { // Normal scoped synthetic beans should never return null @@ -963,21 +964,37 @@ protected void implementCreate(ClassOutput classOutput, ClassCreator beanCreator .getMethodCreator("createSynthetic", providerType.descriptorName(), CreationalContext.class) .setModifiers(ACC_PRIVATE); bean.getCreatorConsumer().accept(createSynthetic); - ResultHandle ret = create.invokeVirtualMethod(createSynthetic.getMethodDescriptor(), create.getThis(), - create.getMethodParam(0)); - BytecodeCreator nullBeanInstance = create.ifNull(ret).trueBranch(); + ResultHandle ret = doCreate.invokeVirtualMethod(createSynthetic.getMethodDescriptor(), doCreate.getThis(), + doCreate.getMethodParam(0)); + BytecodeCreator nullBeanInstance = doCreate.ifNull(ret).trueBranch(); StringBuilderGenerator message = Gizmo.newStringBuilder(nullBeanInstance); message.append("Null contextual instance was produced by a normal scoped synthetic bean: "); message.append(Gizmo.toString(nullBeanInstance, nullBeanInstance.getThis())); ResultHandle e = nullBeanInstance.newInstance( MethodDescriptor.ofConstructor(CreationException.class, String.class), message.callToString()); nullBeanInstance.throwException(e); - create.returnValue(ret); + doCreate.returnValue(ret); } else { - bean.getCreatorConsumer().accept(create); + bean.getCreatorConsumer().accept(doCreate); } } + MethodCreator create = beanCreator.getMethodCreator("create", providerType.descriptorName(), CreationalContext.class) + .setModifiers(ACC_PUBLIC); + TryBlock tryBlock = create.tryBlock(); + tryBlock.returnValue( + tryBlock.invokeSpecialMethod(doCreate.getMethodDescriptor(), tryBlock.getThis(), tryBlock.getMethodParam(0))); + // `Reflections.newInstance()` throws `CreationException` on its own, + // but that's handled like all other `RuntimeException`s + // also ignore custom Throwables, they are virtually never used in practice + CatchBlockCreator catchBlock = tryBlock.addCatch(Exception.class); + catchBlock.ifFalse(catchBlock.instanceOf(catchBlock.getCaughtException(), RuntimeException.class)) + .falseBranch().throwException(catchBlock.getCaughtException()); + ResultHandle creationException = catchBlock.newInstance( + MethodDescriptor.ofConstructor(CreationException.class, Throwable.class), + catchBlock.getCaughtException()); + catchBlock.throwException(creationException); + // Bridge method needed MethodCreator bridgeCreate = beanCreator.getMethodCreator("create", Object.class, CreationalContext.class) .setModifiers(ACC_PUBLIC | ACC_BRIDGE); diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Reflections.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Reflections.java index 465a338fd01d7..804d48862b104 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Reflections.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Reflections.java @@ -21,6 +21,8 @@ import java.util.Set; import java.util.function.Function; +import javax.enterprise.inject.CreationException; + /** * Neither the class nor its methods are considered a public API and should only be used internally. */ @@ -129,7 +131,17 @@ public static Object newInstance(Class clazz, Class[] parameterTypes, Obje } try { return constructor.newInstance(args); - } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } + if (cause instanceof Error) { + throw (Error) cause; + } + // this method is only used to instantiate beans, so throwing `CreationException` is fine + throw new CreationException(cause); + } catch (InstantiationException | IllegalAccessException | IllegalArgumentException e) { throw new RuntimeException("Cannot invoke constructor: " + clazz.getName(), e); } } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/create/BeanCreateErrorTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/create/BeanCreateErrorTest.java new file mode 100644 index 0000000000000..57c04fbc7fc79 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/create/BeanCreateErrorTest.java @@ -0,0 +1,86 @@ +package io.quarkus.arc.test.bean.create; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.math.BigDecimal; +import java.math.BigInteger; + +import javax.enterprise.context.Dependent; +import javax.enterprise.inject.CreationException; +import javax.enterprise.inject.Produces; +import javax.inject.Inject; + +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 BeanCreateErrorTest { + @RegisterExtension + ArcTestContainer container = new ArcTestContainer(CheckedExceptionBean.class, CheckedExceptionProducerBean.class, + UncheckedExceptionBean.class, UncheckedExceptionProducerBean.class); + + @Test + public void checkedExceptionWrapped() { + // Test that the checked exception is wrapped + + CreationException exception = assertThrows(CreationException.class, () -> { + Arc.container().instance(CheckedExceptionBean.class); + }); + assertEquals("foo", exception.getCause().getMessage()); + + exception = assertThrows(CreationException.class, () -> { + Arc.container().instance(BigInteger.class); + }); + assertEquals("bar", exception.getCause().getMessage()); + } + + @Test + public void uncheckedExceptionNotWrapped() { + // Test that the unchecked exception is _not_ wrapped + + assertThrows(IllegalArgumentException.class, () -> { + Arc.container().instance(UncheckedExceptionBean.class); + }); + + assertThrows(IllegalStateException.class, () -> { + Arc.container().instance(BigDecimal.class); + }); + } + + @Dependent + static class CheckedExceptionBean { + @Inject + public CheckedExceptionBean() throws Exception { + throw new Exception("foo"); + } + } + + @Dependent + static class CheckedExceptionProducerBean { + @Produces + @Dependent + BigInteger produce() throws Exception { + throw new Exception("bar"); + } + } + + @Dependent + static class UncheckedExceptionBean { + @Inject + public UncheckedExceptionBean() { + throw new IllegalArgumentException(); + } + } + + @Dependent + static class UncheckedExceptionProducerBean { + @Produces + @Dependent + BigDecimal produce() { + throw new IllegalStateException(); + } + } +} From ac7c0b0bd0ed79e3aa465f73f04af0b727c8d272 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 27 Jan 2023 15:12:52 +0100 Subject: [PATCH 3/5] ArC: add support for static disposer methods This commit also fixes a NPE in case of static producer method declared on a `@Dependent` bean. --- .../quarkus/arc/processor/BeanGenerator.java | 44 ++++--- .../producer/disposer/StaticDisposerTest.java | 112 ++++++++++++++++++ 2 files changed, 140 insertions(+), 16 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/producer/disposer/StaticDisposerTest.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 279f4b60c6687..fea18c51b3f30 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 @@ -846,6 +846,7 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide // Invoke the disposer method // declaringProvider.get(new CreationalContextImpl<>()).dispose() MethodInfo disposerMethod = bean.getDisposer().getDisposerMethod(); + boolean isStatic = Modifier.isStatic(disposerMethod.flags()); ResultHandle declaringProviderSupplierHandle = destroy.readInstanceField( FieldDescriptor.of(beanCreator.getClassName(), FIELD_NAME_DECLARING_PROVIDER_SUPPLIER, @@ -855,15 +856,21 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide MethodDescriptors.SUPPLIER_GET, declaringProviderSupplierHandle); ResultHandle ctxHandle = destroy.newInstance( MethodDescriptor.ofConstructor(CreationalContextImpl.class, Contextual.class), destroy.loadNull()); - ResultHandle declaringProviderInstanceHandle = destroy.invokeInterfaceMethod( - MethodDescriptors.INJECTABLE_REF_PROVIDER_GET, declaringProviderHandle, - ctxHandle); - - if (bean.getDeclaringBean().getScope().isNormal()) { - // We need to unwrap the client proxy + ResultHandle declaringProviderInstanceHandle; + if (isStatic) { + // for static disposers, we don't need to resolve this handle + // the `null` will only be used for reflective invocation in case the disposer is private, which is OK + declaringProviderInstanceHandle = destroy.loadNull(); + } else { declaringProviderInstanceHandle = destroy.invokeInterfaceMethod( - MethodDescriptors.CLIENT_PROXY_GET_CONTEXTUAL_INSTANCE, - declaringProviderInstanceHandle); + MethodDescriptors.INJECTABLE_REF_PROVIDER_GET, declaringProviderHandle, + ctxHandle); + if (bean.getDeclaringBean().getScope().isNormal()) { + // We need to unwrap the client proxy + declaringProviderInstanceHandle = destroy.invokeInterfaceMethod( + MethodDescriptors.CLIENT_PROXY_GET_CONTEXTUAL_INSTANCE, + declaringProviderInstanceHandle); + } } ResultHandle[] referenceHandles = new ResultHandle[disposerMethod.parametersCount()]; @@ -904,6 +911,8 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide destroy.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, destroy.loadClass(disposerMethod.declaringClass().name().toString()), destroy.load(disposerMethod.name()), paramTypesArray, declaringProviderInstanceHandle, argsArray); + } else if (isStatic) { + destroy.invokeStaticMethod(MethodDescriptor.of(disposerMethod), referenceHandles); } else { destroy.invokeVirtualMethod(MethodDescriptor.of(disposerMethod), declaringProviderInstanceHandle, referenceHandles); @@ -912,8 +921,8 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide // Destroy @Dependent instances injected into method parameters of a disposer method destroy.invokeInterfaceMethod(MethodDescriptors.CREATIONAL_CTX_RELEASE, ctxHandle); - // If the declaring bean is @Dependent we must destroy the instance afterwards - if (BuiltinScope.DEPENDENT.is(bean.getDisposer().getDeclaringBean().getScope())) { + // If the declaring bean is @Dependent and the disposer is not static, we must destroy the instance afterwards + if (BuiltinScope.DEPENDENT.is(bean.getDisposer().getDeclaringBean().getScope()) && !isStatic) { destroy.invokeInterfaceMethod(MethodDescriptors.INJECTABLE_BEAN_DESTROY, declaringProviderHandle, declaringProviderInstanceHandle, ctxHandle); } @@ -927,7 +936,7 @@ protected void implementDestroy(BeanInfo bean, ClassCreator beanCreator, Provide // Bridge method needed MethodCreator bridgeDestroy = beanCreator.getMethodCreator("destroy", void.class, Object.class, CreationalContext.class) - .setModifiers(ACC_PUBLIC); + .setModifiers(ACC_PUBLIC | ACC_BRIDGE); bridgeDestroy.returnValue(bridgeDestroy.invokeVirtualMethod(destroy.getMethodDescriptor(), bridgeDestroy.getThis(), bridgeDestroy.getMethodParam(0), bridgeDestroy.getMethodParam(1))); @@ -1241,6 +1250,8 @@ void implementCreateForProducerMethod(ClassOutput classOutput, ClassCreator bean AssignableResultHandle instanceHandle; MethodInfo producerMethod = bean.getTarget().get().asMethod(); + boolean isStatic = Modifier.isStatic(producerMethod.flags()); + instanceHandle = create.createVariable(DescriptorUtils.extToInt(providerType.className())); // instance = declaringProviderSupplier.get().get(new CreationalContextImpl<>()).produce() ResultHandle ctxHandle = create.newInstance( @@ -1252,8 +1263,9 @@ void implementCreateForProducerMethod(ClassOutput classOutput, ClassCreator bean create.getThis()); ResultHandle declaringProviderHandle = create.invokeInterfaceMethod( MethodDescriptors.SUPPLIER_GET, declaringProviderSupplierHandle); - if (Modifier.isStatic(producerMethod.flags())) { - // for static producers, we don't need to resolve this this handle + if (isStatic) { + // for static producers, we don't need to resolve this handle + // the `null` will only be used for reflective invocation in case the producer is private, which is OK declaringProviderInstanceHandle = create.loadNull(); } else { declaringProviderInstanceHandle = create.invokeInterfaceMethod( @@ -1309,7 +1321,7 @@ void implementCreateForProducerMethod(ClassOutput classOutput, ClassCreator bean argsArray)); } else { ResultHandle invokeMethodHandle; - if (Modifier.isStatic(producerMethod.flags())) { + if (isStatic) { invokeMethodHandle = create.invokeStaticMethod(MethodDescriptor.of(producerMethod), referenceHandles); } else { @@ -1326,8 +1338,8 @@ void implementCreateForProducerMethod(ClassOutput classOutput, ClassCreator bean bean.getTarget().get().asMethod().name() + "()"); } - // If the declaring bean is @Dependent we must destroy the instance afterwards - if (BuiltinScope.DEPENDENT.is(bean.getDeclaringBean().getScope())) { + // If the declaring bean is @Dependent and the producer is not static, we must destroy the instance afterwards + if (BuiltinScope.DEPENDENT.is(bean.getDeclaringBean().getScope()) && !isStatic) { create.invokeInterfaceMethod(MethodDescriptors.INJECTABLE_BEAN_DESTROY, declaringProviderHandle, declaringProviderInstanceHandle, ctxHandle); } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/producer/disposer/StaticDisposerTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/producer/disposer/StaticDisposerTest.java new file mode 100644 index 0000000000000..264ce8ab98c7a --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/producer/disposer/StaticDisposerTest.java @@ -0,0 +1,112 @@ +package io.quarkus.arc.test.producer.disposer; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import java.math.BigDecimal; +import java.math.BigInteger; + +import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; +import javax.enterprise.context.Dependent; +import javax.enterprise.inject.Disposes; +import javax.enterprise.inject.Produces; + +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 StaticDisposerTest { + @RegisterExtension + ArcTestContainer container = new ArcTestContainer(Producers.class, Dependency.class); + + @Test + public void testProducersDisposers() { + assertState(0, 0, false, false, 0, 0); + + InstanceHandle intHandle = Arc.container().instance(BigInteger.class); + assertEquals(1, intHandle.get().intValue()); // instance producer + assertState(1, 1, false, false, 1, 0); + intHandle.destroy(); // static disposer + assertState(1, 1, false, true, 2, 2); + + InstanceHandle decHandle = Arc.container().instance(BigDecimal.class); + assertEquals(2, decHandle.get().intValue()); // static producer + assertState(1, 1, false, true, 3, 2); + decHandle.destroy(); // instance disposer + assertState(2, 2, true, true, 4, 4); + } + + private void assertState(int expectedInstancesCreated, int expectedInstancesDestroyed, + boolean expectedInstanceDisposerCalled, boolean expectedStaticDisposerCalled, + int expectedDependencyCreated, int expectedDependencyDestroyed) { + assertEquals(expectedInstancesCreated, Producers.instancesCreated); + assertEquals(expectedInstancesDestroyed, Producers.instancesDestroyed); + assertEquals(expectedInstanceDisposerCalled, Producers.instanceDisposerCalled); + assertEquals(expectedStaticDisposerCalled, Producers.staticDisposerCalled); + assertEquals(expectedDependencyCreated, Dependency.created); + assertEquals(expectedDependencyDestroyed, Dependency.destroyed); + } + + @Dependent + static class Producers { + static int instancesCreated = 0; + static int instancesDestroyed = 0; + + static boolean instanceDisposerCalled = false; + static boolean staticDisposerCalled = false; + + @PostConstruct + void created() { + instancesCreated++; + } + + @PreDestroy + void destroy() { + instancesDestroyed++; + } + + @Produces + @Dependent + BigInteger instanceProducer(Dependency ignored) { + return new BigInteger("1"); + } + + @Produces + @Dependent + static BigDecimal staticProducer(Dependency ignored) { + return new BigDecimal("2.0"); + } + + void instanceDisposer(@Disposes BigDecimal disposed, Dependency ignored) { + assertFalse(instanceDisposerCalled); + assertEquals(2, disposed.intValue()); + instanceDisposerCalled = true; + } + + static void staticDisposer(@Disposes BigInteger disposed, Dependency ignored) { + assertFalse(staticDisposerCalled); + assertEquals(1, disposed.intValue()); + staticDisposerCalled = true; + } + } + + @Dependent + static class Dependency { + static int created = 0; + static int destroyed = 0; + + @PostConstruct + void created() { + created++; + } + + @PreDestroy + void destroyed() { + destroyed++; + } + } +} From 7ad0ccc581b41047dbe49e9c60e7d2c257c0705e Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 27 Jan 2023 15:23:02 +0100 Subject: [PATCH 4/5] ArC: improve runtime representation of array bean types Previously, we always used `GenericArrayType` to represent array bean types at runtime. This is unexpected for people, and also for at least one TCK test that expects a `java.lang.Class` representation of `String[]`. This commit makes sure that arrays of primitive types and simple class types are represented using an array-typed `java.lang.Class` object. --- .../java/io/quarkus/arc/processor/Types.java | 28 ++++++-- .../test/bean/types/ArrayBeanTypesTest.java | 72 +++++++++++++++++++ 2 files changed, 93 insertions(+), 7 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/types/ArrayBeanTypesTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java index 5066d07f874d2..878c6404afb76 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java @@ -21,6 +21,7 @@ import org.jboss.jandex.AnnotationInstance; import org.jboss.jandex.AnnotationTarget; import org.jboss.jandex.AnnotationValue; +import org.jboss.jandex.ArrayType; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.FieldInfo; @@ -186,13 +187,26 @@ private static void getTypeHandle(AssignableResultHandle variable, BytecodeCreat getParameterizedType(variable, creator, tccl, type.asParameterizedType(), cache, typeVariables); } else if (Kind.ARRAY.equals(type.kind())) { - Type componentType = type.asArrayType().component(); - // E.g. String[] -> new GenericArrayTypeImpl(String.class) - AssignableResultHandle componentTypeHandle = creator.createVariable(Object.class); - getTypeHandle(componentTypeHandle, creator, componentType, tccl, cache, typeVariables); - ResultHandle arrayHandle = creator.newInstance( - MethodDescriptor.ofConstructor(GenericArrayTypeImpl.class, java.lang.reflect.Type.class), - componentTypeHandle); + ArrayType array = type.asArrayType(); + Type elementType = array.component(); + while (elementType.kind() == Kind.ARRAY) { + elementType = elementType.asArrayType().component(); + } + + ResultHandle arrayHandle; + if (elementType.kind() == Kind.PRIMITIVE || elementType.kind() == Kind.CLASS) { + // can produce a java.lang.Class representation of the array type + // E.g. String[] -> String[].class + arrayHandle = doLoadClass(creator, array.name().toString(), tccl); + } else { + // E.g. List[] -> new GenericArrayTypeImpl(new ParameterizedTypeImpl(List.class, String.class)) + Type componentType = type.asArrayType().component(); + AssignableResultHandle componentTypeHandle = creator.createVariable(Object.class); + getTypeHandle(componentTypeHandle, creator, componentType, tccl, cache, typeVariables); + arrayHandle = creator.newInstance( + MethodDescriptor.ofConstructor(GenericArrayTypeImpl.class, java.lang.reflect.Type.class), + componentTypeHandle); + } if (cache != null) { cache.put(type, arrayHandle, creator); } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/types/ArrayBeanTypesTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/types/ArrayBeanTypesTest.java new file mode 100644 index 0000000000000..90d3ae1b2b2fb --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/bean/types/ArrayBeanTypesTest.java @@ -0,0 +1,72 @@ +package io.quarkus.arc.test.bean.types; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Type; +import java.util.List; +import java.util.Set; + +import javax.enterprise.context.Dependent; +import javax.enterprise.inject.Produces; +import javax.enterprise.inject.spi.Bean; +import javax.enterprise.util.TypeLiteral; +import javax.inject.Named; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.InjectableBean; +import io.quarkus.arc.test.ArcTestContainer; + +public class ArrayBeanTypesTest { + @RegisterExtension + ArcTestContainer container = new ArcTestContainer(Producer.class); + + @Test + public void test() { + InjectableBean intArray = Arc.container().instance("intArray").getBean(); + assertBeanTypes(intArray, Object.class, int[][].class); + + InjectableBean stringArray = Arc.container().instance("stringArray").getBean(); + assertBeanTypes(stringArray, Object.class, String[].class); + + InjectableBean listArray = Arc.container().instance("listArray").getBean(); + assertBeanTypes(listArray, Object.class, new TypeLiteral[]>() { + }.getType()); + } + + private void assertBeanTypes(Bean bean, Type... expectedTypes) { + Set types = bean.getTypes(); + + assertEquals(expectedTypes.length, types.size()); + for (Type expectedType : expectedTypes) { + assertTrue(types.contains(expectedType)); + } + } + + @Dependent + static class Producer { + @Produces + @Dependent + @Named("intArray") + int[][] intArray() { + return new int[0][0]; + } + + @Produces + @Dependent + @Named("stringArray") + String[] stringArray() { + return new String[0]; + } + + @Produces + @Dependent + @Named("listArray") + List[] listArray() { + return (List[]) new List[0]; + } + } +} From 975224754d2dea415904e69c3487c9ee96201cc7 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 27 Jan 2023 17:11:37 +0100 Subject: [PATCH 5/5] ArC: add validation for array-typed producers --- .../java/io/quarkus/arc/processor/Types.java | 31 +++++++++++-- .../io/quarkus/arc/processor/TypesTest.java | 44 ++++++++++++++----- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java index 878c6404afb76..92f61b5a91c1a 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java @@ -188,10 +188,7 @@ private static void getTypeHandle(AssignableResultHandle variable, BytecodeCreat } else if (Kind.ARRAY.equals(type.kind())) { ArrayType array = type.asArrayType(); - Type elementType = array.component(); - while (elementType.kind() == Kind.ARRAY) { - elementType = elementType.asArrayType().component(); - } + Type elementType = getArrayElementType(array); ResultHandle arrayHandle; if (elementType.kind() == Kind.PRIMITIVE || elementType.kind() == Kind.CLASS) { @@ -351,6 +348,14 @@ static Type getProviderType(ClassInfo classInfo) { } } + static Type getArrayElementType(ArrayType array) { + Type elementType = array.component(); + while (elementType.kind() == Kind.ARRAY) { + elementType = elementType.asArrayType().component(); + } + return elementType; + } + static Set getProducerMethodTypeClosure(MethodInfo producerMethod, BeanDeployment beanDeployment) { Set types; Set unrestrictedBeanTypes = new HashSet<>(); @@ -358,6 +363,9 @@ static Set getProducerMethodTypeClosure(MethodInfo producerMethod, BeanDep if (returnType.kind() == Kind.TYPE_VARIABLE) { throw new DefinitionException("A type variable is not a legal bean type: " + producerMethod); } + if (returnType.kind() == Kind.ARRAY) { + checkArrayType(returnType.asArrayType(), producerMethod); + } if (returnType.kind() == Kind.PRIMITIVE || returnType.kind() == Kind.ARRAY) { types = new HashSet<>(); types.add(returnType); @@ -393,6 +401,9 @@ static Set getProducerFieldTypeClosure(FieldInfo producerField, BeanDeploy if (fieldType.kind() == Kind.TYPE_VARIABLE) { throw new DefinitionException("A type variable is not a legal bean type: " + producerField); } + if (fieldType.kind() == Kind.ARRAY) { + checkArrayType(fieldType.asArrayType(), producerField); + } if (fieldType.kind() == Kind.PRIMITIVE || fieldType.kind() == Kind.ARRAY) { types = new HashSet<>(); types.add(fieldType); @@ -482,6 +493,18 @@ static List getResolvedParameters(ClassInfo classInfo, Map r } } + /** + * Throws {@code DefinitionException} if given {@code producerFieldOrMethod}, + * whose type is given {@code arrayType}, is invalid due to the rules for arrays. + */ + static void checkArrayType(ArrayType arrayType, AnnotationTarget producerFieldOrMethod) { + Type elementType = getArrayElementType(arrayType); + if (elementType.kind() == Kind.TYPE_VARIABLE) { + throw new DefinitionException("A type variable array is not a legal bean type: " + producerFieldOrMethod); + } + containsWildcard(elementType, producerFieldOrMethod, true); + } + /** * Detects wildcard for given type. * In case this is related to a producer field or method, it either logs or throws a {@link DefinitionException} diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/TypesTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/TypesTest.java index 64f9539567b85..bcf5651f7bb2a 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/TypesTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/TypesTest.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -69,18 +70,13 @@ public void testGetTypeClosure() throws IOException { } } ClassInfo producerClass = index.getClassByName(producerName); - final String producersName = "produce"; - assertThrows(DefinitionException.class, - () -> Types.getProducerMethodTypeClosure(producerClass.method(producersName), dummyDeployment)); - assertThrows(DefinitionException.class, - () -> Types.getProducerFieldTypeClosure(producerClass.field(producersName), dummyDeployment)); - - // now assert the same with nested wildcard - final String nestedWildCardProducersName = "produceNested"; - assertThrows(DefinitionException.class, - () -> Types.getProducerMethodTypeClosure(producerClass.method(nestedWildCardProducersName), dummyDeployment)); - assertThrows(DefinitionException.class, - () -> Types.getProducerFieldTypeClosure(producerClass.field(nestedWildCardProducersName), dummyDeployment)); + for (String name : Arrays.asList("produce", "produceNested", "produceTypeVar", + "produceArray", "produceNestedArray", "produceTypeVarArray")) { + assertThrows(DefinitionException.class, + () -> Types.getProducerMethodTypeClosure(producerClass.method(name), dummyDeployment)); + assertThrows(DefinitionException.class, + () -> Types.getProducerFieldTypeClosure(producerClass.field(name), dummyDeployment)); + } // now assert following ones do NOT throw, the wildcard in the hierarchy is just ignored final String wildcardInTypeHierarchy = "eagleProducer"; @@ -140,10 +136,34 @@ public List> produceNested() { return null; } + public T produceTypeVar() { + return null; + } + + public List[][] produceArray() { + return null; + } + + public List>[][] produceNestedArray() { + return null; + } + + public T[][] produceTypeVarArray() { + return null; + } + List produce; List> produceNested; + public T produceTypeVar; + + List[] produceArray; + + List>[] produceNestedArray; + + public T[] produceTypeVarArray; + // following producer should NOT throw an exception because the return types doesn't contain wildcard // but the hierarchy of the return type actually contains one // taken from CDI TCK setup, see https://github.com/jakartaee/cdi-tck/blob/4.0.7/impl/src/main/java/org/jboss/cdi/tck/tests/definition/bean/types/illegal/BeanTypesWithIllegalTypeTest.java