SortTest.java

Xavier Hanin Tue, 19 Jun 2007 05:26:01 -0700

On 6/19/07, Gilles Scokart <[EMAIL PROTECTED]> 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, [EMAIL PROTECTED] <[EMAIL PROTECTED]>:
> 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

Reply via email to