From eccb0e7661f230d892fb34a30e6d9ce18799b88a Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Wed, 8 Feb 2023 16:20:35 +0100 Subject: [PATCH 1/4] ArC: ignore static methods annotated @Inject, similarly to static fields --- .../io/quarkus/arc/processor/Injection.java | 13 ++++- .../StaticMethodInjectionTest.java | 57 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/staticmethod/StaticMethodInjectionTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java index 26ffc24f973e6..6812c9223cfdb 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java @@ -302,6 +302,11 @@ private static boolean hasConstructorInjection(List injections) { private static List getAllInjectionPoints(BeanDeployment beanDeployment, ClassInfo beanClass, DotName name, boolean skipConstructors) { List injectAnnotations = new ArrayList<>(); + + // note that we can't treat static injection as a failure, because + // that would prevent us from even processing the AtInject TCK; + // hence, we just print a warning + for (FieldInfo field : beanClass.fields()) { AnnotationInstance inject = beanDeployment.getAnnotation(field, name); if (inject != null) { @@ -320,7 +325,13 @@ private static List getAllInjectionPoints(BeanDeployment bea } AnnotationInstance inject = beanDeployment.getAnnotation(method, name); if (inject != null) { - injectAnnotations.add(inject); + if (Modifier.isStatic(method.flags())) { + LOGGER.warn("An initializer method must be non-static - ignoring: " + + method.declaringClass().name() + "#" + + method.name() + "()"); + } else { + injectAnnotations.add(inject); + } } } return injectAnnotations; diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/staticmethod/StaticMethodInjectionTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/staticmethod/StaticMethodInjectionTest.java new file mode 100644 index 0000000000000..64dc1a7a342ce --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/staticmethod/StaticMethodInjectionTest.java @@ -0,0 +1,57 @@ +package io.quarkus.arc.test.injection.staticmethod; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.util.concurrent.atomic.AtomicInteger; + +import jakarta.enterprise.context.Dependent; +import jakarta.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 StaticMethodInjectionTest { + + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(Head.class, CombineHarvester.class); + + @Test + public void testInjection() { + assertNotNull(Arc.container().instance(CombineHarvester.class).get()); + assertNotNull(CombineHarvester.head); + assertEquals(1, Head.COUNTER.get()); + } + + @Dependent + static class Head { + + static final AtomicInteger COUNTER = new AtomicInteger(0); + + public Head() { + COUNTER.incrementAndGet(); + } + + } + + @Dependent + static class CombineHarvester { + + static Head head; + + // The parameter is injected + CombineHarvester(Head head) { + CombineHarvester.head = head; + } + + // This one is ignored + @Inject + static void init(Head head) { + CombineHarvester.head = head; + } + + } +} From b8982080ea6b546dd080149a4a24430569546168 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 20 Feb 2023 18:25:46 +0100 Subject: [PATCH 2/4] ArC: fix dependency injection order In case of class beans, dependency injection order is specified by the AtInject specification as follows: - injection points on superclasses are injected before injection points on subclasses; - fields are injected before initializer methods. Since we discover injection points bottom up, this requires reordering after each class in the hierarchy is scanned for injection points. The specification also prescribes that overridden initializer methods are not injected at all. Since we discover injection points bottom up, this requires remembering already seen methods and skipping methods that are overridden by those that were previously seen. This requires proper override detection. The `Methods.isOverridden()` and `Methods.MethodKey` utils are not enough, because they actually do _not_ detect overrides. They are used to skip processing a method in case a method with the same signature has already been processed. To do detect overridden methods properly, it is important to remember that: - private methods cannot be overridden, - package-private methods can only be overridden in the same package. For that reason, this commit introduces a dedicated override detection utility. --- .../quarkus/arc/processor/BeanGenerator.java | 179 +++++++------- .../io/quarkus/arc/processor/Injection.java | 145 ++++++++++- .../io/quarkus/arc/processor/Methods.java | 100 ++------ .../java/io/quarkus/arc/processor/Basics.java | 16 -- .../arc/processor/BeanInfoInjectionsTest.java | 7 +- .../arc/processor/BeanInfoQualifiersTest.java | 3 +- .../arc/processor/BeanInfoTypesTest.java | 3 +- .../quarkus/arc/processor/DotNamesTest.java | 3 +- .../arc/processor/MethodUtilsTest.java | 84 +++---- .../arc/processor/OverrideDetectionTest.java | 226 ++++++++++++++++++ .../processor/SubclassSkipPredicateTest.java | 3 +- .../io/quarkus/arc/processor/TypesTest.java | 3 +- .../quarkus/arc/processor/types/Bottom.java | 42 ++++ .../io/quarkus/arc/processor/types/Top.java | 41 ++++ .../arc/processor/types/extrapkg/Middle.java | 27 +++ .../arc/processor/types/extrapkg/Middle2.java | 42 ++++ .../injection/order/InjectionOrderTest.java | 131 ++++++++++ 17 files changed, 787 insertions(+), 268 deletions(-) create mode 100644 independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/OverrideDetectionTest.java create mode 100644 independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Bottom.java create mode 100644 independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Top.java create mode 100644 independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle.java create mode 100644 independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle2.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/order/InjectionOrderTest.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 861b1b88594b3..1663da365e4b5 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 @@ -1460,16 +1460,6 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat AssignableResultHandle instanceHandle; - List methodInjections = new ArrayList<>(); - List fieldInjections = new ArrayList<>(); - for (Injection injection : bean.getInjections()) { - if (injection.isField()) { - fieldInjections.add(injection); - } else if (injection.isMethod() && !injection.isConstructor()) { - methodInjections.add(injection); - } - } - ResultHandle postConstructsHandle = null; ResultHandle aroundConstructsHandle = null; Map interceptorToWrap = new HashMap<>(); @@ -1651,96 +1641,97 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat } // Perform field and initializer injections - for (Injection fieldInjection : fieldInjections) { - TryBlock tryBlock = create.tryBlock(); - InjectionPointInfo injectionPoint = fieldInjection.injectionPoints.get(0); - ResultHandle providerSupplierHandle = tryBlock.readInstanceField(FieldDescriptor.of(beanCreator.getClassName(), - injectionPointToProviderSupplierField.get(injectionPoint), Supplier.class.getName()), - tryBlock.getThis()); - ResultHandle providerHandle = tryBlock.invokeInterfaceMethod( - MethodDescriptors.SUPPLIER_GET, providerSupplierHandle); - ResultHandle childCtxHandle = tryBlock.invokeStaticMethod(MethodDescriptors.CREATIONAL_CTX_CHILD_CONTEXTUAL, - providerHandle, tryBlock.getMethodParam(0)); - AssignableResultHandle referenceHandle = tryBlock.createVariable(Object.class); - tryBlock.assign(referenceHandle, tryBlock.invokeInterfaceMethod(MethodDescriptors.INJECTABLE_REF_PROVIDER_GET, - providerHandle, childCtxHandle)); - checkPrimitiveInjection(tryBlock, injectionPoint, referenceHandle); - - FieldInfo injectedField = fieldInjection.target.asField(); - // only use reflection fallback if we are not performing transformation - if (isReflectionFallbackNeeded(injectedField, targetPackage, bean)) { - if (Modifier.isPrivate(injectedField.flags())) { - privateMembers.add(isApplicationClass, - String.format("@Inject field %s#%s", fieldInjection.target.asField().declaringClass().name(), - fieldInjection.target.asField().name())); - } - reflectionRegistration.registerField(injectedField); - tryBlock.invokeStaticMethod(MethodDescriptors.REFLECTIONS_WRITE_FIELD, - tryBlock.loadClass(injectedField.declaringClass().name().toString()), - tryBlock.load(injectedField.name()), instanceHandle, referenceHandle); - - } else { - // We cannot use injectionPoint.getRequiredType() because it might be a resolved parameterize type and we could get NoSuchFieldError - tryBlock.writeInstanceField( - FieldDescriptor.of(injectedField.declaringClass().name().toString(), injectedField.name(), - DescriptorUtils.typeToString(injectionPoint.getTarget().asField().type())), - instanceHandle, referenceHandle); - } - CatchBlockCreator catchBlock = tryBlock.addCatch(RuntimeException.class); - catchBlock.throwException(RuntimeException.class, "Error injecting " + fieldInjection.target, - catchBlock.getCaughtException()); - } - for (Injection methodInjection : methodInjections) { - List transientReferences = new ArrayList<>(); - ResultHandle[] referenceHandles = new ResultHandle[methodInjection.injectionPoints.size()]; - int paramIdx = 0; - for (InjectionPointInfo injectionPoint : methodInjection.injectionPoints) { - ResultHandle providerSupplierHandle = create.readInstanceField( - FieldDescriptor.of(beanCreator.getClassName(), - injectionPointToProviderSupplierField.get(injectionPoint), Supplier.class.getName()), - create.getThis()); - ResultHandle providerHandle = create.invokeInterfaceMethod(MethodDescriptors.SUPPLIER_GET, - providerSupplierHandle); - ResultHandle childCtxHandle = create.invokeStaticMethod(MethodDescriptors.CREATIONAL_CTX_CHILD_CONTEXTUAL, - providerHandle, create.getMethodParam(0)); - AssignableResultHandle referenceHandle = create.createVariable(Object.class); - create.assign(referenceHandle, create.invokeInterfaceMethod(MethodDescriptors.INJECTABLE_REF_PROVIDER_GET, + for (Injection injection : bean.getInjections()) { + if (injection.isField()) { + TryBlock tryBlock = create.tryBlock(); + InjectionPointInfo injectionPoint = injection.injectionPoints.get(0); + ResultHandle providerSupplierHandle = tryBlock.readInstanceField(FieldDescriptor.of(beanCreator.getClassName(), + injectionPointToProviderSupplierField.get(injectionPoint), Supplier.class.getName()), + tryBlock.getThis()); + ResultHandle providerHandle = tryBlock.invokeInterfaceMethod( + MethodDescriptors.SUPPLIER_GET, providerSupplierHandle); + ResultHandle childCtxHandle = tryBlock.invokeStaticMethod(MethodDescriptors.CREATIONAL_CTX_CHILD_CONTEXTUAL, + providerHandle, tryBlock.getMethodParam(0)); + AssignableResultHandle referenceHandle = tryBlock.createVariable(Object.class); + tryBlock.assign(referenceHandle, tryBlock.invokeInterfaceMethod(MethodDescriptors.INJECTABLE_REF_PROVIDER_GET, providerHandle, childCtxHandle)); - checkPrimitiveInjection(create, injectionPoint, referenceHandle); - referenceHandles[paramIdx++] = referenceHandle; - // We need to destroy dependent beans for @TransientReference injection points - if (injectionPoint.isDependentTransientReference()) { - transientReferences.add(new TransientReference(providerHandle, referenceHandle, childCtxHandle)); - } - } + checkPrimitiveInjection(tryBlock, injectionPoint, referenceHandle); + + FieldInfo injectedField = injection.target.asField(); + // only use reflection fallback if we are not performing transformation + if (isReflectionFallbackNeeded(injectedField, targetPackage, bean)) { + if (Modifier.isPrivate(injectedField.flags())) { + privateMembers.add(isApplicationClass, + String.format("@Inject field %s#%s", injection.target.asField().declaringClass().name(), + injection.target.asField().name())); + } + reflectionRegistration.registerField(injectedField); + tryBlock.invokeStaticMethod(MethodDescriptors.REFLECTIONS_WRITE_FIELD, + tryBlock.loadClass(injectedField.declaringClass().name().toString()), + tryBlock.load(injectedField.name()), instanceHandle, referenceHandle); - MethodInfo initializerMethod = methodInjection.target.asMethod(); - if (isReflectionFallbackNeeded(initializerMethod, targetPackage)) { - if (Modifier.isPrivate(initializerMethod.flags())) { - privateMembers.add(isApplicationClass, - String.format("@Inject initializer %s#%s()", initializerMethod.declaringClass().name(), - initializerMethod.name())); + } else { + // We cannot use injectionPoint.getRequiredType() because it might be a resolved parameterize type and we could get NoSuchFieldError + tryBlock.writeInstanceField( + FieldDescriptor.of(injectedField.declaringClass().name().toString(), injectedField.name(), + DescriptorUtils.typeToString(injectionPoint.getTarget().asField().type())), + instanceHandle, referenceHandle); } - ResultHandle paramTypesArray = create.newArray(Class.class, create.load(referenceHandles.length)); - ResultHandle argsArray = create.newArray(Object.class, create.load(referenceHandles.length)); - for (int i = 0; i < referenceHandles.length; i++) { - create.writeArrayValue(paramTypesArray, i, - create.loadClass(initializerMethod.parameterType(i).name().toString())); - create.writeArrayValue(argsArray, i, referenceHandles[i]); + CatchBlockCreator catchBlock = tryBlock.addCatch(RuntimeException.class); + catchBlock.throwException(RuntimeException.class, "Error injecting " + injection.target, + catchBlock.getCaughtException()); + } else if (injection.isMethod() && !injection.isConstructor()) { + List transientReferences = new ArrayList<>(); + ResultHandle[] referenceHandles = new ResultHandle[injection.injectionPoints.size()]; + int paramIdx = 0; + for (InjectionPointInfo injectionPoint : injection.injectionPoints) { + ResultHandle providerSupplierHandle = create.readInstanceField( + FieldDescriptor.of(beanCreator.getClassName(), + injectionPointToProviderSupplierField.get(injectionPoint), Supplier.class.getName()), + create.getThis()); + ResultHandle providerHandle = create.invokeInterfaceMethod(MethodDescriptors.SUPPLIER_GET, + providerSupplierHandle); + ResultHandle childCtxHandle = create.invokeStaticMethod(MethodDescriptors.CREATIONAL_CTX_CHILD_CONTEXTUAL, + providerHandle, create.getMethodParam(0)); + AssignableResultHandle referenceHandle = create.createVariable(Object.class); + create.assign(referenceHandle, create.invokeInterfaceMethod(MethodDescriptors.INJECTABLE_REF_PROVIDER_GET, + providerHandle, childCtxHandle)); + checkPrimitiveInjection(create, injectionPoint, referenceHandle); + referenceHandles[paramIdx++] = referenceHandle; + // We need to destroy dependent beans for @TransientReference injection points + if (injectionPoint.isDependentTransientReference()) { + transientReferences.add(new TransientReference(providerHandle, referenceHandle, childCtxHandle)); + } } - reflectionRegistration.registerMethod(initializerMethod); - create.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, - create.loadClass(initializerMethod.declaringClass().name().toString()), - create.load(methodInjection.target.asMethod().name()), - paramTypesArray, instanceHandle, argsArray); - } else { - create.invokeVirtualMethod(MethodDescriptor.of(methodInjection.target.asMethod()), instanceHandle, - referenceHandles); - } + MethodInfo initializerMethod = injection.target.asMethod(); + if (isReflectionFallbackNeeded(initializerMethod, targetPackage)) { + if (Modifier.isPrivate(initializerMethod.flags())) { + privateMembers.add(isApplicationClass, + String.format("@Inject initializer %s#%s()", initializerMethod.declaringClass().name(), + initializerMethod.name())); + } + ResultHandle paramTypesArray = create.newArray(Class.class, create.load(referenceHandles.length)); + ResultHandle argsArray = create.newArray(Object.class, create.load(referenceHandles.length)); + for (int i = 0; i < referenceHandles.length; i++) { + create.writeArrayValue(paramTypesArray, i, + create.loadClass(initializerMethod.parameterType(i).name().toString())); + create.writeArrayValue(argsArray, i, referenceHandles[i]); + } + reflectionRegistration.registerMethod(initializerMethod); + create.invokeStaticMethod(MethodDescriptors.REFLECTIONS_INVOKE_METHOD, + create.loadClass(initializerMethod.declaringClass().name().toString()), + create.load(injection.target.asMethod().name()), + paramTypesArray, instanceHandle, argsArray); - // Destroy injected transient references - destroyTransientReferences(create, transientReferences); + } else { + create.invokeVirtualMethod(MethodDescriptor.of(injection.target.asMethod()), instanceHandle, + referenceHandles); + } + + // Destroy injected transient references + destroyTransientReferences(create, transientReferences); + } } // PostConstruct lifecycle callback interceptors diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java index 6812c9223cfdb..e9ea53f1e70d5 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java @@ -5,7 +5,9 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -18,6 +20,7 @@ import org.jboss.jandex.DotName; import org.jboss.jandex.FieldInfo; import org.jboss.jandex.MethodInfo; +import org.jboss.jandex.Type; import org.jboss.logging.Logger; import io.quarkus.arc.processor.InjectionPointInfo.TypeAndQualifiers; @@ -50,8 +53,8 @@ static Injection forSyntheticBean(Iterable injectionPoints) { static List forBean(AnnotationTarget beanTarget, BeanInfo declaringBean, BeanDeployment beanDeployment, InjectionPointModifier transformer) { if (Kind.CLASS.equals(beanTarget.kind())) { - List injections = new ArrayList<>(); - forClassBean(beanTarget.asClass(), beanTarget.asClass(), beanDeployment, injections, transformer, false); + List injections = forClassBean(beanTarget.asClass(), beanTarget.asClass(), beanDeployment, + transformer, false, new HashSet<>()); Set injectConstructors = injections.stream().filter(Injection::isConstructor) .map(Injection::getTarget).collect(Collectors.toSet()); @@ -142,11 +145,14 @@ static List forBean(AnnotationTarget beanTarget, BeanInfo declaringBe throw new IllegalArgumentException("Unsupported annotation target"); } - private static void forClassBean(ClassInfo beanClass, ClassInfo classInfo, BeanDeployment beanDeployment, - List injections, InjectionPointModifier transformer, boolean skipConstructors) { + // returns injections in the order they should be performed + private static List forClassBean(ClassInfo beanClass, ClassInfo classInfo, BeanDeployment beanDeployment, + InjectionPointModifier transformer, boolean skipConstructors, Set seenMethods) { + + List injections = new ArrayList<>(); List injectAnnotations = getAllInjectionPoints(beanDeployment, classInfo, DotNames.INJECT, - skipConstructors); + skipConstructors, seenMethods); for (AnnotationInstance injectAnnotation : injectAnnotations) { AnnotationTarget injectTarget = injectAnnotation.target(); @@ -186,7 +192,7 @@ private static void forClassBean(ClassInfo beanClass, ClassInfo classInfo, BeanD for (DotName resourceAnnotation : beanDeployment.getResourceAnnotations()) { List resourceAnnotations = getAllInjectionPoints(beanDeployment, classInfo, - resourceAnnotation, true); + resourceAnnotation, true, seenMethods); for (AnnotationInstance resourceAnnotationInstance : resourceAnnotations) { if (Kind.FIELD == resourceAnnotationInstance.target().kind() && resourceAnnotationInstance.target().asField().annotations().stream() @@ -203,10 +209,16 @@ private static void forClassBean(ClassInfo beanClass, ClassInfo classInfo, BeanD if (!classInfo.superName().equals(DotNames.OBJECT)) { ClassInfo info = getClassByName(beanDeployment.getBeanArchiveIndex(), classInfo.superName()); if (info != null) { - forClassBean(beanClass, info, beanDeployment, injections, transformer, true); + List superInjections = forClassBean(beanClass, info, beanDeployment, transformer, true, seenMethods); + + // injections are discovered bottom-up to easily skip overriden methods, + // but they need to be performed top-down per the AtInject specification + superInjections.addAll(injections); + injections = superInjections; } } + return injections; } static Injection forDisposer(MethodInfo disposerMethod, ClassInfo beanClass, BeanDeployment beanDeployment, @@ -300,7 +312,8 @@ private static boolean hasConstructorInjection(List injections) { } private static List getAllInjectionPoints(BeanDeployment beanDeployment, ClassInfo beanClass, - DotName name, boolean skipConstructors) { + DotName annotationName, boolean skipConstructors, Set seenMethods) { + // order is significant: fields must be injected before methods List injectAnnotations = new ArrayList<>(); // note that we can't treat static injection as a failure, because @@ -308,7 +321,7 @@ private static List getAllInjectionPoints(BeanDeployment bea // hence, we just print a warning for (FieldInfo field : beanClass.fields()) { - AnnotationInstance inject = beanDeployment.getAnnotation(field, name); + AnnotationInstance inject = beanDeployment.getAnnotation(field, annotationName); if (inject != null) { if (Modifier.isFinal(field.flags()) || Modifier.isStatic(field.flags())) { LOGGER.warn("An injection field must be non-static and non-final - ignoring: " @@ -319,11 +332,19 @@ private static List getAllInjectionPoints(BeanDeployment bea } } } + for (MethodInfo method : beanClass.methods()) { if (skipConstructors && method.name().equals(Methods.INIT)) { continue; } - AnnotationInstance inject = beanDeployment.getAnnotation(method, name); + + MethodOverrideKey key = new MethodOverrideKey(method); + if (isOverriden(key, seenMethods)) { + continue; + } + seenMethods.add(key); + + AnnotationInstance inject = beanDeployment.getAnnotation(method, annotationName); if (inject != null) { if (Modifier.isStatic(method.flags())) { LOGGER.warn("An initializer method must be non-static - ignoring: " @@ -334,7 +355,111 @@ private static List getAllInjectionPoints(BeanDeployment bea } } } + return injectAnnotations; } + // --- + // this is close to `Methods.MethodKey` and `Methods.isOverridden()`, but it's actually more precise + // the `Methods` code is used on many places to avoid processing methods in case a method with the same + // signature has already been processed, and that's _not_ the definition of overriding + + static class MethodOverrideKey { + final String name; + final List params; + final DotName returnType; + final String visibility; + final MethodInfo method; // this is intentionally ignored for equals/hashCode + + public MethodOverrideKey(MethodInfo method) { + this.method = Objects.requireNonNull(method, "Method must not be null"); + this.name = method.name(); + this.returnType = method.returnType().name(); + this.params = new ArrayList<>(); + for (Type i : method.parameterTypes()) { + params.add(i.name()); + } + if (Modifier.isPublic(method.flags()) || Modifier.isProtected(method.flags())) { + this.visibility = ""; + } else if (Modifier.isPrivate(method.flags())) { + // private methods cannot be overridden + this.visibility = method.declaringClass().name().toString(); + } else { + // package-private methods can only be overridden in the same package + this.visibility = method.declaringClass().name().packagePrefix(); + } + } + + private MethodOverrideKey(String name, List params, DotName returnType, String visibility, MethodInfo method) { + this.name = name; + this.params = params; + this.returnType = returnType; + this.visibility = visibility; + this.method = method; + } + + MethodOverrideKey withoutVisibility() { + return new MethodOverrideKey(name, params, returnType, "", method); + } + + String packageName() { + return method.declaringClass().name().packagePrefix(); + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (!(o instanceof MethodOverrideKey)) + return false; + MethodOverrideKey methodKey = (MethodOverrideKey) o; + return Objects.equals(name, methodKey.name) + && Objects.equals(params, methodKey.params) + && Objects.equals(returnType, methodKey.returnType) + && Objects.equals(visibility, methodKey.visibility); + } + + @Override + public int hashCode() { + return Objects.hash(name, params, returnType, visibility); + } + } + + /** + * Returns whether given {@code method} is overridden by any of given {@code previousMethods}. This method + * only works correctly during a bottom-up traversal of class inheritance hierarchy, during which all seen + * methods are recorded into {@code previousMethods}. + *

+ * This is not entirely precise according to the JLS rules for method overriding, but seems good enough. + */ + static boolean isOverriden(MethodOverrideKey method, Set previousMethods) { + short flags = method.method.flags(); + if (Modifier.isPublic(flags) || Modifier.isProtected(flags)) { + // if there's an override, it must be public or perhaps protected, + // so it always has the same visibility + return previousMethods.contains(method); + } else if (Modifier.isPrivate(flags)) { + // private methods are never overridden + return false; + } else { // package-private + // if there's an override, it must be in the same package and: + // 1. either package-private (so it has the same visibility) + if (previousMethods.contains(method)) { + return true; + } + + // 2. or public/protected (so it has a different visibility: empty string) + String packageName = method.packageName(); + MethodOverrideKey methodWithoutVisibility = method.withoutVisibility(); + for (MethodOverrideKey previousMethod : previousMethods) { + if (methodWithoutVisibility.equals(previousMethod) + && packageName.equals(previousMethod.packageName())) { + return true; + } + } + + return false; + } + } + } 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 3c8740327d5b3..a98301ea89395 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 @@ -355,100 +355,32 @@ public MethodKey(MethodInfo method) { } @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + name.hashCode(); - result = prime * result + params.hashCode(); - result = prime * result + returnType.hashCode(); - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { + public boolean equals(Object o) { + if (this == o) return true; - } - if (obj == null) { - return false; - } - if (!(obj instanceof MethodKey)) { + if (!(o instanceof MethodKey)) return false; - } - MethodKey other = (MethodKey) obj; - if (!name.equals(other.name)) { - return false; - } - if (!params.equals(other.params)) { - return false; - } - if (!returnType.equals(other.returnType)) { - return false; - } - return true; + MethodKey methodKey = (MethodKey) o; + return Objects.equals(name, methodKey.name) + && Objects.equals(params, methodKey.params) + && Objects.equals(returnType, methodKey.returnType); } - } - - static boolean isOverriden(MethodInfo method, Collection previousMethods) { - for (MethodInfo other : previousMethods) { - if (Methods.matchesSignature(method, other)) { - return true; - } + @Override + public int hashCode() { + return Objects.hash(name, params, returnType); } - return false; } - static boolean isOverriden(Methods.MethodKey method, Collection previousMethods) { + /** + * Note that this in fact does not detect method overrides. It is only useful + * to skip processing of a method in case a method with the same name and signature + * has already been processed. (Same name and signature does not mean override!) + */ + static boolean isOverriden(Methods.MethodKey method, Set previousMethods) { return previousMethods.contains(method); } - static boolean matchesSignature(MethodInfo method, MethodInfo subclassMethod) { - if (!method.name().equals(subclassMethod.name())) { - return false; - } - List parameters = method.parameterTypes(); - List subParameters = subclassMethod.parameterTypes(); - - int paramCount = parameters.size(); - if (paramCount != subParameters.size()) { - return false; - } - - if (paramCount == 0) { - return true; - } - - for (int i = 0; i < paramCount; i++) { - if (!Methods.isTypeEqual(parameters.get(i), subParameters.get(i))) { - return false; - } - } - - return true; - } - - static boolean isTypeEqual(Type a, Type b) { - return Methods.toRawType(a).equals(Methods.toRawType(b)); - } - - static DotName toRawType(Type a) { - switch (a.kind()) { - case CLASS: - case PRIMITIVE: - case ARRAY: - return a.name(); - case PARAMETERIZED_TYPE: - return a.asParameterizedType().name(); - case TYPE_VARIABLE: - case UNRESOLVED_TYPE_VARIABLE: - case TYPE_VARIABLE_REFERENCE: - case WILDCARD_TYPE: - default: - return DotNames.OBJECT; - } - } - static void addDelegateTypeMethods(IndexView index, ClassInfo delegateTypeClass, Set methods) { if (delegateTypeClass != null) { for (MethodInfo method : delegateTypeClass.methods()) { diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/Basics.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/Basics.java index d6ad13dc4bba0..1f4cd9ad16e40 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/Basics.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/Basics.java @@ -1,11 +1,6 @@ package io.quarkus.arc.processor; -import java.io.IOException; -import java.io.InputStream; - import org.jboss.jandex.DotName; -import org.jboss.jandex.Index; -import org.jboss.jandex.Indexer; public final class Basics { @@ -13,15 +8,4 @@ public static DotName name(Class clazz) { return DotName.createSimple(clazz.getName()); } - public static Index index(Class... classes) throws IOException { - Indexer indexer = new Indexer(); - for (Class clazz : classes) { - try (InputStream stream = Basics.class.getClassLoader() - .getResourceAsStream(clazz.getName().replace('.', '/') + ".class")) { - indexer.index(stream); - } - } - return indexer.complete(); - } - } diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoInjectionsTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoInjectionsTest.java index ff22bbd568845..fe9167d315c76 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoInjectionsTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoInjectionsTest.java @@ -1,6 +1,5 @@ package io.quarkus.arc.processor; -import static io.quarkus.arc.processor.Basics.index; import static io.quarkus.arc.processor.Basics.name; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -13,7 +12,6 @@ import java.util.List; import org.jboss.jandex.ClassInfo; -import org.jboss.jandex.DotName; import org.jboss.jandex.Index; import org.jboss.jandex.ParameterizedType; import org.jboss.jandex.Type; @@ -33,11 +31,10 @@ public class BeanInfoInjectionsTest { @Test public void testInjections() throws IOException { - Index index = index(Foo.class, Bar.class, FooQualifier.class, AbstractList.class, AbstractCollection.class, + Index index = Index.of(Foo.class, Bar.class, FooQualifier.class, AbstractList.class, AbstractCollection.class, Collection.class, List.class, Iterable.class, Object.class, String.class); - DotName barName = name(Bar.class); - ClassInfo barClass = index.getClassByName(barName); + ClassInfo barClass = index.getClassByName(Bar.class); Type fooType = Type.create(name(Foo.class), Kind.CLASS); Type listStringType = ParameterizedType.create(name(List.class), new Type[] { Type.create(name(String.class), Kind.CLASS) }, null); diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoQualifiersTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoQualifiersTest.java index 3c95a2c557f76..28d7cac0c0c7a 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoQualifiersTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoQualifiersTest.java @@ -1,6 +1,5 @@ package io.quarkus.arc.processor; -import static io.quarkus.arc.processor.Basics.index; import static io.quarkus.arc.processor.Basics.name; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -30,7 +29,7 @@ public class BeanInfoQualifiersTest { @Test public void testQualifiers() throws IOException { - Index index = index(Foo.class, Bar.class, FooQualifier.class, AbstractList.class, AbstractCollection.class, + Index index = Index.of(Foo.class, Bar.class, FooQualifier.class, AbstractList.class, AbstractCollection.class, List.class, Collection.class, Object.class, String.class, Iterable.class); DotName fooName = name(Foo.class); DotName fooQualifierName = name(FooQualifier.class); diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoTypesTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoTypesTest.java index bc53674cd78a9..dc8b317d1fc46 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoTypesTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/BeanInfoTypesTest.java @@ -1,6 +1,5 @@ package io.quarkus.arc.processor; -import static io.quarkus.arc.processor.Basics.index; import static io.quarkus.arc.processor.Basics.name; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -33,7 +32,7 @@ public class BeanInfoTypesTest { @Test public void testResolver() throws IOException { - Index index = index(Foo.class, Bar.class, FooQualifier.class, AbstractList.class, AbstractCollection.class, + Index index = Index.of(Foo.class, Bar.class, FooQualifier.class, AbstractList.class, AbstractCollection.class, Collection.class, List.class, Iterable.class, Object.class, String.class); diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/DotNamesTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/DotNamesTest.java index 7d83fccdb6a6c..36d9ef5369faf 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/DotNamesTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/DotNamesTest.java @@ -1,6 +1,5 @@ package io.quarkus.arc.processor; -import static io.quarkus.arc.processor.Basics.index; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -18,7 +17,7 @@ public class DotNamesTest { @Test public void testSimpleName() throws IOException { - Index index = index(Nested.class, NestedNested.class, DotNamesTest.class); + Index index = Index.of(Nested.class, NestedNested.class, DotNamesTest.class); Assertions.assertEquals("Nested", DotNames.simpleName(index.getClassByName(DotName.createSimple(Nested.class.getName())))); assertEquals("DotNamesTest$Nested", diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/MethodUtilsTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/MethodUtilsTest.java index 4ad312d5ca884..5b29872f6337b 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/MethodUtilsTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/MethodUtilsTest.java @@ -1,120 +1,104 @@ package io.quarkus.arc.processor; -import static io.quarkus.arc.processor.Basics.index; import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; import java.util.Arrays; -import java.util.Collection; import java.util.List; import java.util.Set; import java.util.stream.Collectors; import org.jboss.jandex.ClassInfo; -import org.jboss.jandex.DotName; import org.jboss.jandex.Index; import org.jboss.jandex.MethodInfo; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -/** - * - * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com - *
- * Date: 17/07/2019 - */ +import io.quarkus.arc.processor.types.Bottom; +import io.quarkus.arc.processor.types.Top; +import io.quarkus.arc.processor.types.extrapkg.Middle; +import io.quarkus.arc.processor.types.extrapkg.Middle2; + public class MethodUtilsTest { private Index index; @BeforeEach public void setUp() throws IOException { - index = index(SomeClass.class, SuperClass.class, SuperSuperClass.class, TheRoot.class); + index = Index.of(SomeClass.class, SuperClass.class, SuperSuperClass.class, TheRoot.class, + Top.class, Middle.class, Middle2.class, Bottom.class); } - private Set gatherMethodsFromClasses(Class... classes) { + private Set gatherMethodsFromClasses(Class... classes) { return Arrays.stream(classes) - .map(this::classInfo) + .map(index::getClassByName) .map(ClassInfo::methods) .flatMap(List::stream) + .map(Methods.MethodKey::new) .collect(Collectors.toSet()); } @Test public void shouldFindDirectOverriding() { - ClassInfo aClass = classInfo(SomeClass.class.getName()); - ClassInfo superClass = classInfo(SuperClass.class.getName()); + Set methods = gatherMethodsFromClasses(SomeClass.class); + + MethodInfo parentMethod = index.getClassByName(SuperClass.class).firstMethod("fromSuperClass"); - MethodInfo parentMethod = superClass.firstMethod("fromSuperClass"); - assertThat(Methods.isOverriden(parentMethod, aClass.methods())).isTrue(); + assertThat(Methods.isOverriden(new Methods.MethodKey(parentMethod), methods)).isTrue(); } @Test public void shouldFindGenericOverriding() { - Collection methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); - - ClassInfo grandma = classInfo(SuperSuperClass.class); + Set methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); - MethodInfo genericMethod = grandma.firstMethod("generic"); + MethodInfo genericMethod = index.getClassByName(SuperSuperClass.class).firstMethod("generic"); - assertThat(Methods.isOverriden(genericMethod, methods)).isTrue(); + assertThat(Methods.isOverriden(new Methods.MethodKey(genericMethod), methods)).isTrue(); } @Test public void shouldNotFindNonOverridenFromSuperClass() { - ClassInfo aClass = classInfo(SomeClass.class.getName()); - ClassInfo superClass = classInfo(SuperClass.class.getName()); + Set methods = gatherMethodsFromClasses(SomeClass.class); - MethodInfo parentMethod = superClass.firstMethod("notOverridenFromSuperClass"); - assertThat(Methods.isOverriden(parentMethod, aClass.methods())).isFalse(); + MethodInfo parentMethod = index.getClassByName(SuperClass.class).firstMethod("notOverridenFromSuperClass"); + + assertThat(Methods.isOverriden(new Methods.MethodKey(parentMethod), methods)).isFalse(); } @Test public void shouldNotFindNonGenericNonOverridenFromSuperSuperClass() { - Collection methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); + Set methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); - ClassInfo grandma = classInfo(SuperSuperClass.class.getName()); + MethodInfo parentMethod = index.getClassByName(SuperSuperClass.class).firstMethod("notOverridenNonGeneric"); - MethodInfo parentMethod = grandma.firstMethod("notOverridenNonGeneric"); - assertThat(Methods.isOverriden(parentMethod, methods)).isFalse(); + assertThat(Methods.isOverriden(new Methods.MethodKey(parentMethod), methods)).isFalse(); } @Test public void shouldNotFindGenericNonOverridenFromSuperSuperClass() { - Collection methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); + Set methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); - ClassInfo grandma = classInfo(SuperSuperClass.class.getName()); + MethodInfo parentMethod = index.getClassByName(SuperSuperClass.class).firstMethod("notOverridenGeneric"); - MethodInfo parentMethod = grandma.firstMethod("notOverridenGeneric"); - assertThat(Methods.isOverriden(parentMethod, methods)).isFalse(); + assertThat(Methods.isOverriden(new Methods.MethodKey(parentMethod), methods)).isFalse(); } @Test public void shouldNotFindAlmostMatchingGeneric() { - Collection methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); + Set methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class); - ClassInfo grandma = classInfo(SuperSuperClass.class.getName()); + MethodInfo parentMethod = index.getClassByName(SuperSuperClass.class).firstMethod("almostMatchingGeneric"); - MethodInfo parentMethod = grandma.firstMethod("almostMatchingGeneric"); - assertThat(Methods.isOverriden(parentMethod, methods)).isFalse(); + assertThat(Methods.isOverriden(new Methods.MethodKey(parentMethod), methods)).isFalse(); } @Test public void shouldFindOverridenInTheMiddleOfHierarchy() { - Collection methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class, SuperSuperClass.class); - - ClassInfo root = classInfo(TheRoot.class.getName()); + Set methods = gatherMethodsFromClasses(SomeClass.class, SuperClass.class, SuperSuperClass.class); - MethodInfo parentMethod = root.firstMethod("generic"); - assertThat(Methods.isOverriden(parentMethod, methods)).isTrue(); - } - - private ClassInfo classInfo(Class aClass) { - return index.getClassByName(DotName.createSimple(aClass.getName())); - } + MethodInfo parentMethod = index.getClassByName(TheRoot.class).firstMethod("generic"); - private ClassInfo classInfo(String name) { - return index.getClassByName(DotName.createSimple(name)); + assertThat(Methods.isOverriden(new Methods.MethodKey(parentMethod), methods)).isTrue(); } public static class SomeClass extends SuperClass { @@ -140,7 +124,6 @@ void notOverridenFromSuperClass(int param) { } void almostMatchingGeneric(V param) { - } } @@ -163,7 +146,6 @@ void notOverridenNonGeneric(String param) { public static class TheRoot { void generic(X param) { - } } } diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/OverrideDetectionTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/OverrideDetectionTest.java new file mode 100644 index 0000000000000..4aa68e12ff292 --- /dev/null +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/OverrideDetectionTest.java @@ -0,0 +1,226 @@ +package io.quarkus.arc.processor; + +import static io.quarkus.arc.processor.Injection.isOverriden; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.Index; +import org.jboss.jandex.MethodInfo; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.quarkus.arc.processor.Injection.MethodOverrideKey; +import io.quarkus.arc.processor.types.Bottom; +import io.quarkus.arc.processor.types.Top; +import io.quarkus.arc.processor.types.extrapkg.Middle; +import io.quarkus.arc.processor.types.extrapkg.Middle2; + +public class OverrideDetectionTest { + private Index index; + + @BeforeEach + public void setUp() throws IOException { + index = Index.of(SomeClass.class, SuperClass.class, SuperSuperClass.class, TheRoot.class, + Top.class, Middle.class, Middle2.class, Bottom.class); + } + + private Set allMethodsOf(Class... classes) { + return Arrays.stream(classes) + .map(index::getClassByName) + .map(ClassInfo::methods) + .flatMap(List::stream) + .map(MethodOverrideKey::new) + .collect(Collectors.toSet()); + } + + @Test + public void shouldFindDirectOverriding() { + Set methods = allMethodsOf(SomeClass.class); + + MethodInfo parentMethod = index.getClassByName(SuperClass.class).firstMethod("fromSuperClass"); + + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + } + + @Test + public void shouldFindGenericOverriding() { + Set methods = allMethodsOf(SomeClass.class, SuperClass.class); + + MethodInfo genericMethod = index.getClassByName(SuperSuperClass.class).firstMethod("generic"); + + assertThat(isOverriden(new MethodOverrideKey(genericMethod), methods)).isTrue(); + } + + @Test + public void shouldNotFindNonOverridenFromSuperClass() { + Set methods = allMethodsOf(SomeClass.class); + + MethodInfo parentMethod = index.getClassByName(SuperClass.class).firstMethod("notOverridenFromSuperClass"); + + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + } + + @Test + public void shouldNotFindNonGenericNonOverridenFromSuperSuperClass() { + Set methods = allMethodsOf(SomeClass.class, SuperClass.class); + + MethodInfo parentMethod = index.getClassByName(SuperSuperClass.class).firstMethod("notOverridenNonGeneric"); + + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + } + + @Test + public void shouldNotFindGenericNonOverridenFromSuperSuperClass() { + Set methods = allMethodsOf(SomeClass.class, SuperClass.class); + + MethodInfo parentMethod = index.getClassByName(SuperSuperClass.class).firstMethod("notOverridenGeneric"); + + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + } + + @Test + public void shouldNotFindAlmostMatchingGeneric() { + Set methods = allMethodsOf(SomeClass.class, SuperClass.class); + + MethodInfo parentMethod = index.getClassByName(SuperSuperClass.class).firstMethod("almostMatchingGeneric"); + + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + } + + @Test + public void shouldFindOverridenInTheMiddleOfHierarchy() { + Set methods = allMethodsOf(SomeClass.class, SuperClass.class, SuperSuperClass.class); + + MethodInfo parentMethod = index.getClassByName(TheRoot.class).firstMethod("generic"); + + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + } + + @Test + public void visibilityMiddleBottom() { + Set methods = allMethodsOf(Bottom.class); + + ClassInfo parent = index.getClassByName(Middle.class); + + MethodInfo parentMethod = parent.firstMethod("packagePrivateMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + + parentMethod = parent.firstMethod("privateMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + + parentMethod = parent.firstMethod("packagePrivateMethodToBecomeProtected"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + + parentMethod = parent.firstMethod("packagePrivateMethodToBecomePublic"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + } + + @Test + public void visibilityTopMiddleBottom() { + Set methods = allMethodsOf(Middle.class, Bottom.class); + + ClassInfo parent = index.getClassByName(Top.class); + + MethodInfo parentMethod = parent.firstMethod("publicMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("protectedMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("packagePrivateMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("privateMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + + parentMethod = parent.firstMethod("protectedMethodToBecomePublic"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("packagePrivateMethodToBecomeProtected"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("packagePrivateMethodToBecomePublic"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + } + + @Test + public void visibilityTopMiddle2() { + Set methods = allMethodsOf(Middle2.class); + + ClassInfo parent = index.getClassByName(Top.class); + + MethodInfo parentMethod = parent.firstMethod("publicMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("protectedMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("packagePrivateMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + + parentMethod = parent.firstMethod("privateMethod"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + + parentMethod = parent.firstMethod("protectedMethodToBecomePublic"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isTrue(); + + parentMethod = parent.firstMethod("packagePrivateMethodToBecomeProtected"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + + parentMethod = parent.firstMethod("packagePrivateMethodToBecomePublic"); + assertThat(isOverriden(new MethodOverrideKey(parentMethod), methods)).isFalse(); + } + + public static class SomeClass extends SuperClass { + @Override + void generic(Integer param) { + } + + @Override + void nonGeneric(String param) { + } + + @Override + void fromSuperClass(int param) { + } + } + + public static class SuperClass extends SuperSuperClass { + void fromSuperClass(int param) { + } + + void notOverridenFromSuperClass(int param) { + } + + void almostMatchingGeneric(V param) { + } + } + + public static class SuperSuperClass extends TheRoot { + void generic(V arg) { + } + + void almostMatchingGeneric(Integer arg) { + } + + void nonGeneric(String param) { + } + + void notOverridenGeneric(V arg) { + } + + void notOverridenNonGeneric(String param) { + } + } + + public static class TheRoot { + void generic(X param) { + } + } +} diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java index 683231f1d8b4b..5b73e11ba2f31 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java @@ -13,6 +13,7 @@ import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; +import org.jboss.jandex.Index; import org.jboss.jandex.IndexView; import org.jboss.jandex.MethodInfo; import org.junit.jupiter.api.Test; @@ -23,7 +24,7 @@ public class SubclassSkipPredicateTest { @Test public void testPredicate() throws IOException { - IndexView index = Basics.index(Base.class, Submarine.class, Long.class, Number.class); + IndexView index = Index.of(Base.class, Submarine.class, Long.class, Number.class); AssignabilityCheck assignabilityCheck = new AssignabilityCheck(index, null); SubclassSkipPredicate predicate = new SubclassSkipPredicate(assignabilityCheck::isAssignableFrom, null, Collections.emptySet()); 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 b9153c14d6ad3..da29c9dea820c 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 @@ -20,6 +20,7 @@ import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; +import org.jboss.jandex.Index; import org.jboss.jandex.IndexView; import org.jboss.jandex.ParameterizedType; import org.jboss.jandex.Type; @@ -30,7 +31,7 @@ public class TypesTest { @Test public void testGetTypeClosure() throws IOException { - IndexView index = Basics.index(Foo.class, Baz.class, Producer.class, Object.class, List.class, Collection.class, + IndexView index = Index.of(Foo.class, Baz.class, Producer.class, Object.class, List.class, Collection.class, Iterable.class, Set.class, Eagle.class, Bird.class, Animal.class, AnimalHolder.class, MyRawBean.class, MyBean.class, MyInterface.class, MySuperInterface.class); DotName bazName = DotName.createSimple(Baz.class.getName()); diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Bottom.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Bottom.java new file mode 100644 index 0000000000000..98f24240baba5 --- /dev/null +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Bottom.java @@ -0,0 +1,42 @@ +package io.quarkus.arc.processor.types; + +import io.quarkus.arc.processor.types.extrapkg.Middle; + +public class Bottom extends Middle { + @Override + public String publicMethod(String param) { + return null; + } + + @Override + protected String protectedMethod(String param) { + return null; + } + + @Override + String packagePrivateMethod(String param) { + return null; + } + + // no override + private String privateMethod(String param) { + return null; + } + + // --- + + @Override + public String protectedMethodToBecomePublic(String param) { + return null; + } + + @Override + protected String packagePrivateMethodToBecomeProtected(String param) { + return null; + } + + @Override + public String packagePrivateMethodToBecomePublic(String param) { + return null; + } +} diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Top.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Top.java new file mode 100644 index 0000000000000..d04a889a736d0 --- /dev/null +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/Top.java @@ -0,0 +1,41 @@ +package io.quarkus.arc.processor.types; + +// inheritance hierarchy: +// +// Top------>Middle------>Bottom +// \ +// +---->Middle2 +// +// Top and Bottom are in the same package, +// Middle and Middle2 are in a different package +public class Top { + public String publicMethod(String param) { + return null; + } + + protected String protectedMethod(String param) { + return null; + } + + String packagePrivateMethod(String param) { + return null; + } + + private String privateMethod(String param) { + return null; + } + + // --- + + protected String protectedMethodToBecomePublic(String param) { + return null; + } + + String packagePrivateMethodToBecomeProtected(String param) { + return null; + } + + String packagePrivateMethodToBecomePublic(String param) { + return null; + } +} diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle.java new file mode 100644 index 0000000000000..c0f3534e1504b --- /dev/null +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle.java @@ -0,0 +1,27 @@ +package io.quarkus.arc.processor.types.extrapkg; + +import io.quarkus.arc.processor.types.Top; + +public class Middle extends Top { + // no override + String packagePrivateMethod(String param) { + return null; + } + + // no override + private String privateMethod(String param) { + return null; + } + + // --- + + // no override + String packagePrivateMethodToBecomeProtected(String param) { + return null; + } + + // no override + String packagePrivateMethodToBecomePublic(String param) { + return null; + } +} diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle2.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle2.java new file mode 100644 index 0000000000000..732d504c0846d --- /dev/null +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/types/extrapkg/Middle2.java @@ -0,0 +1,42 @@ +package io.quarkus.arc.processor.types.extrapkg; + +import io.quarkus.arc.processor.types.Top; + +public class Middle2 extends Top { + @Override + public String publicMethod(String param) { + return null; + } + + @Override + protected String protectedMethod(String param) { + return null; + } + + // no override + String packagePrivateMethod(String param) { + return null; + } + + // no override + private String privateMethod(String param) { + return null; + } + + // --- + + @Override + public String protectedMethodToBecomePublic(String param) { + return null; + } + + // no override + protected String packagePrivateMethodToBecomeProtected(String param) { + return null; + } + + // no override + public String packagePrivateMethodToBecomePublic(String param) { + return null; + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/order/InjectionOrderTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/order/InjectionOrderTest.java new file mode 100644 index 0000000000000..32ee6aa2023a3 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/injection/order/InjectionOrderTest.java @@ -0,0 +1,131 @@ +package io.quarkus.arc.test.injection.order; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import jakarta.enterprise.context.Dependent; +import jakarta.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 InjectionOrderTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(Consumer.class, Dependency.class); + + @Test + public void test() { + Consumer consumer = Arc.container().select(Consumer.class).get(); + + assertFalse(ConsumerSuperclass.superConstructorInjected); // subclass calls different ctor + assertTrue(consumer.superFieldInjected()); + assertFalse(ConsumerSuperclass.superInitializerInjected); // overridden in a subclass + assertTrue(ConsumerSuperclass.superPrivateInitializerInjected); // not overridden, it's private + assertFalse(ConsumerSuperclass.superPrivateInitializerInjectedBeforeField); + + assertTrue(Consumer.constructorInjected); + assertTrue(consumer.fieldInjected()); + assertTrue(Consumer.initializerInjected); + assertFalse(Consumer.initializerInjectedBeforeField); + assertTrue(Consumer.privateInitializerInjected); + + assertFalse(Consumer.subclassInjectedBeforeSuperclass); + } + + static class ConsumerSuperclass { + static boolean superConstructorInjected; + static boolean superInitializerInjected; + static boolean superPrivateInitializerInjected; + static boolean superPrivateInitializerInjectedBeforeField; + + @Inject + private Dependency dependency; + + ConsumerSuperclass() { + onSuperInjection(); + } + + @Inject + ConsumerSuperclass(Dependency dependency) { + superConstructorInjected = true; + onSuperInjection(); + } + + @Inject + void init(Dependency dependency) { + superInitializerInjected = true; + onSuperInjection(); + } + + @Inject + private void privateInit(Dependency dependency) { + superPrivateInitializerInjected = true; + if (this.dependency == null) { + superPrivateInitializerInjectedBeforeField = true; + } + onSuperInjection(); + } + + boolean superFieldInjected() { + return this.dependency != null; + } + + void onSuperInjection() { + } + } + + @Dependent + static class Consumer extends ConsumerSuperclass { + static boolean constructorInjected; + static boolean initializerInjected; + static boolean initializerInjectedBeforeField; + static boolean privateInitializerInjected; + static boolean privateInitializerInjectedBeforeField; + + static boolean subclassInjectedBeforeSuperclass; + + @Inject + private Dependency dependency; + + @Inject + Consumer(Dependency dependency) { + super(); + constructorInjected = true; + } + + @Inject + @Override + void init(Dependency dependency) { + initializerInjected = true; + if (this.dependency == null) { + initializerInjectedBeforeField = true; + } + } + + @Inject + private void privateInit(Dependency dependency) { + privateInitializerInjected = true; + if (this.dependency == null) { + privateInitializerInjectedBeforeField = true; + } + } + + boolean fieldInjected() { + return this.dependency != null; + } + + @Override + void onSuperInjection() { + if (Consumer.initializerInjected || Consumer.privateInitializerInjected || fieldInjected()) { + Consumer.subclassInjectedBeforeSuperclass = true; + } + } + } + + @Dependent + static class Dependency { + } +} From b9250857d98146d88b01ba741e0325cf87e45f77 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 13 Feb 2023 17:27:19 +0100 Subject: [PATCH 3/4] ArC: implement CDI 4.0 startup/shutdown events --- .../runtime/src/main/java/io/quarkus/runtime/ShutdownEvent.java | 2 +- core/runtime/src/main/java/io/quarkus/runtime/StartupEvent.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/runtime/src/main/java/io/quarkus/runtime/ShutdownEvent.java b/core/runtime/src/main/java/io/quarkus/runtime/ShutdownEvent.java index bc35ce3419419..16ef6d99e60e0 100644 --- a/core/runtime/src/main/java/io/quarkus/runtime/ShutdownEvent.java +++ b/core/runtime/src/main/java/io/quarkus/runtime/ShutdownEvent.java @@ -14,7 +14,7 @@ * * The annotated method can access other injected beans. */ -public class ShutdownEvent { +public class ShutdownEvent extends jakarta.enterprise.event.Shutdown { private final ShutdownReason shutdownReason; diff --git a/core/runtime/src/main/java/io/quarkus/runtime/StartupEvent.java b/core/runtime/src/main/java/io/quarkus/runtime/StartupEvent.java index 138e271161ea9..02c66f036cfa2 100644 --- a/core/runtime/src/main/java/io/quarkus/runtime/StartupEvent.java +++ b/core/runtime/src/main/java/io/quarkus/runtime/StartupEvent.java @@ -17,5 +17,5 @@ * The annotated method can access other injected beans. * */ -public class StartupEvent { +public class StartupEvent extends jakarta.enterprise.event.Startup { } From a8af85b01686b3916c5007d2262e972bc0ccc6be Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Tue, 14 Feb 2023 15:00:31 +0100 Subject: [PATCH 4/4] ArC: improve bean archive detection --- .../quarkus/arc/deployment/ArcProcessor.java | 4 +- .../arc/deployment/BeanArchiveProcessor.java | 47 ++++++++++++++++++- .../quarkus/arc/processor/BeanDeployment.java | 5 ++ .../io/quarkus/arc/processor/DotNames.java | 2 + 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java index 8e8ebf3f2e1d6..700ae0e278041 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java @@ -682,7 +682,9 @@ void initTestApplicationClassPredicateBean(ArcRecorder recorder, BeanContainerBu @BuildStep List marker() { return Arrays.asList(new AdditionalApplicationArchiveMarkerBuildItem("META-INF/beans.xml"), - new AdditionalApplicationArchiveMarkerBuildItem("META-INF/services/jakarta.enterprise.inject.spi.Extension")); + new AdditionalApplicationArchiveMarkerBuildItem("META-INF/services/jakarta.enterprise.inject.spi.Extension"), + new AdditionalApplicationArchiveMarkerBuildItem( + "META-INF/services/jakarta.enterprise.inject.build.compatible.spi.BuildCompatibleExtension")); } @BuildStep diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/BeanArchiveProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/BeanArchiveProcessor.java index cb75954e4b327..5e8158993fafd 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/BeanArchiveProcessor.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/BeanArchiveProcessor.java @@ -1,5 +1,8 @@ package io.quarkus.arc.deployment; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -17,6 +20,7 @@ import org.jboss.jandex.DotName; import org.jboss.jandex.IndexView; import org.jboss.jandex.Indexer; +import org.jboss.logging.Logger; import io.quarkus.arc.processor.BeanArchives; import io.quarkus.arc.processor.BeanDefiningAnnotation; @@ -37,6 +41,8 @@ public class BeanArchiveProcessor { + private static final Logger LOGGER = Logger.getLogger(BeanArchiveProcessor.class); + @BuildStep public BeanArchiveIndexBuildItem build(ArcConfig config, ApplicationArchivesBuildItem applicationArchivesBuildItem, List additionalBeanDefiningAnnotations, @@ -127,8 +133,12 @@ private IndexView buildApplicationIndex(ArcConfig config, ApplicationArchivesBui if (isApplicationArchiveExcluded(config, excludeDependencyBuildItems, archive)) { continue; } + if (!possiblyBeanArchive(archive)) { + continue; + } IndexView index = archive.getIndex(); - if (isExplicitBeanArchive(archive) || isImplicitBeanArchive(index, beanDefiningAnnotations) + if (isExplicitBeanArchive(archive) + || isImplicitBeanArchive(index, beanDefiningAnnotations) || isAdditionalBeanArchive(archive, beanArchivePredicates)) { indexes.add(index); } @@ -144,6 +154,7 @@ private boolean isExplicitBeanArchive(ApplicationArchive archive) { private boolean isImplicitBeanArchive(IndexView index, Set beanDefiningAnnotations) { // NOTE: Implicit bean archive without beans.xml contains one or more bean classes with a bean defining annotation and no extension return index.getAllKnownImplementors(DotNames.EXTENSION).isEmpty() + && index.getAllKnownImplementors(DotNames.BUILD_COMPATIBLE_EXTENSION).isEmpty() && containsBeanDefiningAnnotation(index, beanDefiningAnnotations); } @@ -157,6 +168,40 @@ private boolean isAdditionalBeanArchive(ApplicationArchive archive, return false; } + private boolean possiblyBeanArchive(ApplicationArchive archive) { + return archive.apply(tree -> { + boolean result = true; + for (String beansXml : List.of("META-INF/beans.xml", "WEB-INF/beans.xml")) { + result &= tree.apply(beansXml, pathVisit -> { + if (pathVisit == null) { + return true; + } + + // crude but enough + try { + String text = Files.readString(pathVisit.getPath()); + if (text.contains("bean-discovery-mode='none'") + || text.contains("bean-discovery-mode=\"none\"")) { + return false; + } + + if (text.contains("bean-discovery-mode='all'") + || text.contains("bean-discovery-mode=\"all\"")) { + LOGGER.warnf("Detected bean archive with bean discovery mode of 'all', " + + "this is not portable in CDI Lite and is treated as 'annotated' in Quarkus! " + + "Path to beans.xml: %s", pathVisit.getPath()); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + + return true; + }); + } + return result; + }); + } + private boolean isApplicationArchiveExcluded(ArcConfig config, List excludeDependencyBuildItems, ApplicationArchive archive) { if (archive.getKey() != null) { 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 7e986296102a6..da182d80f3591 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 @@ -946,6 +946,11 @@ private List findBeans(Collection beanDefiningAnnotations, Li continue; } + if (beanClass.interfaceNames().contains(DotNames.BUILD_COMPATIBLE_EXTENSION)) { + // Skip build compatible extensions + continue; + } + boolean hasBeanDefiningAnnotation = false; if (annotationStore.hasAnyAnnotation(beanClass, beanDefiningAnnotations)) { hasBeanDefiningAnnotation = true; diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/DotNames.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/DotNames.java index 2959a12dfe7ce..713f6e06c2be9 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/DotNames.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/DotNames.java @@ -32,6 +32,7 @@ import jakarta.enterprise.inject.TransientReference; import jakarta.enterprise.inject.Typed; import jakarta.enterprise.inject.Vetoed; +import jakarta.enterprise.inject.build.compatible.spi.BuildCompatibleExtension; import jakarta.enterprise.inject.spi.Bean; import jakarta.enterprise.inject.spi.BeanManager; import jakarta.enterprise.inject.spi.EventMetadata; @@ -108,6 +109,7 @@ public final class DotNames { public static final DotName CLASS = create(Class.class); public static final DotName ENUM = create(Enum.class); public static final DotName EXTENSION = create(Extension.class); + public static final DotName BUILD_COMPATIBLE_EXTENSION = create(BuildCompatibleExtension.class); public static final DotName OPTIONAL = create(Optional.class); public static final DotName OPTIONAL_INT = create(OptionalInt.class); public static final DotName OPTIONAL_LONG = create(OptionalLong.class);