You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ivy-commits@incubator.apache.org by gs...@apache.org on 2007/06/19 11:32:43 UTC

svn commit: r548695 - in /incubator/ivy/core/trunk: src/java/org/apache/ivy/Ivy.java src/java/org/apache/ivy/core/sort/SortEngine.java test/java/org/apache/ivy/core/sort/SortTest.java

Author: gscokart
Date: Tue Jun 19 04:32:43 2007
New Revision: 548695

URL: http://svn.apache.org/viewvc?view=rev&rev=548695
Log:
refactor: remove dependency between sort engine and IvySettings

Modified:
    incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
    incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
    incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java

Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
URL: http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java?view=diff&rev=548695&r1=548694&r2=548695
==============================================================================
--- incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java (original)
+++ incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java Tue Jun 19 04:32:43 2007
@@ -159,7 +159,8 @@
             eventManager = new EventManager();
         }
         if (sortEngine == null) {
-            sortEngine = new SortEngine(settings);
+            sortEngine = new SortEngine();
+            //Settings element are injected in the getSortEngine method. 
         }
         if (searchEngine == null) {
             searchEngine = new SearchEngine(settings);
@@ -329,7 +330,7 @@
      * Sorts the collection of IvyNode from the less dependent to the more dependent
      */
     public List sortNodes(Collection nodes) {
-        return sortEngine.sortNodes(nodes);
+        return getSortEngine().sortNodes(nodes);
     }
 
     /**
@@ -344,15 +345,10 @@
      *            revision of an other modules present in the of modules to sort with a different
      *            revision.
      * @return a List of sorted ModuleDescriptors
-     * @deprecated Use sortModuleDescriptors(Collection,NonMatchingVersionReporter)
      */
-    public List sortModuleDescriptors(Collection moduleDescriptors) {
-        return sortEngine.sortModuleDescriptors(moduleDescriptors, new SilentNonMatchingVersionReporter());
-    }
-
     public List sortModuleDescriptors(Collection moduleDescriptors,
             NonMatchingVersionReporter nonMatchingVersionReporter) {
-        return sortEngine.sortModuleDescriptors(moduleDescriptors, nonMatchingVersionReporter);
+        return getSortEngine().sortModuleDescriptors(moduleDescriptors, nonMatchingVersionReporter);
     }
 
     // ///////////////////////////////////////////////////////////////////////
@@ -569,6 +565,8 @@
     }
 
     public SortEngine getSortEngine() {
+        sortEngine.setCircularDependencyStrategy(settings.getCircularDependencyStrategy());
+        sortEngine.setVersionMatcher(settings.getVersionMatcher());
         return sortEngine;
     }
 

Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
URL: http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java?view=diff&rev=548695&r1=548694&r2=548695
==============================================================================
--- incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java (original)
+++ incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java Tue Jun 19 04:32:43 2007
@@ -26,18 +26,29 @@
 
 import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
 import org.apache.ivy.core.resolve.IvyNode;
