refactored BundleResolver to first find all requirements then explicitly sort them before resolving dependencies (FELIX-1664)
also minor refactoring to add isOptional/setOptional to parent IRequirementModelElement interface


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@820299 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/sigil/common/core/src/org/apache/felix/sigil/core/repository/BundleResolver.java b/sigil/common/core/src/org/apache/felix/sigil/core/repository/BundleResolver.java
index fcc4c57..613512d 100644
--- a/sigil/common/core/src/org/apache/felix/sigil/core/repository/BundleResolver.java
+++ b/sigil/common/core/src/org/apache/felix/sigil/core/repository/BundleResolver.java
@@ -35,6 +35,8 @@
 import org.apache.felix.sigil.core.BldCore;
 import org.apache.felix.sigil.model.ICompoundModelElement;
 import org.apache.felix.sigil.model.IModelElement;
+import org.apache.felix.sigil.model.IModelWalker;
+import org.apache.felix.sigil.model.IRequirementModelElement;
 import org.apache.felix.sigil.model.eclipse.ILibrary;
 import org.apache.felix.sigil.model.eclipse.ILibraryImport;
 import org.apache.felix.sigil.model.eclipse.ISigilBundle;
@@ -56,7 +58,7 @@
 public class BundleResolver implements IBundleResolver
 {
 
-    private class BundleOrderComparator implements Comparator<ISigilBundle>
+    private static class BundleOrderComparator implements Comparator<ISigilBundle>
     {
         private IModelElement requirement;
 
@@ -155,7 +157,7 @@
 
     }
 
-    private class ResolutionContext
+    private static class ResolutionContext
     {
         private final IModelElement root;
         private final ResolutionConfig config;
@@ -210,18 +212,18 @@
         }
 
 
-        public void startRequirement( IModelElement element )
+        public void startRequirement( IRequirementModelElement element )
         {
             requirements.add( element );
             monitor.startResolution( element );
         }
 
 
-        public void endRequirement( IModelElement element )
+        public void endRequirement( IRequirementModelElement element )
         {
             ISigilBundle provider = resolution.getProvider( element );
 
-            setValid( provider != null || isOptional( element ) || config.isIgnoreErrors() );
+            setValid( provider != null || element.isOptional() || config.isIgnoreErrors() );
 
             if ( isValid() )
             {
@@ -234,7 +236,7 @@
         }
     }
 
-    private class Resolution implements IResolution
+    private static class Resolution implements IResolution
     {
         private Map<ISigilBundle, List<IModelElement>> providees = new HashMap<ISigilBundle, List<IModelElement>>();
         private Map<IModelElement, ISigilBundle> providers = new HashMap<IModelElement, ISigilBundle>();
@@ -359,6 +361,30 @@
         }
     };
 
+    private static final Comparator<IRequirementModelElement> REQUIREMENT_COMPARATOR = new Comparator<IRequirementModelElement>()
+    {
+        public int compare(IRequirementModelElement o1, IRequirementModelElement o2)
+        {
+            if ( o1 instanceof IPackageImport ) {
+                if (o2 instanceof IPackageImport ) {
+                    return 0;
+                }
+                else {
+                    return -1;
+                }
+            }
+            else // assumes only alternative is IRequiredBundle
+            {
+                if ( o2 instanceof IRequiredBundle) {
+                    return 0;
+                }
+                else {
+                    return 1;
+                }
+            }
+        }
+    };
+
     private IRepositoryManager repositoryManager;
 
 
@@ -387,53 +413,56 @@
         return ctx.resolution;
     }
 
