Skip to content

Commit

Permalink
Resolves mojohaus#921: Refactoring segments; work in progress
Browse files Browse the repository at this point in the history
  • Loading branch information
jarmoniuk committed May 19, 2023
1 parent dcf704e commit 2c0c141
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public abstract class AbstractVersionDetails implements VersionDetails {
protected AbstractVersionDetails() {}

@Override
public Restriction restrictionFor(Optional<Segment> scope) 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;
Expand All @@ -87,13 +87,13 @@ public Restriction restrictionFor(Optional<Segment> scope) throws InvalidSegment
}

final ArtifactVersion currentVersion = highestLowerBound;
ArtifactVersion nextVersion = scope.filter(s -> s.isMajorTo(SUBINCREMENTAL))
ArtifactVersion nextVersion = unchangedSegment.filter(s -> s.isMajorTo(SUBINCREMENTAL))
.map(s -> (ArtifactVersion) new BoundArtifactVersion(currentVersion, Segment.of(s.value() + 1)))
.orElse(currentVersion);
return new Restriction(
nextVersion,
false,
scope.filter(MAJOR::isMajorTo)
unchangedSegment.filter(MAJOR::isMajorTo)
.map(s -> (ArtifactVersion) new BoundArtifactVersion(currentVersion, s))
.orElse(null),
false);
Expand Down Expand Up @@ -267,7 +267,7 @@ public final ArtifactVersion[] getVersions(
public final ArtifactVersion getNewestUpdate(
ArtifactVersion currentVersion, Optional<Segment> updateScope, boolean includeSnapshots) {
try {
return getNewestVersion(restrictionFor(updateScope), includeSnapshots);
return getNewestVersion(restrictionForUnchangedSegment(updateScope), includeSnapshots);
} catch (InvalidSegmentException e) {
return null;
}
Expand All @@ -277,7 +277,7 @@ public final ArtifactVersion getNewestUpdate(
public final ArtifactVersion[] getAllUpdates(
ArtifactVersion currentVersion, Optional<Segment> updateScope, boolean includeSnapshots) {
try {
return getVersions(restrictionFor(updateScope), includeSnapshots);
return getVersions(restrictionForUnchangedSegment(updateScope), includeSnapshots);
} catch (InvalidSegmentException e) {
return null;
}
Expand Down Expand Up @@ -435,7 +435,7 @@ public final ArtifactVersion[] getReportUpdates(Optional<Segment> updateScope, b
private Stream<ArtifactVersion> getArtifactVersionStream(Optional<Segment> updateScope, boolean includeSnapshots) {
if (isCurrentVersionDefined()) {
try {
Restriction restriction = restrictionFor(updateScope);
Restriction restriction = restrictionForUnchangedSegment(updateScope);

return Arrays.stream(getVersions(includeSnapshots))
.filter(candidate -> isVersionInRestriction(restriction, candidate));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ ArtifactVersion[] getAllUpdates(Optional<Segment> updateScope, boolean includeSn

/**
* <p>Returns a {@linkplain Restriction} object for computing version <em>upgrades</em>
* with the given segment allowing updates, with all more major segments locked in place.</p>
* with the given segment being the least major segment to be left unchanged,
* with all more major segments locked in place.</p>
* <p>The resulting restriction could be thought of as one
* retaining the versions on positions up to the held position,
* the position right after the position held in place will be incremented by one,
Expand All @@ -340,12 +341,13 @@ ArtifactVersion[] getAllUpdates(Optional<Segment> updateScope, boolean includeSn
* <p>This will allow matching the required versions while not matching versions which are considered
* inferior than the zeroth version, i.e. versions with a qualifier.</p>
*
* @param scope most major segment where updates are allowed Optional.empty() for no restriction
* @param unchangedSegment least major segment where updates are allowed; {@code Optional.empty()}
* for no restriction
* @return {@linkplain Restriction} object based on the arguments
* @throws InvalidSegmentException if the requested segment is outside the bounds (less than 1 or greater than
* the segment count)
*/
Restriction restrictionFor(Optional<Segment> scope) throws InvalidSegmentException;
Restriction restrictionForUnchangedSegment(Optional<Segment> unchangedSegment) throws InvalidSegmentException;

/**
* Returns the {@link Restriction} objects for a segemnt scope which is to be <b>ignored</b>.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
package org.codehaus.mojo.versions.ordering;

import java.util.ArrayList;
import java.util.List;

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> artifact version with all segments <em>major</em> to the given segment
* <p>Represents an <b>immutable</b> upper bound artifact version
* where all segments <em>major or equal</em> to the given segment
* held in place. It can be thought of as an artifact having +∞ as its upper bound
* on all segments minor to the held segment.</p>
* on all segments minor or equal to the held segment.</p>
* <p>For example:</p>
* <p>A {@link BoundArtifactVersion} of {@code [1.2.3-2, INCREMENTAL]} can be seen as {@code 1.2.+∞}
* and will be greater than all versions matching the {@code 1.2.*} pattern.</p>
* <p>A {@link BoundArtifactVersion} of {@code [1.2.3-2, SUBINCREMENTAL]} will be greater
* * than all versions matching the {@code 1.2.3-2.*} pattern.</p>
* * than all versions matching the {@code 1.2.3.*} pattern.</p>
* <p>When compared to another artifact versions, this results with the other object
* with the segment versions up to the held segment being equal,
* always comparing lower than this object.</p>
Expand All @@ -27,42 +29,50 @@
*/
public class BoundArtifactVersion implements ArtifactVersion {
/**
* Most major segment that can change, i.e. not held in place.
* All segments that are more major than this one are held in place.
* Least major segment that may not change
* All segments that are more major than this one are also held in place.
* If equal to {@code null}, no restrictions are in place, meaning that all version
* segments can change freely.
*/
private final Segment segment;
private final Segment unchangedSegment;

private final ArtifactVersion comparable;

/**
* Constructs the instance given the version in a text format.
* @param artifactVersion version in a text format
* @param segment most major segment that can change, i.e. <em>not</em> held in place
* @param unchangedSegment most major segment that may not change, may be {@code null} meaning no restrictions
*/
public BoundArtifactVersion(String artifactVersion, Segment segment) {
this.segment = segment;
public BoundArtifactVersion(String artifactVersion, Segment unchangedSegment) {
this.unchangedSegment = unchangedSegment;
comparable = createComparable(artifactVersion, unchangedSegment);
}

static ArtifactVersion createComparable(String artifactVersion, Segment unchangedSegment) {
final ArtifactVersion comparable;
StringBuilder versionBuilder = new StringBuilder();
String[] segments = tokens(artifactVersion);
for (int segNr = 0;
segNr <= segments.length || segNr <= Segment.SUBINCREMENTAL.value();
segNr++, versionBuilder.append(".")) {
if (segNr < segment.value()) {
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);
}
}
versionBuilder.append(Integer.MAX_VALUE);
comparable = DefaultArtifactVersionCache.of(versionBuilder.toString());
return comparable;
}

/**
* Constructs the instance given a {@link ArtifactVersion instance}
* @param artifactVersion artifact version containing the segment version values
* @param segment most major segment that can change, i.e. <em>not</em> held in place
* @param unchangedSegment least major segment that may not change
*/
public BoundArtifactVersion(ArtifactVersion artifactVersion, Segment segment) {
this(artifactVersion.toString(), segment);
public BoundArtifactVersion(ArtifactVersion artifactVersion, Segment unchangedSegment) {
this(artifactVersion.toString(), unchangedSegment);
}

/**
Expand Down Expand Up @@ -108,8 +118,8 @@ private static String integerItemOrZero(String item) {
* All segments that are more major than this one are held in place.
* @return segment that can change
*/
public Segment getSegment() {
return segment;
public Segment getUnchangedSegment() {
return unchangedSegment;
}

@Override
Expand All @@ -135,7 +145,7 @@ public boolean equals(Object o) {

return new EqualsBuilder()
.appendSuper(super.equals(o))
.append(getSegment(), that.getSegment())
.append(getUnchangedSegment(), that.getUnchangedSegment())
.append(comparable, that.comparable)
.isEquals();
}
Expand All @@ -144,7 +154,7 @@ public boolean equals(Object o) {
public int hashCode() {
return new HashCodeBuilder(17, 37)
.appendSuper(super.hashCode())
.append(getSegment())
.append(getUnchangedSegment())
.append(comparable)
.toHashCode();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,65 +27,95 @@
import static org.codehaus.mojo.versions.api.Segment.MINOR;
import static org.codehaus.mojo.versions.api.Segment.SUBINCREMENTAL;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.Matchers.*;

/**
* Unit tests for {@link BoundArtifactVersion}
*/
public class BoundArtifactVersionTest {
@Test
public void testMajorUpperBoundGreaterThanNextMajor() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.2.3", MAJOR);
public void testCreateComparableForNoRestrictions() {
ArtifactVersion comparable = BoundArtifactVersion.createComparable("1.2.3", null);
assertThat(comparable.toString(), matchesPattern("^\\Q2147483647.2147483647.2147483647\\E.*"));
}

@Test
public void testCreateComparableForMajor() {
ArtifactVersion comparable = BoundArtifactVersion.createComparable("1.2.3", MAJOR);
assertThat(comparable.toString(), matchesPattern("^\\Q1.2147483647.2147483647\\E.*"));
}

@Test
public void testCreateComparableForMinor() {
ArtifactVersion comparable = BoundArtifactVersion.createComparable("1.2.3", MINOR);
assertThat(comparable.toString(), matchesPattern("^\\Q1.2.2147483647\\E.*"));
}

@Test
public void testCreateComparableForIncremental() {
ArtifactVersion comparable = BoundArtifactVersion.createComparable("1.2.3", INCREMENTAL);
assertThat(comparable.toString(), matchesPattern("^\\Q1.2.3\\E.*"));
}

@Test
public void testCreateComparableForSubIncremental() {
ArtifactVersion comparable = BoundArtifactVersion.createComparable("1.2.3-4", SUBINCREMENTAL);
assertThat(comparable.toString(), matchesPattern("^\\Q1.2.3.4\\E.*"));
}


@Test
public void testNullLockedGreaterThanNextMajor() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.2.3", null);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("2.0.0");
assertThat(bound.compareTo(artifactVersion), greaterThan(0));
}

@Test
public void testSubIncrementalUpperBoundGreaterThanNextSubIncremental() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.2.3-2", SUBINCREMENTAL);
public void testIncrementalLockedGreaterThanNextSubIncremental() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.2.3-2", INCREMENTAL);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("1.2.3-3");
assertThat(bound.compareTo(artifactVersion), greaterThan(0));
}

@Test
public void testVersionShorterThanSegment() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", INCREMENTAL);
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", MINOR);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("1.1.3");
assertThat(bound.compareTo(artifactVersion), greaterThan(0));
}

@Test
public void testVersionBoundArtifactVersionShorterThanConcreteVersionAndSegment() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", SUBINCREMENTAL);
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", INCREMENTAL);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("1.1.3");
assertThat(bound.compareTo(artifactVersion), lessThan(0));
}

@Test
public void testVersionSubIncrementalBoundGreaterThanSubIncremental() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", SUBINCREMENTAL);
public void testVersionHeldIncrementalBoundGreaterThanSubIncremental() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", INCREMENTAL);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("1.1.0-2");
assertThat(bound.compareTo(artifactVersion), greaterThan(0));
}

@Test
public void testVersionSubIncrementalBoundGreaterThanIncremental() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", INCREMENTAL);
public void testVersionHeldMinorBoundGreaterThanIncremental() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", MINOR);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("1.1.3");
assertThat(bound.compareTo(artifactVersion), greaterThan(0));
}

@Test
public void testVersionSubIncrementalBoundGreaterThanMinor() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", MINOR);
public void testVersionHeldMajorBoundGreaterThanMinor() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.1", MAJOR);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("1.3");
assertThat(bound.compareTo(artifactVersion), greaterThan(0));
}

@Test
public void testSnapshotWithSubIncremental() {
BoundArtifactVersion bound = new BoundArtifactVersion("1.0.0-SNAPSHOT", MINOR);
BoundArtifactVersion bound = new BoundArtifactVersion("1.0.0-SNAPSHOT", MAJOR);
ArtifactVersion artifactVersion = DefaultArtifactVersionCache.of("1.0.0");
assertThat(bound.compareTo(artifactVersion), greaterThan(0));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,6 @@ public class DisplayDependencyUpdatesMojo extends AbstractVersionsDisplayMojo {
/**
* Whether to allow the major version number to be changed.
*
* <p><b>Note: {@code false} also implies {@linkplain #allowAnyUpdates}
* to be {@code false}</b></p>
*
* @since 2.5
*/
@Parameter(property = "allowMajorUpdates", defaultValue = "true")
Expand All @@ -217,8 +214,8 @@ public class DisplayDependencyUpdatesMojo extends AbstractVersionsDisplayMojo {
/**
* <p>Whether to allow the minor version number to be changed.</p>
*
* <p><b>Note: {@code false} also implies {@linkplain #allowAnyUpdates}
* and {@linkplain #allowMajorUpdates} to be {@code false}</b></p>
* <p><b>Note: {@code false} also implies {@linkplain #allowMajorUpdates}
* to be {@code false}</b></p>
*
* @since 2.5
*/
Expand All @@ -228,32 +225,14 @@ public class DisplayDependencyUpdatesMojo extends AbstractVersionsDisplayMojo {
/**
* <p>Whether to allow the incremental version number to be changed.</p>
*
* <p><b>Note: {@code false} also implies {@linkplain #allowAnyUpdates},
* {@linkplain #allowMajorUpdates}, and {@linkplain #allowMinorUpdates}
* to be {@code false}</b></p>
* <p><b>Note: {@code false} also implies {@linkplain #allowMajorUpdates}
* and {@linkplain #allowMinorUpdates} to be {@code false}</b></p>
*
* @since 2.5
*/
@Parameter(property = "allowIncrementalUpdates", defaultValue = "true")
private boolean allowIncrementalUpdates = true;

/**
* <p>Ignored -- largely replaced by {@linkplain #allowMajorUpdates},
* {@linkplain #allowMinorUpdates} and {@linkplain #allowIncrementalUpdates},
* which are equal to {@code true} by default.</p>
*
* <p><b>Please note: Prior to version 2.16.0, leaving this parameter at its default
* value ({@code true}) would mean that the plugin would <u>ignore</u>
* {@linkplain #allowMajorUpdates}, {@linkplain #allowMinorUpdates}, and {@linkplain #allowIncrementalUpdates},
* which confused many users.</b></p>
*
* @since 2.5
* @deprecated This will be removed with version 3.0.0
*/
@Deprecated
@Parameter(property = "allowAnyUpdates", defaultValue = "true")
private boolean allowAnyUpdates = true;

/**
* Whether to show additional information such as dependencies that do not need updating. Defaults to false.
*
Expand Down Expand Up @@ -484,11 +463,6 @@ protected void validateInput() throws MojoExecutionException {
validateGAVList(pluginDependencyExcludes, 3, "pluginDependencyExcludes");
validateGAVList(pluginManagementDependencyIncludes, 3, "pluginManagementDependencyIncludes");
validateGAVList(pluginManagementDependencyExcludes, 3, "pluginManagementDependencyExcludes");
if (getLog() != null
&& allowAnyUpdates
&& !(allowMajorUpdates && allowMinorUpdates && allowIncrementalUpdates)) {
getLog().warn("Assuming allowAnyUpdates false because one or more other \"allow\" switches is false.");
}
}

/**
Expand Down
Loading

0 comments on commit 2c0c141

Please sign in to comment.