From e08e3d8c548819b862fc831403ce4d10bfb24298 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Fri, 1 Nov 2019 09:54:26 +0100 Subject: [PATCH] Intercepted methods of a bean may not be declared final - otherwise a deployment exception is thrown - final methods declared on bean classes that require client proxies are ignored though --- .../io/quarkus/arc/processor/BeanInfo.java | 19 ++++-- .../io/quarkus/arc/processor/Methods.java | 21 +++--- .../finalmethod/FinalMethodIgnoredTest.java | 45 +++++++++++++ .../FinalInterceptedMethodTest.java | 56 ++++++++++++++++ .../FinalNonInterceptedMethodTest.java | 65 +++++++++++++++++++ 5 files changed, 190 insertions(+), 16 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/clientproxy/finalmethod/FinalMethodIgnoredTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalInterceptedMethodTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalNonInterceptedMethodTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java index 92daeaba796dd..beee4628749a3 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java @@ -19,7 +19,9 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; +import java.util.stream.Collectors; import javax.enterprise.inject.spi.DefinitionException; +import javax.enterprise.inject.spi.DeploymentException; import javax.enterprise.inject.spi.InterceptionType; import org.jboss.jandex.AnnotationInstance; import org.jboss.jandex.AnnotationTarget; @@ -405,8 +407,10 @@ void init(List errors) { if (disposer != null) { disposer.init(errors); } - interceptedMethods.putAll(initInterceptedMethods()); - lifecycleInterceptors.putAll(initLifecycleInterceptors()); + interceptedMethods.putAll(initInterceptedMethods(errors)); + if (errors.isEmpty()) { + lifecycleInterceptors.putAll(initLifecycleInterceptors()); + } } protected String getType() { @@ -421,7 +425,7 @@ protected String getType() { } } - private Map initInterceptedMethods() { + private Map initInterceptedMethods(List errors) { if (!isInterceptor() && isClassBean()) { Map interceptedMethods = new HashMap<>(); Map> candidates = new HashMap<>(); @@ -434,7 +438,14 @@ private Map initInterceptedMethods() { } } - Methods.addInterceptedMethodCandidates(beanDeployment, target.get().asClass(), candidates, classLevelBindings); + Set finalMethods = Methods.addInterceptedMethodCandidates(beanDeployment, target.get().asClass(), + candidates, classLevelBindings); + if (!finalMethods.isEmpty()) { + errors.add(new DeploymentException(String.format( + "Intercepted methods of the bean %s may not be declared final:\n\t- %s", getBeanClass(), + finalMethods.stream().map(Object::toString).sorted().collect(Collectors.joining("\n\t- "))))); + return Collections.emptyMap(); + } for (Entry> entry : candidates.entrySet()) { List interceptors = beanDeployment.getInterceptorResolver() 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 6f927a8493a54..5a1e23d383703 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 @@ -100,9 +100,9 @@ private static boolean skipForClientProxy(MethodInfo method) { if (Modifier.isFinal(method.flags())) { String className = method.declaringClass().name().toString(); if (!className.startsWith("java.")) { - LOGGER.warn( - String.format("Method %s.%s() is final, skipped during generation of the corresponding client proxy", - className, method.name())); + LOGGER.warn(String.format( + "Final method %s.%s() is ignored during proxy generation and should never be invoked upon the proxy instance!", + className, method.name())); } return true; } @@ -113,9 +113,10 @@ static boolean isObjectToString(MethodInfo method) { return method.declaringClass().name().equals(DotNames.OBJECT) && method.name().equals(TO_STRING); } - static void addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo, + static Set addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo, Map> candidates, List classLevelBindings) { + Set finalMethods = new HashSet<>(); for (MethodInfo method : classInfo.methods()) { if (skipForSubclass(method)) { continue; @@ -134,13 +135,7 @@ static void addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassI } if (!merged.isEmpty()) { if (Modifier.isFinal(method.flags())) { - String className = method.declaringClass().name().toString(); - if (!className.startsWith("java.")) { - LOGGER.warn( - String.format( - "Method %s.%s() is final, skipped during generation of the corresponding intercepted subclass", - className, method.name())); - } + finalMethods.add(method); } else { candidates.computeIfAbsent(new Methods.MethodKey(method), key -> merged); } @@ -149,9 +144,11 @@ static void addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassI if (classInfo.superClassType() != null) { ClassInfo superClassInfo = getClassByName(beanDeployment.getIndex(), classInfo.superName()); if (superClassInfo != null) { - addInterceptedMethodCandidates(beanDeployment, superClassInfo, candidates, classLevelBindings); + finalMethods + .addAll(addInterceptedMethodCandidates(beanDeployment, superClassInfo, candidates, classLevelBindings)); } } + return finalMethods; } private static boolean skipForSubclass(MethodInfo method) { diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/clientproxy/finalmethod/FinalMethodIgnoredTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/clientproxy/finalmethod/FinalMethodIgnoredTest.java new file mode 100644 index 0000000000000..40d808267a276 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/clientproxy/finalmethod/FinalMethodIgnoredTest.java @@ -0,0 +1,45 @@ +package io.quarkus.arc.test.clientproxy.finalmethod; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ClientProxy; +import io.quarkus.arc.test.ArcTestContainer; +import java.io.IOException; +import javax.annotation.PostConstruct; +import javax.enterprise.context.ApplicationScoped; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public class FinalMethodIgnoredTest { + + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(Moo.class); + + @Test + public void testProducer() throws IOException { + Moo moo = Arc.container().instance(Moo.class).get(); + assertTrue(moo instanceof ClientProxy); + assertEquals(0, moo.getVal()); + assertEquals(10, ((Moo) ((ClientProxy) moo).arc_contextualInstance()).val); + } + + @ApplicationScoped + static class Moo { + + private int val; + + @PostConstruct + void init() { + this.val = 10; + } + + // will return 0 if invoked upon a client proxy + final int getVal() { + return val; + } + + } + +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalInterceptedMethodTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalInterceptedMethodTest.java new file mode 100644 index 0000000000000..350c67aa36401 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalInterceptedMethodTest.java @@ -0,0 +1,56 @@ +package io.quarkus.arc.test.interceptors.finalmethod; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.quarkus.arc.test.ArcTestContainer; +import io.quarkus.arc.test.interceptors.Simple; +import javax.annotation.Priority; +import javax.enterprise.inject.spi.DeploymentException; +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 FinalInterceptedMethodTest { + + @RegisterExtension + public ArcTestContainer container = ArcTestContainer.builder().beanClasses(Simple.class, SimpleBean.class, + SimpleInterceptor.class).shouldFail().build(); + + @Test + public void testFailure() { + Throwable t = container.getFailure(); + assertNotNull(t); + assertTrue(t instanceof DeploymentException); + assertTrue(t.getMessage().contains("foo")); + assertTrue(t.getMessage().contains("bar")); + } + + @Simple + @Singleton + static class SimpleBean { + + final String foo() { + return "foo"; + } + + final void bar() { + } + + } + + @Simple + @Priority(1) + @Interceptor + static class SimpleInterceptor { + + @AroundInvoke + Object mySuperCoolAroundInvoke(InvocationContext ctx) throws Exception { + return ctx.proceed(); + } + } + +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalNonInterceptedMethodTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalNonInterceptedMethodTest.java new file mode 100644 index 0000000000000..a4ce410c69cc7 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/finalmethod/FinalNonInterceptedMethodTest.java @@ -0,0 +1,65 @@ +package io.quarkus.arc.test.interceptors.finalmethod; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; +import io.quarkus.arc.test.interceptors.Simple; +import javax.annotation.PostConstruct; +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 FinalNonInterceptedMethodTest { + + static final String VAL = "ping"; + + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(Simple.class, SimpleBean.class, + SimpleInterceptor.class); + + @Test + public void testInvocation() { + SimpleBean bean = Arc.container().instance(SimpleBean.class).get(); + assertEquals(VAL, bean.foo()); + assertEquals("a" + VAL, bean.bar()); + } + + @Singleton + static class SimpleBean { + + private String val; + + @PostConstruct + void init() { + val = VAL; + } + + // This method is final but not intercepted = OK + final String foo() { + return val; + } + + @Simple + String bar() { + return val; + } + + } + + @Simple + @Priority(1) + @Interceptor + static class SimpleInterceptor { + + @AroundInvoke + Object mySuperCoolAroundInvoke(InvocationContext ctx) throws Exception { + return "a" + ctx.proceed(); + } + } + +}