Resolves mojohaus#921: Refactoring segments; work in progress

versions-common/src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java

@@ -19,16 +19,6 @@
  * under the License.
  */
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.TreeSet;
-import java.util.regex.Pattern;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-
 import org.apache.maven.artifact.ArtifactUtils;
 import org.apache.maven.artifact.versioning.ArtifactVersion;
 import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
@@ -39,6 +29,11 @@
 import org.codehaus.mojo.versions.ordering.VersionComparator;
 import org.codehaus.mojo.versions.utils.DefaultArtifactVersionCache;
 
+import java.util.*;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
 import static java.util.Collections.reverseOrder;
 import static java.util.Optional.empty;
 import static java.util.Optional.of;
@@ -69,7 +64,8 @@ public abstract class AbstractVersionDetails implements VersionDetails {
     protected AbstractVersionDetails() {}
 
     @Override
-    public Restriction restrictionForUnchangedSegment(Optional<Segment> unchangedSegment) throws InvalidSegmentException {
+    public Restriction restrictionForUnchangedSegment(Optional<Segment> unchangedSegment)
+            throws InvalidSegmentException {
         // one range spec can have multiple restrictions, and multiple 'lower bound', we want the highest one.
         // [1.0,2.0),[3.0,4.0) -> 3.0
         ArtifactVersion highestLowerBound = currentVersion;
@@ -87,21 +83,26 @@ public Restriction restrictionForUnchangedSegment(Optional<Segment> unchangedSeg
         }
 
         final ArtifactVersion currentVersion = highestLowerBound;
-        ArtifactVersion nextVersion = unchangedSegment.filter(s -> s.isMajorTo(SUBINCREMENTAL))
-                .map(s -> (ArtifactVersion) new BoundArtifactVersion(currentVersion, Segment.of(s.value() + 1)))
+        ArtifactVersion nextVersion = unchangedSegment
+                .filter(s -> s.isGreaterThan(SUBINCREMENTAL))
+                .map(Segment::ofLesserThan)
+                .map(s -> (ArtifactVersion) new BoundArtifactVersion(currentVersion, s))
                 .orElse(currentVersion);
         return new Restriction(
                 nextVersion,
                 false,
-                unchangedSegment.filter(MAJOR::isMajorTo)
+                unchangedSegment
+                        .filter(MAJOR::isGreaterThan)
                         .map(s -> (ArtifactVersion) new BoundArtifactVersion(currentVersion, s))
                         .orElse(null),
                 false);
     }
 
     @Override
     public Restriction restrictionForIgnoreScope(Optional<Segment> ignored) {
-        ArtifactVersion nextVersion = ignored.map(s -> (ArtifactVersion) new BoundArtifactVersion(currentVersion, s))
+        ArtifactVersion nextVersion = ignored
+                .map(Segment::ofGreaterThan)
+                .map(s -> (ArtifactVersion) new BoundArtifactVersion(currentVersion, s))
                 .orElse(currentVersion);
         return new Restriction(nextVersion, false, null, false);
     }
@@ -200,9 +201,9 @@ public final ArtifactVersion[] getNewerVersions(String version, boolean includeS
     @Deprecated
     @Override
     public final ArtifactVersion[] getNewerVersions(
-            String version, Optional<Segment> upperBoundSegment, boolean includeSnapshots)
+            String version, Optional<Segment> unchangedSegment, boolean includeSnapshots)
             throws InvalidSegmentException {
-        return getNewerVersions(version, upperBoundSegment, includeSnapshots, false);
+        return getNewerVersions(version, unchangedSegment, includeSnapshots, false);
     }
 
     @Override
@@ -217,7 +218,7 @@ public final ArtifactVersion[] getNewerVersions(
                 : currentVersion;
         ArtifactVersion upperBound = unchangedSegment
                 .map(s -> (ArtifactVersion) new BoundArtifactVersion(
-                        currentVersion, s.isMajorTo(SUBINCREMENTAL) ? Segment.of(s.value() + 1) : s))
+                        currentVersion, s.isGreaterThan(SUBINCREMENTAL) ? Segment.of(s.value() + 1) : s))
                 .orElse(null);
 
         Restriction restriction = new Restriction(lowerBound, allowDowngrade, upperBound, allowDowngrade);
@@ -236,7 +237,7 @@ public Optional<ArtifactVersion> getNewestVersion(
                 : currentVersion;
         ArtifactVersion upperBound = upperBoundSegment
                 .map(s -> (ArtifactVersion) new BoundArtifactVersion(
-                        currentVersion, s.isMajorTo(SUBINCREMENTAL) ? Segment.of(s.value() + 1) : s))
+                        currentVersion, s.isGreaterThan(SUBINCREMENTAL) ? Segment.of(s.value() + 1) : s))
                 .orElse(null);
 
         Restriction restriction = new Restriction(lowerBound, allowDowngrade, upperBound, allowDowngrade);
