Testing Deployers with new Reflect + ClassPool| JBoss.org Content Archive (Read Only)

15. Re: Testing Deployers with new Reflect + ClassPool

alesj Oct 28, 2009 9:45 AM (in response to alesj)

"flavia.rainone@jboss.com" wrote:

I'll start adding a test to reproduce the failure I caught yesterday.

Which failure is that?

16. Re: Testing Deployers with new Reflect + ClassPool

flavia.rainone Oct 28, 2009 9:56 AM (in response to alesj)

"alesj" wrote:
"flavia.rainone@jboss.com" wrote:

I'll start adding a test to reproduce the failure I caught yesterday.

Which failure is that?

The bug that was causing the test failure you found:
https://jira.jboss.org/jira/browse/JBREFLECT-65

17. Re: Testing Deployers with new Reflect + ClassPool

alesj Oct 28, 2009 12:59 PM (in response to alesj)

"alesj" wrote:

* CL isolation check

This one fails with JavassistTIF.

"alesj" wrote:

* Domain hierarchy usage

Passes for both.

"alesj" wrote:

* Import from another bundle, although class exists in our jar
* Do things still properly work after re-deploy; e.g. no stale stuff left over

WIP.

18. Re: Testing Deployers with new Reflect + ClassPool

alesj Oct 28, 2009 4:21 PM (in response to alesj)

"alesj" wrote:
"alesj" wrote:

* CL isolation check

This one fails with JavassistTIF.

"alesj" wrote:

* Domain hierarchy usage

Passes for both.

Actually, they both fail.
Looks sort of random, if I disable 1st the 2nd fails, and vice-versa.
Leading to conclusion that we probably don't do cleanup properly.

19. Re: Testing Deployers with new Reflect + ClassPool

flavia.rainone Oct 29, 2009 8:48 AM (in response to alesj)

"alesj" wrote:
Actually, they both fail.
Looks sort of random, if I disable 1st the 2nd fails, and vice-versa.
Leading to conclusion that we probably don't do cleanup properly.

Thanks. I'll take a look at that.

20. Re: Testing Deployers with new Reflect + ClassPool

flavia.rainone Nov 6, 2009 6:34 AM (in response to alesj)

"flavia.rainone@jboss.com" wrote:
I think that we should look into the class pool tests and see what they are missing before we proceed to deployers tests. I'll start adding a test to reproduce the failure I caught yesterday.

This is related to JBREFLECT-65.

As a matter of fact, I found out that there is already a test that should catch this failure in the classpool tests. The tests Kabir wrote are very complete :)

The difference between the classpool test environment and the deployers one is that this one is lacking the calls to DomainRegistry.initMapsForLoader(loader, module, parentUnitLoader) method. Fixing that completely replaces the ugly hack, because parentUnitLoader is the loader we want when defining the parent of a class pool domain.

In JBoss AOP, the initMapsForLoader method is invoked by the AOPClassLoaderInitializer class. This class is indirectly invoked by the AOPDeployer and uses the DeploymentUnit to get to the arguments needed for initMapsForLoader. I've created a temporary deployer to test this on ClassPoolTestCase:

public void deploy(VFSDeploymentUnit unit) throws DeploymentException
 {
 if (unit.isTopLevel() || unit.getParent().getClassLoader() != unit.getClassLoader())
 {
 //Only bother doing all this if we are a different loader from the parent unit
 Module module = getModuleRecursively(unit);
 if (module == null)
 {
 throw new IllegalStateException("No " + Module.class.getName() +
 " attachment could be found in the following deployment unit or its parents: " + unit);
 }
 ClassLoader parentUnitLoader = unit.isTopLevel() ? null : unit.getParent().getClassLoader();
 domainRegistry.initMapsForLoader(unit.getClassLoader(), module, parentUnitLoader);
 }
 }

It works :).
However, I'm afraid this is not the correct way of doing this. We are not deploying anything at all, only registering the classloader structure created for the deployed unit. I thought on using a ModuleRegistry, but I can't find out how to get to the parameters required for initMapsForLoader from the module. I'm clueless on what is the correct approach for calling initMapsForLoader method. Any ideas?

21. Re: Testing Deployers with new Reflect + ClassPool

kabirkhan Nov 6, 2009 7:02 AM (in response to alesj)

org.jboss.aop.asintegration.jboss5.AOPClassLoaderDeployer is set up in AS to do this.

It calls AOPClassLoaderInitializer.initializeForUnit() which in turn ends up in DomainRegistry.initMapsForLoader()

22. Re: Testing Deployers with new Reflect + ClassPool

alesj Nov 6, 2009 7:11 AM (in response to alesj)

"flavia.rainone@jboss.com" wrote:
I thought on using a ModuleRegistry, but I can't find out how to get to the parameters required for initMapsForLoader from the module. I'm clueless on what is the correct approach for calling initMapsForLoader method. Any ideas?

ModuleRegistry gets Module passed in. Perhaps this then:

ClassLoader cl = ClassLoading.getClassLoaderForModule(module);
DeploymentUnit unit = AbstractDeploymentClassLoaderPolicyModule.class.cast(module).getDeploymentUnit();
ClassLoader parentUnitLoader = unit.isTopLevel() ? null : unit.getParent().getClassLoader();

