Re-implemented relative web fragment ordering using a topological sort

Conflicts:
	jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Ordering.java

5 files changed, 256 insertions, 488 deletions

diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/AbsoluteOrdering.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/AbsoluteOrdering.java
new file mode 100644
index 0000000000..0dd989e0eb
--- /dev/null
+++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/AbsoluteOrdering.java
@@ -0,0 +1,100 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2015 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.webapp;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.eclipse.jetty.util.resource.Resource;
+
+/**
+ * AbsoluteOrdering
+ *
+ * An <absolute-order> element in web.xml
+ */
+public class AbsoluteOrdering implements Ordering
+{
+    public static final String OTHER = "@@-OTHER-@@";
+    protected List<String> _order = new ArrayList<String>();
+    protected boolean _hasOther = false;
+    protected MetaData _metaData;
+
+    public AbsoluteOrdering (MetaData metaData)
+    {
+        _metaData = metaData;
+    }
+    
+    /** 
+     * Order the list of jars in WEB-INF/lib according to the ordering declarations in the descriptors
+     * @see org.eclipse.jetty.webapp.Ordering#order(java.util.List)
+     */
+    @Override
+    public List<Resource> order(List<Resource> jars)
+    {           
+        List<Resource> orderedList = new ArrayList<Resource>();
+        List<Resource> tmp = new ArrayList<Resource>(jars);
+      
+        //1. put everything into the list of named others, and take the named ones out of there,
+        //assuming we will want to use the <other> clause
+        Map<String,FragmentDescriptor> others = new HashMap<String,FragmentDescriptor>(_metaData.getNamedFragments());
+        
+        //2. for each name, take out of the list of others, add to tail of list
+        int index = -1;
+        for (String item:_order)
+        {
+            if (!item.equals(OTHER))
+            {
+                FragmentDescriptor f = others.remove(item);
+                if (f != null)
+                {
+                    Resource jar = _metaData.getJarForFragment(item);
+                    orderedList.add(jar); //take from others and put into final list in order, ignoring duplicate names
+                    //remove resource from list for resource matching name of descriptor
+                    tmp.remove(jar);
+                }
+            }
+            else
+                index = orderedList.size(); //remember the index at which we want to add in all the others
+        }
+        
+        //3. if <other> was specified, insert rest of the fragments 
+        if (_hasOther)
+        {
+            orderedList.addAll((index < 0? 0: index), tmp);
+        }
+        
+        return orderedList;
+    }
+    
+    public void add (String name)
+    {
+        _order.add(name); 
+    }
+    
+    public void addOthers ()
+    {
+        if (_hasOther)
+            throw new IllegalStateException ("Duplicate <other> element in absolute ordering");
+        
+        _hasOther = true;
+        _order.add(OTHER);
+    }
+}
\ No newline at end of file
diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java
index 78088acb48..0a9003e773 100644
--- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java
+++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java
@@ -166,15 +166,15 @@ public class MetaData
         {
             Ordering ordering = getOrdering();
             if (ordering == null)
-               ordering = new Ordering.AbsoluteOrdering(this);
+               ordering = new AbsoluteOrdering(this);
 
             List<String> order = _webDefaultsRoot.getOrdering();
             for (String s:order)
             {
                 if (s.equalsIgnoreCase("others"))
-                    ((Ordering.AbsoluteOrdering)ordering).addOthers();
+                    ((AbsoluteOrdering)ordering).addOthers();
                 else
-                    ((Ordering.AbsoluteOrdering)ordering).add(s);
+                    ((AbsoluteOrdering)ordering).add(s);
             }
             
             //(re)set the ordering to cause webinf jar order to be recalculated
@@ -193,15 +193,15 @@ public class MetaData
         {
             Ordering ordering = getOrdering();
             if (ordering == null)
-                ordering = new Ordering.AbsoluteOrdering(this);
+                ordering = new AbsoluteOrdering(this);
 
             List<String> order = _webXmlRoot.getOrdering();
             for (String s:order)
             {
                 if (s.equalsIgnoreCase("others"))
-                    ((Ordering.AbsoluteOrdering)ordering).addOthers();
+                    ((AbsoluteOrdering)ordering).addOthers();
                 else
-                    ((Ordering.AbsoluteOrdering)ordering).add(s);
+                    ((AbsoluteOrdering)ordering).add(s);
             }
             
             //(re)set the ordering to cause webinf jar order to be recalculated
@@ -233,15 +233,15 @@ public class MetaData
             Ordering ordering = getOrdering();
             
             if (ordering == null)
-               ordering = new Ordering.AbsoluteOrdering(this);
+               ordering = new AbsoluteOrdering(this);
 
             List<String> order = webOverrideRoot.getOrdering();
             for (String s:order)
             {
                 if (s.equalsIgnoreCase("others"))
-                    ((Ordering.AbsoluteOrdering)ordering).addOthers();
+                    ((AbsoluteOrdering)ordering).addOthers();
                 else
-                    ((Ordering.AbsoluteOrdering)ordering).add(s);
+                    ((AbsoluteOrdering)ordering).add(s);
             }
             
             //set or reset the ordering to cause the webinf jar ordering to be recomputed
@@ -286,7 +286,7 @@ public class MetaData
         //only accept an ordering from the fragment if there is no ordering already established
         if (_ordering == null && descriptor.isOrdered())
         {
-            setOrdering(new Ordering.RelativeOrdering(this));
+            setOrdering(new RelativeOrdering(this));
             return;
         }
         
diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Ordering.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Ordering.java
index 5ee01be945..86143ba234 100644
--- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Ordering.java
+++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Ordering.java
@@ -18,12 +18,7 @@
 
 package org.eclipse.jetty.webapp;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 
 import org.eclipse.jetty.util.resource.Resource;
 
@@ -34,460 +29,5 @@ import org.eclipse.jetty.util.resource.Resource;
 public interface Ordering
 {  
     public List<Resource> order(List<Resource> fragments);
-    public boolean isAbsolute ();
-    public boolean hasOther();
-
-    /**
-     * AbsoluteOrdering
-     *
-     * An &lt;absolute-order&gt; element in web.xml
-     */
-    public static class AbsoluteOrdering implements Ordering
-    {
-        public static final String OTHER = "@@-OTHER-@@";
-        protected List<String> _order = new ArrayList<String>();
-        protected boolean _hasOther = false;
-        protected MetaData _metaData;
-    
-        public AbsoluteOrdering (MetaData metaData)
-        {
-            _metaData = metaData;
-        }
-        
-        /** 
-         * Order the list of jars in WEB-INF/lib according to the ordering declarations in the descriptors
-         * @see org.eclipse.jetty.webapp.Ordering#order(java.util.List)
-         */
-        @Override
-        public List<Resource> order(List<Resource> jars)
-        {           
-            List<Resource> orderedList = new ArrayList<Resource>();
-            List<Resource> tmp = new ArrayList<Resource>(jars);
-          
-            //1. put everything into the list of named others, and take the named ones out of there,
-            //assuming we will want to use the <other> clause
-            Map<String,FragmentDescriptor> others = new HashMap<String,FragmentDescriptor>(_metaData.getNamedFragments());
-            
-            //2. for each name, take out of the list of others, add to tail of list
-            int index = -1;
-            for (String item:_order)
-            {
-                if (!item.equals(OTHER))
-                {
-                    FragmentDescriptor f = others.remove(item);
-                    if (f != null)
-                    {
-                        Resource jar = _metaData.getJarForFragment(item);
-                        orderedList.add(jar); //take from others and put into final list in order, ignoring duplicate names
-                        //remove resource from list for resource matching name of descriptor
-                        tmp.remove(jar);
-                    }
-                }
-                else
-                    index = orderedList.size(); //remember the index at which we want to add in all the others
-            }
-            
-            //3. if <other> was specified, insert rest of the fragments 
-            if (_hasOther)
-            {
-                orderedList.addAll((index < 0? 0: index), tmp);
-            }
-            
-            return orderedList;
-        }
-        
-        @Override
-        public boolean isAbsolute()
-        {
-            return true;
-        }
-        
-        public void add (String name)
-        {
-            _order.add(name); 
-        }
-        
-        public void addOthers ()
-        {
-            if (_hasOther)
-                throw new IllegalStateException ("Duplicate <other> element in absolute ordering");
-            
-            _hasOther = true;
-            _order.add(OTHER);
-        }
-        
-        @Override
-        public boolean hasOther ()
-        {
-            return _hasOther;
-        }
-    }
-    /**
-     * RelativeOrdering
-     *
-     * A set of &lt;order&gt; elements in web-fragment.xmls.
-     */
-    public static class RelativeOrdering implements Ordering
-    {
-        protected MetaData _metaData;
-        protected LinkedList<Resource> _beforeOthers = new LinkedList<Resource>();
-        protected LinkedList<Resource> _afterOthers = new LinkedList<Resource>();
-        protected LinkedList<Resource> _noOthers = new LinkedList<Resource>();
-        
-        public RelativeOrdering (MetaData metaData)
-        {
-            _metaData = metaData;
-        }
-        /** 
-         * Order the list of jars according to the ordering declared
-         * in the various web-fragment.xml files.
-         * @see org.eclipse.jetty.webapp.Ordering#order(java.util.List)
-         */
-        @Override
-        public List<Resource> order(List<Resource> jars)
-        {         
-            _beforeOthers.clear();
-            _afterOthers.clear();
-            _noOthers.clear();
-
-            //for each jar, put it into the ordering according to the fragment ordering
-            for (Resource jar:jars)
-            {
-                //check if the jar has a fragment descriptor
-                FragmentDescriptor descriptor = _metaData.getFragment(jar);
-                if (descriptor != null)
-                {
-                    switch (descriptor.getOtherType())
-                    {
-                        case None:
-                        {
-                            addNoOthers(jar);
-                            break;
-                        }
-                        case Before:
-                        { 
-                            addBeforeOthers(jar);
-                            break;
-                        }
-                        case After:
-                        {
-                            addAfterOthers(jar);
-                            break;
-                        }
-                    } 
-                }
-                else
-                {
-                    //jar fragment has no descriptor, but there is a relative ordering in place, so it must be part of the others
-                    addNoOthers(jar);
-                }
-            }            
-                
-            //now apply the ordering
-            List<Resource> orderedList = new ArrayList<Resource>(); 
-            int maxIterations = 2;
-            boolean done = false;
-            do
-            {
-                //1. order the before-others according to any explicit before/after relationships 
-                boolean changesBefore = orderList(_beforeOthers);
-    
-                //2. order the after-others according to any explicit before/after relationships
-                boolean changesAfter = orderList(_afterOthers);
-    
-                //3. order the no-others according to their explicit before/after relationships
-                boolean changesNone = orderList(_noOthers);
-                
-                //we're finished on a clean pass through with no ordering changes
-                done = (!changesBefore && !changesAfter && !changesNone);
-            }
-            while (!done && (--maxIterations >0));
-            
-            //4. merge before-others + no-others +after-others
-            if (!done)
-                throw new IllegalStateException("Circular references for fragments");
-            
-            for (Resource r: _beforeOthers)
-                orderedList.add(r);
-            for (Resource r: _noOthers)
-                orderedList.add(r);
-            for(Resource r: _afterOthers)
-                orderedList.add(r);
-            
-            return orderedList;
-        }
-        
-        @Override
-        public boolean isAbsolute ()
-        {
-            return false;
-        }
-        
-        @Override
-        public boolean hasOther ()
-        {
-            return !_beforeOthers.isEmpty() || !_afterOthers.isEmpty();
-        }
-        
-        public void addBeforeOthers (Resource r)
-        {
-            _beforeOthers.addLast(r);
-        }
-        
-        public void addAfterOthers (Resource r)
-        {
-            _afterOthers.addLast(r);
-        }
-        
-        public void addNoOthers (Resource r)
-        {
-            _noOthers.addLast(r);
-        }
-        
-       protected boolean orderList (LinkedList<Resource> list)
-       {
-           //Take a copy of the list so we can iterate over it and at the same time do random insertions
-           boolean changes = false;
-           List<Resource> iterable = new ArrayList<Resource>(list);
-           Iterator<Resource> itor = iterable.iterator();
-           
-           while (itor.hasNext())
-           {
-               Resource r = itor.next();
-               FragmentDescriptor f = _metaData.getFragment(r);
-               if (f == null)
-               {
-                   //no fragment for this resource so cannot have any ordering directives
-                   continue;
-               }
-                
-               //Handle any explicit <before> relationships for the fragment we're considering
-               List<String> befores = f.getBefores();
-               if (befores != null && !befores.isEmpty())
-               {
-                   for (String b: befores)
-                   {
-                       //Fragment we're considering must be before b
-                       //Check that we are already before it, if not, move us so that we are.
-                       //If the name does not exist in our list, then get it out of the no-other list
-                       if (!isBefore(list, f.getName(), b))
-                       {
-                           //b is not already before name, move it so that it is
-                           int idx1 = getIndexOf(list, f.getName());
-                           int idx2 = getIndexOf(list, b);
-    
-                           //if b is not in the same list
-                           if (idx2 < 0)
-                           {
-                               changes = true;
-                               // must be in the noOthers list or it would have been an error
-                               Resource bResource = _metaData.getJarForFragment(b);
-                               if (bResource != null)
-                               {
-                                   //If its in the no-others list, insert into this list so that we are before it
-                                   if (_noOthers.remove(bResource))
-                                   {
-                                       insert(list, idx1+1, b);
-                                      
-                                   }
-                               }
-                           }
-                           else
-                           {
-                               //b is in the same list but b is before name, so swap it around
-                               list.remove(idx1);
-                               insert(list, idx2, f.getName());
-                               changes = true;
-                           }
-                       }
-                   }
-               }
-    
-               //Handle any explicit <after> relationships
-               List<String> afters = f.getAfters();
-               if (afters != null && !afters.isEmpty())
-               {
-                   for (String a: afters)
-                   {
-                       //Check that fragment we're considering is after a, moving it if possible if its not
-                       if (!isAfter(list, f.getName(), a))
-                       {
-                           //name is not after a, move it
-                           int idx1 = getIndexOf(list, f.getName());
-                           int idx2 = getIndexOf(list, a);
-                           
-                           //if a is not in the same list as name
-                           if (idx2 < 0)
-                           {
-                               changes = true;
-                               //take it out of the noOthers list and put it in the right place in this list
-                               Resource aResource = _metaData.getJarForFragment(a);
-                               if (aResource != null)
-                               {
-                                   if (_noOthers.remove(aResource))
-                                   {
-                                       insert(list,idx1, aResource);       
-                                   }
-                               }
-                           }
-                           else
-                           {
-                               //a is in the same list as name, but in the wrong place, so move it
-                               list.remove(idx2);
-                               insert(list,idx1, a);
-                               changes = true;
-                           }
-                       }
-                       //Name we're considering must be after this name
-                       //Check we're already after it, if not, move us so that we are.
-                       //If the name does not exist in our list, then get it out of the no-other list
-                   }
-               }
-           }
-    
-           return changes;
-       }
-    
-       /**
-        * Is fragment with name a before fragment with name b?
-        * 
-        * @param list the list of resources
-        * @param fragNameA the first fragment
-        * @param fragNameB the second fragment
-        * @return true if fragment name A is before fragment name B 
-        */
-       protected boolean isBefore (List<Resource> list, String fragNameA, String fragNameB)
-       {
-           //check if a and b are already in the same list, and b is already
-           //before a 
-           int idxa = getIndexOf(list, fragNameA);
-           int idxb = getIndexOf(list, fragNameB);
-           
-           
-           if (idxb >=0 && idxb < idxa)
-           {
-               //a and b are in the same list but a is not before b
-               return false;
-           }
-           
-           if (idxb < 0)
-           {
-               //a and b are not in the same list, but it is still possible that a is before
-               //b, depending on which list we're examining
-               if (list == _beforeOthers)
-               {
-                   //The list we're looking at is the beforeOthers.If b is in the _afterOthers or the _noOthers, then by
-                   //definition a is before it
-                   return true;
-               }
-               else if (list == _afterOthers)
-               {
-                   //The list we're looking at is the afterOthers, then a will be the tail of
-                   //the final list.  If b is in the beforeOthers list, then b will be before a and an error.
-                   if (_beforeOthers.contains(fragNameB))
-                       throw new IllegalStateException("Incorrect relationship: "+fragNameA+" before "+fragNameB);
-                   else
-                       return false; //b could be moved to the list
-               }
-           }
-          
-           //a and b are in the same list and a is already before b
-           return true;
-       }
-    
-    
-       /**
-        * Is fragment name "a" after fragment name "b"?
-        * 
-        * @param list the list of resources
-        * @param fragNameA the first fragment
-        * @param fragNameB the second fragment
-        * @return true if fragment name A is after fragment name B
-        */
-       protected boolean isAfter(List<Resource> list, String fragNameA, String fragNameB)
-       {
-           int idxa = getIndexOf(list, fragNameA);
-           int idxb = getIndexOf(list, fragNameB);
-           
-           if (idxb >=0 && idxa < idxb)
-           {
-               //a and b are both in the same list, but a is before b
-               return false;
-           }
-           
-           if (idxb < 0)
-           {
-               //a and b are in different lists. a could still be after b depending on which list it is in.
-    
-               if (list == _afterOthers)
-               {
-                   //The list we're looking at is the afterOthers. If b is in the beforeOthers or noOthers then
-                   //by definition a is after b because a is in the afterOthers list.
-                   return true;
-               }
-               else if (list == _beforeOthers)
-               {
-                   //The list we're looking at is beforeOthers, and contains a and will be before
-                   //everything else in the final ist. If b is in the afterOthers list, then a cannot be before b.
-                   if (_afterOthers.contains(fragNameB))
-                       throw new IllegalStateException("Incorrect relationship: "+fragNameB+" after "+fragNameA);
-                   else
-                       return false; //b could be moved from noOthers list
-               }
-           }
-    
-           return true; //a and b in the same list, a is after b
-       }
-    
-       /**
-        * Insert the resource matching the fragName into the list of resources
-        * at the location indicated by index.
-        * 
-        * @param list the list of resources
-        * @param index the index to insert into
-        * @param fragName the fragment name to insert
-        */
-       protected void insert(List<Resource> list, int index, String fragName)
-       {
-           Resource jar = _metaData.getJarForFragment(fragName);
-           if (jar == null)
-               throw new IllegalStateException("No jar for insertion");
-           
-           insert(list, index, jar);
-       }
-    
-       protected void insert(List<Resource> list, int index, Resource resource)
-       {
-           if (list == null)
-               throw new IllegalStateException("List is null for insertion");
-           
-           //add it at the end
-           if (index > list.size())
-               list.add(resource);
-           else
-               list.add(index, resource);
-       }
-    
-       protected void remove (List<Resource> resources, Resource r)
-       {
-           if (resources == null)
-               return;
-           resources.remove(r);
-       }
-    
-       protected int getIndexOf(List<Resource> resources, String fragmentName)
-       {
-          FragmentDescriptor fd = _metaData.getFragment(fragmentName);
-          if (fd == null)
-              return -1;
-          
-          
-          Resource r = _metaData.getJarForFragment(fragmentName);
-          if (r == null)
-              return -1;
-          
-          return resources.indexOf(r);
-       }
-    }
   
 }
diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/RelativeOrdering.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/RelativeOrdering.java
new file mode 100644
index 0000000000..374c9701ce
--- /dev/null
+++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/RelativeOrdering.java
@@ -0,0 +1,144 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2015 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.webapp;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Consumer;
+
+import org.eclipse.jetty.util.TopologicalSort;
+import org.eclipse.jetty.util.resource.Resource;
+
+/**
+ * Relative Fragment Ordering
+ * <p>Uses a {@link TopologicalSort} to order the fragments.</p>
+ */
+public class RelativeOrdering implements Ordering
+{
+    protected MetaData _metaData;
+    
+    public RelativeOrdering (MetaData metaData)
+    {
+        _metaData = metaData;
+    }
+
+    @Override
+    public List<Resource> order(List<Resource> jars)
+    {    
+        TopologicalSort<Resource> sort = new TopologicalSort<>();
+        List<Resource> sorted = new ArrayList<>(jars);
+        Set<Resource> others = new HashSet<>();
+        Set<Resource> before_others = new HashSet<>();
+        Set<Resource> after_others = new HashSet<>();
+        
+        // Pass 1: split the jars into 'before others', 'others' or 'after others'
+        for (Resource jar : jars)
+        {
+            FragmentDescriptor fragment=_metaData.getFragment(jar);
+            
+            if (fragment == null)
+                others.add(jar);
+            else
+            {
+                switch (fragment.getOtherType())
+                {
+                    case None:
+                        others.add(jar);
+                        break;
+                    case Before:
+                        before_others.add(jar);
+                        break;
+                    case After:
+                        after_others.add(jar);
+                        break;
+                } 
+            }
+        }
+        
+        // Pass 2: Add sort dependencies for each jar
+        Set<Resource> referenced = new HashSet<>();
+        for (Resource jar : jars)
+        {
+            FragmentDescriptor fragment=_metaData.getFragment(jar);
+
+            if (fragment != null)
+            {
+                // Add each explicit 'after' ordering as a sort dependency
+                // and remember that the dependency has been referenced.
+                for (String name: fragment.getAfters())
+                {
+                    Resource after=_metaData.getJarForFragment(name);
+                    sort.addDependency(jar,after);
+                    referenced.add(after);
+                }
+
+                // Add each explicit 'before' ordering as a sort dependency
+                // and remember that the dependency has been referenced.
+                for (String name: fragment.getBefores())
+                {
+                    Resource before=_metaData.getJarForFragment(name);
+                    sort.addDependency(before,jar);
+                    referenced.add(before);
+                }
+
+                // handle the others
+                switch (fragment.getOtherType())
+                {
+                    case None:
+                        break;
+                    case Before:
+                        // Add a dependency on this jar from all 
+                        // jars in the 'others' and 'after others' sets, but
+                        // exclude any jars we have already explicitly 
+                        // referenced above.
+                        Consumer<Resource> add_before = other -> 
+                        {
+                            if (!referenced.contains(other)) 
+                                sort.addDependency(other,jar);
+                        };
+                        others.forEach(add_before);
+                        after_others.forEach(add_before);
+                        break;
+                        
+                    case After:
+                        // Add a dependency from this jar to all 
+                        // jars in the 'before others' and 'others' sets, but
+                        // exclude any jars we have already explicitly 
+                        // referenced above.
+                        Consumer<Resource> add_after = other -> 
+                        {
+                            if (!referenced.contains(other)) 
+                                sort.addDependency(jar,other);
+                        };
+                        before_others.forEach(add_after);
+                        others.forEach(add_after);
+                        break;
+                } 
+            }
+            referenced.clear();
+        }
+
+        // sort the jars according to the added dependencies
+        sort.sort(sorted);
+        
+        return sorted;
+    }
+}
\ No newline at end of file
diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/OrderingTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/OrderingTest.java
index 7aca0ead12..4ad040f030 100644
--- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/OrderingTest.java
+++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/OrderingTest.java
@@ -32,8 +32,6 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.jetty.util.resource.Resource;
-import org.eclipse.jetty.webapp.Ordering.AbsoluteOrdering;
-import org.eclipse.jetty.webapp.Ordering.RelativeOrdering;
 import org.junit.Test;
 
 /**
@@ -185,7 +183,6 @@ public class OrderingTest
     throws Exception
     {
         //Example from ServletSpec p.70
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         List<Resource> resources = new ArrayList<Resource>();
         metaData._ordering = new RelativeOrdering(metaData);
@@ -279,7 +276,6 @@ public class OrderingTest
     throws Exception
     {
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new RelativeOrdering(metaData);
 
@@ -364,7 +360,7 @@ public class OrderingTest
                              "BEFplainDC",
                              "EBFplainCD",
                              "EBFplainDC",
-                             "EBFDplain"};
+                             "EBFDplainC"};
 
         String orderedNames = "";
         for (Resource r:orderedList)
@@ -379,7 +375,6 @@ public class OrderingTest
     throws Exception
     {
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new RelativeOrdering(metaData);
 
@@ -454,7 +449,6 @@ public class OrderingTest
     throws Exception
     {
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new RelativeOrdering(metaData);
 
@@ -529,7 +523,6 @@ public class OrderingTest
         //A: after B
         //B: after A
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new RelativeOrdering(metaData);
 
@@ -559,7 +552,7 @@ public class OrderingTest
 
         try
         {
-            List<Resource> orderedList = metaData._ordering.order(resources);
+            metaData._ordering.order(resources);
             fail("No circularity detected");
         }
         catch (Exception e)
@@ -575,7 +568,6 @@ public class OrderingTest
     throws Exception
     {
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new RelativeOrdering(metaData);
 
@@ -637,7 +629,6 @@ public class OrderingTest
         // A,B,C,others
         //
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new AbsoluteOrdering(metaData);
         ((AbsoluteOrdering)metaData._ordering).add("A");
@@ -711,7 +702,6 @@ public class OrderingTest
         // C,B,A
         List<Resource> resources = new ArrayList<Resource>();
 
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new AbsoluteOrdering(metaData);
         ((AbsoluteOrdering)metaData._ordering).add("C");
@@ -783,7 +773,6 @@ public class OrderingTest
     {
         //empty <absolute-ordering>
 
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new AbsoluteOrdering(metaData);
         List<Resource> resources = new ArrayList<Resource>();
@@ -801,7 +790,6 @@ public class OrderingTest
     {
         //B,A,C other jars with no fragments
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new RelativeOrdering(metaData);
 
@@ -867,7 +855,6 @@ public class OrderingTest
     {
         //web.xml has no ordering, jar A has fragment after others, jar B is plain, jar C is plain
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new RelativeOrdering(metaData);
         
@@ -907,7 +894,6 @@ public class OrderingTest
         // A,B,C,others
         //
         List<Resource> resources = new ArrayList<Resource>();
-        WebAppContext wac = new WebAppContext();
         MetaData metaData = new MetaData();
         metaData._ordering = new AbsoluteOrdering(metaData);
         ((AbsoluteOrdering)metaData._ordering).add("A");
@@ -981,8 +967,6 @@ public class OrderingTest
             fail("No outcome matched "+result);
     }
 
-
-
     public boolean checkResult (String result, String[] outcomes)
     {
         boolean matched = false;