From 65f1d1146c650c0057861b05a215089d80a5e5d4 Mon Sep 17 00:00:00 2001 From: Matej Novotny Date: Thu, 14 Mar 2024 16:55:04 +0100 Subject: [PATCH] WELD-2785 Skip superclass declarations of final methods when creating proxies --- .../jboss/weld/bean/proxy/ProxyFactory.java | 18 +++++-- .../inheritance/AbstractSuperClass.java | 6 +++ .../inheritance/AbstractSuperClass2.java | 9 ++++ .../inheritance/ImplBean.java | 11 ++++ ...roxyIgnoreInvalidInheritedMethodsTest.java | 50 +++++++++++++++++++ .../inheritance/Secure.java | 19 +++++++ .../inheritance/SecureInterceptor.java | 20 ++++++++ 7 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass2.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ImplBean.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ProxyIgnoreInvalidInheritedMethodsTest.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/Secure.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/SecureInterceptor.java diff --git a/impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java b/impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java index 743016e657..39393761ac 100644 --- a/impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java +++ b/impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java @@ -52,6 +52,8 @@ import org.jboss.classfilewriter.util.Boxing; import org.jboss.classfilewriter.util.DescriptorUtils; import org.jboss.weld.Container; +import org.jboss.weld.annotated.enhanced.MethodSignature; +import org.jboss.weld.annotated.enhanced.jlr.MethodSignatureImpl; import org.jboss.weld.bean.AbstractProducerBean; import org.jboss.weld.bean.builtin.AbstractBuiltInBean; import org.jboss.weld.config.WeldConfiguration; @@ -625,13 +627,16 @@ protected void addMethodsFromClass(ClassFile proxyClassType, ClassMethod staticC // In rare cases, the bean class may be abstract - in this case we have to add methods from all interfaces implemented by any abstract class // from the hierarchy boolean isBeanClassAbstract = Modifier.isAbstract(cls.getModifiers()); + // a final method might have a non-final declaration in abstract superclass + // hence we need to remember which we saw and skip those in superclasses + Set foundFinalMethods = new HashSet<>(); while (cls != null) { - addMethods(cls, proxyClassType, staticConstructor); + addMethods(cls, proxyClassType, staticConstructor, foundFinalMethods); if (isBeanClassAbstract && Modifier.isAbstract(cls.getModifiers())) { for (Class implementedInterface : Reflections.getInterfaceClosure(cls)) { if (!additionalInterfaces.contains(implementedInterface)) { - addMethods(implementedInterface, proxyClassType, staticConstructor); + addMethods(implementedInterface, proxyClassType, staticConstructor, foundFinalMethods); } } } @@ -660,9 +665,14 @@ protected void addMethodsFromClass(ClassFile proxyClassType, ClassMethod staticC } } - private void addMethods(Class cls, ClassFile proxyClassType, ClassMethod staticConstructor) { + private void addMethods(Class cls, ClassFile proxyClassType, ClassMethod staticConstructor, + Set foundFinalmethods) { for (Method method : AccessController.doPrivileged(new GetDeclaredMethodsAction(cls))) { - if (isMethodAccepted(method, getProxySuperclass())) { + MethodSignature methodSignature = new MethodSignatureImpl(method); + if (Modifier.isFinal(method.getModifiers())) { + foundFinalmethods.add(methodSignature); + } + if (isMethodAccepted(method, getProxySuperclass()) && !foundFinalmethods.contains(methodSignature)) { try { MethodInformation methodInfo = new RuntimeMethodInformation(method); ClassMethod classMethod = proxyClassType.addMethod(method); diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass.java new file mode 100644 index 0000000000..37a6a909fa --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass.java @@ -0,0 +1,6 @@ +package org.jboss.weld.tests.proxy.ignoreinvalidmethods.inheritance; + +public abstract class AbstractSuperClass { + + protected abstract void ping(); +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass2.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass2.java new file mode 100644 index 0000000000..d0bfe1783b --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/AbstractSuperClass2.java @@ -0,0 +1,9 @@ +package org.jboss.weld.tests.proxy.ignoreinvalidmethods.inheritance; + +public abstract class AbstractSuperClass2 extends AbstractSuperClass { + + @Override + public final void ping() { + // this method is NOT proxyable + } +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ImplBean.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ImplBean.java new file mode 100644 index 0000000000..9a9ec9a5f5 --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ImplBean.java @@ -0,0 +1,11 @@ +package org.jboss.weld.tests.proxy.ignoreinvalidmethods.inheritance; + +import jakarta.enterprise.context.ApplicationScoped; + +@ApplicationScoped +@Secure +public class ImplBean extends AbstractSuperClass2 { + + public void pong() { + } +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ProxyIgnoreInvalidInheritedMethodsTest.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ProxyIgnoreInvalidInheritedMethodsTest.java new file mode 100644 index 0000000000..03f8e8fd26 --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/ProxyIgnoreInvalidInheritedMethodsTest.java @@ -0,0 +1,50 @@ +package org.jboss.weld.tests.proxy.ignoreinvalidmethods.inheritance; + +import jakarta.inject.Inject; + +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.junit.Arquillian; +import org.jboss.shrinkwrap.api.Archive; +import org.jboss.shrinkwrap.api.BeanArchive; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.weld.config.ConfigurationKey; +import org.jboss.weld.test.util.Utils; +import org.jboss.weld.tests.util.PropertiesBuilder; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** + * A bean class implementing a hierarchy of abstract classes where one introduces a method and the other implements it + * as final. This prevents proxying, but we should still be able to ignore such method when creating proxy. + * + * See WELD-2785 + */ +@RunWith(Arquillian.class) +public class ProxyIgnoreInvalidInheritedMethodsTest { + + @Deployment + public static Archive createTestArchive() { + return ShrinkWrap.create(BeanArchive.class, Utils.getDeploymentNameAsHash(ProxyIgnoreInvalidInheritedMethodsTest.class)) + .addPackage(ProxyIgnoreInvalidInheritedMethodsTest.class.getPackage()) + .addAsResource(PropertiesBuilder.newBuilder() + .set(ConfigurationKey.PROXY_IGNORE_FINAL_METHODS.get(), + ImplBean.class.getName() + "|" + AbstractSuperClass2.class.getName()) + .build(), "weld.properties"); + } + + @Inject + ImplBean implBean; + + @Test + public void testProxy() { + // firstly, the test should be able to deploy and execute, i.e. to create the proxy + // then we verify that interception happens only for one of methods + Assert.assertEquals(0, SecureInterceptor.timesInvoked); + implBean.pong(); + Assert.assertEquals(1, SecureInterceptor.timesInvoked); + implBean.ping(); + Assert.assertEquals(1, SecureInterceptor.timesInvoked); + } + +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/Secure.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/Secure.java new file mode 100644 index 0000000000..b1d8ba08da --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/Secure.java @@ -0,0 +1,19 @@ +package org.jboss.weld.tests.proxy.ignoreinvalidmethods.inheritance; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.interceptor.InterceptorBinding; + +@InterceptorBinding +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Inherited +@Target({ ElementType.METHOD, ElementType.TYPE }) +public @interface Secure { + +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/SecureInterceptor.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/SecureInterceptor.java new file mode 100644 index 0000000000..f47bcd85dd --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/ignoreinvalidmethods/inheritance/SecureInterceptor.java @@ -0,0 +1,20 @@ +package org.jboss.weld.tests.proxy.ignoreinvalidmethods.inheritance; + +import jakarta.annotation.Priority; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InvocationContext; + +@Priority(1) +@Secure +@Interceptor +public class SecureInterceptor { + + public static int timesInvoked = 0; + + @AroundInvoke + public Object aroundInvoke(InvocationContext context) throws Exception { + timesInvoked++; + return context.proceed(); + } +}