-import org.apache.ivy.core.settings.IvySettings;
 import org.apache.ivy.plugins.circular.CircularDependencyException;
 import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
 import org.apache.ivy.plugins.version.VersionMatcher;
 
 public class SortEngine {
-    private IvySettings settings;
 
-    public SortEngine(IvySettings settings) {
-        this.settings = settings;
+    private CircularDependencyStrategy circularStrategy;
+
+    private VersionMatcher versionMatcher;
+
+    public SortEngine() {
+    }
+
+    
+    public void setCircularDependencyStrategy(CircularDependencyStrategy circularStrategy) {
+        this.circularStrategy = circularStrategy;
     }
 
+    public void setVersionMatcher(VersionMatcher versionMatcher) {
+        this.versionMatcher = versionMatcher;
+    }
+
+
     public List sortNodes(Collection nodes) throws CircularDependencyException {
         /*
          * here we want to use the sort algorithm which work on module descriptors : so we first put
@@ -92,10 +103,8 @@
     public List sortModuleDescriptors(Collection moduleDescriptors,
             NonMatchingVersionReporter nonMatchingVersionReporter)
             throws CircularDependencyException {
-        VersionMatcher versionMatcher = settings.getVersionMatcher();
-        CircularDependencyStrategy circularDepStrategy = settings.getCircularDependencyStrategy();
         ModuleDescriptorSorter sorter = new ModuleDescriptorSorter(moduleDescriptors,
-                versionMatcher, nonMatchingVersionReporter, circularDepStrategy);
+                versionMatcher, nonMatchingVersionReporter, circularStrategy);
         return sorter.sortModuleDescriptors();
     }
 

Modified: incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
URL: http://svn.apache.org/viewvc/incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java?view=diff&rev=548695&r1=548694&r2=548695
==============================================================================
--- incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java (original)
+++ incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java Tue Jun 19 04:32:43 2007
@@ -27,7 +27,6 @@
 import junit.framework.Assert;
 import junit.framework.TestCase;
 
-import org.apache.ivy.Ivy;
 import org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor;
 import org.apache.ivy.core.module.descriptor.DefaultModuleDescriptor;
 import org.apache.ivy.core.module.descriptor.DependencyDescriptor;
@@ -35,6 +34,9 @@
 import org.apache.ivy.core.module.id.ModuleRevisionId;
 import org.apache.ivy.plugins.circular.CircularDependencyHelper;
 import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
+import org.apache.ivy.plugins.circular.WarnCircularDependencyStrategy;
+import org.apache.ivy.plugins.version.ExactVersionMatcher;
+import org.apache.ivy.plugins.version.LatestVersionMatcher;
 
 public class SortTest extends TestCase {
 
@@ -46,9 +48,9 @@
 
     private DefaultModuleDescriptor md4;
 
-    private static Ivy ivy;
-    
-    
+    private SortEngine sortEngine;
+
+    private SilentNonMatchingVersionReporter nonMatchReporter;
 
     /*
      * (non-Javadoc)
@@ -63,8 +65,11 @@
         md3 = createModuleDescriptorToSort("md3", "rev3");
         md4 = createModuleDescriptorToSort("md4", "rev4");
 
-        ivy = new Ivy();
-        ivy.configureDefault();
+        sortEngine = new SortEngine();
+        sortEngine.setCircularDependencyStrategy(WarnCircularDependencyStrategy.getInstance());
+        sortEngine.setVersionMatcher(new ExactVersionMatcher());
+        
+        nonMatchReporter = new SilentNonMatchingVersionReporter();
     }
 
     public void testSort() throws Exception {
@@ -78,7 +83,7 @@
         Collection permutations = getAllLists(md1, md3, md2, md4);
         for (Iterator it = permutations.iterator(); it.hasNext();) {
             List toSort = (List) it.next();
-            assertSorted(expectedOrder, ivy.sortModuleDescriptors(toSort));
+            assertSorted(expectedOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
         }
     }
 
@@ -100,7 +105,7 @@
         Collection permutations = getAllLists(md1, md3, md2, md4);
         for (Iterator it = permutations.iterator(); it.hasNext();) {
             List toSort = (List) it.next();
-            assertSorted(possibleOrder, ivy.sortModuleDescriptors(toSort));
+            assertSorted(possibleOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
         }
     }
 
@@ -117,7 +122,7 @@
         Collection permutations = getAllLists(md1, md3, md2, md4);
         for (Iterator it = permutations.iterator(); it.hasNext();) {
             List toSort = (List) it.next();
-            assertSorted(possibleOrder, ivy.sortModuleDescriptors(toSort));
+            assertSorted(possibleOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
         }
     }
 
@@ -160,10 +165,10 @@
             }
         }
         CircularDependencyReporterMock circularDepReportMock = new CircularDependencyReporterMock();
-        ivy.getSettings().setCircularDependencyStrategy(circularDepReportMock);
+        sortEngine.setCircularDependencyStrategy(circularDepReportMock);
 
         List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3, md2, md1});
-        ivy.sortModuleDescriptors(toSort);
+        sortEngine.sortModuleDescriptors(toSort, nonMatchReporter);
 
         circularDepReportMock.validate();
     }
@@ -178,13 +183,15 @@
         addDependency(md3, "md2", "latest.integration");
         addDependency(md4, "md3", "latest.integration");
 
+        sortEngine.setVersionMatcher(new LatestVersionMatcher());
+        
         DefaultModuleDescriptor[][] expectedOrder = new DefaultModuleDescriptor[][] {{md1, md2,
                 md3, md4}};
 
         Collection permutations = getAllLists(md1, md3, md2, md4);
         for (Iterator it = permutations.iterator(); it.hasNext();) {
             List toSort = (List) it.next();
-            assertSorted(expectedOrder, ivy.sortModuleDescriptors(toSort));
+            assertSorted(expectedOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
         }
 
     }
@@ -210,7 +217,7 @@
         Collection permutations = getAllLists(md1, md3, md2, md4);
         for (Iterator it = permutations.iterator(); it.hasNext();) {
             List toSort = (List) it.next();
-            assertSorted(possibleOrder, ivy.sortModuleDescriptors(toSort));
+            assertSorted(possibleOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
         }
 
     }
@@ -244,7 +251,7 @@
         NonMatchingVersionReporterMock nonMatchingVersionReporterMock = 
             new NonMatchingVersionReporterMock();
         List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3, md2, md1});
-        ivy.sortModuleDescriptors(toSort, nonMatchingVersionReporterMock);
+        sortEngine.sortModuleDescriptors(toSort, nonMatchingVersionReporterMock);
         nonMatchingVersionReporterMock.validate();
     }
 



Re: svn commit: r548695 - in /incubator/ivy/core/trunk: src/java/org/apache/ivy/Ivy.java src/java/org/apache/ivy/core/sort/SortEngine.java test/java/org/apache/ivy/core/sort/SortTest.java

Posted by Xavier Hanin <xa...@gmail.com>.
On 6/20/07, Gilles Scokart <gs...@gmail.com> wrote:
>
> I now submit the other aproach : using a dedicated settings interface
> for each settings.
>
> This has a more limited impact on the current implementation (the Ivy
> facade is not impacted).  Si I think this aproach might be better.
>
> Note that I put the SortEngineSettings in the sort package so that the
> engine is really isolated from the full settings.  However, it will
> make the settings package dependent on all engines package.  I don't
> think it is a problem,  but WDYT?


It's ok for me, I think it's cleaner like that.

Xavier

Gilles
>
> 2007/6/19, Xavier Hanin <xa...@gmail.com>:
> > On 6/19/07, Gilles Scokart <gs...@gmail.com> wrote:
> > >
> > > Please, have a look at this change (Mostly to Ivy).
> > >
> > > I did this change in order to eleminate the dependency between the
> > > SortEngine and the Settings.  Now, the Ivy class has responsability to
> > > inject the settings into the engine.
> > >
> > > This eliminate the dependency, clarify the engine, and clarify the
> > > test a little bit.  Howerver the side effect is that the setter
> > > 'setSortEngine' doesn't work like before.  Previously, when the
> > > setSortEngine method was used, the settings was not injected in the
> > > engine.  Now it is.
> > >
> > > WDYT?  Do you think the same apoach could be taken for the other
> > > engines or do you think we should roll back?
> >
> >
> > The problem with this approach IMO is that it doesn't scale very well.
> If
> > sort engine requires a lot of settings, setting properties one by one
> will
> > be cumbersome. A solution in between would be to define a Settings
> interface
> > for each specific settings. Then we could keep only class implementing
> all
> > interfaces, or have one central setting giving access to each
> > implementation, or rely on a mock object for tests, ... That's the
> approach
> > used by Wicket:
> >
> https://svn.apache.org/repos/asf/incubator/wicket/trunk/jdk-1.4/wicket/src/main/java/org/apache/wicket/settings/Settings.java
> >
> > Another point concerning your change: you set the properties in the
> getter
> > which is public, meaning that the setters are called each time get is
> > called, which is too much IMO.
> >
> > Xavier
> >
> > Gilles
> > >
> > > 2007/6/19, gscokart@apache.org <gs...@apache.org>:
> > > > Author: gscokart
> > > > Date: Tue Jun 19 04:32:43 2007
> > > > New Revision: 548695
> > > >
> > > > URL: http://svn.apache.org/viewvc?view=rev&rev=548695
> > > > Log:
> > > > refactor: remove dependency between sort engine and IvySettings
> > > >
> > > > Modified:
> > > >     incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> > > >
> > >
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > > >
> > >
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > > >
> > > > Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> > > > URL:
> > >
> http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java?view=diff&rev=548695&r1=548694&r2=548695
> > > >
> > >
> ==============================================================================
> > > > --- incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> (original)
> > > > +++ incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java Tue
> Jun 19
> > > 04:32:43 2007
> > > > @@ -159,7 +159,8 @@
> > > >              eventManager = new EventManager();
> > > >          }
> > > >          if (sortEngine == null) {
> > > > -            sortEngine = new SortEngine(settings);
> > > > +            sortEngine = new SortEngine();
> > > > +            //Settings element are injected in the getSortEngine
> > > method.
> > > >          }
> > > >          if (searchEngine == null) {
> > > >              searchEngine = new SearchEngine(settings);
> > > > @@ -329,7 +330,7 @@
> > > >       * Sorts the collection of IvyNode from the less dependent to
> the
> > > more dependent
> > > >       */
> > > >      public List sortNodes(Collection nodes) {
> > > > -        return sortEngine.sortNodes(nodes);
> > > > +        return getSortEngine().sortNodes(nodes);
> > > >      }
> > > >
> > > >      /**
> > > > @@ -344,15 +345,10 @@
> > > >       *            revision of an other modules present in the of
> > > modules to sort with a different
> > > >       *            revision.
> > > >       * @return a List of sorted ModuleDescriptors
> > > > -     * @deprecated Use
> > > sortModuleDescriptors(Collection,NonMatchingVersionReporter)
> > > >       */
> > > > -    public List sortModuleDescriptors(Collection moduleDescriptors)
> {
> > > > -        return sortEngine.sortModuleDescriptors(moduleDescriptors,
> new
> > > SilentNonMatchingVersionReporter());
> > > > -    }
> > > > -
> > > >      public List sortModuleDescriptors(Collection moduleDescriptors,
> > > >              NonMatchingVersionReporter nonMatchingVersionReporter)
> {
> > > > -        return sortEngine.sortModuleDescriptors(moduleDescriptors,
> > > nonMatchingVersionReporter);
> > > > +        return
> getSortEngine().sortModuleDescriptors(moduleDescriptors,
> > > nonMatchingVersionReporter);
> > > >      }
> > > >
> > > >      //
> > >
> ///////////////////////////////////////////////////////////////////////
> > > > @@ -569,6 +565,8 @@
> > > >      }
> > > >
> > > >      public SortEngine getSortEngine() {
> > > > +        sortEngine.setCircularDependencyStrategy(
> > > settings.getCircularDependencyStrategy());
> > > > +        sortEngine.setVersionMatcher(settings.getVersionMatcher());
> > > >          return sortEngine;
> > > >      }
> > > >
> > > >
> > > > Modified:
> > >
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > > > URL:
> > >
> http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java?view=diff&rev=548695&r1=548694&r2=548695
> > > >
> > >
> ==============================================================================
> > > > ---
> > >
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > > (original)
> > > > +++
> > >
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > > Tue Jun 19 04:32:43 2007
> > > > @@ -26,18 +26,29 @@
> > > >
> > > >  import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
> > > >  import org.apache.ivy.core.resolve.IvyNode;
> > > > -import org.apache.ivy.core.settings.IvySettings;
> > > >  import org.apache.ivy.plugins.circular.CircularDependencyException;
> > > >  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> > > >  import org.apache.ivy.plugins.version.VersionMatcher;
> > > >
> > > >  public class SortEngine {
> > > > -    private IvySettings settings;
> > > >
> > > > -    public SortEngine(IvySettings settings) {
> > > > -        this.settings = settings;
> > > > +    private CircularDependencyStrategy circularStrategy;
> > > > +
> > > > +    private VersionMatcher versionMatcher;
> > > > +
> > > > +    public SortEngine() {
> > > > +    }
> > > > +
> > > > +
> > > > +    public void
> > > setCircularDependencyStrategy(CircularDependencyStrategy
> circularStrategy) {
> > > > +        this.circularStrategy = circularStrategy;
> > > >      }
> > > >
> > > > +    public void setVersionMatcher(VersionMatcher versionMatcher) {
> > > > +        this.versionMatcher = versionMatcher;
> > > > +    }
> > > > +
> > > > +
> > > >      public List sortNodes(Collection nodes) throws
> > > CircularDependencyException {
> > > >          /*
> > > >           * here we want to use the sort algorithm which work on
> module
> > > descriptors : so we first put
> > > > @@ -92,10 +103,8 @@
> > > >      public List sortModuleDescriptors(Collection moduleDescriptors,
> > > >              NonMatchingVersionReporter nonMatchingVersionReporter)
> > > >              throws CircularDependencyException {
> > > > -        VersionMatcher versionMatcher = settings.getVersionMatcher
> ();
> > > > -        CircularDependencyStrategy circularDepStrategy =
> > > settings.getCircularDependencyStrategy();
> > > >          ModuleDescriptorSorter sorter = new
> > > ModuleDescriptorSorter(moduleDescriptors,
> > > > -                versionMatcher, nonMatchingVersionReporter,
> > > circularDepStrategy);
> > > > +                versionMatcher, nonMatchingVersionReporter,
> > > circularStrategy);
> > > >          return sorter.sortModuleDescriptors();
> > > >      }
> > > >
> > > >
> > > > Modified:
> > >
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > > > URL:
> > >
> http://svn.apache.org/viewvc/incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java?view=diff&rev=548695&r1=548694&r2=548695
> > > >
> > >
> ==============================================================================
> > > > ---
> > >
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > > (original)
> > > > +++
> > >
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > > Tue Jun 19 04:32:43 2007
> > > > @@ -27,7 +27,6 @@
> > > >  import junit.framework.Assert;
> > > >  import junit.framework.TestCase;
> > > >
> > > > -import org.apache.ivy.Ivy;
> > > >  import
> > > org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor;
> > > >  import
> org.apache.ivy.core.module.descriptor.DefaultModuleDescriptor;
> > > >  import org.apache.ivy.core.module.descriptor.DependencyDescriptor;
> > > > @@ -35,6 +34,9 @@
> > > >  import org.apache.ivy.core.module.id.ModuleRevisionId;
> > > >  import org.apache.ivy.plugins.circular.CircularDependencyHelper;
> > > >  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> > > > +import
> org.apache.ivy.plugins.circular.WarnCircularDependencyStrategy;
> > > > +import org.apache.ivy.plugins.version.ExactVersionMatcher;
> > > > +import org.apache.ivy.plugins.version.LatestVersionMatcher;
> > > >
> > > >  public class SortTest extends TestCase {
> > > >
> > > > @@ -46,9 +48,9 @@
> > > >
> > > >      private DefaultModuleDescriptor md4;
> > > >
> > > > -    private static Ivy ivy;
> > > > -
> > > > -
> > > > +    private SortEngine sortEngine;
> > > > +
> > > > +    private SilentNonMatchingVersionReporter nonMatchReporter;
> > > >
> > > >      /*
> > > >       * (non-Javadoc)
> > > > @@ -63,8 +65,11 @@
> > > >          md3 = createModuleDescriptorToSort("md3", "rev3");
> > > >          md4 = createModuleDescriptorToSort("md4", "rev4");
> > > >
> > > > -        ivy = new Ivy();
> > > > -        ivy.configureDefault();
> > > > +        sortEngine = new SortEngine();
> > > > +        sortEngine.setCircularDependencyStrategy(
> > > WarnCircularDependencyStrategy.getInstance());
> > > > +        sortEngine.setVersionMatcher(new ExactVersionMatcher());
> > > > +
> > > > +        nonMatchReporter = new SilentNonMatchingVersionReporter();
> > > >      }
> > > >
> > > >      public void testSort() throws Exception {
> > > > @@ -78,7 +83,7 @@
> > > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > > >          for (Iterator it = permutations.iterator(); it.hasNext();)
> {
> > > >              List toSort = (List) it.next();
> > > > -            assertSorted(expectedOrder, ivy.sortModuleDescriptors
> > > (toSort));
> > > > +            assertSorted(expectedOrder,
> > > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > > >          }
> > > >      }
> > > >
> > > > @@ -100,7 +105,7 @@
> > > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > > >          for (Iterator it = permutations.iterator(); it.hasNext();)
> {
> > > >              List toSort = (List) it.next();
> > > > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> > > (toSort));
> > > > +            assertSorted(possibleOrder,
> > > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > > >          }
> > > >      }
> > > >
> > > > @@ -117,7 +122,7 @@
> > > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > > >          for (Iterator it = permutations.iterator(); it.hasNext();)
> {
> > > >              List toSort = (List) it.next();
> > > > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> > > (toSort));
> > > > +            assertSorted(possibleOrder,
> > > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > > >          }
> > > >      }
> > > >
> > > > @@ -160,10 +165,10 @@
> > > >              }
> > > >          }
> > > >          CircularDependencyReporterMock circularDepReportMock = new
> > > CircularDependencyReporterMock();
> > > > -        ivy.getSettings
> > > ().setCircularDependencyStrategy(circularDepReportMock);
> > > > +        sortEngine.setCircularDependencyStrategy
> > > (circularDepReportMock);
> > > >
> > > >          List toSort = Arrays.asList(new ModuleDescriptor[] {md4,
> md3,
> > > md2, md1});
> > > > -        ivy.sortModuleDescriptors(toSort);
> > > > +        sortEngine.sortModuleDescriptors(toSort, nonMatchReporter);
> > > >
> > > >          circularDepReportMock.validate();
> > > >      }
> > > > @@ -178,13 +183,15 @@
> > > >          addDependency(md3, "md2", "latest.integration");
> > > >          addDependency(md4, "md3", "latest.integration");
> > > >
> > > > +        sortEngine.setVersionMatcher(new LatestVersionMatcher());
> > > > +
> > > >          DefaultModuleDescriptor[][] expectedOrder = new
> > > DefaultModuleDescriptor[][] {{md1, md2,
> > > >                  md3, md4}};
> > > >
> > > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > > >          for (Iterator it = permutations.iterator(); it.hasNext();)
> {
> > > >              List toSort = (List) it.next();
> > > > -            assertSorted(expectedOrder, ivy.sortModuleDescriptors
> > > (toSort));
> > > > +            assertSorted(expectedOrder,
> > > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > > >          }
> > > >
> > > >      }
> > > > @@ -210,7 +217,7 @@
> > > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > > >          for (Iterator it = permutations.iterator(); it.hasNext();)
> {
> > > >              List toSort = (List) it.next();
> > > > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> > > (toSort));
> > > > +            assertSorted(possibleOrder,
> > > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > > >          }
> > > >
> > > >      }
> > > > @@ -244,7 +251,7 @@
> > > >          NonMatchingVersionReporterMock
> nonMatchingVersionReporterMock =
> > > >              new NonMatchingVersionReporterMock();
> > > >          List toSort = Arrays.asList(new ModuleDescriptor[] {md4,
> md3,
> > > md2, md1});
> > > > -        ivy.sortModuleDescriptors(toSort,
> > > nonMatchingVersionReporterMock);
> > > > +        sortEngine.sortModuleDescriptors(toSort,
> > > nonMatchingVersionReporterMock);
> > > >          nonMatchingVersionReporterMock.validate();
> > > >      }
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > Gilles SCOKART
> > >
> >
> >
> >
> > --
> > Xavier Hanin - Independent Java Consultant
> > Manage your dependencies with Ivy!
> > http://incubator.apache.org/ivy/
> >
>
>
> --
> Gilles SCOKART
>



-- 
Xavier Hanin - Independent Java Consultant
Manage your dependencies with Ivy!
http://incubator.apache.org/ivy/

Re: svn commit: r548695 - in /incubator/ivy/core/trunk: src/java/org/apache/ivy/Ivy.java src/java/org/apache/ivy/core/sort/SortEngine.java test/java/org/apache/ivy/core/sort/SortTest.java

Posted by Gilles Scokart <gs...@gmail.com>.
I now submit the other aproach : using a dedicated settings interface
for each settings.

This has a more limited impact on the current implementation (the Ivy
facade is not impacted).  Si I think this aproach might be better.

Note that I put the SortEngineSettings in the sort package so that the
engine is really isolated from the full settings.  However, it will
make the settings package dependent on all engines package.  I don't
think it is a problem,  but WDYT?

Gilles

2007/6/19, Xavier Hanin <xa...@gmail.com>:
> On 6/19/07, Gilles Scokart <gs...@gmail.com> wrote:
> >
> > Please, have a look at this change (Mostly to Ivy).
> >
> > I did this change in order to eleminate the dependency between the
> > SortEngine and the Settings.  Now, the Ivy class has responsability to
> > inject the settings into the engine.
> >
> > This eliminate the dependency, clarify the engine, and clarify the
> > test a little bit.  Howerver the side effect is that the setter
> > 'setSortEngine' doesn't work like before.  Previously, when the
> > setSortEngine method was used, the settings was not injected in the
> > engine.  Now it is.
> >
> > WDYT?  Do you think the same apoach could be taken for the other
> > engines or do you think we should roll back?
>
>
> The problem with this approach IMO is that it doesn't scale very well. If
> sort engine requires a lot of settings, setting properties one by one will
> be cumbersome. A solution in between would be to define a Settings interface
> for each specific settings. Then we could keep only class implementing all
> interfaces, or have one central setting giving access to each
> implementation, or rely on a mock object for tests, ... That's the approach
> used by Wicket:
> https://svn.apache.org/repos/asf/incubator/wicket/trunk/jdk-1.4/wicket/src/main/java/org/apache/wicket/settings/Settings.java
>
> Another point concerning your change: you set the properties in the getter
> which is public, meaning that the setters are called each time get is
> called, which is too much IMO.
>
> Xavier
>
> Gilles
> >
> > 2007/6/19, gscokart@apache.org <gs...@apache.org>:
> > > Author: gscokart
> > > Date: Tue Jun 19 04:32:43 2007
> > > New Revision: 548695
> > >
> > > URL: http://svn.apache.org/viewvc?view=rev&rev=548695
> > > Log:
> > > refactor: remove dependency between sort engine and IvySettings
> > >
> > > Modified:
> > >     incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> > >
> > incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > >
> > incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > >
> > > Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> > > URL:
> > http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java?view=diff&rev=548695&r1=548694&r2=548695
> > >
> > ==============================================================================
> > > --- incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java (original)
> > > +++ incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java Tue Jun 19
> > 04:32:43 2007
> > > @@ -159,7 +159,8 @@
> > >              eventManager = new EventManager();
> > >          }
> > >          if (sortEngine == null) {
> > > -            sortEngine = new SortEngine(settings);
> > > +            sortEngine = new SortEngine();
> > > +            //Settings element are injected in the getSortEngine
> > method.
> > >          }
> > >          if (searchEngine == null) {
> > >              searchEngine = new SearchEngine(settings);
> > > @@ -329,7 +330,7 @@
> > >       * Sorts the collection of IvyNode from the less dependent to the
> > more dependent
> > >       */
> > >      public List sortNodes(Collection nodes) {
> > > -        return sortEngine.sortNodes(nodes);
> > > +        return getSortEngine().sortNodes(nodes);
> > >      }
> > >
> > >      /**
> > > @@ -344,15 +345,10 @@
> > >       *            revision of an other modules present in the of
> > modules to sort with a different
> > >       *            revision.
> > >       * @return a List of sorted ModuleDescriptors
> > > -     * @deprecated Use
> > sortModuleDescriptors(Collection,NonMatchingVersionReporter)
> > >       */
> > > -    public List sortModuleDescriptors(Collection moduleDescriptors) {
> > > -        return sortEngine.sortModuleDescriptors(moduleDescriptors, new
> > SilentNonMatchingVersionReporter());
> > > -    }
> > > -
> > >      public List sortModuleDescriptors(Collection moduleDescriptors,
> > >              NonMatchingVersionReporter nonMatchingVersionReporter) {
> > > -        return sortEngine.sortModuleDescriptors(moduleDescriptors,
> > nonMatchingVersionReporter);
> > > +        return getSortEngine().sortModuleDescriptors(moduleDescriptors,
> > nonMatchingVersionReporter);
> > >      }
> > >
> > >      //
> > ///////////////////////////////////////////////////////////////////////
> > > @@ -569,6 +565,8 @@
> > >      }
> > >
> > >      public SortEngine getSortEngine() {
> > > +        sortEngine.setCircularDependencyStrategy(
> > settings.getCircularDependencyStrategy());
> > > +        sortEngine.setVersionMatcher(settings.getVersionMatcher());
> > >          return sortEngine;
> > >      }
> > >
> > >
> > > Modified:
> > incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > > URL:
> > http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java?view=diff&rev=548695&r1=548694&r2=548695
> > >
> > ==============================================================================
> > > ---
> > incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > (original)
> > > +++
> > incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > Tue Jun 19 04:32:43 2007
> > > @@ -26,18 +26,29 @@
> > >
> > >  import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
> > >  import org.apache.ivy.core.resolve.IvyNode;
> > > -import org.apache.ivy.core.settings.IvySettings;
> > >  import org.apache.ivy.plugins.circular.CircularDependencyException;
> > >  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> > >  import org.apache.ivy.plugins.version.VersionMatcher;
> > >
> > >  public class SortEngine {
> > > -    private IvySettings settings;
> > >
> > > -    public SortEngine(IvySettings settings) {
> > > -        this.settings = settings;
> > > +    private CircularDependencyStrategy circularStrategy;
> > > +
> > > +    private VersionMatcher versionMatcher;
> > > +
> > > +    public SortEngine() {
> > > +    }
> > > +
> > > +
> > > +    public void
> > setCircularDependencyStrategy(CircularDependencyStrategy circularStrategy) {
> > > +        this.circularStrategy = circularStrategy;
> > >      }
> > >
> > > +    public void setVersionMatcher(VersionMatcher versionMatcher) {
> > > +        this.versionMatcher = versionMatcher;
> > > +    }
> > > +
> > > +
> > >      public List sortNodes(Collection nodes) throws
> > CircularDependencyException {
> > >          /*
> > >           * here we want to use the sort algorithm which work on module
> > descriptors : so we first put
> > > @@ -92,10 +103,8 @@
> > >      public List sortModuleDescriptors(Collection moduleDescriptors,
> > >              NonMatchingVersionReporter nonMatchingVersionReporter)
> > >              throws CircularDependencyException {
> > > -        VersionMatcher versionMatcher = settings.getVersionMatcher();
> > > -        CircularDependencyStrategy circularDepStrategy =
> > settings.getCircularDependencyStrategy();
> > >          ModuleDescriptorSorter sorter = new
> > ModuleDescriptorSorter(moduleDescriptors,
> > > -                versionMatcher, nonMatchingVersionReporter,
> > circularDepStrategy);
> > > +                versionMatcher, nonMatchingVersionReporter,
> > circularStrategy);
> > >          return sorter.sortModuleDescriptors();
> > >      }
> > >
> > >
> > > Modified:
> > incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > > URL:
> > http://svn.apache.org/viewvc/incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java?view=diff&rev=548695&r1=548694&r2=548695
> > >
> > ==============================================================================
> > > ---
> > incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > (original)
> > > +++
> > incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > Tue Jun 19 04:32:43 2007
> > > @@ -27,7 +27,6 @@
> > >  import junit.framework.Assert;
> > >  import junit.framework.TestCase;
> > >
> > > -import org.apache.ivy.Ivy;
> > >  import
> > org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor;
> > >  import org.apache.ivy.core.module.descriptor.DefaultModuleDescriptor;
> > >  import org.apache.ivy.core.module.descriptor.DependencyDescriptor;
> > > @@ -35,6 +34,9 @@
> > >  import org.apache.ivy.core.module.id.ModuleRevisionId;
> > >  import org.apache.ivy.plugins.circular.CircularDependencyHelper;
> > >  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> > > +import org.apache.ivy.plugins.circular.WarnCircularDependencyStrategy;
> > > +import org.apache.ivy.plugins.version.ExactVersionMatcher;
> > > +import org.apache.ivy.plugins.version.LatestVersionMatcher;
> > >
> > >  public class SortTest extends TestCase {
> > >
> > > @@ -46,9 +48,9 @@
> > >
> > >      private DefaultModuleDescriptor md4;
> > >
> > > -    private static Ivy ivy;
> > > -
> > > -
> > > +    private SortEngine sortEngine;
> > > +
> > > +    private SilentNonMatchingVersionReporter nonMatchReporter;
> > >
> > >      /*
> > >       * (non-Javadoc)
> > > @@ -63,8 +65,11 @@
> > >          md3 = createModuleDescriptorToSort("md3", "rev3");
> > >          md4 = createModuleDescriptorToSort("md4", "rev4");
> > >
> > > -        ivy = new Ivy();
> > > -        ivy.configureDefault();
> > > +        sortEngine = new SortEngine();
> > > +        sortEngine.setCircularDependencyStrategy(
> > WarnCircularDependencyStrategy.getInstance());
> > > +        sortEngine.setVersionMatcher(new ExactVersionMatcher());
> > > +
> > > +        nonMatchReporter = new SilentNonMatchingVersionReporter();
> > >      }
> > >
> > >      public void testSort() throws Exception {
> > > @@ -78,7 +83,7 @@
> > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> > >              List toSort = (List) it.next();
> > > -            assertSorted(expectedOrder, ivy.sortModuleDescriptors
> > (toSort));
> > > +            assertSorted(expectedOrder,
> > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > >          }
> > >      }
> > >
> > > @@ -100,7 +105,7 @@
> > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> > >              List toSort = (List) it.next();
> > > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> > (toSort));
> > > +            assertSorted(possibleOrder,
> > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > >          }
> > >      }
> > >
> > > @@ -117,7 +122,7 @@
> > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> > >              List toSort = (List) it.next();
> > > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> > (toSort));
> > > +            assertSorted(possibleOrder,
> > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > >          }
> > >      }
> > >
> > > @@ -160,10 +165,10 @@
> > >              }
> > >          }
> > >          CircularDependencyReporterMock circularDepReportMock = new
> > CircularDependencyReporterMock();
> > > -        ivy.getSettings
> > ().setCircularDependencyStrategy(circularDepReportMock);
> > > +        sortEngine.setCircularDependencyStrategy
> > (circularDepReportMock);
> > >
> > >          List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3,
> > md2, md1});
> > > -        ivy.sortModuleDescriptors(toSort);
> > > +        sortEngine.sortModuleDescriptors(toSort, nonMatchReporter);
> > >
> > >          circularDepReportMock.validate();
> > >      }
> > > @@ -178,13 +183,15 @@
> > >          addDependency(md3, "md2", "latest.integration");
> > >          addDependency(md4, "md3", "latest.integration");
> > >
> > > +        sortEngine.setVersionMatcher(new LatestVersionMatcher());
> > > +
> > >          DefaultModuleDescriptor[][] expectedOrder = new
> > DefaultModuleDescriptor[][] {{md1, md2,
> > >                  md3, md4}};
> > >
> > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> > >              List toSort = (List) it.next();
> > > -            assertSorted(expectedOrder, ivy.sortModuleDescriptors
> > (toSort));
> > > +            assertSorted(expectedOrder,
> > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > >          }
> > >
> > >      }
> > > @@ -210,7 +217,7 @@
> > >          Collection permutations = getAllLists(md1, md3, md2, md4);
> > >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> > >              List toSort = (List) it.next();
> > > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> > (toSort));
> > > +            assertSorted(possibleOrder,
> > sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > >          }
> > >
> > >      }
> > > @@ -244,7 +251,7 @@
> > >          NonMatchingVersionReporterMock nonMatchingVersionReporterMock =
> > >              new NonMatchingVersionReporterMock();
> > >          List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3,
> > md2, md1});
> > > -        ivy.sortModuleDescriptors(toSort,
> > nonMatchingVersionReporterMock);
> > > +        sortEngine.sortModuleDescriptors(toSort,
> > nonMatchingVersionReporterMock);
> > >          nonMatchingVersionReporterMock.validate();
> > >      }
> > >
> > >
> > >
> > >
> >
> >
> > --
> > Gilles SCOKART
> >
>
>
>
> --
> Xavier Hanin - Independent Java Consultant
> Manage your dependencies with Ivy!
> http://incubator.apache.org/ivy/
>


-- 
Gilles SCOKART

Re: svn commit: r548695 - in /incubator/ivy/core/trunk: src/java/org/apache/ivy/Ivy.java src/java/org/apache/ivy/core/sort/SortEngine.java test/java/org/apache/ivy/core/sort/SortTest.java

Posted by Xavier Hanin <xa...@gmail.com>.
On 6/19/07, Gilles Scokart <gs...@gmail.com> wrote:
>
> Please, have a look at this change (Mostly to Ivy).
>
> I did this change in order to eleminate the dependency between the
> SortEngine and the Settings.  Now, the Ivy class has responsability to
> inject the settings into the engine.
>
> This eliminate the dependency, clarify the engine, and clarify the
> test a little bit.  Howerver the side effect is that the setter
> 'setSortEngine' doesn't work like before.  Previously, when the
> setSortEngine method was used, the settings was not injected in the
> engine.  Now it is.
>
> WDYT?  Do you think the same apoach could be taken for the other
> engines or do you think we should roll back?


The problem with this approach IMO is that it doesn't scale very well. If
sort engine requires a lot of settings, setting properties one by one will
be cumbersome. A solution in between would be to define a Settings interface
for each specific settings. Then we could keep only class implementing all
interfaces, or have one central setting giving access to each
implementation, or rely on a mock object for tests, ... That's the approach
used by Wicket:
https://svn.apache.org/repos/asf/incubator/wicket/trunk/jdk-1.4/wicket/src/main/java/org/apache/wicket/settings/Settings.java

Another point concerning your change: you set the properties in the getter
which is public, meaning that the setters are called each time get is
called, which is too much IMO.

Xavier

Gilles
>
> 2007/6/19, gscokart@apache.org <gs...@apache.org>:
> > Author: gscokart
> > Date: Tue Jun 19 04:32:43 2007
> > New Revision: 548695
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=548695
> > Log:
> > refactor: remove dependency between sort engine and IvySettings
> >
> > Modified:
> >     incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> >
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> >
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> >
> > Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> > URL:
> http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java?view=diff&rev=548695&r1=548694&r2=548695
> >
> ==============================================================================
> > --- incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java (original)
> > +++ incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java Tue Jun 19
> 04:32:43 2007
> > @@ -159,7 +159,8 @@
> >              eventManager = new EventManager();
> >          }
> >          if (sortEngine == null) {
> > -            sortEngine = new SortEngine(settings);
> > +            sortEngine = new SortEngine();
> > +            //Settings element are injected in the getSortEngine
> method.
> >          }
> >          if (searchEngine == null) {
> >              searchEngine = new SearchEngine(settings);
> > @@ -329,7 +330,7 @@
> >       * Sorts the collection of IvyNode from the less dependent to the
> more dependent
> >       */
> >      public List sortNodes(Collection nodes) {
> > -        return sortEngine.sortNodes(nodes);
> > +        return getSortEngine().sortNodes(nodes);
> >      }
> >
> >      /**
> > @@ -344,15 +345,10 @@
> >       *            revision of an other modules present in the of
> modules to sort with a different
> >       *            revision.
> >       * @return a List of sorted ModuleDescriptors
> > -     * @deprecated Use
> sortModuleDescriptors(Collection,NonMatchingVersionReporter)
> >       */
> > -    public List sortModuleDescriptors(Collection moduleDescriptors) {
> > -        return sortEngine.sortModuleDescriptors(moduleDescriptors, new
> SilentNonMatchingVersionReporter());
> > -    }
> > -
> >      public List sortModuleDescriptors(Collection moduleDescriptors,
> >              NonMatchingVersionReporter nonMatchingVersionReporter) {
> > -        return sortEngine.sortModuleDescriptors(moduleDescriptors,
> nonMatchingVersionReporter);
> > +        return getSortEngine().sortModuleDescriptors(moduleDescriptors,
> nonMatchingVersionReporter);
> >      }
> >
> >      //
> ///////////////////////////////////////////////////////////////////////
> > @@ -569,6 +565,8 @@
> >      }
> >
> >      public SortEngine getSortEngine() {
> > +        sortEngine.setCircularDependencyStrategy(
> settings.getCircularDependencyStrategy());
> > +        sortEngine.setVersionMatcher(settings.getVersionMatcher());
> >          return sortEngine;
> >      }
> >
> >
> > Modified:
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > URL:
> http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java?view=diff&rev=548695&r1=548694&r2=548695
> >
> ==============================================================================
> > ---
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> (original)
> > +++
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> Tue Jun 19 04:32:43 2007
> > @@ -26,18 +26,29 @@
> >
> >  import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
> >  import org.apache.ivy.core.resolve.IvyNode;
> > -import org.apache.ivy.core.settings.IvySettings;
> >  import org.apache.ivy.plugins.circular.CircularDependencyException;
> >  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> >  import org.apache.ivy.plugins.version.VersionMatcher;
> >
> >  public class SortEngine {
> > -    private IvySettings settings;
> >
> > -    public SortEngine(IvySettings settings) {
> > -        this.settings = settings;
> > +    private CircularDependencyStrategy circularStrategy;
> > +
> > +    private VersionMatcher versionMatcher;
> > +
> > +    public SortEngine() {
> > +    }
> > +
> > +
> > +    public void
> setCircularDependencyStrategy(CircularDependencyStrategy circularStrategy) {
> > +        this.circularStrategy = circularStrategy;
> >      }
> >
> > +    public void setVersionMatcher(VersionMatcher versionMatcher) {
> > +        this.versionMatcher = versionMatcher;
> > +    }
> > +
> > +
> >      public List sortNodes(Collection nodes) throws
> CircularDependencyException {
> >          /*
> >           * here we want to use the sort algorithm which work on module
> descriptors : so we first put
> > @@ -92,10 +103,8 @@
> >      public List sortModuleDescriptors(Collection moduleDescriptors,
> >              NonMatchingVersionReporter nonMatchingVersionReporter)
> >              throws CircularDependencyException {
> > -        VersionMatcher versionMatcher = settings.getVersionMatcher();
> > -        CircularDependencyStrategy circularDepStrategy =
> settings.getCircularDependencyStrategy();
> >          ModuleDescriptorSorter sorter = new
> ModuleDescriptorSorter(moduleDescriptors,
> > -                versionMatcher, nonMatchingVersionReporter,
> circularDepStrategy);
> > +                versionMatcher, nonMatchingVersionReporter,
> circularStrategy);
> >          return sorter.sortModuleDescriptors();
> >      }
> >
> >
> > Modified:
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > URL:
> http://svn.apache.org/viewvc/incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java?view=diff&rev=548695&r1=548694&r2=548695
> >
> ==============================================================================
> > ---
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> (original)
> > +++
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> Tue Jun 19 04:32:43 2007
> > @@ -27,7 +27,6 @@
> >  import junit.framework.Assert;
> >  import junit.framework.TestCase;
> >
> > -import org.apache.ivy.Ivy;
> >  import
> org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor;
> >  import org.apache.ivy.core.module.descriptor.DefaultModuleDescriptor;
> >  import org.apache.ivy.core.module.descriptor.DependencyDescriptor;
> > @@ -35,6 +34,9 @@
> >  import org.apache.ivy.core.module.id.ModuleRevisionId;
> >  import org.apache.ivy.plugins.circular.CircularDependencyHelper;
> >  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> > +import org.apache.ivy.plugins.circular.WarnCircularDependencyStrategy;
> > +import org.apache.ivy.plugins.version.ExactVersionMatcher;
> > +import org.apache.ivy.plugins.version.LatestVersionMatcher;
> >
> >  public class SortTest extends TestCase {
> >
> > @@ -46,9 +48,9 @@
> >
> >      private DefaultModuleDescriptor md4;
> >
> > -    private static Ivy ivy;
> > -
> > -
> > +    private SortEngine sortEngine;
> > +
> > +    private SilentNonMatchingVersionReporter nonMatchReporter;
> >
> >      /*
> >       * (non-Javadoc)
> > @@ -63,8 +65,11 @@
> >          md3 = createModuleDescriptorToSort("md3", "rev3");
> >          md4 = createModuleDescriptorToSort("md4", "rev4");
> >
> > -        ivy = new Ivy();
> > -        ivy.configureDefault();
> > +        sortEngine = new SortEngine();
> > +        sortEngine.setCircularDependencyStrategy(
> WarnCircularDependencyStrategy.getInstance());
> > +        sortEngine.setVersionMatcher(new ExactVersionMatcher());
> > +
> > +        nonMatchReporter = new SilentNonMatchingVersionReporter();
> >      }
> >
> >      public void testSort() throws Exception {
> > @@ -78,7 +83,7 @@
> >          Collection permutations = getAllLists(md1, md3, md2, md4);
> >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> >              List toSort = (List) it.next();
> > -            assertSorted(expectedOrder, ivy.sortModuleDescriptors
> (toSort));
> > +            assertSorted(expectedOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> >          }
> >      }
> >
> > @@ -100,7 +105,7 @@
> >          Collection permutations = getAllLists(md1, md3, md2, md4);
> >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> >              List toSort = (List) it.next();
> > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> (toSort));
> > +            assertSorted(possibleOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> >          }
> >      }
> >
> > @@ -117,7 +122,7 @@
> >          Collection permutations = getAllLists(md1, md3, md2, md4);
> >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> >              List toSort = (List) it.next();
> > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> (toSort));
> > +            assertSorted(possibleOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> >          }
> >      }
> >
> > @@ -160,10 +165,10 @@
> >              }
> >          }
> >          CircularDependencyReporterMock circularDepReportMock = new
> CircularDependencyReporterMock();
> > -        ivy.getSettings
> ().setCircularDependencyStrategy(circularDepReportMock);
> > +        sortEngine.setCircularDependencyStrategy
> (circularDepReportMock);
> >
> >          List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3,
> md2, md1});
> > -        ivy.sortModuleDescriptors(toSort);
> > +        sortEngine.sortModuleDescriptors(toSort, nonMatchReporter);
> >
> >          circularDepReportMock.validate();
> >      }
> > @@ -178,13 +183,15 @@
> >          addDependency(md3, "md2", "latest.integration");
> >          addDependency(md4, "md3", "latest.integration");
> >
> > +        sortEngine.setVersionMatcher(new LatestVersionMatcher());
> > +
> >          DefaultModuleDescriptor[][] expectedOrder = new
> DefaultModuleDescriptor[][] {{md1, md2,
> >                  md3, md4}};
> >
> >          Collection permutations = getAllLists(md1, md3, md2, md4);
> >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> >              List toSort = (List) it.next();
> > -            assertSorted(expectedOrder, ivy.sortModuleDescriptors
> (toSort));
> > +            assertSorted(expectedOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> >          }
> >
> >      }
> > @@ -210,7 +217,7 @@
> >          Collection permutations = getAllLists(md1, md3, md2, md4);
> >          for (Iterator it = permutations.iterator(); it.hasNext();) {
> >              List toSort = (List) it.next();
> > -            assertSorted(possibleOrder, ivy.sortModuleDescriptors
> (toSort));
> > +            assertSorted(possibleOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> >          }
> >
> >      }
> > @@ -244,7 +251,7 @@
> >          NonMatchingVersionReporterMock nonMatchingVersionReporterMock =
> >              new NonMatchingVersionReporterMock();
> >          List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3,
> md2, md1});
> > -        ivy.sortModuleDescriptors(toSort,
> nonMatchingVersionReporterMock);
> > +        sortEngine.sortModuleDescriptors(toSort,
> nonMatchingVersionReporterMock);
> >          nonMatchingVersionReporterMock.validate();
> >      }
> >
> >
> >
> >
>
>
> --
> Gilles SCOKART
>



-- 
Xavier Hanin - Independent Java Consultant
Manage your dependencies with Ivy!
http://incubator.apache.org/ivy/

Re: svn commit: r548695 - in /incubator/ivy/core/trunk: src/java/org/apache/ivy/Ivy.java src/java/org/apache/ivy/core/sort/SortEngine.java test/java/org/apache/ivy/core/sort/SortTest.java

Posted by Gilles Scokart <gs...@gmail.com>.
Please, have a look at this change (Mostly to Ivy).

I did this change in order to eleminate the dependency between the
SortEngine and the Settings.  Now, the Ivy class has responsability to
inject the settings into the engine.

This eliminate the dependency, clarify the engine, and clarify the
test a little bit.  Howerver the side effect is that the setter
'setSortEngine' doesn't work like before.  Previously, when the
setSortEngine method was used, the settings was not injected in the
engine.  Now it is.

WDYT?  Do you think the same apoach could be taken for the other
engines or do you think we should roll back?

Gilles

2007/6/19, gscokart@apache.org <gs...@apache.org>:
> Author: gscokart
> Date: Tue Jun 19 04:32:43 2007
> New Revision: 548695
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=548695
> Log:
> refactor: remove dependency between sort engine and IvySettings
>
> Modified:
>     incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
>     incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
>     incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
>
> Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> URL: http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java?view=diff&rev=548695&r1=548694&r2=548695
> ==============================================================================
> --- incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java (original)
> +++ incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java Tue Jun 19 04:32:43 2007
> @@ -159,7 +159,8 @@
>              eventManager = new EventManager();
>          }
>          if (sortEngine == null) {
> -            sortEngine = new SortEngine(settings);
> +            sortEngine = new SortEngine();
> +            //Settings element are injected in the getSortEngine method.
>          }
>          if (searchEngine == null) {
>              searchEngine = new SearchEngine(settings);
> @@ -329,7 +330,7 @@
>       * Sorts the collection of IvyNode from the less dependent to the more dependent
>       */
>      public List sortNodes(Collection nodes) {
> -        return sortEngine.sortNodes(nodes);
> +        return getSortEngine().sortNodes(nodes);
>      }
>
>      /**
> @@ -344,15 +345,10 @@
>       *            revision of an other modules present in the of modules to sort with a different
>       *            revision.
>       * @return a List of sorted ModuleDescriptors
> -     * @deprecated Use sortModuleDescriptors(Collection,NonMatchingVersionReporter)
>       */
> -    public List sortModuleDescriptors(Collection moduleDescriptors) {
> -        return sortEngine.sortModuleDescriptors(moduleDescriptors, new SilentNonMatchingVersionReporter());
> -    }
> -
>      public List sortModuleDescriptors(Collection moduleDescriptors,
>              NonMatchingVersionReporter nonMatchingVersionReporter) {
> -        return sortEngine.sortModuleDescriptors(moduleDescriptors, nonMatchingVersionReporter);
> +        return getSortEngine().sortModuleDescriptors(moduleDescriptors, nonMatchingVersionReporter);
>      }
>
>      // ///////////////////////////////////////////////////////////////////////
> @@ -569,6 +565,8 @@
>      }
>
>      public SortEngine getSortEngine() {
> +        sortEngine.setCircularDependencyStrategy(settings.getCircularDependencyStrategy());
> +        sortEngine.setVersionMatcher(settings.getVersionMatcher());
>          return sortEngine;
>      }
>
>
> Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> URL: http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java?view=diff&rev=548695&r1=548694&r2=548695
> ==============================================================================
> --- incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java (original)
> +++ incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java Tue Jun 19 04:32:43 2007
> @@ -26,18 +26,29 @@
>
>  import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
>  import org.apache.ivy.core.resolve.IvyNode;
> -import org.apache.ivy.core.settings.IvySettings;
>  import org.apache.ivy.plugins.circular.CircularDependencyException;
>  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
>  import org.apache.ivy.plugins.version.VersionMatcher;
>
>  public class SortEngine {
> -    private IvySettings settings;
>
> -    public SortEngine(IvySettings settings) {
> -        this.settings = settings;
> +    private CircularDependencyStrategy circularStrategy;
> +
> +    private VersionMatcher versionMatcher;
> +
> +    public SortEngine() {
> +    }
> +
> +
> +    public void setCircularDependencyStrategy(CircularDependencyStrategy circularStrategy) {
> +        this.circularStrategy = circularStrategy;
>      }
>
> +    public void setVersionMatcher(VersionMatcher versionMatcher) {
> +        this.versionMatcher = versionMatcher;
> +    }
> +
> +
>      public List sortNodes(Collection nodes) throws CircularDependencyException {
>          /*
>           * here we want to use the sort algorithm which work on module descriptors : so we first put
> @@ -92,10 +103,8 @@
>      public List sortModuleDescriptors(Collection moduleDescriptors,
>              NonMatchingVersionReporter nonMatchingVersionReporter)
>              throws CircularDependencyException {
> -        VersionMatcher versionMatcher = settings.getVersionMatcher();
> -        CircularDependencyStrategy circularDepStrategy = settings.getCircularDependencyStrategy();
>          ModuleDescriptorSorter sorter = new ModuleDescriptorSorter(moduleDescriptors,
> -                versionMatcher, nonMatchingVersionReporter, circularDepStrategy);
> +                versionMatcher, nonMatchingVersionReporter, circularStrategy);
>          return sorter.sortModuleDescriptors();
>      }
>
>
> Modified: incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> URL: http://svn.apache.org/viewvc/incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java?view=diff&rev=548695&r1=548694&r2=548695
> ==============================================================================
> --- incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java (original)
> +++ incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java Tue Jun 19 04:32:43 2007
> @@ -27,7 +27,6 @@
>  import junit.framework.Assert;
>  import junit.framework.TestCase;
>
> -import org.apache.ivy.Ivy;
>  import org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor;
>  import org.apache.ivy.core.module.descriptor.DefaultModuleDescriptor;
>  import org.apache.ivy.core.module.descriptor.DependencyDescriptor;
> @@ -35,6 +34,9 @@
>  import org.apache.ivy.core.module.id.ModuleRevisionId;
>  import org.apache.ivy.plugins.circular.CircularDependencyHelper;
>  import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> +import org.apache.ivy.plugins.circular.WarnCircularDependencyStrategy;
> +import org.apache.ivy.plugins.version.ExactVersionMatcher;
> +import org.apache.ivy.plugins.version.LatestVersionMatcher;
>
>  public class SortTest extends TestCase {
>
> @@ -46,9 +48,9 @@
>
>      private DefaultModuleDescriptor md4;
>
> -    private static Ivy ivy;
> -
> -
> +    private SortEngine sortEngine;
> +
> +    private SilentNonMatchingVersionReporter nonMatchReporter;
>
>      /*
>       * (non-Javadoc)
> @@ -63,8 +65,11 @@
>          md3 = createModuleDescriptorToSort("md3", "rev3");
>          md4 = createModuleDescriptorToSort("md4", "rev4");
>
> -        ivy = new Ivy();
> -        ivy.configureDefault();
> +        sortEngine = new SortEngine();
> +        sortEngine.setCircularDependencyStrategy(WarnCircularDependencyStrategy.getInstance());
> +        sortEngine.setVersionMatcher(new ExactVersionMatcher());
> +
> +        nonMatchReporter = new SilentNonMatchingVersionReporter();
>      }
>
>      public void testSort() throws Exception {
> @@ -78,7 +83,7 @@
>          Collection permutations = getAllLists(md1, md3, md2, md4);
>          for (Iterator it = permutations.iterator(); it.hasNext();) {
>              List toSort = (List) it.next();
> -            assertSorted(expectedOrder, ivy.sortModuleDescriptors(toSort));
> +            assertSorted(expectedOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
>          }
>      }
>
> @@ -100,7 +105,7 @@
>          Collection permutations = getAllLists(md1, md3, md2, md4);
>          for (Iterator it = permutations.iterator(); it.hasNext();) {
>              List toSort = (List) it.next();
> -            assertSorted(possibleOrder, ivy.sortModuleDescriptors(toSort));
> +            assertSorted(possibleOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
>          }
>      }
>
> @@ -117,7 +122,7 @@
>          Collection permutations = getAllLists(md1, md3, md2, md4);
>          for (Iterator it = permutations.iterator(); it.hasNext();) {
>              List toSort = (List) it.next();
> -            assertSorted(possibleOrder, ivy.sortModuleDescriptors(toSort));
> +            assertSorted(possibleOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
>          }
>      }
>
> @@ -160,10 +165,10 @@
>              }
>          }
>          CircularDependencyReporterMock circularDepReportMock = new CircularDependencyReporterMock();
> -        ivy.getSettings().setCircularDependencyStrategy(circularDepReportMock);
> +        sortEngine.setCircularDependencyStrategy(circularDepReportMock);
>
>          List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3, md2, md1});
> -        ivy.sortModuleDescriptors(toSort);
> +        sortEngine.sortModuleDescriptors(toSort, nonMatchReporter);
>
>          circularDepReportMock.validate();
>      }
> @@ -178,13 +183,15 @@
>          addDependency(md3, "md2", "latest.integration");
>          addDependency(md4, "md3", "latest.integration");
>
> +        sortEngine.setVersionMatcher(new LatestVersionMatcher());
> +
>          DefaultModuleDescriptor[][] expectedOrder = new DefaultModuleDescriptor[][] {{md1, md2,
>                  md3, md4}};
>
>          Collection permutations = getAllLists(md1, md3, md2, md4);
>          for (Iterator it = permutations.iterator(); it.hasNext();) {
>              List toSort = (List) it.next();
> -            assertSorted(expectedOrder, ivy.sortModuleDescriptors(toSort));
> +            assertSorted(expectedOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
>          }
>
>      }
> @@ -210,7 +217,7 @@
>          Collection permutations = getAllLists(md1, md3, md2, md4);
>          for (Iterator it = permutations.iterator(); it.hasNext();) {
>              List toSort = (List) it.next();
> -            assertSorted(possibleOrder, ivy.sortModuleDescriptors(toSort));
> +            assertSorted(possibleOrder, sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
>          }
>
>      }
> @@ -244,7 +251,7 @@
>          NonMatchingVersionReporterMock nonMatchingVersionReporterMock =
>              new NonMatchingVersionReporterMock();
>          List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3, md2, md1});
> -        ivy.sortModuleDescriptors(toSort, nonMatchingVersionReporterMock);
> +        sortEngine.sortModuleDescriptors(toSort, nonMatchingVersionReporterMock);
>          nonMatchingVersionReporterMock.validate();
>      }
>
>
>
>


-- 
Gilles SCOKART