From 68e2db9c28b3b8bb5d804ef9a42d29669234e35e Mon Sep 17 00:00:00 2001 From: Matej Novotny Date: Wed, 18 Oct 2023 16:45:14 +0200 Subject: [PATCH] WELD-2763 Correct how Weld chooses proxy package when a bean has unassignable interface type --- .../jboss/weld/bean/proxy/ProxyFactory.java | 16 ++++-- .../unimplemented/interfaces/MyExtension.java | 16 ++++++ ...CdiBeanWithUnimplementedInterfaceTest.java | 45 ++++++++++++++++ .../ProxyCreationForEjbLocalTest.java | 53 +++++++++++++++++++ .../interfaces/ifaces/BeanIface.java | 5 ++ .../interfaces/ifaces/LocalInterface1.java | 5 ++ .../interfaces/ifaces/LocalInterface2.java | 5 ++ .../NotImplementedButDeclaredInterface.java | 8 +++ .../ifaces/NotImplementedIface.java | 5 ++ .../interfaces/impl/CDIBean.java | 20 +++++++ .../interfaces/impl/StatelessLocalBean.java | 31 +++++++++++ 11 files changed, 204 insertions(+), 5 deletions(-) create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/MyExtension.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForCdiBeanWithUnimplementedInterfaceTest.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForEjbLocalTest.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/BeanIface.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface1.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface2.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedButDeclaredInterface.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedIface.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/CDIBean.java create mode 100644 tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/StatelessLocalBean.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 4887679ff7..743016e657 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 @@ -209,11 +209,11 @@ static String getProxyName(String contextId, Class proxiedBeanType, Set proxiedBeanType, Set bean, TypeInfo typeInfo, StringBuilder name) { + return createCompoundProxyName(contextId, bean, typeInfo, name, null); + } + + private static ProxyNameHolder createCompoundProxyName(String contextId, Bean bean, TypeInfo typeInfo, + StringBuilder name, String knownProxyPackage) { String className; - String proxyPackage = null; + String proxyPackage = knownProxyPackage; // we need a sorted collection without repetition, hence LinkedHashSet final Set interfaces = new LinkedHashSet<>(); // for producers, try to determine the most specific class and make sure the proxy starts with the same package and class if (bean != null && bean instanceof AbstractProducerBean) { Class mostSpecificClass = ((AbstractProducerBean) bean).getType(); + // for producers, always override the proxy package proxyPackage = mostSpecificClass.getPackage().getName(); if (mostSpecificClass.getDeclaringClass() != null) { interfaces.add(mostSpecificClass.getDeclaringClass().getSimpleName()); diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/MyExtension.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/MyExtension.java new file mode 100644 index 0000000000..b4de59f8f8 --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/MyExtension.java @@ -0,0 +1,16 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces; + +import jakarta.enterprise.event.Observes; +import jakarta.enterprise.inject.spi.Extension; +import jakarta.enterprise.inject.spi.ProcessBeanAttributes; + +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.NotImplementedIface; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.impl.CDIBean; + +public class MyExtension implements Extension { + + public void pba(@Observes ProcessBeanAttributes pba) { + // add the type of the interface the class doesn't directly implement + pba.configureBeanAttributes().addType(NotImplementedIface.class); + } +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForCdiBeanWithUnimplementedInterfaceTest.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForCdiBeanWithUnimplementedInterfaceTest.java new file mode 100644 index 0000000000..0c2c787c24 --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForCdiBeanWithUnimplementedInterfaceTest.java @@ -0,0 +1,45 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces; + +import jakarta.enterprise.inject.spi.Extension; +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.test.util.Utils; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.BeanIface; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.NotImplementedIface; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.impl.CDIBean; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(Arquillian.class) +public class ProxyCreationForCdiBeanWithUnimplementedInterfaceTest { + + @Deployment + public static Archive deploy() { + return ShrinkWrap.create(BeanArchive.class, Utils.getDeploymentNameAsHash(ProxyCreationForEjbLocalTest.class)) + .addClasses(MyExtension.class, ProxyCreationForCdiBeanWithUnimplementedInterfaceTest.class, CDIBean.class, + NotImplementedIface.class, BeanIface.class) + .addAsServiceProvider(Extension.class, MyExtension.class); + } + + @Inject + NotImplementedIface cdiBean; + + @Test + public void testProxyPackageMatchesTheClass() { + // sanity check of the testing setup + Assert.assertEquals(NotImplementedIface.class.getSimpleName(), cdiBean.ping3()); + + // The assertion is based solely on inspecting the proxy format - expected package and first mentioned class + // We cannot rely on verifying that the class can be defined because if this runs on WFLY, it is a non-issue + // due to using ClassLoader#defineClass. The mismatch only shows when using MethodHandles.Lookup + // see https://github.com/jakartaee/platform-tck/issues/1194 for more information + Assert.assertEquals(CDIBean.class.getPackage(), cdiBean.getClass().getPackage()); + Assert.assertTrue(cdiBean.getClass().getName().startsWith(CDIBean.class.getName())); + } +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForEjbLocalTest.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForEjbLocalTest.java new file mode 100644 index 0000000000..14e08f423d --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ProxyCreationForEjbLocalTest.java @@ -0,0 +1,53 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces; + +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.test.util.Utils; +import org.jboss.weld.tests.category.Integration; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.LocalInterface1; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.LocalInterface2; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.NotImplementedButDeclaredInterface; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.impl.StatelessLocalBean; +import org.junit.Assert; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; + +@Category(Integration.class) +@RunWith(Arquillian.class) +public class ProxyCreationForEjbLocalTest { + + @Deployment + public static Archive deploy() { + return ShrinkWrap.create(BeanArchive.class, Utils.getDeploymentNameAsHash(ProxyCreationForEjbLocalTest.class)) + .addClasses(ProxyCreationForEjbLocalTest.class, LocalInterface1.class, LocalInterface2.class, + NotImplementedButDeclaredInterface.class, StatelessLocalBean.class); + } + + @Inject + StatelessLocalBean bean1; + + @Inject + NotImplementedButDeclaredInterface bean2; + + @Test + public void testProxyPackageMatchesTheClass() { + // sanity check of the testing setup + Assert.assertEquals(LocalInterface1.class.getSimpleName(), bean1.ping1()); + + // also assert invoking the method from the interface bean doesn't implement directly + Assert.assertEquals(NotImplementedButDeclaredInterface.class.getSimpleName(), bean2.ping3()); + + // The assertion is based solely on inspecting the proxy format - expected package and first mentioned class + // We cannot rely on verifying that the class can be defined because this runs on WFLY which directly uses + // ClassLoader#defineClass in which case it's a non-issue. The mismatch only shows when using MethodHandles.Lookup + // see https://github.com/jakartaee/platform-tck/issues/1194 for more information + Assert.assertEquals(StatelessLocalBean.class.getPackage(), bean1.getClass().getPackage()); + Assert.assertTrue(bean1.getClass().getName().startsWith(StatelessLocalBean.class.getName())); + } +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/BeanIface.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/BeanIface.java new file mode 100644 index 0000000000..fe656f66fe --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/BeanIface.java @@ -0,0 +1,5 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces; + +public interface BeanIface { + String ping1(); +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface1.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface1.java new file mode 100644 index 0000000000..278c521c53 --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface1.java @@ -0,0 +1,5 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces; + +public interface LocalInterface1 { + String ping1(); +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface2.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface2.java new file mode 100644 index 0000000000..3039dc6aaf --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/LocalInterface2.java @@ -0,0 +1,5 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces; + +public interface LocalInterface2 { + String ping2(); +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedButDeclaredInterface.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedButDeclaredInterface.java new file mode 100644 index 0000000000..ecabb73128 --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedButDeclaredInterface.java @@ -0,0 +1,8 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces; + +import jakarta.ejb.Local; + +@Local +public interface NotImplementedButDeclaredInterface extends LocalInterface1 { + String ping3(); +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedIface.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedIface.java new file mode 100644 index 0000000000..98d9dd5e7f --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/ifaces/NotImplementedIface.java @@ -0,0 +1,5 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces; + +public interface NotImplementedIface { + String ping3(); +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/CDIBean.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/CDIBean.java new file mode 100644 index 0000000000..34d728dce0 --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/CDIBean.java @@ -0,0 +1,20 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces.impl; + +import jakarta.enterprise.context.ApplicationScoped; + +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.BeanIface; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.NotImplementedIface; + +// Plain CDI bean which doesn't implement one interface but has its method +// A CDI extension attempts to add this type programatically +@ApplicationScoped +public class CDIBean implements BeanIface { + @Override + public String ping1() { + return BeanIface.class.getSimpleName(); + } + + public String ping3() { + return NotImplementedIface.class.getSimpleName(); + } +} diff --git a/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/StatelessLocalBean.java b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/StatelessLocalBean.java new file mode 100644 index 0000000000..bb070de04f --- /dev/null +++ b/tests-arquillian/src/test/java/org/jboss/weld/tests/classDefining/unimplemented/interfaces/impl/StatelessLocalBean.java @@ -0,0 +1,31 @@ +package org.jboss.weld.tests.classDefining.unimplemented.interfaces.impl; + +import jakarta.ejb.Local; +import jakarta.ejb.LocalBean; +import jakarta.ejb.Stateless; + +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.LocalInterface1; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.LocalInterface2; +import org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces.NotImplementedButDeclaredInterface; + +// NOTE: the bean intentionally declares NotImplementedButDeclaredInterface but does *not* implement it directly +@Stateless +@LocalBean +@Local({ LocalInterface1.class, LocalInterface2.class, + NotImplementedButDeclaredInterface.class }) +public class StatelessLocalBean implements LocalInterface1, LocalInterface2 { + + @Override + public String ping1() { + return LocalInterface1.class.getSimpleName(); + } + + @Override + public String ping2() { + return LocalInterface2.class.getSimpleName(); + } + + public String ping3() { + return NotImplementedButDeclaredInterface.class.getSimpleName(); + } +}