-
1. Re: Precedence
bill.burke Jul 2, 2007 11:25 AM (in response to adrian.brock)"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 Jul 2, 2007 11:35 AM (in response to adrian.brock)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 Jul 2, 2007 11:59 AM (in response to adrian.brock)"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
alesj Jul 3, 2007 9:16 AM (in response to adrian.brock)"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
alesj Jul 3, 2007 9:20 AM (in response to adrian.brock)Or O(n*log(n)) vs. O(log(n)) if we 'exaggerate' on the implementation. ;-)
-
6. Re: Precedence
adrian.brock Jul 3, 2007 10:04 AM (in response to adrian.brock)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 Jul 3, 2007 10:50 AM (in response to adrian.brock)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
alesj Jul 3, 2007 10:59 AM (in response to adrian.brock)"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
alesj Jul 6, 2007 7:30 AM (in response to adrian.brock)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 Jul 6, 2007 8:21 AM (in response to adrian.brock)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 Jul 6, 2007 9:16 AM (in response to adrian.brock)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 Jul 6, 2007 9:19 AM (in response to adrian.brock)If you're introducing new tests then do that, don't change the original tests. :-)
-
13. Re: Precedence
alesj Jul 6, 2007 9:21 AM (in response to adrian.brock)"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
alesj Jul 6, 2007 9:25 AM (in response to adrian.brock)"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'.