Merge pull request #31348 from manovotn/validateProducersInInterceptors

Arc - Validate whether interceptors declare producer methods
quarkusio · Feb 23, 2023 · 1b8fc61 · 1b8fc61
2 parents f6d6823 + 174b2c6
commit 1b8fc61
Show file tree

Hide file tree

Showing 5 changed files with 244 additions and 5 deletions.
diff --git a/...endent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/AnnotationStore.java b/...endent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/AnnotationStore.java
@@ -114,7 +114,7 @@ private Collection<AnnotationInstance> getOriginalAnnotations(AnnotationTarget t
         Collection<AnnotationInstance> annotations;
         switch (target.kind()) {
             case CLASS:
-                annotations = target.asClass().classAnnotations();
+                annotations = target.asClass().declaredAnnotations();
                 break;
             case METHOD:
                 // Note that the returning collection also contains method params annotations

diff --git a/...endent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorInfo.java b/...endent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorInfo.java
@@ -9,12 +9,14 @@
 import java.util.List;
 import java.util.Set;
 
+import jakarta.enterprise.inject.spi.DefinitionException;
 import jakarta.enterprise.inject.spi.InterceptionType;
 
 import org.jboss.jandex.AnnotationInstance;
 import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.ClassInfo;
 import org.jboss.jandex.DotName;
+import org.jboss.jandex.FieldInfo;
 import org.jboss.jandex.MethodInfo;
 import org.jboss.jandex.Type;
 import org.jboss.jandex.Type.Kind;
@@ -46,6 +48,7 @@ public class InterceptorInfo extends BeanInfo implements Comparable<InterceptorI
                 Sets.singletonHashSet(Type.create(target.asClass().name(), Kind.CLASS)), new HashSet<>(), injections,
                 null, null, false, Collections.emptyList(), null, false, null, priority);
         this.bindings = bindings;
+        AnnotationStore store = beanDeployment.getAnnotationStore();
         List<MethodInfo> aroundInvokes = new ArrayList<>();
         List<MethodInfo> aroundConstructs = new ArrayList<>();
         List<MethodInfo> postConstructs = new ArrayList<>();
@@ -57,20 +60,34 @@ public class InterceptorInfo extends BeanInfo implements Comparable<InterceptorI
                 if (Modifier.isStatic(method.flags())) {
                     continue;
                 }
-                if (method.hasAnnotation(DotNames.AROUND_INVOKE)) {
+                if (store.hasAnnotation(method, DotNames.PRODUCES) || store.hasAnnotation(method, DotNames.DISPOSES)) {
+                    // according to spec, finding @Produces or @Disposes on a method is a DefinitionException
+                    throw new DefinitionException(
+                            "An interceptor method cannot be marked @Produces or @Disposes - " + method + " in class: "
+                                    + aClass);
+                }
+                if (store.hasAnnotation(method, DotNames.AROUND_INVOKE)) {
                     aroundInvokes.add(validateSignature(method));
                 }
-                if (method.hasAnnotation(DotNames.AROUND_CONSTRUCT)) {
+                if (store.hasAnnotation(method, DotNames.AROUND_CONSTRUCT)) {
                     aroundConstructs.add(validateSignature(method));
                 }
-                if (method.hasAnnotation(DotNames.POST_CONSTRUCT)) {
+                if (store.hasAnnotation(method, DotNames.POST_CONSTRUCT)) {
                     postConstructs.add(validateSignature(method));
                 }
-                if (method.hasAnnotation(DotNames.PRE_DESTROY)) {
+                if (store.hasAnnotation(method, DotNames.PRE_DESTROY)) {
                     preDestroys.add(validateSignature(method));
                 }
             }
 
+            for (FieldInfo field : aClass.fields()) {
+                if (store.hasAnnotation(field, DotNames.PRODUCES)) {
+                    // according to spec, finding @Produces on a field is a DefinitionException
+                    throw new DefinitionException(
+                            "An interceptor field cannot be marked @Produces - " + field + " in class: " + aClass);
+                }
+            }
+
             DotName superTypeName = aClass.superName();
             aClass = superTypeName == null || DotNames.OBJECT.equals(superTypeName) ? null
                     : getClassByName(beanDeployment.getBeanArchiveIndex(), superTypeName);

diff --git a/...rc/test/java/io/quarkus/arc/test/producer/disposer/illegal/DisposerInInterceptorTest.java b/...rc/test/java/io/quarkus/arc/test/producer/disposer/illegal/DisposerInInterceptorTest.java
@@ -0,0 +1,73 @@
+package io.quarkus.arc.test.producer.disposer.illegal;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import jakarta.annotation.Priority;
+import jakarta.enterprise.context.Dependent;
+import jakarta.enterprise.inject.Disposes;
+import jakarta.enterprise.inject.spi.DefinitionException;
+import jakarta.interceptor.AroundInvoke;
+import jakarta.interceptor.Interceptor;
+import jakarta.interceptor.InterceptorBinding;
+import jakarta.interceptor.InvocationContext;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.test.ArcTestContainer;
+
+public class DisposerInInterceptorTest {
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder()
+            .beanClasses(BadInterceptor.class, FooBean.class, MyBinding.class).shouldFail()
+            .build();
+
+    @Test
+    public void testFailure() {
+        Throwable error = container.getFailure();
+        assertNotNull(error);
+        assertTrue(error instanceof DefinitionException);
+    }
+
+    @Interceptor
+    @MyBinding
+    @Priority(1)
+    static class BadInterceptor {
+
+        @AroundInvoke
+        public Object aroundInvoke(InvocationContext ic) throws Exception {
+            return ic.proceed();
+        }
+
+        // declaring a disposer inside an interceptor should raise DefinitionException
+        void dispose(@Disposes String ignored) {
+        }
+
+    }
+
+    @Dependent
+    @MyBinding
+    static class FooBean {
+
+        public String ping() {
+            return FooBean.class.getSimpleName();
+        }
+
+    }
+
+    @Target({ TYPE, METHOD })
+    @Retention(RUNTIME)
+    @Documented
+    @InterceptorBinding
+    @interface MyBinding {
+
+    }
+}
diff --git a/...ts/src/test/java/io/quarkus/arc/test/producer/illegal/ProducerFieldInInterceptorTest.java b/...ts/src/test/java/io/quarkus/arc/test/producer/illegal/ProducerFieldInInterceptorTest.java
@@ -0,0 +1,73 @@
+package io.quarkus.arc.test.producer.illegal;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import jakarta.annotation.Priority;
+import jakarta.enterprise.context.Dependent;
+import jakarta.enterprise.inject.Produces;
+import jakarta.enterprise.inject.spi.DefinitionException;
+import jakarta.interceptor.AroundInvoke;
+import jakarta.interceptor.Interceptor;
+import jakarta.interceptor.InterceptorBinding;
+import jakarta.interceptor.InvocationContext;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.test.ArcTestContainer;
+
+public class ProducerFieldInInterceptorTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder()
+            .beanClasses(BadInterceptor.class, FooBean.class, MyBinding.class).shouldFail()
+            .build();
+
+    @Test
+    public void testFailure() {
+        Throwable error = container.getFailure();
+        assertNotNull(error);
+        assertTrue(error instanceof DefinitionException);
+    }
+
+    @Interceptor
+    @MyBinding
+    @Priority(1)
+    static class BadInterceptor {
+
+        @AroundInvoke
+        public Object aroundInvoke(InvocationContext ic) throws Exception {
+            return ic.proceed();
+        }
+
+        // declaring a producer inside an interceptor should raise DefinitionException
+        @Produces
+        String val = "42";
+    }
+
+    @Dependent
+    @MyBinding
+    static class FooBean {
+
+        public String ping() {
+            return ProducerMethodInInterceptorTest.FooBean.class.getSimpleName();
+        }
+
+    }
+
+    @Target({ TYPE, METHOD })
+    @Retention(RUNTIME)
+    @Documented
+    @InterceptorBinding
+    @interface MyBinding {
+
+    }
+}
diff --git a/...s/src/test/java/io/quarkus/arc/test/producer/illegal/ProducerMethodInInterceptorTest.java b/...s/src/test/java/io/quarkus/arc/test/producer/illegal/ProducerMethodInInterceptorTest.java
@@ -0,0 +1,76 @@
+package io.quarkus.arc.test.producer.illegal;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import jakarta.annotation.Priority;
+import jakarta.enterprise.context.Dependent;
+import jakarta.enterprise.inject.Produces;
+import jakarta.enterprise.inject.spi.DefinitionException;
+import jakarta.interceptor.AroundInvoke;
+import jakarta.interceptor.Interceptor;
+import jakarta.interceptor.InterceptorBinding;
+import jakarta.interceptor.InvocationContext;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.test.ArcTestContainer;
+
+public class ProducerMethodInInterceptorTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder()
+            .beanClasses(BadInterceptor.class, FooBean.class, MyBinding.class).shouldFail()
+            .build();
+
+    @Test
+    public void testFailure() {
+        Throwable error = container.getFailure();
+        assertNotNull(error);
+        assertTrue(error instanceof DefinitionException);
+    }
+
+    @Interceptor
+    @MyBinding
+    @Priority(1)
+    static class BadInterceptor {
+
+        @AroundInvoke
+        public Object aroundInvoke(InvocationContext ic) throws Exception {
+            return ic.proceed();
+        }
+
+        // declaring a producer inside an interceptor should raise DefinitionException
+        @Produces
+        String generateString() {
+            return "42";
+        }
+
+    }
+
+    @Dependent
+    @MyBinding
+    static class FooBean {
+
+        public String ping() {
+            return FooBean.class.getSimpleName();
+        }
+
+    }
+
+    @Target({ TYPE, METHOD })
+    @Retention(RUNTIME)
+    @Documented
+    @InterceptorBinding
+    @interface MyBinding {
+
+    }
+}