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 index fb439049f29b2..2dd9d5ac1570b 100644 --- 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 @@ -21,6 +21,7 @@ import jakarta.enterprise.inject.UnsatisfiedResolutionException; import jakarta.enterprise.inject.spi.DefinitionException; import jakarta.enterprise.inject.spi.DeploymentException; +import jakarta.enterprise.inject.spi.InterceptionType; import org.jboss.jandex.AnnotationInstance; import org.jboss.jandex.AnnotationTarget; @@ -710,8 +711,17 @@ static void addImplicitQualifiers(Set qualifiers) { } static List getCallbacks(ClassInfo beanClass, DotName annotation, IndexView index) { + InterceptionType interceptionType = null; + if (DotNames.POST_CONSTRUCT.equals(annotation)) { + interceptionType = InterceptionType.POST_CONSTRUCT; + } else if (DotNames.PRE_DESTROY.equals(annotation)) { + interceptionType = InterceptionType.PRE_DESTROY; + } else { + throw new IllegalArgumentException("Unexpected callback annotation: " + annotation); + } + List callbacks = new ArrayList<>(); - collectCallbacks(beanClass, callbacks, annotation, index, new HashSet<>()); + collectCallbacks(beanClass, callbacks, annotation, index, new HashSet<>(), interceptionType); Collections.reverse(callbacks); return callbacks; } @@ -729,7 +739,8 @@ static List getAroundInvokes(ClassInfo beanClass, BeanDeployment dep continue; } if (store.hasAnnotation(method, DotNames.AROUND_INVOKE)) { - InterceptorInfo.addInterceptorMethod(allMethods, methods, method); + InterceptorInfo.addInterceptorMethod(allMethods, methods, method, InterceptionType.AROUND_INVOKE, + InterceptorPlacement.TARGET_CLASS); if (++aroundInvokesFound > 1) { throw new DefinitionException( "Multiple @AroundInvoke interceptor methods declared on class: " + aClass); @@ -1042,24 +1053,18 @@ private static void fetchType(Type type, BeanDeployment beanDeployment) { } private static void collectCallbacks(ClassInfo clazz, List callbacks, DotName annotation, IndexView index, - Set knownMethods) { + Set knownMethods, InterceptionType interceptionType) { for (MethodInfo method : clazz.methods()) { 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."); - } + InterceptorInfo.validateSignature(method, interceptionType, InterceptorPlacement.TARGET_CLASS); + callbacks.add(method); } knownMethods.add(method.name()); } if (clazz.superName() != null) { ClassInfo superClass = getClassByName(index, clazz.superName()); if (superClass != null) { - collectCallbacks(superClass, callbacks, annotation, index, knownMethods); + collectCallbacks(superClass, callbacks, annotation, index, knownMethods, interceptionType); } } } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorInfo.java index f2ef90f7c26c0..91a468bdeba58 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorInfo.java @@ -5,6 +5,7 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -106,28 +107,32 @@ public class InterceptorInfo extends BeanInfo implements Comparable 1) { throw new DefinitionException( "Multiple @AroundInvoke interceptor methods declared on class: " + aClass); } } if (store.hasAnnotation(method, DotNames.AROUND_CONSTRUCT)) { - addInterceptorMethod(allMethods, aroundConstructs, method); + addInterceptorMethod(allMethods, aroundConstructs, method, InterceptionType.AROUND_CONSTRUCT, + InterceptorPlacement.INTERCEPTOR_CLASS); if (++aroundConstructsFound > 1) { throw new DefinitionException( "Multiple @AroundConstruct interceptor methods declared on class: " + aClass); } } if (store.hasAnnotation(method, DotNames.POST_CONSTRUCT)) { - addInterceptorMethod(allMethods, postConstructs, method); + addInterceptorMethod(allMethods, postConstructs, method, InterceptionType.POST_CONSTRUCT, + InterceptorPlacement.INTERCEPTOR_CLASS); if (++postConstructsFound > 1) { throw new DefinitionException( "Multiple @PostConstruct interceptor methods declared on class: " + aClass); } } if (store.hasAnnotation(method, DotNames.PRE_DESTROY)) { - addInterceptorMethod(allMethods, preDestroys, method); + addInterceptorMethod(allMethods, preDestroys, method, InterceptionType.PRE_DESTROY, + InterceptorPlacement.INTERCEPTOR_CLASS); if (++preDestroysFound > 1) { throw new DefinitionException( "Multiple @PreDestroy interceptor methods declared on class: " + aClass); @@ -297,8 +302,9 @@ public int compareTo(InterceptorInfo other) { return getTarget().toString().compareTo(other.getTarget().toString()); } - static void addInterceptorMethod(List allMethods, List interceptorMethods, MethodInfo method) { - validateSignature(method); + static void addInterceptorMethod(List allMethods, List interceptorMethods, MethodInfo method, + InterceptionType interceptionType, InterceptorPlacement interceptorPlacement) { + validateSignature(method, interceptionType, interceptorPlacement); if (!isInterceptorMethodOverriden(allMethods, method)) { interceptorMethods.add(method); } @@ -319,19 +325,105 @@ static boolean hasInterceptorMethodParameter(MethodInfo method) { || method.parameterType(0).name().equals(DotNames.ARC_INVOCATION_CONTEXT)); } - private static MethodInfo validateSignature(MethodInfo method) { - if (!hasInterceptorMethodParameter(method)) { - throw new IllegalStateException( - "An interceptor method must accept exactly one parameter of type jakarta.interceptor.InvocationContext: " - + method + " declared on " + method.declaringClass()); + private enum InterceptorMethodError { + MUST_HAVE_PARAMETER, + MUST_NOT_HAVE_PARAMETER, + WRONG_RETURN_TYPE, + } + + static void validateSignature(MethodInfo method, InterceptionType interceptionType, + InterceptorPlacement interceptorPlacement) { + boolean isLifecycleCallback = interceptionType == InterceptionType.AROUND_CONSTRUCT + || interceptionType == InterceptionType.POST_CONSTRUCT + || interceptionType == InterceptionType.PRE_DESTROY; + + boolean mustHaveParameter = !isLifecycleCallback || interceptorPlacement == InterceptorPlacement.INTERCEPTOR_CLASS; + boolean mustNotHaveParameter = isLifecycleCallback && interceptorPlacement == InterceptorPlacement.TARGET_CLASS; + boolean mayReturnVoid = isLifecycleCallback; + boolean mayReturnObject = !isLifecycleCallback || interceptorPlacement == InterceptorPlacement.INTERCEPTOR_CLASS; + + Set errors = EnumSet.noneOf(InterceptorMethodError.class); + if (mustHaveParameter && !hasInterceptorMethodParameter(method)) { + errors.add(InterceptorMethodError.MUST_HAVE_PARAMETER); + } + if (mustNotHaveParameter && method.parametersCount() > 0) { + errors.add(InterceptorMethodError.MUST_NOT_HAVE_PARAMETER); + } + + boolean wrongReturnType = true; + if (mayReturnVoid && method.returnType().kind().equals(Kind.VOID)) { + wrongReturnType = false; + } + if (mayReturnObject && method.returnType().name().equals(DotNames.OBJECT)) { + wrongReturnType = false; + } + if (wrongReturnType) { + errors.add(InterceptorMethodError.WRONG_RETURN_TYPE); } - if (!method.returnType().kind().equals(Type.Kind.VOID) && - !method.returnType().name().equals(DotNames.OBJECT)) { - throw new IllegalStateException( - "The return type of an interceptor method must be java.lang.Object or void: " - + method + " declared on " + method.declaringClass()); + + if (!errors.isEmpty()) { + StringBuilder msg = new StringBuilder(); + switch (interceptionType) { + case AROUND_CONSTRUCT: + msg.append("@AroundConstruct"); + break; + case AROUND_INVOKE: + msg.append("@AroundInvoke"); + break; + case POST_CONSTRUCT: + msg.append("@PostConstruct"); + break; + case PRE_DESTROY: + msg.append("@PreDestroy"); + break; + default: + throw new IllegalArgumentException("Unknown interception type: " + interceptionType); + } + if (isLifecycleCallback) { + msg.append(" lifecycle callback method"); + } else { + msg.append(" interceptor method"); + } + msg.append(" declared in "); + switch (interceptorPlacement) { + case INTERCEPTOR_CLASS: + msg.append("an interceptor class"); + break; + case TARGET_CLASS: + msg.append("a target class"); + break; + default: + throw new IllegalArgumentException("Unknown interceptor placement: " + interceptorPlacement); + } + msg.append(" must "); + + if (errors.contains(InterceptorMethodError.MUST_HAVE_PARAMETER)) { + msg.append("have exactly one parameter of type jakarta.interceptor.InvocationContext"); + } else if (errors.contains(InterceptorMethodError.MUST_NOT_HAVE_PARAMETER)) { + msg.append("have zero parameters"); + } + + if (errors.contains(InterceptorMethodError.WRONG_RETURN_TYPE)) { + if (errors.contains(InterceptorMethodError.MUST_HAVE_PARAMETER) + || errors.contains(InterceptorMethodError.MUST_NOT_HAVE_PARAMETER)) { + msg.append(" and must "); + } + msg.append("have a return type of "); + if (mayReturnVoid) { + msg.append("void"); + } + if (mayReturnVoid && mayReturnObject) { + msg.append(" or "); + } + if (mayReturnObject) { + msg.append("java.lang.Object"); + } + } + + msg.append(": ").append(method).append(" declared in ").append(method.declaringClass().name()); + + throw new DefinitionException(msg.toString()); } - return method; } } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorPlacement.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorPlacement.java new file mode 100644 index 0000000000000..c7935f6e9e647 --- /dev/null +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InterceptorPlacement.java @@ -0,0 +1,6 @@ +package io.quarkus.arc.processor; + +enum InterceptorPlacement { + INTERCEPTOR_CLASS, + TARGET_CLASS, +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/illegal/InterceptorWithoutParameterTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/illegal/InterceptorWithoutParameterTest.java new file mode 100644 index 0000000000000..40e232f6c4c90 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/illegal/InterceptorWithoutParameterTest.java @@ -0,0 +1,59 @@ +package io.quarkus.arc.test.interceptors.illegal; + +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import jakarta.annotation.Priority; +import jakarta.enterprise.inject.spi.DefinitionException; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.test.ArcTestContainer; + +public class InterceptorWithoutParameterTest { + @RegisterExtension + public ArcTestContainer container = ArcTestContainer.builder() + .beanClasses(MyInterceptor.class, MyInterceptorBinding.class) + .shouldFail() + .build(); + + @Test + public void trigger() { + Throwable error = container.getFailure(); + assertNotNull(error); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains( + "@AroundInvoke interceptor method declared in an interceptor class must have exactly one parameter")); + assertTrue(error.getMessage().contains("intercept()")); + assertTrue(error.getMessage().contains("InterceptorWithoutParameterTest$MyInterceptor")); + } + + @Target({ TYPE, METHOD, FIELD, PARAMETER }) + @Retention(RUNTIME) + @InterceptorBinding + @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Interceptor + @Priority(1) + static class MyInterceptor { + @AroundInvoke + Object intercept() throws Exception { + return null; + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructTest.java index 333784b69af00..259a38de10ff3 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructTest.java @@ -1,5 +1,6 @@ package io.quarkus.arc.test.validation; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -23,7 +24,11 @@ public class InvalidPostConstructTest { public void testFailure() { Throwable error = container.getFailure(); assertNotNull(error); - assertTrue(error instanceof DefinitionException); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains( + "@PostConstruct lifecycle callback method declared in a target class must have a return type of void")); + assertTrue(error.getMessage().contains("invalid()")); + assertTrue(error.getMessage().contains("InvalidPostConstructTest$InvalidBean")); } @ApplicationScoped diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructWithParametersTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructWithParametersTest.java index 22670f7406192..26a693472ab1c 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructWithParametersTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPostConstructWithParametersTest.java @@ -1,5 +1,6 @@ package io.quarkus.arc.test.validation; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -7,7 +8,6 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.spi.DefinitionException; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -23,10 +23,11 @@ public class InvalidPostConstructWithParametersTest { 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")); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains( + "@PostConstruct lifecycle callback method declared in a target class must have zero parameters")); + assertTrue(error.getMessage().contains("invalid(java.lang.String ignored)")); + assertTrue(error.getMessage().contains("InvalidPostConstructWithParametersTest$InvalidBean")); } @ApplicationScoped diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPreDestroyTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPreDestroyTest.java index e966915d3108e..55b7a8a83ce16 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPreDestroyTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/validation/InvalidPreDestroyTest.java @@ -1,5 +1,6 @@ package io.quarkus.arc.test.validation; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -7,7 +8,6 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.spi.DefinitionException; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -24,10 +24,11 @@ public class InvalidPreDestroyTest { 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")); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains( + "@PreDestroy lifecycle callback method declared in a target class must have a return type of void")); + assertTrue(error.getMessage().contains("invalid()")); + assertTrue(error.getMessage().contains("InvalidPreDestroyTest$InvalidBean")); } @ApplicationScoped