JavaEE Deployers need redoing again
At the last reload of the deployers I went through all the deployers
and tried to fix all the braindeaths. Unfortunately, most of them have
resurfaced in different guises.
Some examples:
1) MISUSE OF INIT
The parsing deployers are doing things in init they should not be doing.
e.g. The JBossEJBParsingDeployer is doing this (I know it is deprecated)
@Override
protected void init(VFSDeploymentUnit unit, JBossMetaData metaData, VirtualFile file) throws Exception
{
super.init(unit, metaData, file);
// For legacy
ApplicationMetaData amd = new ApplicationMetaData(metaData);
unit.addAttachment(ApplicationMetaData.class, amd);
// Wrap in the standardjboss.xml
JBossMetaData stdMetaData = getStandardMetaData(unit);
// Save this as a transient(non-managed) attachment
unit.addAttachment("standardjboss.xml", stdMetaData);
// Add the loader repository config
LoaderRepositoryConfig loaderRepositoryConfig = createLoaderRepositoryConfig(metaData);
if (loaderRepositoryConfig != null)
unit.addAttachment(LoaderRepositoryConfig.class.getName(), loaderRepositoryConfig);
}
None of this is initialization of the parsed model, e.g. inserting the URL
into the model which is something the xml processing can't do.
If the model is passed in programmatically (profile service) none of this work gets done.
Even worse, this is the place that parses standardjboss.xml so no jboss.xml
means no standardjboss.xml means a failed deployment
Caused by: java.lang.IllegalStateException: Container configuration not found: Standard Stateless SessionBean available: [] at org.jboss.metadata.ejb.jboss.JBossEnterpriseBeanMetaData.determineContainerConfiguration(JBossEnterpriseBeanMetaData.java:648) at org.jboss.ejb.deployers.EjbDeployer.deploy(EjbDeployer.java:261)
I don't know why standardjboss.xml has to be an attachment. A single
deployer that does both the parsing (during bootstrap) and merge
would suffice.
It should be after all the other mechanisms to create ejb deployment metadata
including the annotation build.
2) MISUSE OF createMetaData()
JBossAppParsingDeployer does this
@Override
protected void createMetaData(DeploymentUnit unit, String name, String suffix) throws DeploymentException
{
super.createMetaData(unit, name, suffix);
EarMetaData specMetaData = unit.getAttachment(EarMetaData.class);
JBossAppMetaData metaData = unit.getAttachment(JBossAppMetaData.class);
if(specMetaData == null && metaData == null)
return;
// If there no JBossMetaData was created from a jboss-app.xml, create one
if (metaData == null)
{
metaData = new JBossAppMetaData();
}
// Create a merged view
JBossAppMetaData mergedMetaData = new JBossAppMetaData();
mergedMetaData.merge(metaData, specMetaData);
// Set the merged as the output
unit.getTransientManagedObjects().addAttachment(JBossAppMetaData.class, mergedMetaData);
// Keep the raw parsed metadata as well
unit.addAttachment("Raw"+JBossAppMetaData.class.getName(), metaData, JBossAppMetaData.class);
}
This is to resolve the "init" not invoked problem in (1), but it isn't
the purpose of createMetaData(), which is define different ways
to look for metadata files. All of which have now been parameterized as
config on the deployer anyway, so we could make this method final,
but then people couldn't subclass it to introduce a new mechanism of
locating a metadata file.
This processing should be in a seperate deployer it is not parsing work.
I want to kill the allowReparse() option once we have the parsing
deployers that can parse multiple metadata files.
3) AbstractVFSRealDeployer is deprecated
Anything that is not following the deployer model and is accessing the
VFS/ deployer structure in undefined ways should extend this deprecated deployer.
It means we haven't thought hard enough about how to include what is
going on in the deployer model and we need to come up with some abstraction
such that it can be done safely and others can share the working code.
AnnotatedMetaDataDeployer does not extend the class, instead it
just reproduces it's code. Thus there is no deprecated warning to tell
us to review this processing.
Similar functionality will also be required for MBeans/POJO
descriptors from annotations and JCA when it is defined in the spec.
Probably other places as well, e.g. AOP metadata from annotations?
4) Scalability and error proneness
AnnotatedMetaDataDeployer has this horrible code:
ClassLoader loader = unit.getClassLoader();
List<VirtualFile> classpath = unit.getClassPath();
if(classpath == null)
return;
try
{
Map<VirtualFile, Class<?>> classpathClasses = new HashMap<VirtualFile, Class<?>>();
for(VirtualFile path : classpath)
{
AnnotatedClassFilter classVisitor = new AnnotatedClassFilter(unit, loader, path);
path.visit(classVisitor);
Map<VirtualFile, Class<?>> classes = classVisitor.getAnnotatedClasses();
if(classes != null && classes.size() > 0)
{
log.info("Annotated classes: "+classes);
classpathClasses.putAll(classes);
}
}
What's the point in having a visitor pattern that scales like iterators do
but without the usual error proneness of iterators if people are just
going to use it to load a collection and then iterate?
This code does not scale. For large deployments, the classes map is going to be huge.
It's also not very extensible.
a) There is no way to add new deployment types.
e.g. Where do I include aop checking for isMetadataComplete()
and looking at the classes as they are visited?
b) Where I do add/modify the processors either for my own custom
annotations that create metadata or to replace a buggy processor
to fix a problem?
5) The merge code is horrible and not defined well.
There is currently:
EjbParsingDeployer
JBossEjbParsingDeployer
StandardJBossMetaDataDeployer
MergedJBossMetaDataDeployer
AnnotatedMetaDataDeployer
All doing different aspects of creating and merging ejb metadata.
Besides the no jboss.xml means no standardjboss.xml problem mentioned above,
there's almost certainly a problem where the AnnotatedMetaDataDeployer
does not run or merge standardjboss.xml if there is no ejb-jar.xml or jboss.xml
6) PostClassLoader
If we need a seperate stage before the REAL deployers to analyse the whole
deployment, we should define a new stage, e.g. PRE-REAL or something.
This would also be the point where add dependencies to the REAL
stage of a deployment for things like ejb-ref dependencies.
PostClassLoader is for AOP and other class enhancement code,
so it can modify the classes before somebody tries to load them.
Currently there is an ill-defined race condition between AOP enhancing
the ejb classes and the javaee deployers analyzing them for annotations, etc.
GOING FORWARD:
I'd like to fix all this, but I guess it is too close to Beta3.
Instead this is something I'm going to work on for RC1 once Beta3 is out.
One thing this does show is that we really need some tests that
simulate what the profile service is going to do when it is fully up and running.
i.e. programmatic deployments from the modifications of the attachments
created from an initial deployment.
I'm pretty sure this is all broken with the current state of the deployers. :-(
