1 2 Previous Next 28 Replies Latest reply on Jun 10, 2008 8:33 AM by Adrian Brock

    Annotation Tests in deployers-vfs

    Adrian Brock Master

      The annotation tests in deployers-vfs are failing for me under eclipse.

      I don't know why they are working under Maven (command line)
      probably a difference in classpath issue.

      When I look at these tests I see a number of problems

      These tests should be using the classes in target/classes compiled from src/main
      with the deployment constructed using multiple bases.
      i.e. using the AssembledDirectory vfs

      Major Problems I see:

      1) Use of ClassLoaderSystem.getInstance()
      Each test should be constructing its own ClassLoaderSystem not
      sharing the default one. Using the default one is bound to introduce leaks across tests.
      See for example the ClassLoaderDependenciesTest in deployers-impl
      on how to do this.

      2) Isolated classloading. It should also be setting the DefaultDomain
      in its own ClassLoaderSystem to be BEFORE_BUT_JAVA_ONLY
      again see ClassLoaderDependenciesTest
      otherwise you're going to load classes from the testsuite classpath
      rather than the deployment so there's no real test at all

      3) BeanScanningDeployer using setRelativeOrder is a hack
      and the component handling is fundamentally broken/brain dead.

       public BeanScanningDeployer(KernelDeploymentDeployer kdd)
       {
       super(AnnotationEnvironment.class);
       setInputs(BeanMetaData.class);
       setOutput(BeanMetaData.class);
       if (kdd != null)
       setRelativeOrder(kdd.getRelativeOrder() + 10);
       else
       setRelativeOrder(getRelativeOrder() + 10);
       }
      


      This breaks the orthogonality of the deployers. One deployer should
      not depend upon another. This needs doing in a different way.

      This can be seen further with the "mapComponents" stuff.
      This deployer should either be a component deployer or a top level deployer
      not mixing/matching and hacking in some bizarre way.

      It certainly shouldn't be creating components by delegation to a different deployer
      that has run earlier.

      Iteration and delegation, {insert explitive here :-) }
       for (BeanMetaData bfb : bfBeans)
       {
       KernelDeploymentDeployer.addBeanComponent(unit, bfb);
      


      Who knows what happens when addBeanComponent throws an error?

      With these current hacks it is impossible for me to insert some extra behaviour
      between these deployers because it is doing a stupid almost indeterminate dance.
      Nor can I replace one of these deployers with alternate behaviour because
      they are too tied together.

      4) Debugging

      Even if I do
      enableTrace("org.jboss")

      I get no inclination why this test is failing.
      Where is the debug information?

      5) Others

      There's probably more wrong? But this is just from a basic look.
      e.g. why do these tests depend upon the VFS anyway?

      As far as I can tell, these tests would work just as well under the Mock classloading
      in deployers-impl if written properly?

        • 1. Re: Annotation Tests in deployers-vfs
          Adrian Brock Master

          My guess is that these deployers needs rewriting such the flow goes something like:

          1) Deployer that collects Bean/BeanFactory annotations as attachments
          2) KernelDeploymentDeployer is ammended to create components
          for any Bean/BeanFactory (if metadata doesn't exist).

          The association between the two deployers would be some collection
          of annotations or the annotation itself with an associated factory for each.

          In pseudo code:

          An abstraction that defines how to create a BeanMetaData
          from a Bean or collection of Beans (or in fact something more generic)
          
          public interface AnnotationProcessor<Class<A extends A>, Class<B extends Collection<A>>, C>
          {
          ...;
          }
          
          With a concrete implementation for BeanMetaData
          
          public class BeanProcessor implements AnnoationProcessor<Bean, Beans, BeanMetaData>
          {
          ...;
          }
          
          KernelDeploymentDeployer would take the processor(s)
          This could probably be done generically in the abstract component deployer
          
           public KernelDeploymentDeployer(List<AnnotationProcessor> processors)
           {
           setDeploymentVisitor(new KernelDeploymentVisitor());
           setComponentVisitor(new BeanMetaDataVisitor());
          
           // Add setInput for Bean/Beans, i.e. processor.getAnnotation/s()
           // Add setOutput for processor.getOutput(), i.e. BeanMetaData
           setProcessors(processors);
           }
          


          The component deployer would then visit the Bean/Beans annotations
          invoke run the processor then call the relevant the addComponent method
          of the normal visitor.

          Of course you need a way to handle duplicate names but that should be
          known by the processor.

          With this generic processing in place, we are back to a more reasonable flow.

          * A proper depdendency between deployers
          * You can easily insert between the deployers and understand what is happening
          * You can replace deployers to get different behaviour
          * Using a visitor pattern for the processors makes error handling easy/less error prone
          * Other deployers can reuse the new metadata in an understandable way

          e.g. There would be nothing to stop me writing a deployer that does
          Bean bean = AnnotationFactory.createAnnotations("@Bean ...");
          unit.addAttachment(Bean.class, bean);
          

          and it would get processed naturally.

          Although I don't know why you want to do this. :-)

          • 2. Re: Annotation Tests in deployers-vfs
            Adrian Brock Master

            My suggestion also has the advantage that it is extensible.
            e.g. I could an annotation processor for some other annotation

            • 3. Re: Annotation Tests in deployers-vfs
              Ales Justin Master

               

              "adrian@jboss.org" wrote:

              These tests should be using the classes in target/classes compiled from src/main
              with the deployment constructed using multiple bases.
              i.e. using the AssembledDirectory vfs

              Major Problems I see:

              1) Use of ClassLoaderSystem.getInstance()
              Each test should be constructing its own ClassLoaderSystem not
              sharing the default one. Using the default one is bound to introduce leaks across tests.
              See for example the ClassLoaderDependenciesTest in deployers-impl
              on how to do this.

              2) Isolated classloading. It should also be setting the DefaultDomain
              in its own ClassLoaderSystem to be BEFORE_BUT_JAVA_ONLY
              again see ClassLoaderDependenciesTest
              otherwise you're going to load classes from the testsuite classpath
              rather than the deployment so there's no real test at all

              Yup, it's on my todo list.
              That's been puzzling me from the beginning.
              But since it worked, in both - IDEA and maven - I didn't go and investigate in details. :-)

              "adrian@jboss.org" wrote:

              3) BeanScanningDeployer using setRelativeOrder is a hack
              and the component handling is fundamentally broken/brain dead.

               public BeanScanningDeployer(KernelDeploymentDeployer kdd)
               {
               super(AnnotationEnvironment.class);
               setInputs(BeanMetaData.class);
               setOutput(BeanMetaData.class);
               if (kdd != null)
               setRelativeOrder(kdd.getRelativeOrder() + 10);
               else
               setRelativeOrder(getRelativeOrder() + 10);
               }
              


              This breaks the orthogonality of the deployers. One deployer should
              not depend upon another. This needs doing in a different way.

              This can be seen further with the "mapComponents" stuff.
              This deployer should either be a component deployer or a top level deployer
              not mixing/matching and hacking in some bizarre way.

              It certainly shouldn't be creating components by delegation to a different deployer that has run earlier.

              Iteration and delegation, {insert explitive here :-) }
               for (BeanMetaData bfb : bfBeans)
               {
               KernelDeploymentDeployer.addBeanComponent(unit, bfb);
              


              Who knows what happens when addBeanComponent throws an error?

              With these current hacks it is impossible for me to insert some extra behaviour
              between these deployers because it is doing a stupid almost indeterminate dance.
              Nor can I replace one of these deployers with alternate behaviour because
              they are too tied together.

              (A bit of cardboard posting ... again)
              OK, just saw your 2nd post to this issue. :-)

              "adrian@jboss.org" wrote:

              4) Debugging

              Even if I do
              enableTrace("org.jboss")

              I get no inclination why this test is failing.
              Where is the debug information?

              Coming up. ;-)

              "adrian@jboss.org" wrote:

              5) Others

              There's probably more wrong? But this is just from a basic look.
              e.g. why do these tests depend upon the VFS anyway?

              As far as I can tell, these tests would work just as well under the Mock classloading
              in deployers-impl if written properly?

              OK, I'll leave the current one's as they are, and add a few there.

              • 4. Re: Annotation Tests in deployers-vfs
                Ales Justin Master

                 

                "adrian@jboss.org" wrote:
                My suggestion also has the advantage that it is extensible.
                e.g. I could an annotation processor for some other annotation

                This should be then there already. ;-)
                So there would be no need for my crappy prototype.

                • 5. Re: Annotation Tests in deployers-vfs
                  Ales Justin Master

                   

                  "adrian@jboss.org" wrote:

                  Iteration and delegation, {insert explitive here :-) }
                   for (BeanMetaData bfb : bfBeans)
                   {
                   KernelDeploymentDeployer.addBeanComponent(unit, bfb);
                  


                  Who knows what happens when addBeanComponent throws an error?

                  Why do we treat component failure on deploy (AbstractComponentDeployer)
                   try
                   {
                   deployComponents(unit);
                   }
                   catch (Throwable t)
                   {
                   undeployComponents(unit);
                   throw DeploymentException.rethrowAsDeploymentException("Error deploying: " + unit.getName(), t);
                   }
                  

                  differently then in AbstractRealDeployerWithInput
                   List<T> visited = new ArrayList();
                   try
                   {
                   Set<? extends T> deployments = unit.getAllMetaData(getInput());
                   for (T deployment : deployments)
                   {
                   visitor.deploy(unit, deployment);
                   visited.add(deployment);
                   }
                   }
                   catch (Throwable t)
                   {
                   for (int i = visited.size()-1; i >= 0; --i)
                   {
                   try
                   {
                   visitor.undeploy(unit, visited.get(i));
                   }
                   catch (Throwable ignored)
                   {
                   log.warn("Error during undeploy: " + unit.getName(), ignored);
                   }
                   }
                   throw DeploymentException.rethrowAsDeploymentException("Error deploying: " + unit.getName(), t);
                   }
                  


                  • 6. Re: Annotation Tests in deployers-vfs
                    Ales Justin Master

                     

                    "adrian@jboss.org" wrote:
                    Of course you need a way to handle duplicate names but that should be
                    known by the processor.

                    Can I add?
                    DeploymentUnit::getComponent(String name) --> DeploymentUnit
                    

                    To ease up on component iteration regarding code duplication. ;-)

                    • 7. Re: Annotation Tests in deployers-vfs
                      Adrian Brock Master

                       

                      "alesj" wrote:

                      Why do we treat component failure on deploy (AbstractComponentDeployer)
                      ...
                      differently then in AbstractRealDeployerWithInput


                      I didn't know we did :-)

                      Looks like I never did the unwind of the components properly, instead it tries
                      to undeploy all the components whether they got created or not.

                      • 8. Re: Annotation Tests in deployers-vfs
                        Ales Justin Master

                         

                        "adrian@jboss.org" wrote:
                        Looks like I never did the unwind of the components properly, instead it tries
                        to undeploy all the components whether they got created or not.

                        I fixed it here:
                        - http://www.jboss.com/index.html?module=bb&op=viewtopic&t=136534
                        ;-)

                        • 9. Re: Annotation Tests in deployers-vfs
                          Adrian Brock Master

                           

                          "alesj" wrote:

                          Can I add?
                          DeploymentUnit::getComponent(String name) --> DeploymentUnit
                          

                          To ease up on component iteration regarding code duplication. ;-)


                          I guarantee somebody will misuse it. :-)

                          • 10. Re: Annotation Tests in deployers-vfs
                            Ales Justin Master

                             

                            "adrian@jboss.org" wrote:
                            I guarantee somebody will misuse it. :-)

                            They can already do that.
                            It's only that it will require more code for them to do it.
                            Which might prevent them from trying. :-)

                            • 11. Re: Annotation Tests in deployers-vfs
                              Ales Justin Master

                              This is what I came up with:

                              public abstract class AbstractAnnotationDeployer<D, C> extends AbstractComponentDeployer<D, C>
                              {
                               /** The annotation processors */
                               private AnnotationProcessor<?, C>[] processors;
                              
                               public AbstractAnnotationDeployer(AnnotationProcessor<?, C>... processors)
                               {
                               super();
                               if (processors != null && processors.length > 0)
                               {
                               addInput(AnnotationEnvironment.class);
                               this.processors = processors;
                               for (AnnotationProcessor processor : processors)
                               {
                               addInput(processor.getAnnotation());
                               }
                               }
                               }
                              
                               public void internalDeploy(DeploymentUnit unit) throws DeploymentException
                               {
                               super.internalDeploy(unit);
                              
                               if (processors != null && compVisitor != null)
                               {
                               try
                               {
                               AnnotationEnvironment env = unit.getAttachment(AnnotationEnvironment.class);
                               for (AnnotationProcessor<?, C> processor : processors)
                               {
                               List<C> components = new ArrayList<C>();
                              
                               if (env != null)
                               {
                               // from classes
                               Set<Class<?>> classes = env.classIsAnnotatedWith(processor.getAnnotation());
                               for (Class clazz : classes)
                               {
                               C component = processor.createComponent(clazz);
                               if (component != null)
                               components.add(component);
                               }
                               }
                              
                               // from attachments
                               Annotation annotation = unit.getAttachment(processor.getAnnotation());
                               if (annotation != null)
                               {
                               C component = processor.createComponent(annotation);
                               if (component != null)
                               components.add(component);
                               }
                              
                               int i = 0;
                               try
                               {
                               for (i = 0; i < components.size(); i++)
                               {
                               deployComponent(unit, processor, components.get(i));
                               }
                               }
                               catch (Throwable t)
                               {
                               for (; i >= 0; i--)
                               {
                               try
                               {
                               compVisitor.undeploy(unit, components.get(i));
                               }
                               catch (Throwable ignored)
                               {
                               }
                               }
                               throw DeploymentException.rethrowAsDeploymentException("Error deploying: " + unit.getName(), t);
                               }
                               }
                               }
                               catch (Throwable t)
                               {
                               super.internalUndeploy(unit);
                               throw DeploymentException.rethrowAsDeploymentException("Error deploying: " + unit.getName(), t);
                               }
                               }
                               }
                              
                               protected void deployComponent(DeploymentUnit unit, AnnotationProcessor<?, C> processor, C component) throws DeploymentException
                               {
                               String name = processor.getComponentName(component);
                               DeploymentUnit componentUnit = unit.getComponent(name);
                               C previous = null;
                               if (componentUnit != null)
                               previous = componentUnit.getAttachment(getOutput());
                              
                               if (previous != null)
                               {
                               processor.merge(previous, component);
                               }
                               else if (processor.isValidComponent(component))
                               {
                               compVisitor.deploy(unit, component);
                               }
                               }
                              
                               public void internalUndeploy(DeploymentUnit unit)
                               {
                               if (processors != null && compVisitor != null)
                               {
                               try
                               {
                               AnnotationEnvironment env = unit.getAttachment(AnnotationEnvironment.class);
                               for (AnnotationProcessor<?, C> processor : processors)
                               {
                               List<C> components = new ArrayList<C>();
                              
                               if (env != null)
                               {
                               // from classes
                               Set<Class<?>> classes = env.classIsAnnotatedWith(processor.getAnnotation());
                               for (Class clazz : classes)
                               {
                               C component = processor.createUndeployableComponent(clazz);
                               if (component != null)
                               components.add(component);
                               }
                               }
                              
                               // from attachments
                               Annotation annotation = unit.getAttachment(processor.getAnnotation());
                               if (annotation != null)
                               {
                               C component = processor.createUndeployableComponent(annotation);
                               if (component != null)
                               components.add(component);
                               }
                              
                               for (int i = components.size() - 1; i >= 0; i--)
                               {
                               try
                               {
                               compVisitor.undeploy(unit, components.get(i));
                               }
                               catch (Throwable ignored)
                               {
                               log.warn("Error during undeploy: " + unit.getName(), ignored);
                               }
                               }
                               }
                               }
                               catch (Throwable ignored)
                               {
                               log.warn("Error during undeploy: " + unit.getName(), ignored);
                               }
                               }
                              
                               super.internalUndeploy(unit);
                               }
                              }
                              


                              The two issues that I see here are
                              - I'm not able to differ between previous (merged) component while undeploying
                              - I have to re-create component for undeploy (that's why I use that createUndeployableComponent method)

                              Does this somehow fit into the picture you have/had?


                              • 12. Re: Annotation Tests in deployers-vfs
                                Adrian Brock Master

                                 

                                "alesj" wrote:

                                Does this somehow fit into the picture you have/had?


                                No not really.

                                I wanted you to seperate the annotation scanning from the component creation.
                                i.e the annotation @Bean or a "collection" of them @Beans?
                                becomes an attachment in its own right.

                                The component deployer should not care where the annotation(s) come from.
                                One possible source is the annoation scanning.

                                Schematically:
                                BeanAnnotationScanner: class scan -> annotation attachments
                                Component deployer: metadata or annotations -> BeanMetaData components

                                That way if I dont want annotation scanning for beans or I want to use
                                some other logic I can remove, replace or insert a new deployer to do what I want.

                                e.g. Hypothetically: suppose I want all beans scanned from classes to be "on-demand"
                                I can add a deployer inbetween that modifies the annotations but doesn't touch
                                those created using normal metadata.

                                Like I said before, I don't really think anybody will do this kind of thing,
                                but we should do it properly just in case.

                                Also it keeps the processing to "TO DO ONE THING WELL"
                                i.e. one deployer knows how to scan for bean annotations
                                one knows how to create components from a set of such annotations
                                (mod those that already have metadata).

                                • 13. Re: Annotation Tests in deployers-vfs
                                  Adrian Brock Master

                                   

                                  "adrian@jboss.org" wrote:

                                  I wanted you to seperate the annotation scanning from the component creation.
                                  i.e the annotation @Bean or a "collection" of them @Beans?
                                  becomes an attachment in its own right.


                                  The attachment doesn't actually have to be the annotation if you need other
                                  information from the class that is annotated. e.g. It could be a collection of classes.

                                  • 14. Re: Annotation Tests in deployers-vfs
                                    Ales Justin Master

                                     

                                    "adrian@jboss.org" wrote:
                                    The attachment doesn't actually have to be the annotation if you need other
                                    information from the class that is annotated. e.g. It could be a collection of classes.

                                    Yup, exactly what I wanted to point out.
                                    Since in my @Bean case, I need the actual class that is annoatted.
                                    Otherwise there is no point in having @Bean if you could define the class there. ;-)

                                    1 2 Previous Next