-
     private void resolveElement( IModelElement element, ResolutionContext ctx ) throws ResolutionException
     {
-        if ( isRequirement( element ) )
-        {
-            resolveRequirement( element, ctx );
-        }
-
-        if ( ctx.isValid() && element instanceof ICompoundModelElement )
-        {
-            resolveCompound( ( ICompoundModelElement ) element, ctx );
-        }
-    }
-
-
-    private void resolveCompound( ICompoundModelElement compound, ResolutionContext ctx ) throws ResolutionException
-    {
-        for ( IModelElement element : compound.children() )
-        {
-            if ( ctx.isNewModelElement( element ) )
-            {
-                if ( isRequirement( element ) )
-                {
-                    resolveRequirement( element, ctx );
-                }
-                else if ( element instanceof ICompoundModelElement )
-                {
-                    if ( !ctx.monitor.isCanceled() )
-                    {
-                        ctx.enterModelElement( element );
-                        resolveElement( ( ICompoundModelElement ) element, ctx );
-                        ctx.exitModelElement( element );
-                    }
-                }
-
-                if ( !ctx.isValid() )
-                {
+        if ( ctx.isValid() ) {
+            for (IRequirementModelElement req : findRequirements(element, ctx)) {
+                if ( ctx.monitor.isCanceled())
                     break;
-                }
+                resolveRequirement(req, ctx);
             }
         }
     }
-
-
-    private void resolveRequirement( IModelElement requirement, ResolutionContext ctx ) throws ResolutionException
+    
+    private List<IRequirementModelElement> findRequirements(IModelElement element,
+        final ResolutionContext ctx)
     {
-        if ( ctx.config.isOptional() || !isOptional( requirement ) )
+        final LinkedList<IRequirementModelElement> reqs = new LinkedList<IRequirementModelElement>();
+        
+        if (element instanceof IRequirementModelElement)
+        {
+            reqs.add((IRequirementModelElement) element);
+        }
+        else if (element instanceof ICompoundModelElement)
+        {
+            ICompoundModelElement compound = (ICompoundModelElement) element;
+            compound.visit(new IModelWalker()
+            {
+                public boolean visit(IModelElement element)
+                {
+                    if (element instanceof IRequirementModelElement)
+                    {
+                        reqs.add((IRequirementModelElement) element);
+                    }
+                    else if ( element instanceof ILibrary ) {
+                        ILibrary lib = repositoryManager.resolveLibrary( ( ILibraryImport ) element );
+                        reqs.addAll( lib.getImports() );
+                    }
+                    
+                    return !ctx.monitor.isCanceled();
+                }
+            });
+        }
+
+        if ( !ctx.monitor.isCanceled() ) {
+            Collections.sort(reqs, REQUIREMENT_COMPARATOR);
+        }        
+        return reqs;
+    }
+
+    private void resolveRequirement( IRequirementModelElement requirement, ResolutionContext ctx ) throws ResolutionException
+    {
+        if ( ctx.config.isOptional() || !requirement.isOptional() )
         {
             ctx.startRequirement( requirement );
 
@@ -488,7 +517,7 @@
     }
 
 
-    private List<ISigilBundle> findProvidersAtPriority( int i, IModelElement requirement, ResolutionContext ctx )
+    private List<ISigilBundle> findProvidersAtPriority( int i, IRequirementModelElement requirement, ResolutionContext ctx )
         throws ResolutionException
     {
         ArrayList<ISigilBundle> providers = new ArrayList<ISigilBundle>();
@@ -506,7 +535,7 @@
     }
 
 
-    private Collection<ISigilBundle> findProviders( IModelElement requirement, ResolutionConfig config,
+    private Collection<ISigilBundle> findProviders( IRequirementModelElement requirement, ResolutionConfig config,
         IBundleRepository rep ) throws ResolutionException
     {
         ArrayList<ISigilBundle> found = new ArrayList<ISigilBundle>();
@@ -521,14 +550,6 @@
             IRequiredBundle rb = ( IRequiredBundle ) requirement;
             found.addAll( rep.findAllProviders( rb, config.getOptions() ) );
         }
-        else if ( requirement instanceof ILibraryImport )
-        {
-            ILibrary lib = repositoryManager.resolveLibrary( ( ILibraryImport ) requirement );
-            if ( lib != null )
-            {
-                found.addAll( rep.findProviders( lib, config.getOptions() ) );
-            }
-        }
         else
         {
             // shouldn't get here - developer error if do
@@ -538,43 +559,4 @@
 
         return found;
     }
-
-
-    private boolean isOptional( IModelElement element )
-    {
-        if ( element instanceof IPackageImport )
-        {
-            return ( ( IPackageImport ) element ).isOptional();
-        }
-        else if ( element instanceof IRequiredBundle )
-        {
-            return ( ( IRequiredBundle ) element ).isOptional();
-        }
-        else if ( element instanceof ILibraryImport )
-        {
-            ILibrary lib = repositoryManager.resolveLibrary( ( ILibraryImport ) element );
-            for ( IPackageImport pi : lib.getImports() )
-            {
-                if ( !isOptional( pi ) )
-                {
-                    return false;
-                }
-            }
-            return true;
-        }
-        else
-        {
-            // should never get this due to isRequirement test prior to calling this
-            // developer error if found
-            throw new IllegalStateException( "Invalid optional element test for " + element );
-        }
-    }
-
-
-    private boolean isRequirement( IModelElement element )
-    {
-        return element instanceof IPackageImport || element instanceof IRequiredBundle
-            || element instanceof ILibraryImport;
-    }
-
 }
diff --git a/sigil/common/core/src/org/apache/felix/sigil/model/IRequirementModelElement.java b/sigil/common/core/src/org/apache/felix/sigil/model/IRequirementModelElement.java
index 64419c9..0f73b66 100644
--- a/sigil/common/core/src/org/apache/felix/sigil/model/IRequirementModelElement.java
+++ b/sigil/common/core/src/org/apache/felix/sigil/model/IRequirementModelElement.java
@@ -20,7 +20,15 @@
 package org.apache.felix.sigil.model;
 
 
-public interface IRequirementModelElement
+public interface IRequirementModelElement extends IModelElement
 {
     boolean accepts( IModelElement provider );
+    
+    /**
+     * indicates whether the OSGi attribute "resolution=optional" is specified.
+     */
+    boolean isOptional();
+
+
+    void setOptional( boolean optional );
 }
diff --git a/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IPackageImport.java b/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IPackageImport.java
index 1cec175..8d6de03 100644
--- a/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IPackageImport.java
+++ b/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IPackageImport.java
@@ -27,15 +27,6 @@
     Comparable<IPackageImport>
 {
     /**
-     * indicates whether the OSGi attribute "resolution=optional" is specified.
-     */
-    boolean isOptional();
-
-
-    void setOptional( boolean optional );
-
-
-    /**
      * indicates whether import is needed at compile-time.
      * Default true. Used in conjunction with OSGiHeader.ALWAYS,
      * to add an OSGI import, without creating a dependency.
diff --git a/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IRequiredBundle.java b/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IRequiredBundle.java
index fa64308..dc04b37 100644
--- a/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IRequiredBundle.java
+++ b/sigil/common/core/src/org/apache/felix/sigil/model/osgi/IRequiredBundle.java
@@ -37,10 +37,4 @@
 
 
     void setVersions( VersionRange versions );
-
-
-    boolean isOptional();
-
-
-    void setOptional( boolean optional );
 }
\ No newline at end of file