diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java index adb1c7c825ada..4cae282858698 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java @@ -159,7 +159,7 @@ static Set addInterceptedMethodCandidates(BeanDeployment beanDeploym Map> candidates, List classLevelBindings, Consumer bytecodeTransformerConsumer, boolean transformUnproxyableClasses) { - return addInterceptedMethodCandidates(beanDeployment, classInfo, classInfo, candidates, classLevelBindings, + return addInterceptedMethodCandidates(beanDeployment, classInfo, classInfo, candidates, Set.copyOf(classLevelBindings), bytecodeTransformerConsumer, transformUnproxyableClasses, new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom, beanDeployment.getBeanArchiveIndex()), @@ -169,7 +169,7 @@ static Set addInterceptedMethodCandidates(BeanDeployment beanDeploym static Set addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo, ClassInfo originalClassInfo, Map> candidates, - List classLevelBindings, Consumer bytecodeTransformerConsumer, + Set classLevelBindings, Consumer bytecodeTransformerConsumer, boolean transformUnproxyableClasses, SubclassSkipPredicate skipPredicate, boolean ignoreMethodLevelBindings) { Set methodsFromWhichToRemoveFinal = new HashSet<>(); @@ -177,52 +177,22 @@ static Set addInterceptedMethodCandidates(BeanDeployment beanDeploym skipPredicate.startProcessing(classInfo, originalClassInfo); for (MethodInfo method : classInfo.methods()) { - if (skipPredicate.test(method)) { + Set merged = mergeBindings(beanDeployment, originalClassInfo, classLevelBindings, + ignoreMethodLevelBindings, method); + if (merged.isEmpty() || skipPredicate.test(method)) { continue; } - Set merged = new HashSet<>(); - if (ignoreMethodLevelBindings) { - merged.addAll(classLevelBindings); - } else { - Collection methodAnnnotations = beanDeployment.getAnnotations(method); - List methodLevelBindings = methodAnnnotations.stream() - .flatMap(a -> beanDeployment.extractInterceptorBindings(a).stream()) - .collect(Collectors.toList()); - if (Modifier.isPrivate(method.flags()) - && !methodLevelBindings.isEmpty() - && !Annotations.contains(methodAnnnotations, DotNames.OBSERVES) - && !Annotations.contains(methodAnnnotations, DotNames.OBSERVES_ASYNC)) { - if (methodLevelBindings.size() == 1) { - LOGGER.warnf("%s will have no effect on method %s.%s() because the method is private", - methodLevelBindings.iterator().next(), classInfo.name(), method.name()); - } else { - LOGGER.warnf("Annotations %s will have no effect on method %s.%s() because the method is private", - methodLevelBindings.stream().map(AnnotationInstance::toString).collect(Collectors.joining(",")), - classInfo.name(), method.name()); - } - - } - merged.addAll(methodLevelBindings); - for (AnnotationInstance classLevelBinding : classLevelBindings) { - if (methodLevelBindings.isEmpty() - || methodLevelBindings.stream().noneMatch(a -> classLevelBinding.name().equals(a.name()))) { - merged.add(classLevelBinding); - } + boolean addToCandidates = true; + if (Modifier.isFinal(method.flags())) { + if (transformUnproxyableClasses) { + methodsFromWhichToRemoveFinal.add(NameAndDescriptor.fromMethodInfo(method)); + } else { + addToCandidates = false; + finalMethodsFoundAndNotChanged.add(method); } } - if (!merged.isEmpty()) { - boolean addToCandidates = true; - if (Modifier.isFinal(method.flags())) { - if (transformUnproxyableClasses) { - methodsFromWhichToRemoveFinal.add(NameAndDescriptor.fromMethodInfo(method)); - } else { - addToCandidates = false; - finalMethodsFoundAndNotChanged.add(method); - } - } - if (addToCandidates) { - candidates.computeIfAbsent(new Methods.MethodKey(method), key -> merged); - } + if (addToCandidates) { + candidates.computeIfAbsent(new Methods.MethodKey(method), key -> merged); } } skipPredicate.methodsProcessed(); @@ -255,6 +225,49 @@ static Set addInterceptedMethodCandidates(BeanDeployment beanDeploym return finalMethodsFoundAndNotChanged; } + private static Set mergeBindings(BeanDeployment beanDeployment, ClassInfo classInfo, + Set classLevelBindings, boolean ignoreMethodLevelBindings, MethodInfo method) { + if (ignoreMethodLevelBindings) { + return classLevelBindings; + } + + Collection methodAnnotations = beanDeployment.getAnnotations(method); + if (methodAnnotations.isEmpty()) { + // No annotations declared on the method + return classLevelBindings; + } + List methodLevelBindings = new ArrayList<>(); + for (AnnotationInstance annotation : methodAnnotations) { + methodLevelBindings.addAll(beanDeployment.extractInterceptorBindings(annotation)); + } + + Set merged; + if (methodLevelBindings.isEmpty()) { + merged = classLevelBindings; + } else { + merged = new HashSet<>(methodLevelBindings); + for (AnnotationInstance binding : classLevelBindings) { + if (methodLevelBindings.stream().noneMatch(a -> binding.name().equals(a.name()))) { + merged.add(binding); + } + } + + if (Modifier.isPrivate(method.flags()) + && !Annotations.contains(methodAnnotations, DotNames.OBSERVES) + && !Annotations.contains(methodAnnotations, DotNames.OBSERVES_ASYNC)) { + if (methodLevelBindings.size() == 1) { + LOGGER.warnf("%s will have no effect on method %s.%s() because the method is private", + methodLevelBindings.iterator().next(), classInfo.name(), method.name()); + } else { + LOGGER.warnf("Annotations %s will have no effect on method %s.%s() because the method is private", + methodLevelBindings.stream().map(AnnotationInstance::toString).collect(Collectors.joining(",")), + classInfo.name(), method.name()); + } + } + } + return merged; + } + static class NameAndDescriptor { private final String name; private final String descriptor; @@ -536,17 +549,30 @@ public boolean test(MethodInfo method) { if (!parameters.isEmpty() && (beanArchiveIndex != null)) { String originalClassPackage = DotNames.packageName(originalClazz.name()); for (Type type : parameters) { - ClassInfo parameterClassInfo = beanArchiveIndex.getClassByName(type.name()); - if (parameterClassInfo == null) { - continue; // hope for the best + if (type.kind() == Kind.PRIMITIVE) { + continue; } - if (Modifier.isPrivate(parameterClassInfo.flags())) { - return true; // parameters whose class is private can not be loaded, as we would end up with IllegalAccessError when trying to access the use the load the class + DotName typeName = type.name(); + if (type.kind() == Kind.ARRAY) { + typeName = type.asArrayType().component().name(); + } + ClassInfo param = beanArchiveIndex.getClassByName(typeName); + if (param == null) { + LOGGER.warn(String.format( + "Parameter type info not available: %s - unable to validate the parameter type's visibility for method %s declared on %s", + type.name(), method.name(), method.declaringClass().name())); + continue; } - if (!Modifier.isPublic(parameterClassInfo.flags())) { - // parameters whose class is package-private and the package is not the same as the package of the method for which we are checking can not be loaded, - // as we would end up with IllegalAccessError when trying to access the use the load the class - return !DotNames.packageName(parameterClassInfo.name()).equals(originalClassPackage); + if (Modifier.isPublic(param.flags()) || Modifier.isProtected(param.flags())) { + continue; + } + // e.g. parameters whose class is package-private and the package is not the same as the package of the method for which we are checking can not be loaded, + // as we would end up with IllegalAccessError when trying to access the use the load the class + if (!DotNames.packageName(param.name()).equals(originalClassPackage)) { + LOGGER.warn(String.format( + "A method %s() declared on %s has a non-public parameter of type %s which prevents it from being intercepted. Please change the parameter type visibility in order to make it intercepted.", + method.name(), method.declaringClass().name(), type)); + return true; } } } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/nonpublicparams/NonPublicParametersTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/nonpublicparams/NonPublicParametersTest.java new file mode 100644 index 0000000000000..9d82fb04a4150 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/nonpublicparams/NonPublicParametersTest.java @@ -0,0 +1,69 @@ +package io.quarkus.arc.test.interceptors.nonpublicparams; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; +import io.quarkus.arc.test.interceptors.Simple; +import io.quarkus.arc.test.interceptors.nonpublicparams.charlie.Charlie; +import javax.annotation.Priority; +import javax.inject.Singleton; +import javax.interceptor.AroundInvoke; +import javax.interceptor.Interceptor; +import javax.interceptor.InvocationContext; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public class NonPublicParametersTest { + + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(Simple.class, SimpleBean.class, + SimpleInterceptor.class); + + @Test + public void testInterception() { + ArcContainer arc = Arc.container(); + SimpleBean simpleBean = arc.instance(SimpleBean.class).get(); + assertEquals("FOO", + simpleBean.foo(null, new Bar(), new Baz(), null)); + } + + @Singleton + static class SimpleBean extends Charlie { + + @Simple + String foo(Foo foo, Bar bar, Baz baz, CharlieParam charlie) { + return "foo"; + } + + @Simple + String fooArray(Foo[] foo, boolean isOk) { + return "foo"; + } + + private static class Foo { + } + + } + + static class Bar { + + } + + protected static class Baz { + + } + + @Simple + @Priority(1) + @Interceptor + public static class SimpleInterceptor { + + @AroundInvoke + Object mySuperCoolAroundInvoke(InvocationContext ctx) throws Exception { + return ctx.proceed().toString().toUpperCase(); + } + } + +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/nonpublicparams/charlie/Charlie.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/nonpublicparams/charlie/Charlie.java new file mode 100644 index 0000000000000..e04b49bd3881e --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/nonpublicparams/charlie/Charlie.java @@ -0,0 +1,9 @@ +package io.quarkus.arc.test.interceptors.nonpublicparams.charlie; + +public class Charlie { + + protected class CharlieParam { + + } + +}