Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WELD-2785 Skip superclass declarations of final methods when creating proxies #2949

Merged
merged 1 commit into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<MethodSignature> 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);
}
}
}
Expand Down Expand Up @@ -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<MethodSignature> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.jboss.weld.tests.proxy.ignoreinvalidmethods.inheritance;

public abstract class AbstractSuperClass {

protected abstract void ping();
}
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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() {
}
}
Original file line number Diff line number Diff line change
@@ -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);
}

}
Original file line number Diff line number Diff line change
@@ -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 {

}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading