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-2763 Correct how Weld chooses proxy package when a bean has unassignable interface type #2895

Merged
merged 1 commit into from
Oct 20, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 11 additions & 5 deletions impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,11 @@ static String getProxyName(String contextId, Class<?> proxiedBeanType, Set<? ext
}
}
if (typeModified) {
//this bean has interfaces that the base type is not assignable to
//which can happen with some creative use of the SPI
//interface only bean.
// This bean has interfaces that the base type is not assignable to.
// One example of this is an EJB bean using @Local and declaring an interface it doesn't implement.
// Another case is a CDI bean with type added via ProcessBeanAttributes which isn't directly implemented.
StringBuilder name = new StringBuilder(typeInfo.getSuperClass().getSimpleName() + "$");
holder = createCompoundProxyName(contextId, bean, typeInfo, name);
holder = createCompoundProxyName(contextId, bean, typeInfo, name, bean.getBeanClass().getPackage().getName());
} else {
holder = new ProxyNameHolder(null, typeInfo.getSuperClass().getSimpleName(), bean);
}
Expand Down Expand Up @@ -247,13 +247,19 @@ static String getProxyName(String contextId, Class<?> proxiedBeanType, Set<? ext

private static ProxyNameHolder createCompoundProxyName(String contextId, Bean<?> 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<String> 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());
Expand Down
Original file line number Diff line number Diff line change
@@ -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<CDIBean> pba) {
// add the type of the interface the class doesn't directly implement
pba.configureBeanAttributes().addType(NotImplementedIface.class);
}
}
Original file line number Diff line number Diff line change
@@ -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()));
}
}
Original file line number Diff line number Diff line change
@@ -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()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces;

public interface BeanIface {
String ping1();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces;

public interface LocalInterface1 {
String ping1();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces;

public interface LocalInterface2 {
String ping2();
}
Original file line number Diff line number Diff line change
@@ -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();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package org.jboss.weld.tests.classDefining.unimplemented.interfaces.ifaces;

public interface NotImplementedIface {
String ping3();
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}