Skip to content

Commit

Permalink
WELD-2763 Correct how Weld chooses proxy package when a bean has unas…
Browse files Browse the repository at this point in the history
…signable interface type
  • Loading branch information
manovotn committed Oct 19, 2023
1 parent 2b19d4b commit 68e2db9
Show file tree
Hide file tree
Showing 11 changed files with 204 additions and 5 deletions.
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();
}
}

0 comments on commit 68e2db9

Please sign in to comment.