1 2 Previous Next 23 Replies Latest reply on Jul 6, 2007 12:46 PM by Ales Justin

    Precedence

    Adrian Brock Master

       

      "bill.burke@jboss.com" wrote:
      How do you define precedence rules for deployer ordering? For example, let's take a hypothetical Seam deployer that needs to augment an EJB deployment based on additional metadata within the deployment unit. It needs to happen after EJB XML and Annotation processing, but before component processing.

      I liked the install/uninstall approach (see below if you dont' know what I'm talking about) because we could have added precedence metadata to the addDeployer method. Precendence metadata like, I have to come before the EJB component processing, but after the XML/annotation processing.

      <install bean="MainDeployer" method="addDeployer">
       <parameter><this/></parameter>
      </install>
      <uninstall bean="MainDeployer" method="removeDeployer">
       <parameter><this/></parameter>
      </uninstall>
      




      See the part where I describe attachment flows.
      For your example (assuming JMX components) you would do something like:
      public MySeamDeployer extends AbstractSimpleRealDeployer<SeamMetaData.class>
      {
       public MySeamDeployer()
       {
       // We are after everything that creates and modifies SeamMetaData
       super(SeamMetaData.class);
      
       // We need the EJB deployment and Annotation metadata
       setInputs(EJB3Deployment.class, AnnotationCache.class);
       // We output a -service.xml description so we are before that deployer
       setOutputs(ServiceDeploymentMetaData.class);
       }
      }
      


        • 1. Re: Precedence
          Bill Burke Master

           

          "adrian@jboss.org" wrote:
           // We output a -service.xml description so we are before that deployer
           setOutputs(ServiceDeploymentMetaData.class);
           }
          }
          


          still don't see how an augmenting deployer (a deploeyr that augments a metamodel) can be sure to come before the component deployer.

          If your input and output is EJB metadata, this will work?

          • 2. Re: Precedence
            Bill Burke Master

            I'm guess what I"m saying is:

            
            setInputs(EJBMetaData.class, SeamMetaData.class);
            setOutputs(EJBMetaData.class)
            
            


            Will this even work?

            Again, this is a deployer that modifies existing metadata. For instance, adds an interceptor because you have annotated your ejb with @org.jboss.seam.Name.

            And we still have to be sure that the EJB component deployer comes after this augmentation.

            • 3. Re: Precedence
              Adrian Brock Master

               

              "bill.burke@jboss.com" wrote:
              I'm guess what I"m saying is:

              
              setInputs(EJBMetaData.class, SeamMetaData.class);
              setOutputs(EJBMetaData.class)
              
              


              Will this even work?


              Yes. But you're welcome to examine the code and try to come up with new testcases
              where it breaks. The only thing that should be not supported is loops. :-)
              See DeployersImpl.sort()

              The deployer above comes before anything that accepts EJBMetaData and after
              anything that outputs EJBMetaData (along as the other is not itself a "filter" -
              inputs/outputs EJBMetaData).

              If there are multiple "filter"s then the getRelativeOrder() can be used to control ordering
              (assuming it is important - but it is probably bad design?).

              • 4. Re: Precedence
                Ales Justin Master

                 

                "adrian@jboss.org" wrote:

                Yes. But you're welcome to examine the code and try to come up with new testcases
                where it breaks. The only thing that should be not supported is loops. :-)
                See DeployersImpl.sort()


                Do we really need to check all current deployers (O(n^2))?
                Since the original set is already ordered, thus we only need to find the position of the newly added deployer (O(n)).


                • 5. Re: Precedence
                  Ales Justin Master

                  Or O(n*log(n)) vs. O(log(n)) if we 'exaggerate' on the implementation. ;-)

                  • 6. Re: Precedence
                    Adrian Brock Master

                    You'll be suprised how fast a "bubble sort" is with modern cpus that do caching. :-)

                    When we get to more than 100 deployers and it doesn't fit in the cpu cache
                    then we might want to look at optimizing it. :-)

                    It's actually much less than O(n^2) since (n-1) of the deployers are already in the correct
                    order anyway. :-)

                    • 7. Re: Precedence
                      Adrian Brock Master

                      Since Carlo just found a bug in the sort, :-(
                      do you want to fix it?
                      http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4059992#4059992

                      • 8. Re: Precedence
                        Ales Justin Master

                         

                        "adrian@jboss.org" wrote:
                        Since Carlo just found a bug in the sort, :-(
                        do you want to fix it?
                        http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4059992#4059992


                        A chance to write that in O(log(n)). :-)
                        I already assigned a task to me.

                        • 9. Re: Precedence
                          Ales Justin Master

                          I've re-written sorting.
                          I'm using simple transitive closure.
                          It sorts a lot better. :-)

                          The issue I'm having is with DeployerFlowUnitTestCase.testInputOutputMultipleTransient();

                          Should we make this work?
                          Since in my opinion this _is_ a loop.

                          • 10. Re: Precedence
                            Adrian Brock Master

                            It's not a loop. The deployers are:

                            1) input=null, output=test
                            2) input=test, output=test
                            3) input=test, output=test
                            4) input=test, output=null

                            (2) and (3) are "filters", they input and output the same attachment.
                            If a deployer inputs and outputs the same attachment then it shouldn't
                            be considered as creating a loop.

                            The above is the correct order.
                            (2) and (3) looks ambigous, e.g. they could be other way around
                            but they are actually resolved using the Ordered.COMPARATOR.

                            In this case, the toString() on the deployer "3" is after "2" since the relativeOrders
                            are the same.

                            • 11. Re: Precedence
                              Adrian Brock Master

                              What is this? It is not a fix to change the test.
                              You're supposed to make the tests pass, not change them!

                              The orders in those tests were the correct ones.

                              There's NO POINT having regression tests if you modify the tests
                              because you introduced a regression. :-)

                              
                              > Modified: projects/microcontainer/trunk/deployers-impl/src/tests/org/jboss/test/deployers/deployer/test/DeployerFlowUnitTestCase.java
                              > ===================================================================
                              > --- projects/microcontainer/trunk/deployers-impl/src/tests/org/jboss/test/deployers/deployer/test/DeployerFlowUnitTestCase.java 2007-07-06 12:53:54 UTC (rev 63864)
                              > +++ projects/microcontainer/trunk/deployers-impl/src/tests/org/jboss/test/deployers/deployer/test/DeployerFlowUnitTestCase.java 2007-07-06 13:05:54 UTC (rev 63865)
                              > @@ -288,7 +288,7 @@
                              > assertEquals(6, deployer3.getUndeployOrder());
                              > assertEquals(5, deployer4.getUndeployOrder());
                              > }
                              > -
                              > +
                              > public void testMultipleOutput() throws Exception
                              > {
                              > DeployerClient main = createMainDeployer();
                              > @@ -453,11 +453,11 @@
                              > main.addDeployment(deployment);
                              > main.process();
                              >
                              > - assertEquals(1, deployer1.getDeployOrder());
                              > - assertEquals(2, deployer2.getDeployOrder());
                              > - assertEquals(3, deployer3.getDeployOrder());
                              > - assertEquals(4, deployer4.getDeployOrder());
                              > - assertEquals(5, deployer5.getDeployOrder());
                              > + assertEquals(5, deployer1.getDeployOrder());
                              > + assertEquals(4, deployer2.getDeployOrder());
                              > + assertEquals(1, deployer3.getDeployOrder());
                              > + assertEquals(2, deployer4.getDeployOrder());
                              > + assertEquals(3, deployer5.getDeployOrder());
                              > assertEquals(6, deployer6.getDeployOrder());
                              > assertEquals(-1, deployer1.getUndeployOrder());
                              > assertEquals(-1, deployer2.getUndeployOrder());
                              > @@ -469,33 +469,33 @@
                              > main.removeDeployment(deployment);
                              > main.process();
                              >
                              > - assertEquals(1, deployer1.getDeployOrder());
                              > - assertEquals(2, deployer2.getDeployOrder());
                              > - assertEquals(3, deployer3.getDeployOrder());
                              > - assertEquals(4, deployer4.getDeployOrder());
                              > - assertEquals(5, deployer5.getDeployOrder());
                              > + assertEquals(5, deployer1.getDeployOrder());
                              > + assertEquals(4, deployer2.getDeployOrder());
                              > + assertEquals(1, deployer3.getDeployOrder());
                              > + assertEquals(2, deployer4.getDeployOrder());
                              > + assertEquals(3, deployer5.getDeployOrder());
                              > assertEquals(6, deployer6.getDeployOrder());
                              > - assertEquals(12, deployer1.getUndeployOrder());
                              > - assertEquals(11, deployer2.getUndeployOrder());
                              > - assertEquals(10, deployer3.getUndeployOrder());
                              > - assertEquals(9, deployer4.getUndeployOrder());
                              > - assertEquals(8, deployer5.getUndeployOrder());
                              > + assertEquals(8, deployer1.getUndeployOrder());
                              > + assertEquals(9, deployer2.getUndeployOrder());
                              > + assertEquals(12, deployer3.getUndeployOrder());
                              > + assertEquals(11, deployer4.getUndeployOrder());
                              > + assertEquals(10, deployer5.getUndeployOrder());
                              > assertEquals(7, deployer6.getUndeployOrder());
                              >
                              > main.addDeployment(deployment);
                              > main.process();
                              >
                              > - assertEquals(13, deployer1.getDeployOrder());
                              > - assertEquals(14, deployer2.getDeployOrder());
                              > - assertEquals(15, deployer3.getDeployOrder());
                              > - assertEquals(16, deployer4.getDeployOrder());
                              > - assertEquals(17, deployer5.getDeployOrder());
                              > + assertEquals(17, deployer1.getDeployOrder());
                              > + assertEquals(16, deployer2.getDeployOrder());
                              > + assertEquals(13, deployer3.getDeployOrder());
                              > + assertEquals(14, deployer4.getDeployOrder());
                              > + assertEquals(15, deployer5.getDeployOrder());
                              > assertEquals(18, deployer6.getDeployOrder());
                              > - assertEquals(12, deployer1.getUndeployOrder());
                              > - assertEquals(11, deployer2.getUndeployOrder());
                              > - assertEquals(10, deployer3.getUndeployOrder());
                              > - assertEquals(9, deployer4.getUndeployOrder());
                              > - assertEquals(8, deployer5.getUndeployOrder());
                              > + assertEquals(8, deployer1.getUndeployOrder());
                              > + assertEquals(9, deployer2.getUndeployOrder());
                              > + assertEquals(12, deployer3.getUndeployOrder());
                              > + assertEquals(11, deployer4.getUndeployOrder());
                              > + assertEquals(10, deployer5.getUndeployOrder());
                              > assertEquals(7, deployer6.getUndeployOrder());
                              


                              • 12. Re: Precedence
                                Adrian Brock Master

                                If you're introducing new tests then do that, don't change the original tests. :-)

                                • 13. Re: Precedence
                                  Ales Justin Master

                                   

                                  "adrian@jboss.org" wrote:
                                  What is this? It is not a fix to change the test.
                                  You're supposed to make the tests pass, not change them!

                                  The orders in those tests were the correct ones.

                                  There's NO POINT having regression tests if you modify the tests
                                  because you introduced a regression. :-)

                                  :-)

                                  The new sort satisfies the given order.
                                  In the cases where the position doesn't matter, it currently doesn't do name compare.
                                  I'll see how I can push that in as well.

                                  • 14. Re: Precedence
                                    Ales Justin Master

                                     

                                    "adrian@jboss.org" wrote:
                                    If you're introducing new tests then do that, don't change the original tests. :-)

                                    It was easier to fix your tests than introduce name compare into the sorting algorithm. :-)

                                    I'll revert the changes and add name compare on the one's that are not 'comparable'.

                                    1 2 Previous Next