From d34eb5adda6cd3fe534e538bbf16170b5f6df212 Mon Sep 17 00:00:00 2001 From: Matej Novotny Date: Sun, 18 Feb 2024 12:15:31 +0100 Subject: [PATCH] Correct how we process injection point transformers for SR CP. Correct gRPC injection point transformer to expect method AnnotationTarget instead of method param. --- .../grpc/deployment/GrpcClientProcessor.java | 11 +- .../SmallRyeContextPropagationProcessor.java | 239 ++++++++++++------ .../cdi/ConfiguredAndSharedBeansTest.java | 39 +++ .../arc/processor/InjectionPointInfo.java | 2 +- .../arc/processor/InjectionPointModifier.java | 17 +- 5 files changed, 221 insertions(+), 87 deletions(-) diff --git a/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcClientProcessor.java b/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcClientProcessor.java index 54f481ff16420..e66fef8f2e7ef 100644 --- a/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcClientProcessor.java +++ b/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcClientProcessor.java @@ -26,6 +26,7 @@ import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; import org.jboss.jandex.AnnotationInstance; +import org.jboss.jandex.AnnotationTarget; import org.jboss.jandex.AnnotationTarget.Kind; import org.jboss.jandex.AnnotationValue; import org.jboss.jandex.ClassInfo; @@ -345,16 +346,22 @@ public void transform(TransformationContext ctx) { AnnotationInstance clientAnnotation = Annotations.find(ctx.getQualifiers(), GrpcDotNames.GRPC_CLIENT); if (clientAnnotation != null && clientAnnotation.value() == null) { String clientName = null; + AnnotationTarget annotationTarget = ctx.getTarget(); if (ctx.getTarget().kind() == Kind.FIELD) { clientName = clientAnnotation.target().asField().name(); - } else if (ctx.getTarget().kind() == Kind.METHOD_PARAMETER) { + } else if (ctx.getTarget().kind() == Kind.METHOD + && clientAnnotation.target().kind().equals(Kind.METHOD_PARAMETER)) { MethodParameterInfo param = clientAnnotation.target().asMethodParameter(); + annotationTarget = param; // We don't need to check if parameter names are recorded - that's validated elsewhere clientName = param.method().parameterName(param.position()); } if (clientName != null) { ctx.transform().remove(GrpcDotNames::isGrpcClient) - .add(GrpcDotNames.GRPC_CLIENT, AnnotationValue.createStringValue("value", clientName)).done(); + .add(AnnotationInstance.builder(GrpcDotNames.GRPC_CLIENT) + .value(clientName) + .buildWithTarget(annotationTarget)) + .done(); } } } diff --git a/extensions/smallrye-context-propagation/deployment/src/main/java/io/quarkus/smallrye/context/deployment/SmallRyeContextPropagationProcessor.java b/extensions/smallrye-context-propagation/deployment/src/main/java/io/quarkus/smallrye/context/deployment/SmallRyeContextPropagationProcessor.java index bcbe69dda868f..f923fc6dcaf70 100644 --- a/extensions/smallrye-context-propagation/deployment/src/main/java/io/quarkus/smallrye/context/deployment/SmallRyeContextPropagationProcessor.java +++ b/extensions/smallrye-context-propagation/deployment/src/main/java/io/quarkus/smallrye/context/deployment/SmallRyeContextPropagationProcessor.java @@ -4,7 +4,6 @@ import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -22,6 +21,7 @@ import org.jboss.jandex.AnnotationTarget; import org.jboss.jandex.AnnotationValue; import org.jboss.jandex.DotName; +import org.jboss.jandex.MethodParameterInfo; import org.jboss.jandex.Type; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; @@ -52,6 +52,8 @@ */ class SmallRyeContextPropagationProcessor { + private static final String NAME_DELIMITER = "/"; + @BuildStep void registerBean(BuildProducer additionalBeans) { additionalBeans @@ -130,41 +132,69 @@ public void transform(TransformationContext transformationContext) { && !ann.name().equals(io.quarkus.arc.processor.DotNames.DEFAULT))) { return; } - // create a unique name based on the injection point - String mpConfigIpName; AnnotationTarget target = transformationContext.getTarget(); - final String nameDelimiter = "/"; - switch (target.kind()) { - case FIELD: - mpConfigIpName = target.asField().declaringClass().name().toString() - + nameDelimiter + if (target.kind().equals(AnnotationTarget.Kind.FIELD)) { + AnnotationInstance meConfigInstance = Annotations.find(transformationContext.getAllAnnotations(), + DotNames.MANAGED_EXECUTOR_CONFIG); + AnnotationInstance tcConfigInstance = Annotations.find(transformationContext.getAllAnnotations(), + DotNames.THREAD_CONTEXT_CONFIG); + + if (meConfigInstance != null || tcConfigInstance != null) { + // create a unique name based on the injection point + String mpConfigIpName = target.asField().declaringClass().name().toString() + + NAME_DELIMITER + target.asField().name(); - break; - case METHOD_PARAMETER: - mpConfigIpName = target.asMethodParameter().method().declaringClass().name().toString() - + nameDelimiter - + target.asMethodParameter().method().name() - + nameDelimiter - + (target.asMethodParameter().position() + 1); - break; - // any other value is unexpected and we skip that - default: - return; - } - AnnotationInstance meConfigInstance = Annotations.find(transformationContext.getAllAnnotations(), - DotNames.MANAGED_EXECUTOR_CONFIG); - AnnotationInstance tcConfigInstance = Annotations.find(transformationContext.getAllAnnotations(), - DotNames.THREAD_CONTEXT_CONFIG); - if (meConfigInstance != null || tcConfigInstance != null) { - // add @NamedInstance with the generated name - transformationContext.transform() - .add(DotNames.NAMED_INSTANCE, AnnotationValue.createStringValue("value", mpConfigIpName)).done(); + // add @NamedInstance with the generated name + transformationContext.transform() + .add(DotNames.NAMED_INSTANCE, AnnotationValue.createStringValue("value", mpConfigIpName)) + .done(); + } + } else if (target.kind().equals(AnnotationTarget.Kind.METHOD)) { + // If it's method, we can have multiple parameters that we might need to configure and + // each injection point needs its own unique @NamedInstance. + // Finally, we register these annotation instance with the transformer. Note that when creating + // each annotation instance, we have to use AnnotationTarget of the _method parameter_ + Collection annotationsToAdd = new ArrayList<>(); + createRequiredAnnotationInstances(Annotations.getAnnotations(AnnotationTarget.Kind.METHOD_PARAMETER, + DotNames.MANAGED_EXECUTOR_CONFIG, transformationContext.getAllAnnotations()), + transformationContext.getQualifiers(), annotationsToAdd); + createRequiredAnnotationInstances(Annotations.getAnnotations(AnnotationTarget.Kind.METHOD_PARAMETER, + DotNames.THREAD_CONTEXT_CONFIG, transformationContext.getAllAnnotations()), + transformationContext.getQualifiers(), annotationsToAdd); + transformationContext.transform().addAll(annotationsToAdd).done(); } + } }); } + private void createRequiredAnnotationInstances(Collection configAnnotationInstances, + Collection knownQualifiers, + Collection instancesToAdd) { + for (AnnotationInstance annotationInstance : configAnnotationInstances) { + if (annotationInstance.target().kind().equals(AnnotationTarget.Kind.METHOD_PARAMETER)) { + MethodParameterInfo methodParameterInfo = annotationInstance.target().asMethodParameter(); + // skip if the method param injection point has custom qualifiers on it (including @NamedInstance) + if (methodParameterInfo.annotations().stream() + .anyMatch(ann -> knownQualifiers.contains(ann) + && !ann.name().equals(io.quarkus.arc.processor.DotNames.ANY) + && !ann.name().equals(io.quarkus.arc.processor.DotNames.DEFAULT))) { + continue; + } + String mpConfigIpName = methodParameterInfo.method().declaringClass().name().toString() + + NAME_DELIMITER + + methodParameterInfo.method().name() + + NAME_DELIMITER + + (methodParameterInfo.position() + 1); + // create a new AnnotationInstance with annotation target set to the respective _method parameter_ + instancesToAdd.add(AnnotationInstance.builder(DotNames.NAMED_INSTANCE) + .value(mpConfigIpName) + .buildWithTarget(methodParameterInfo)); + } + } + } + @BuildStep @Record(ExecutionTime.RUNTIME_INIT) void createSynthBeansForConfiguredInjectionPoints(BuildProducer syntheticBeanBuildItemBuildProducer, @@ -174,54 +204,111 @@ void createSynthBeansForConfiguredInjectionPoints(BuildProducer threadContextMap = new HashMap<>(); Set unconfiguredContextIPs = new HashSet<>(); for (InjectionPointInfo ipInfo : bdFinishedBuildItem.getInjectionPoints()) { - AnnotationInstance namedAnnotation = ipInfo.getRequiredQualifier(DotNames.NAMED_INSTANCE); - // only look for IP with @NamedInstance on it because the IP transformation made sure it's there - if (namedAnnotation == null) { - continue; - } - // furthermore, we only look for any IP that doesn't have other custom qualifier - if (ipInfo.getRequiredQualifiers().stream() - .anyMatch(ann -> !ann.name().equals(DotNames.NAMED_INSTANCE) - && !ann.name().equals(io.quarkus.arc.processor.DotNames.ANY) - && !ann.name().equals(io.quarkus.arc.processor.DotNames.DEFAULT))) { - continue; - } + if (AnnotationTarget.Kind.FIELD.equals(ipInfo.getTarget().kind())) { + AnnotationInstance namedAnnotation = ipInfo.getRequiredQualifier(DotNames.NAMED_INSTANCE); + // only look for IP with @NamedInstance on it because the IP transformation made sure it's there + if (namedAnnotation == null) { + continue; + } + // furthermore, we only look for any IP that doesn't have other custom qualifier + if (ipInfo.getRequiredQualifiers().stream() + .anyMatch(ann -> !ann.name().equals(DotNames.NAMED_INSTANCE) + && !ann.name().equals(io.quarkus.arc.processor.DotNames.ANY) + && !ann.name().equals(io.quarkus.arc.processor.DotNames.DEFAULT))) { + continue; + } + AnnotationInstance meConfigInstance = Annotations.find(ipInfo.getTarget().asField().annotations(), + DotNames.MANAGED_EXECUTOR_CONFIG); + AnnotationInstance tcConfigInstance = Annotations.find(ipInfo.getTarget().asField().annotations(), + DotNames.THREAD_CONTEXT_CONFIG); - AnnotationInstance meConfigInstance = Annotations.find(extractAnnotations(ipInfo.getTarget()), - DotNames.MANAGED_EXECUTOR_CONFIG); - AnnotationInstance tcConfigInstance = Annotations.find(extractAnnotations(ipInfo.getTarget()), - DotNames.THREAD_CONTEXT_CONFIG); + // get the name from @NamedInstance qualifier + String nameValue = namedAnnotation.value().asString(); - // get the name from @NamedInstance qualifier - String nameValue = namedAnnotation.value().asString(); + if (meConfigInstance == null && tcConfigInstance == null) { + // injection point with @NamedInstance on it but no configuration + if (ipInfo.getType().name().equals(DotNames.MANAGED_EXECUTOR)) { + unconfiguredExecutorIPs.add(nameValue); + } else { + unconfiguredContextIPs.add(nameValue); + } + continue; + } + // we are looking for injection points with @ManagedExecutorConfig/@ThreadContextConfig + if (meConfigInstance != null || tcConfigInstance != null) { + if (meConfigInstance != null) { + // parse ME config annotation and store in a map + executorMap.putIfAbsent(nameValue, + new ExecutorConfig(meConfigInstance.value("cleared"), + meConfigInstance.value("propagated"), + meConfigInstance.value("maxAsync"), + meConfigInstance.value("maxQueued"))); - if (meConfigInstance == null && tcConfigInstance == null) { - // injection point with @NamedInstance on it but no configuration - if (ipInfo.getType().name().equals(DotNames.MANAGED_EXECUTOR)) { - unconfiguredExecutorIPs.add(nameValue); - } else { - unconfiguredContextIPs.add(nameValue); + } else if (tcConfigInstance != null) { + // parse TC config annotation + threadContextMap.putIfAbsent(nameValue, + new ThreadConfig(tcConfigInstance.value("cleared"), + tcConfigInstance.value("propagated"), + tcConfigInstance.value("unchanged"))); + } } - continue; - } - // we are looking for injection points with @ManagedExecutorConfig/@ThreadContextConfig - if (meConfigInstance != null || tcConfigInstance != null) { - if (meConfigInstance != null) { - // parse ME config annotation and store in a map - executorMap.putIfAbsent(nameValue, - new ExecutorConfig(meConfigInstance.value("cleared"), - meConfigInstance.value("propagated"), - meConfigInstance.value("maxAsync"), - meConfigInstance.value("maxQueued"))); - - } else if (tcConfigInstance != null) { - // parse TC config annotation - threadContextMap.putIfAbsent(nameValue, - new ThreadConfig(tcConfigInstance.value("cleared"), - tcConfigInstance.value("propagated"), - tcConfigInstance.value("unchanged"))); + } else if (AnnotationTarget.Kind.METHOD.equals(ipInfo.getTarget().kind())) { + // for a method, we need to process each parameter as a separate injection point + for (AnnotationInstance annotationInstance : ipInfo.getRequiredQualifiers()) { + // just METHOD_PARAMETER and filter to only @NamedInstance + if (annotationInstance.target() == null + || !AnnotationTarget.Kind.METHOD_PARAMETER.equals(annotationInstance.target().kind()) + || !annotationInstance.name().equals(DotNames.NAMED_INSTANCE)) { + continue; + } + MethodParameterInfo methodParameterInfo = annotationInstance.target().asMethodParameter(); + // there should be no other custom qualifiers in this injection point + // furthermore, we only look for any IP that doesn't have other custom qualifier + if (methodParameterInfo.annotations().stream() + .anyMatch(ann -> ipInfo.getRequiredQualifiers().contains(ann) + && !ann.name().equals(DotNames.NAMED_INSTANCE) + && !ann.name().equals(io.quarkus.arc.processor.DotNames.ANY) + && !ann.name().equals(io.quarkus.arc.processor.DotNames.DEFAULT))) { + continue; + } + AnnotationInstance meConfigInstance = Annotations.find(methodParameterInfo.annotations(), + DotNames.MANAGED_EXECUTOR_CONFIG); + AnnotationInstance tcConfigInstance = Annotations.find(methodParameterInfo.annotations(), + DotNames.THREAD_CONTEXT_CONFIG); + + // get the name from @NamedInstance qualifier + String nameValue = annotationInstance.value().asString(); + + if (meConfigInstance == null && tcConfigInstance == null) { + // injection point with @NamedInstance on it but no configuration + if (ipInfo.getType().name().equals(DotNames.MANAGED_EXECUTOR)) { + unconfiguredExecutorIPs.add(nameValue); + } else { + unconfiguredContextIPs.add(nameValue); + } + continue; + } + // we are looking for injection points with @ManagedExecutorConfig/@ThreadContextConfig + if (meConfigInstance != null || tcConfigInstance != null) { + if (meConfigInstance != null) { + // parse ME config annotation and store in a map + executorMap.putIfAbsent(nameValue, + new ExecutorConfig(meConfigInstance.value("cleared"), + meConfigInstance.value("propagated"), + meConfigInstance.value("maxAsync"), + meConfigInstance.value("maxQueued"))); + + } else if (tcConfigInstance != null) { + // parse TC config annotation + threadContextMap.putIfAbsent(nameValue, + new ThreadConfig(tcConfigInstance.value("cleared"), + tcConfigInstance.value("propagated"), + tcConfigInstance.value("unchanged"))); + } + } } } + } // check all unconfigured IPs, if we also found same name and configured ones, then drop these from the set unconfiguredExecutorIPs.removeAll(unconfiguredExecutorIPs.stream() @@ -297,18 +384,6 @@ void createSynthBeansForConfiguredInjectionPoints(BuildProducer extractAnnotations(AnnotationTarget target) { - switch (target.kind()) { - case FIELD: - return target.asField().annotations(); - case METHOD_PARAMETER: - return target.asMethodParameter().method().annotations(); - // any other value is unexpected and we skip that - default: - return Collections.EMPTY_SET; - } - } - class ExecutorConfig { String[] cleared; diff --git a/extensions/smallrye-context-propagation/deployment/src/test/java/io/quarkus/smallrye/context/deployment/test/cdi/ConfiguredAndSharedBeansTest.java b/extensions/smallrye-context-propagation/deployment/src/test/java/io/quarkus/smallrye/context/deployment/test/cdi/ConfiguredAndSharedBeansTest.java index 6f007ef980060..aec7f969753e1 100644 --- a/extensions/smallrye-context-propagation/deployment/src/test/java/io/quarkus/smallrye/context/deployment/test/cdi/ConfiguredAndSharedBeansTest.java +++ b/extensions/smallrye-context-propagation/deployment/src/test/java/io/quarkus/smallrye/context/deployment/test/cdi/ConfiguredAndSharedBeansTest.java @@ -18,6 +18,7 @@ import java.util.Set; import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.Dependent; import jakarta.enterprise.inject.Produces; import jakarta.inject.Inject; import jakarta.inject.Qualifier; @@ -80,6 +81,8 @@ public void test() { bean.assertSharedThreadContextsAreTheSame(); bean.assertUserDefinedProducersAreRespected(); + + bean.assertConfiguredInjectionPointsInConstructor(); } @Qualifier @@ -93,6 +96,19 @@ public void test() { @Singleton static class SomeBean { + final ManagedExecutor ctorExecutor1; + final ManagedExecutor ctorExecutor2; + final ThreadContext ctorThreadContext; + + // c-tor injection, three out of four params should be configured + public SomeBean(@ManagedExecutorConfig(maxAsync = 8, maxQueued = 8) ManagedExecutor ctorExecutor1, + @ManagedExecutorConfig(maxAsync = 3, maxQueued = 3) ManagedExecutor ctorExecutor2, Foo foo, + @ThreadContextConfig(cleared = "CDI") ThreadContext ctorTc) { + this.ctorExecutor1 = ctorExecutor1; + this.ctorExecutor2 = ctorExecutor2; + this.ctorThreadContext = ctorTc; + } + @Inject @ManagedExecutorConfig ManagedExecutor defaultExecutor; @@ -231,6 +247,29 @@ public void assertUserDefinedProducersAreRespected() { propagated = providersToStringSet(plan.propagatedProviders); assertTrue(propagated.isEmpty()); } + + public void assertConfiguredInjectionPointsInConstructor() { + SmallRyeManagedExecutor exec = unwrapExecutor(ctorExecutor1); + assertEquals(8, exec.getMaxAsync()); + assertEquals(8, exec.getMaxQueued()); + + exec = unwrapExecutor(ctorExecutor2); + assertEquals(3, exec.getMaxAsync()); + assertEquals(3, exec.getMaxQueued()); + + SmallRyeThreadContext context = unwrapThreadContext(ctorThreadContext); + ThreadContextProviderPlan plan = context.getPlan(); + assertEquals(0, plan.unchangedProviders.size()); + Set propagated = providersToStringSet(plan.propagatedProviders); + Set cleared = providersToStringSet(plan.clearedProviders); + assertTrue(propagated.isEmpty()); + assertTrue(cleared.contains(ThreadContext.CDI)); + } + } + + @Dependent + static class Foo { + } @ApplicationScoped diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointInfo.java index e79adfdaa8da4..312c1d3943e8d 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointInfo.java @@ -100,7 +100,7 @@ static List fromMethod(MethodInfo method, ClassInfo beanClas } Type type = resolveType(paramType, beanClass, method.declaringClass(), beanDeployment); injectionPoints.add(new InjectionPointInfo(type, - transformer.applyTransformers(type, method, paramQualifiers), + transformer.applyTransformers(type, method, position, paramQualifiers), method, position, contains(paramAnnotations, DotNames.TRANSIENT_REFERENCE), contains(paramAnnotations, DotNames.DELEGATE))); } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointModifier.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointModifier.java index 9133a81f86bf8..53dedded292d0 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointModifier.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointModifier.java @@ -3,6 +3,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.jboss.jandex.AnnotationInstance; import org.jboss.jandex.AnnotationTarget; @@ -28,7 +29,8 @@ public class InjectionPointModifier { this.transformers = transformers; } - public Set applyTransformers(Type type, AnnotationTarget target, Set qualifiers) { + public Set applyTransformers(Type type, AnnotationTarget target, Integer paramPosition, + Set qualifiers) { // with no transformers, we just immediately return original set of qualifiers if (transformers.isEmpty()) { return qualifiers; @@ -39,7 +41,18 @@ public Set applyTransformers(Type type, AnnotationTarget tar transformer.transform(transformationContext); } } - return transformationContext.getQualifiers(); + if (paramPosition != null && AnnotationTarget.Kind.METHOD.equals(target.kind())) { + // only return set of qualifiers related to the given method parameter + return transformationContext.getQualifiers().stream().filter( + annotationInstance -> target.asMethod().parameters().get(paramPosition).equals(annotationInstance.target())) + .collect(Collectors.toSet()); + } else { + return transformationContext.getQualifiers(); + } + } + + public Set applyTransformers(Type type, AnnotationTarget target, Set qualifiers) { + return applyTransformers(type, target, null, qualifiers); } static class TransformationContextImpl extends AnnotationsTransformationContext>