5 Replies Latest reply on Dec 4, 2007 4:17 PM by Scott Stark

    JavaEE Deployers need redoing again

    Adrian Brock Master

      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. :-(

        • 1. Re: JavaEE Deployers need redoing again
          Scott Stark Master

           

          "adrian@jboss.org" wrote:

          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.

          It could be moved into the MergedJBossMetaDataDeployer.

          "adrian@jboss.org" wrote:

          2) MISUSE OF createMetaData()

          JBossAppParsingDeployer does ...

          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.


          "adrian@jboss.org" wrote:

          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.

          AnnotationMetaDataDeployer does not extend the class, ...

          Obtaining the annotations that affect deployment is a basic feature of deployment that needs to be supported.

          "adrian@jboss.org" wrote:

          4) Scalability and error proneness

          AnnotationMetaDataDeployer has this horrible code:
          ...
          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?

          Currently the annotation to metadata processors require the root classes to build the associated metadata structure. It cannot be integrated into a visitor pattern. Whether this becomes an extensible annotation based deployer, or deployers for each deployment type is added is the question. We need better support for annotations in the base deployment layers.

          "adrian@jboss.org" wrote:

          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.

          Yes, we need this stage.

          "adrian@jboss.org" wrote:

          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. :-(

          Yes, the current deployment tests in the mc are way too simple. We need tests that exercise deployers at every stage.


          • 2. Re: JavaEE Deployers need redoing again
            Ales Justin Master

             

            "scott.stark@jboss.org" wrote:

            Yes, the current deployment tests in the mc are way too simple. We need tests that exercise deployers at every stage.

            I can work on this (for CR1), just give me initial requirements.
            Should this tests be in MC or AS or both?

            • 3. Re: JavaEE Deployers need redoing again
              Scott Stark Master

              We need tests in both. The AS tests should initially focus on the various combinations of metadata sources:

              - annotation only deployment
              - standard descriptor, metadata complete deployment
              - standard descriptor + annotations
              - jboss descriptor + annotations
              ...
              - profile service type of metadata override to see that parsing type deployers are bypassed

              • 4. Re: JavaEE Deployers need redoing again
                Ales Justin Master

                 

                "scott.stark@jboss.org" wrote:
                We need tests in both. The AS tests should initially focus on the various combinations of metadata sources:

                What about in MC?

                • 5. Re: JavaEE Deployers need redoing again
                  Scott Stark Master

                  The mc tests need to focus on the types of deployer chains we are using (merging multiple sources of metadata), the new pre-real deployer state, annotation support, support for metadata complete deployments that bypass parsing deployers, etc.