@@ -265,36 +266,36 @@ public final ArtifactVersion[] getVersions(
 
     @Override
     public final ArtifactVersion getNewestUpdate(
-            ArtifactVersion currentVersion, Optional<Segment> updateScope, boolean includeSnapshots) {
+            ArtifactVersion currentVersion, Optional<Segment> unchangedSegment, boolean includeSnapshots) {
         try {
-            return getNewestVersion(restrictionForUnchangedSegment(updateScope), includeSnapshots);
+            return getNewestVersion(restrictionForUnchangedSegment(unchangedSegment), includeSnapshots);
         } catch (InvalidSegmentException e) {
             return null;
         }
     }
 
     @Override
     public final ArtifactVersion[] getAllUpdates(
-            ArtifactVersion currentVersion, Optional<Segment> updateScope, boolean includeSnapshots) {
+            ArtifactVersion currentVersion, Optional<Segment> unchangedSegment, boolean includeSnapshots) {
         try {
-            return getVersions(restrictionForUnchangedSegment(updateScope), includeSnapshots);
+            return getVersions(restrictionForUnchangedSegment(unchangedSegment), includeSnapshots);
         } catch (InvalidSegmentException e) {
             return null;
         }
     }
 
     @Override
-    public final ArtifactVersion getNewestUpdate(Optional<Segment> updateScope, boolean includeSnapshots) {
+    public final ArtifactVersion getNewestUpdate(Optional<Segment> unchangedSegment, boolean includeSnapshots) {
         if (isCurrentVersionDefined()) {
-            return getNewestUpdate(getCurrentVersion(), updateScope, includeSnapshots);
+            return getNewestUpdate(getCurrentVersion(), unchangedSegment, includeSnapshots);
         }
         return null;
     }
 
     @Override
-    public final ArtifactVersion[] getAllUpdates(Optional<Segment> updateScope, boolean includeSnapshots) {
+    public final ArtifactVersion[] getAllUpdates(Optional<Segment> unchangedSegment, boolean includeSnapshots) {
         if (isCurrentVersionDefined()) {
-            return getAllUpdates(getCurrentVersion(), updateScope, includeSnapshots);
+            return getAllUpdates(getCurrentVersion(), unchangedSegment, includeSnapshots);
         }
         return null;
     }

versions-common/src/main/java/org/codehaus/mojo/versions/api/PropertyVersions.java

@@ -320,7 +320,7 @@ public ArtifactVersion getNewestVersion(
                 ? null
                 : upperBoundSegment
                         .map(s -> (ArtifactVersion) new BoundArtifactVersion(
-                                currentVersion, s.isMajorTo(SUBINCREMENTAL) ? Segment.of(s.value() + 1) : s))
+                                currentVersion, s.isGreaterThan(SUBINCREMENTAL) ? Segment.of(s.value() + 1) : s))
                         .orElse(null);
         if (helper.getLog().isDebugEnabled()) {
             helper.getLog().debug("Property ${" + property.getName() + "}: upperBound is: " + upperBound);

versions-common/src/main/java/org/codehaus/mojo/versions/api/Segment.java

@@ -19,6 +19,8 @@
  * under the License.
  */
 
+import java.util.Optional;
+
 /**
  * Indicates the segment along with its 0-based index
  *
@@ -40,18 +42,45 @@ public int value() {
     }
 
     public static Segment of(int index) {
-        if (index < 0 || index > 3) {
+        if (index < 0) {
+            return null;
+        }
+        if (index > 3) {
             throw new IllegalArgumentException("Wrong segment index: " + index);
         }
         return values()[index];
     }
 
+    /**
+     * Creates a segment that has a greater scope than the given segment or {@code null}
+     * if the segment is already {@link #MAJOR}
+     * @param other segment that the new segment is to be based on
+     * @return that has a greater scope than the given segment or {@code null}
+     * if the segment is already {@link #MAJOR}
+     */
+    public static Segment ofGreaterThan(Segment other) {
+        return Optional.ofNullable(other)
+                .map(s -> of(s.value() - 1))
+                .orElse(null);
+    }
+
+    /**
+     * Creates a segment that has a lesser scope than the given segment
+     * @param other segment that the new segment is to be based on
+     * @return that has a lesser scope than the given segment
+     */
+    public static Segment ofLesserThan(Segment other) {
+        return Optional.ofNullable(other)
+                .map(s -> of(s.value() + 1))
+                .orElse(MAJOR);
+    }
+
     /**
      * Returns true if the given segment is more major than the other
      * @param other other segment to compare
      * @return true if the given segment is more major
      */
-    public boolean isMajorTo(Segment other) {
+    public boolean isGreaterThan(Segment other) {
         return value() < other.value();
     }
 

versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java

@@ -199,7 +199,7 @@ ArtifactVersion getNewestVersion(
      * should be included.
      *
      * @param versionString current version
-     * @param upperBoundSegment the upper bound segment; empty() means no upper bound
+     * @param unchangedSegment the upper bound segment; empty() means no upper bound
      * @param includeSnapshots whether snapshot versions should be included
      * @param allowDowngrade whether to allow downgrading if the current version is a snapshots and snapshots
      *                       are disallowed
@@ -208,23 +208,23 @@ ArtifactVersion getNewestVersion(
      * the segment count)
      */
     Optional<ArtifactVersion> getNewestVersion(
-            String versionString, Optional<Segment> upperBoundSegment, boolean includeSnapshots, boolean allowDowngrade)
+            String versionString, Optional<Segment> unchangedSegment, boolean includeSnapshots, boolean allowDowngrade)
             throws InvalidSegmentException;
 
     /**
      * Returns the newest version newer than the specified current version, but within the specified update scope or
      * <code>null</code> if no such version exists.
      *
      * @param currentVersion the lower bound or <code>null</code> if the lower limit is unbounded.
-     * @param updateScope the update scope to include.
+     * @param unchangedSegment the update scope to include.
      * @param includeSnapshots <code>true</code> if snapshots are to be included.
      * @return the newest version after currentVersion within the specified update scope or <code>null</code> if no
      *         version is available.
      * @throws InvalidSegmentException thrown if the updateScope is greater than the number of segments
      * @since 1.0-beta-1
      */
     ArtifactVersion getNewestUpdate(
-            ArtifactVersion currentVersion, Optional<Segment> updateScope, boolean includeSnapshots)
+            ArtifactVersion currentVersion, Optional<Segment> unchangedSegment, boolean includeSnapshots)
             throws InvalidSegmentException;
 
     /**
@@ -242,23 +242,23 @@ ArtifactVersion getNewestUpdate(
      * should be included.
      *
      * @param version           current version
-     * @param upperBoundSegment the upper bound segment; empty() means no upper bound
+     * @param unchangedSegment the upper bound segment; empty() means no upper bound
      * @param includeSnapshots  whether snapshot versions should be included
      * @return array of newer versions fulfilling the criteria
      * @throws InvalidSegmentException if the requested segment is outside the bounds (less than 1 or greater than
      *                                 the segment count)
      * @deprecated please use {@link AbstractVersionDetails#getNewerVersions(String, Optional, boolean, boolean)},
      * boolean, boolean)} instead
      */
-    ArtifactVersion[] getNewerVersions(String version, Optional<Segment> upperBoundSegment, boolean includeSnapshots)
+    ArtifactVersion[] getNewerVersions(String version, Optional<Segment> unchangedSegment, boolean includeSnapshots)
             throws InvalidSegmentException;
 
     /**
      * Returns an array of newer versions than the given version, given the upper bound segment and whether snapshots
      * should be included.
      *
      * @param versionString current version
-     * @param upperBoundSegment the upper bound segment; empty() means no upper bound
+     * @param unchangedSegment the upper bound segment; empty() means no upper bound
      * @param includeSnapshots whether snapshot versions should be included
      * @param allowDowngrade whether to allow downgrading if the current version is a snapshots and snapshots
      *                       are disallowed
@@ -267,47 +267,47 @@ ArtifactVersion[] getNewerVersions(String version, Optional<Segment> upperBoundS
      * the segment count)
      */
     ArtifactVersion[] getNewerVersions(
-            String versionString, Optional<Segment> upperBoundSegment, boolean includeSnapshots, boolean allowDowngrade)
+            String versionString, Optional<Segment> unchangedSegment, boolean includeSnapshots, boolean allowDowngrade)
             throws InvalidSegmentException;
 
     /**
      * Returns the all versions newer than the specified current version, but within the specified update scope.
      *
      * @param currentVersion the lower bound or <code>null</code> if the lower limit is unbounded.
-     * @param updateScope the update scope to include.
+     * @param unchangedSegment the update scope to include.
      * @param includeSnapshots <code>true</code> if snapshots are to be included.
      * @return the all versions after currentVersion within the specified update scope.
      * @throws InvalidSegmentException thrown if the updateScope is greater than the number of segments
      * @since 1.0-beta-1
      */
     ArtifactVersion[] getAllUpdates(
-            ArtifactVersion currentVersion, Optional<Segment> updateScope, boolean includeSnapshots)
+            ArtifactVersion currentVersion, Optional<Segment> unchangedSegment, boolean includeSnapshots)
             throws InvalidSegmentException;
 
     /**
      * Returns the newest version newer than the specified current version, but within the specified update scope or
      * <code>null</code> if no such version exists.
      *
-     * @param updateScope the update scope to include.
+     * @param unchangedSegment the update scope to include.
      * @param includeSnapshots <code>true</code> if snapshots are to be included.
      * @return the newest version after currentVersion within the specified update scope or <code>null</code> if no
      *         version is available.
      * @throws InvalidSegmentException thrown if the updateScope is greater than the number of segments
      * @since 1.0-beta-1
      */
-    ArtifactVersion getNewestUpdate(Optional<Segment> updateScope, boolean includeSnapshots)
+    ArtifactVersion getNewestUpdate(Optional<Segment> unchangedSegment, boolean includeSnapshots)
             throws InvalidSegmentException;
 
     /**
      * Returns the all versions newer than the specified current version, but within the specified update scope.
      *
-     * @param updateScope the update scope to include.
+     * @param unchangedSegment the update scope to include.
      * @param includeSnapshots {@code true} if snapshots are to be included.
      * @return the all versions after currentVersion within the specified update scope.
      * @throws InvalidSegmentException thrown if the updateScope is greater than the number of segments
      * @since 1.0-beta-1
      */
-    ArtifactVersion[] getAllUpdates(Optional<Segment> updateScope, boolean includeSnapshots)
+    ArtifactVersion[] getAllUpdates(Optional<Segment> unchangedSegment, boolean includeSnapshots)
             throws InvalidSegmentException;
 
     /**

versions-common/src/main/java/org/codehaus/mojo/versions/ordering/BoundArtifactVersion.java

@@ -1,16 +1,16 @@
 package org.codehaus.mojo.versions.ordering;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.maven.artifact.versioning.ArtifactVersion;
 import org.codehaus.mojo.versions.api.Segment;
 import org.codehaus.mojo.versions.utils.DefaultArtifactVersionCache;
 import org.codehaus.plexus.util.StringUtils;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Optional;
-
 /**
  * <p>Represents an <b>immutable</b> upper bound artifact version
  * where all segments <em>major or equal</em> to the given segment
@@ -55,7 +55,8 @@ static ArtifactVersion createComparable(String artifactVersion, Segment unchange
         for (int segNr = 0;
                 segNr <= segments.length || segNr <= Segment.SUBINCREMENTAL.value();
                 segNr++, versionBuilder.append(".")) {
-            if (segNr <= Optional.ofNullable(unchangedSegment).map(Segment::value).orElse(-1)) {
+            if (segNr
+                    <= Optional.ofNullable(unchangedSegment).map(Segment::value).orElse(-1)) {
                 versionBuilder.append(segNr < segments.length ? integerItemOrZero(segments[segNr]) : "0");
             } else {
                 versionBuilder.append(Integer.MAX_VALUE);
@@ -114,9 +115,9 @@ private static String integerItemOrZero(String item) {
     }
 
     /**
-     * Returns the most major segment that can change.
-     * All segments that are more major than this one are held in place.
-     * @return segment that can change
+     * Returns the least major segment that may not change.
+     * All segments that are more or equally major than this one are held in place.
+     * @return segment that is held in place change
      */
     public Segment getUnchangedSegment() {
         return unchangedSegment;
@@ -145,7 +146,7 @@ public boolean equals(Object o) {
 
         return new EqualsBuilder()
                 .appendSuper(super.equals(o))
-                .append(getUnchangedSegment(), that.getUnchangedSegment())
+                .append(unchangedSegment, that.unchangedSegment)
                 .append(comparable, that.comparable)
                 .isEquals();
     }
@@ -154,7 +155,7 @@ public boolean equals(Object o) {
     public int hashCode() {
         return new HashCodeBuilder(17, 37)
                 .appendSuper(super.hashCode())
-                .append(getUnchangedSegment())
+                .append(unchangedSegment)
                 .append(comparable)
                 .toHashCode();
     }

versions-common/src/test/java/org/codehaus/mojo/versions/api/ArtifactVersionsTest.java

@@ -182,7 +182,7 @@ public void testReportLabels() {
                 new MavenVersionComparator());
 
         assertThat(instance.getNewestUpdate(of(SUBINCREMENTAL), false).toString(), is("1.1.0-2"));
-        assertThat(instance.getNewestUpdate(of(INCREMENTAL), false).toString(), is("1.1.3"));
+        assertThat(instance.getNewestUpdate(of(MINOR), false).toString(), is("1.1.3"));
     }
 
     @Test