From 72a860c78ba79f14e075a73a96208162898cd52a Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Wed, 28 Aug 2024 13:55:11 +0200 Subject: [PATCH] ArC - static methods interception: fix the set of copied annotations - also only consider annotations with RetentionPolicy.RUNTIME because AnnotatedElement#addAnnotation(AnnotationInstance) does not reflect the original RetentionPolicy - fixes #42828 --- .../InterceptedStaticMethodsProcessor.java | 18 +++++- .../InterceptedStaticMethodTest.java | 55 ++++++++++--------- 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java index 1aa676876fe5b..35a38353b95a7 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java @@ -30,6 +30,7 @@ import org.jboss.jandex.DotName; import org.jboss.jandex.IndexView; import org.jboss.jandex.MethodInfo; +import org.jboss.jandex.MethodParameterInfo; import org.jboss.jandex.Type; import org.jboss.logging.Logger; import org.objectweb.asm.ClassVisitor; @@ -59,6 +60,7 @@ import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem; import io.quarkus.deployment.builditem.GeneratedClassBuildItem; import io.quarkus.deployment.builditem.nativeimage.ReflectiveMethodBuildItem; +import io.quarkus.gizmo.AnnotatedElement; import io.quarkus.gizmo.BytecodeCreator; import io.quarkus.gizmo.ClassCreator; import io.quarkus.gizmo.ClassOutput; @@ -454,9 +456,19 @@ public ClassVisitor apply(String className, ClassVisitor outputClassVisitor) { MethodCreator newMethod = transformer.addMethod(originalDescriptor) .setModifiers(interceptedMethod.flags()) .setSignature(interceptedMethod.genericSignatureIfRequired()); - // Copy over all annotations - for (AnnotationInstance annotationInstance : interceptedMethod.annotations()) { - newMethod.addAnnotation(annotationInstance); + // Copy over all annotations with RetentionPolicy.RUNTIME + for (AnnotationInstance annotationInstance : interceptedMethod.declaredAnnotations()) { + if (annotationInstance.runtimeVisible()) { + newMethod.addAnnotation(annotationInstance); + } + } + for (MethodParameterInfo param : interceptedMethod.parameters()) { + AnnotatedElement newParam = newMethod.getParameterAnnotations(param.position()); + for (AnnotationInstance paramAnnotation : param.declaredAnnotations()) { + if (paramAnnotation.runtimeVisible()) { + newParam.addAnnotation(paramAnnotation); + } + } } for (Type exceptionType : interceptedMethod.exceptions()) { newMethod.addException(exceptionType.name().toString()); diff --git a/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/interceptor/staticmethods/InterceptedStaticMethodTest.java b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/interceptor/staticmethods/InterceptedStaticMethodTest.java index 41f8f4aecb730..65af1e0fb2d01 100644 --- a/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/interceptor/staticmethods/InterceptedStaticMethodTest.java +++ b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/interceptor/staticmethods/InterceptedStaticMethodTest.java @@ -2,10 +2,12 @@ import static java.lang.annotation.ElementType.METHOD; import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.CLASS; import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.lang.annotation.Retention; import java.lang.annotation.Target; @@ -19,15 +21,12 @@ import jakarta.interceptor.InterceptorBinding; import jakarta.interceptor.InvocationContext; -import org.jboss.jandex.AnnotationTarget; -import org.jboss.jandex.AnnotationTarget.Kind; -import org.jboss.jandex.MethodInfo; +import org.jboss.jandex.AnnotationTransformation; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.opentest4j.AssertionFailedError; import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem; -import io.quarkus.arc.processor.AnnotationsTransformer; import io.quarkus.builder.BuildChainBuilder; import io.quarkus.builder.BuildContext; import io.quarkus.builder.BuildStep; @@ -37,8 +36,9 @@ public class InterceptedStaticMethodTest { @RegisterExtension static final QuarkusUnitTest config = new QuarkusUnitTest() - .withApplicationRoot((jar) -> jar - .addClasses(InterceptMe.class, Simple.class, AnotherSimple.class, SimpleInterceptor.class)) + .withApplicationRoot(root -> root + .addClasses(InterceptMe.class, NotNull.class, WithClassPolicy.class, Simple.class, AnotherSimple.class, + SimpleInterceptor.class)) .addBuildChainCustomizer(buildCustomizer()); static Consumer buildCustomizer() { @@ -50,23 +50,10 @@ public void accept(BuildChainBuilder builder) { @Override public void execute(BuildContext context) { - context.produce(new AnnotationsTransformerBuildItem(new AnnotationsTransformer() { - - @Override - public boolean appliesTo(Kind kind) { - return AnnotationTarget.Kind.METHOD == kind; - } - - @Override - public void transform(TransformationContext context) { - MethodInfo method = context.getTarget().asMethod(); - if (method.declaringClass().name().toString() - .endsWith("AnotherSimple")) { - context.transform().add(InterceptMe.class).done(); - } - } - - })); + context.produce(new AnnotationsTransformerBuildItem( + AnnotationTransformation.forMethods() + .whenMethod(AnotherSimple.class, "ping") + .transform(tc -> tc.add(InterceptMe.class)))); } }).produces(AnnotationsTransformerBuildItem.class).build(); } @@ -84,8 +71,9 @@ public void testInterceptor() { public static class Simple { + @WithClassPolicy @InterceptMe - public static String ping(String val) { + public static String ping(@NotNull String val) { return val.toUpperCase(); } @@ -124,7 +112,12 @@ Object aroundInvoke(InvocationContext ctx) throws Exception { // verify annotations can be inspected if (ctx.getMethod().getDeclaringClass().getName().equals(Simple.class.getName())) { assertEquals(1, ctx.getMethod().getAnnotations().length); - assertNotNull(ctx.getMethod().getAnnotation(InterceptMe.class)); + assertTrue(ctx.getMethod().isAnnotationPresent(InterceptMe.class)); + assertFalse(ctx.getMethod().isAnnotationPresent(WithClassPolicy.class)); + assertFalse(ctx.getMethod().isAnnotationPresent(NotNull.class)); + if (ctx.getMethod().getName().equals("ping")) { + assertTrue(ctx.getMethod().getParameters()[0].isAnnotationPresent(NotNull.class)); + } } Object ret = ctx.proceed(); if (ret != null) { @@ -150,4 +143,14 @@ Object aroundInvoke(InvocationContext ctx) throws Exception { } + @Retention(RUNTIME) + @interface NotNull { + + } + + @Retention(CLASS) + @interface WithClassPolicy { + + } + }