Dunno how we would then handle if you register CL directly in -beans.xml --> VFSClassLoaderPolicyModule.
But I guess this falls into parentUL = null category.

ClassLoader cl = ClassLoading.getClassLoaderForModule(module);
DeploymentUnit unit = null;
if (module instance of AbstractDeploymentClassLoaderPolicyModule)
 unit = AbstractDeploymentClassLoaderPolicyModule.class.cast(module).getDeploymentUnit();
ClassLoader parentUnitLoader = unit == null || unit.isTopLevel() ? null : unit.getParent().getClassLoader();

23. Re: Testing Deployers with new Reflect + ClassPool

alesj Nov 6, 2009 7:15 AM (in response to alesj)

"kabir.khan@jboss.com" wrote:
org.jboss.aop.asintegration.jboss5.AOPClassLoaderDeployer is set up in AS to do this.

It calls AOPClassLoaderInitializer.initializeForUnit() which in turn ends up in DomainRegistry.initMapsForLoader()

We should do this w/o AOP being present.
And as Flavia suggests - and which is correct, as CLs/Modules can bypass Deployers -
this should be done via ModuleRegistry - as I described it in my previous post.

I guess this calls for a test that would create a CL from -beans.xml,
and see if the Classpools still work.
Flavia, will you create one or should I, and you just fix the issues if they are present?

24. Re: Testing Deployers with new Reflect + ClassPool

flavia.rainone Nov 6, 2009 7:21 AM (in response to alesj)

"kabir.khan@jboss.com" wrote:
org.jboss.aop.asintegration.jboss5.AOPClassLoaderDeployer is set up in AS to do this.

It calls AOPClassLoaderInitializer.initializeForUnit() which in turn ends up in DomainRegistry.initMapsForLoader()

Yes, but that's something AOP-related. I want to have an approach that is independent of JBoss AOP. I plan to split that DomainRegistry related stuff away from AOP, as it is now a classpool related thing (the new AOP impl that uses the classpool project contains already a split, but I think I'll work some more on that before considering it done).

Do you think that using a deployer for only calling DomainRegistry.initMapsForLoader is appropriate? I see that AOPClassLoaderDeployer adds the AspetManager as an attachment to the unit, which IMO justifies the need for a deployer. But I'm not sure about a deployer that is only used for calling initMapsForLoader.

25. Re: Testing Deployers with new Reflect + ClassPool

flavia.rainone Nov 6, 2009 7:25 AM (in response to alesj)

"flavia.rainone@jboss.com" wrote:
But I'm not sure about a deployer that is only used for calling initMapsForLoader.

I see Ales answered that question before I pressed the submit button :)

26. Re: Testing Deployers with new Reflect + ClassPool

flavia.rainone Nov 6, 2009 7:46 AM (in response to alesj)

"alesj" wrote:
I guess this calls for a test that would create a CL from -beans.xml,
and see if the Classpools still work.
Flavia, will you create one or should I, and you just fix the issues if they are present?

You can go ahead and create one, as I think this will be faster.

27. Re: Testing Deployers with new Reflect + ClassPool

flavia.rainone Nov 6, 2009 8:48 AM (in response to alesj)

"alesj" wrote:
ModuleRegistry gets Module passed in. Perhaps this then:

ClassLoader cl = ClassLoading.getClassLoaderForModule(module);
DeploymentUnit unit = AbstractDeploymentClassLoaderPolicyModule.class.cast(module).getDeploymentUnit();
ClassLoader parentUnitLoader = unit.isTopLevel() ? null : unit.getParent().getClassLoader();

Thanks, that works. I'll do a clean up of the previous fix and check some todos related to JBREFLECT-65 before committing.

28. Re: Testing Deployers with new Reflect + ClassPool

alesj Nov 6, 2009 9:00 AM (in response to alesj)

You should use the 2nd code snippet - the one that checks if the Module is of the right instance.
Since, as I wrote, not all Modules are deployment Modules.

29. Re: Testing Deployers with new Reflect + ClassPool

alesj Nov 6, 2009 11:15 AM (in response to alesj)

"flavia.rainone@jboss.com" wrote:
"alesj" wrote:
I guess this calls for a test that would create a CL from -beans.xml,
and see if the Classpools still work.
Flavia, will you create one or should I, and you just fix the issues if they are present?

You can go ahead and create one, as I think this will be faster.

This is done now - see TypeInfoTest and ClassPoolTestCase.

I've left a TODO in ClassPoolTestCase for you:

 try
 {
 Object anys = assertBean("AnyServlet", Object.class);
 Class<?> anysClass = anys.getClass();
 ClassLoader anysCL = anysClass.getClassLoader();

 DeploymentUnit du = getMainDeployerStructure().getDeploymentUnit(deployment.getName(), true);
 ClassLoader cl = getClassLoader(du);

 assertNotSame(cl, anysCL);

 // TODO - Flavia, apply ClassPool tests
 }

And once you delete assert(Not)Equals in JavassistTypeInfoTestCase,
we'll see how this works with your patch.

Currently both types - introspection and javassist - work for this test.