Detect callback methods (such as methods annotated with @PostConstruct …

…and @PreDestroy), and validate their signature. In this case, a DefinitionException is thrown, indicating that the method signature is invalid. Fix quarkusio#27591 according to the discussion.
gsmet · Oct 27, 2022 · cb2c0d5 · cb2c0d5
1 parent efeb47c
commit cb2c0d5
Show file tree

Hide file tree

Showing 6 changed files with 133 additions and 7 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
@@ -156,6 +156,7 @@ AdditionalBeanBuildItem quarkusApplication(CombinedIndexBuildItem combinedIndex)
                 quarkusApplications.add(quarkusApplication.name().toString());
             }
         }
+
         return AdditionalBeanBuildItem.builder().setUnremovable()
                 .setDefaultScope(DotName.createSimple(ApplicationScoped.class.getName()))
                 .addBeanClasses(quarkusApplications)

diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Beans.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Beans.java
@@ -795,13 +795,17 @@ private static void fetchType(Type type, BeanDeployment beanDeployment) {
     private static void collectCallbacks(ClassInfo clazz, List<MethodInfo> callbacks, DotName annotation, IndexView index,
             Set<String> knownMethods) {
         for (MethodInfo method : clazz.methods()) {
-            if (method.returnType().kind() == Kind.VOID && method.parameterTypes().isEmpty()) {
-                if (method.hasAnnotation(annotation) && !knownMethods.contains(method.name())) {
+            if (method.hasAnnotation(annotation) && !knownMethods.contains(method.name())) {
+                if (method.returnType().kind() == Kind.VOID && method.parameterTypes().isEmpty()) {
                     callbacks.add(method);
+                } else {
+                    // invalid signature - build a meaningful message.
+                    throw new DefinitionException("Invalid signature for the method `" + method + "` from class `"
+                            + method.declaringClass() + "`. Methods annotated with `" + annotation + "` must return" +
+                            " `void` and cannot have parameters.");
                 }
-                knownMethods.add(method.name());
             }
-
+            knownMethods.add(method.name());
         }
         if (clazz.superName() != null) {
             ClassInfo superClass = getClassByName(index, clazz.superName());
@@ -866,7 +870,7 @@ private static Integer initAlternativePriority(AnnotationTarget target, Integer
         if (computedPriority != null) {
             if (alternativePriority != null) {
                 LOGGER.infof(
-                        "Computed priority [%s] overrides the priority [%s] declared via @Priority or @AlernativePriority",
+                        "Computed priority [%s] overrides the priority [%s] declared via @Priority or @AlternativePriority",
                         computedPriority, alternativePriority);
             }
             alternativePriority = computedPriority;

diff --git a/...ava/io/quarkus/arc/test/bean/lifecycle/inheritance/BeanLifecycleMethodsOverridenTest.java b/...ava/io/quarkus/arc/test/bean/lifecycle/inheritance/BeanLifecycleMethodsOverridenTest.java
@@ -19,7 +19,7 @@ public class BeanLifecycleMethodsOverridenTest {
     ArcTestContainer container = new ArcTestContainer(Bird.class, Eagle.class, Falcon.class);
 
     @Test
-    public void testOverridenMethodWithNoAnnotation() {
+    public void testOverriddenMethodWithNoAnnotation() {
         resetAll();
         InstanceHandle<Falcon> falconInstanceHandle = Arc.container().instance(Falcon.class);
         falconInstanceHandle.get().ping();
@@ -31,7 +31,7 @@ public void testOverridenMethodWithNoAnnotation() {
     }
 
     @Test
-    public void testOverridenMethodWithLifecycleAnnotation() {
+    public void testOverriddenMethodWithLifecycleAnnotation() {
         resetAll();
         InstanceHandle<Eagle> eagleInstanceHandle = Arc.container().instance(Eagle.class);
         eagleInstanceHandle.get().ping();

diff --git a/...ects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructTest.java b/...ects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructTest.java
@@ -0,0 +1,38 @@
+package io.quarkus.arc.test.validation;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.annotation.PostConstruct;
+import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.inject.spi.DefinitionException;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.Unremovable;
+import io.quarkus.arc.test.ArcTestContainer;
+import io.smallrye.mutiny.Uni;
+
+public class InvalidPostConstructTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder().beanClasses(InvalidBean.class).shouldFail().build();
+
+    @Test
+    public void testFailure() {
+        Throwable error = container.getFailure();
+        assertNotNull(error);
+        assertTrue(error instanceof DefinitionException);
+    }
+
+    @ApplicationScoped
+    @Unremovable
+    public static class InvalidBean {
+
+        @PostConstruct
+        public Uni<Void> invalid() {
+            return Uni.createFrom().nullItem();
+        }
+    }
+}
diff --git a/.../src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructWithParametersTest.java b/.../src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructWithParametersTest.java
@@ -0,0 +1,41 @@
+package io.quarkus.arc.test.validation;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.annotation.PostConstruct;
+import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.inject.spi.DefinitionException;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.Unremovable;
+import io.quarkus.arc.test.ArcTestContainer;
+
+public class InvalidPostConstructWithParametersTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder().beanClasses(InvalidBean.class).shouldFail().build();
+
+    @Test
+    public void testFailure() {
+        Throwable error = container.getFailure();
+        assertNotNull(error);
+        assertTrue(error instanceof DefinitionException);
+        Assertions.assertTrue(error.getMessage().contains("invalid(java.lang.String ignored)"));
+        Assertions.assertTrue(error.getMessage().contains("$InvalidBean"));
+        Assertions.assertTrue(error.getMessage().contains("PostConstruct"));
+    }
+
+    @ApplicationScoped
+    @Unremovable
+    public static class InvalidBean {
+
+        @PostConstruct
+        public void invalid(String ignored) {
+
+        }
+    }
+}
diff --git a/...rojects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPreDestroyTest.java b/...rojects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPreDestroyTest.java
@@ -0,0 +1,42 @@
+package io.quarkus.arc.test.validation;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.annotation.PreDestroy;
+import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.inject.spi.DefinitionException;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.Unremovable;
+import io.quarkus.arc.test.ArcTestContainer;
+import io.smallrye.mutiny.Multi;
+
+public class InvalidPreDestroyTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder().beanClasses(InvalidBean.class).shouldFail().build();
+
+    @Test
+    public void testFailure() {
+        Throwable error = container.getFailure();
+        assertNotNull(error);
+        assertTrue(error instanceof DefinitionException);
+        Assertions.assertTrue(error.getMessage().contains("invalid()"));
+        Assertions.assertTrue(error.getMessage().contains("$InvalidBean"));
+        Assertions.assertTrue(error.getMessage().contains("PreDestroy"));
+    }
+
+    @ApplicationScoped
+    @Unremovable
+    public static class InvalidBean {
+
+        @PreDestroy
+        public Multi<Void> invalid() {
+            return null;
+        }
+    }
+}