Resolves#1169: Clean-up, remove deprecated methods, remove Plexus Str…

…ingUtils in favour of Apache Commons (#1170) * Resolves#1169: Clean up deprecated methods. Move from Plexus StringUtils to Apache Commons. Improved ArtifactVersions::compareTo and added a stricter one for PluginUpdatesDetails to comfort static code checkers.
mojohaus · Oct 28, 2024 · 21a0198 · 21a0198
1 parent bf8db03
commit 21a0198
Show file tree

Hide file tree

Showing 27 changed files with 276 additions and 210 deletions.
diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/ArtifactVersions.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/ArtifactVersions.java
@@ -19,21 +19,21 @@
  * under the License.
  */
 
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
 import org.apache.commons.lang3.builder.EqualsBuilder;
-import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.ArtifactUtils;
 import org.apache.maven.artifact.versioning.ArtifactVersion;
 import org.apache.maven.artifact.versioning.Restriction;
 import org.apache.maven.artifact.versioning.VersionRange;
 import org.codehaus.mojo.versions.ordering.VersionComparator;
 
-import static org.apache.commons.lang3.StringUtils.compare;
-
 /**
  * Holds the results of a search for versions of an artifact.
  *
@@ -65,8 +65,8 @@ public class ArtifactVersions extends AbstractVersionDetails implements Comparab
     /**
      * Creates a new {@link ArtifactVersions} instance.
      *
-     * @param artifact The artifact.
-     * @param versions The versions.
+     * @param artifact          The artifact.
+     * @param versions          The versions.
      * @param versionComparator The version comparison rule.
      * @since 1.0-alpha-3
      */
@@ -93,18 +93,13 @@ public ArtifactVersions(ArtifactVersions other) {
         setCurrentVersionRange(other.getCurrentVersionRange());
     }
 
-    @SuppressWarnings("checkstyle:InnerAssignment")
     public int compareTo(ArtifactVersions that) {
-        int rv;
         return this == that
                 ? 0
-                : that == null || getClass() != that.getClass()
-                        ? 1
-                        : (rv = compare(getGroupId(), that.getGroupId())) != 0
-                                ? rv
-                                : (rv = compare(getArtifactId(), that.getArtifactId())) != 0
-                                        ? rv
-                                        : compare(getVersion(), that.getVersion());
+                : Comparator.nullsLast(Comparator.comparing(ArtifactVersions::getGroupId)
+                                .thenComparing(ArtifactVersions::getArtifactId)
+                                .thenComparing(ArtifactVersions::getVersion))
+                        .compare(this, that);
     }
 
     @Override
@@ -132,19 +127,15 @@ public boolean equals(Object o) {
 
     @Override
     public int hashCode() {
-        return new HashCodeBuilder(17, 37)
-                .append(getArtifact())
-                .append(getVersions(true))
-                .append(getVersionComparator())
-                .toHashCode();
+        return Objects.hash(getArtifact(), Arrays.hashCode(getVersions(true)), getVersionComparator());
     }
 
     /**
      * Checks if the version is in the range (and ensures that the range respects the <code>-!</code> syntax to rule out
      * any qualifiers from range boundaries).
      *
      * @param version the version to check.
-     * @param range the range to check.
+     * @param range   the range to check.
      * @return <code>true</code> if and only if the version is in the range.
      * @since 1.3
      */
@@ -227,9 +218,9 @@ public ArtifactVersion[] getVersions(boolean includeSnapshots) {
      * the method will only count release versions.
      *
      * @param includeSnapshots {@code includeSnapshots} is {@code true}, snapshots will not counted, which means that
-     *      * the method will only count release versions.
+     *                         * the method will only count release versions.
      * @return if {@code includeSnapshots} is {@code true}, returns {@code true} if there are no versions.
-     * if {@code includeSnapshots} is {@code false}, returns {@code true} if there are no releases.
+     *         if {@code includeSnapshots} is {@code false}, returns {@code true} if there are no releases.
      */
     public boolean isEmpty(boolean includeSnapshots) {
         return includeSnapshots

diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java
@@ -49,6 +49,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.maven.artifact.Artifact;
@@ -84,7 +85,6 @@
 import org.codehaus.mojo.versions.utils.VersionsExpressionEvaluator;
 import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
 import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;
-import org.codehaus.plexus.util.StringUtils;
 import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
 import org.eclipse.aether.RepositorySystem;
 import org.eclipse.aether.repository.AuthenticationContext;

diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/PluginUpdatesDetails.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/PluginUpdatesDetails.java
@@ -19,13 +19,16 @@
  * under the License.
  */
 
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.Map;
 import java.util.Objects;
 
 import org.apache.maven.artifact.versioning.ArtifactVersion;
 import org.apache.maven.model.Dependency;
 
 import static java.util.Optional.empty;
+import static java.util.Optional.ofNullable;
 
 /**
  * Details of a plugin's updates.
@@ -47,6 +50,10 @@ public PluginUpdatesDetails(
         this.includeSnapshots = includeSnapshots;
     }
 
+    public boolean isIncludeSnapshots() {
+        return includeSnapshots;
+    }
+
     public Map<Dependency, ArtifactVersions> getDependencyVersions() {
         return dependencyVersions;
     }
@@ -85,4 +92,45 @@ public boolean isDependencyUpdateAvailable() {
     public boolean isUpdateAvailable() {
         return isArtifactUpdateAvailable() || isDependencyUpdateAvailable();
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (o == this) {
+            return true;
+        }
+        if (!(o instanceof PluginUpdatesDetails)) {
+            return false;
+        }
+        PluginUpdatesDetails other = (PluginUpdatesDetails) o;
+        return includeSnapshots == other.includeSnapshots
+                && Objects.equals(dependencyVersions, other.dependencyVersions)
+                && super.equals(o);
+    }
+
+    public int hashCode() {
+        return Objects.hash(
+                getArtifact(),
+                Arrays.hashCode(getVersions(true)),
+                getVersionComparator(),
+                includeSnapshots,
+                dependencyVersions);
+    }
+
+    // added an arbitrary comparison just to be able to differentiate objects having different includeSnapshots
+    // and dependencyVersions while their super.compareTo() returns 0
+    @SuppressWarnings("checkstyle:InnerAssignment")
+    public int compareTo(PluginUpdatesDetails that) {
+        int r;
+        return (r = super.compareTo(that)) != 0
+                ? r
+                : Comparator.comparing(PluginUpdatesDetails::isIncludeSnapshots)
+                        .thenComparing(p -> ofNullable(p.dependencyVersions)
+                                .map(Map::values)
+                                .map(c -> ofNullable(that.dependencyVersions)
+                                        .map(Map::values)
+                                        .map(c::containsAll)
+                                        .orElse(true))
+                                .orElse(false))
+                        .compare(this, that);
+    }
 }
diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java
@@ -52,6 +52,7 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
 import org.apache.maven.artifact.versioning.VersionRange;
 import org.apache.maven.execution.MavenSession;
@@ -76,7 +77,6 @@
 import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
 import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;
 import org.codehaus.plexus.util.IOUtil;
-import org.codehaus.plexus.util.StringUtils;
 import org.codehaus.plexus.util.xml.XmlStreamReader;
 import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
 import org.codehaus.stax2.XMLInputFactory2;

diff --git a/...s-common/src/main/java/org/codehaus/mojo/versions/ordering/AbstractVersionComparator.java b/...s-common/src/main/java/org/codehaus/mojo/versions/ordering/AbstractVersionComparator.java
@@ -19,6 +19,7 @@
  * under the License.
  */
 
+import org.apache.maven.artifact.ArtifactUtils;
 import org.apache.maven.artifact.versioning.ArtifactVersion;
 
 /**
@@ -35,7 +36,7 @@ public final int getSegmentCount(ArtifactVersion v) {
         if (v == null) {
             return 0;
         }
-        if (VersionComparators.isSnapshot(v)) {
+        if (ArtifactUtils.isSnapshot(v.toString())) {
             return innerGetSegmentCount(VersionComparators.stripSnapshot(v));
         }
         return innerGetSegmentCount(v);

diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/BoundArtifactVersion.java b/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/BoundArtifactVersion.java
@@ -18,12 +18,12 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.commons.lang3.StringUtils;
 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;
 
 /**
  * <p>Represents an <b>immutable</b> artifact version with all segments <em>major</em> to the given segment

diff --git a/...ions-common/src/main/java/org/codehaus/mojo/versions/ordering/MavenVersionComparator.java b/...ions-common/src/main/java/org/codehaus/mojo/versions/ordering/MavenVersionComparator.java
@@ -19,11 +19,11 @@
  * under the License.
  */
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.maven.artifact.versioning.ArtifactVersion;
 import org.apache.maven.artifact.versioning.ComparableVersion;
 import org.codehaus.mojo.versions.api.Segment;
 import org.codehaus.mojo.versions.utils.DefaultArtifactVersionCache;
-import org.codehaus.plexus.util.StringUtils;
 
 /**
  * A comparator which uses Maven's version rules, i.e. 1.3.34 &gt; 1.3.9 but 1.3.4.3.2.34 &lt; 1.3.4.3.2.9.

diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/VersionComparators.java b/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/VersionComparators.java
@@ -112,10 +112,6 @@ public static String alphaNumIncrement(String token) {
         }
     }
 
-    static boolean isSnapshot(ArtifactVersion v) {
-        return v != null && SNAPSHOT_PATTERN.matcher(v.toString()).find();
-    }
-
     static ArtifactVersion stripSnapshot(ArtifactVersion v) {
         final String version = v.toString();
         final Matcher matcher = SNAPSHOT_PATTERN.matcher(version);
@@ -124,16 +120,4 @@ static ArtifactVersion stripSnapshot(ArtifactVersion v) {
         }
         return v;
     }
-
-    static ArtifactVersion copySnapshot(ArtifactVersion source, ArtifactVersion destination) {
-        if (isSnapshot(destination)) {
-            destination = stripSnapshot(destination);
-        }
-        final Matcher matcher = SNAPSHOT_PATTERN.matcher(source.toString());
-        if (matcher.find()) {
-            return DefaultArtifactVersionCache.of(destination.toString() + "-" + matcher.group(0));
-        } else {
-            return DefaultArtifactVersionCache.of(destination.toString() + "-SNAPSHOT");
-        }
-    }
 }
diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/utils/DelegatingContextualLog.java b/versions-common/src/main/java/org/codehaus/mojo/versions/utils/DelegatingContextualLog.java
@@ -19,8 +19,9 @@
  * under the License.
  */
 
+import java.util.Objects;
+
 import org.apache.maven.plugin.logging.Log;
-import org.codehaus.plexus.util.StringUtils;
 
 /**
  * Created by IntelliJ IDEA.
@@ -46,7 +47,7 @@ public DelegatingContextualLog(Log delegate) {
     }
 
     public synchronized void setContext(String context) {
-        if (StringUtils.equals(currentContext, context)) {
+        if (Objects.equals(currentContext, context)) {
             return;
         }
         if (currentContext != null) {

diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/utils/DependencyBuilder.java b/versions-common/src/main/java/org/codehaus/mojo/versions/utils/DependencyBuilder.java
@@ -126,47 +126,6 @@ public static DependencyBuilder newBuilder() {
         return new DependencyBuilder();
     }
 
-    /**
-     * Convenience builder method
-     * @param groupId groupId of the dependency
-     * @param artifactId artifactId of the dependency
-     * @param version version of the dependency
-     * @return new instance of {@linkplain Dependency}
-     * @deprecated please use the {@link #newBuilder()} method instead
-     */
-    @Deprecated
-    public static Dependency dependencyWith(String groupId, String artifactId, String version) {
-        return newBuilder()
-                .withGroupId(groupId)
-                .withArtifactId(artifactId)
-                .withVersion(version)
-                .build();
-    }
-
-    /**
-     * Convenience builder method
-     * @param groupId groupId of the dependency
-     * @param artifactId artifactId of the dependency
-     * @param version version of the dependency
-     * @param type type of the dependency
-     * @param classifier classifier of the dependency
-     * @param scope scope of the dependency
-     * @return new instance of {@linkplain Dependency}
-     * @deprecated please use the {@link #newBuilder()} method instead
-     */
-    @Deprecated
-    public static Dependency dependencyWith(
-            String groupId, String artifactId, String version, String type, String classifier, String scope) {
-        return newBuilder()
-                .withGroupId(groupId)
-                .withArtifactId(artifactId)
-                .withVersion(version)
-                .withType(type)
-                .withClassifier(classifier)
-                .withScope(scope)
-                .build();
-    }
-
     /**
      * Builds the {@linkplain Dependency} instance
      * @return {@linkplain Dependency} instance

diff --git a/versions-common/src/test/java/org/codehaus/mojo/versions/filtering/DependencyFilterTest.java b/versions-common/src/test/java/org/codehaus/mojo/versions/filtering/DependencyFilterTest.java
@@ -39,9 +39,21 @@ class DependencyFilterTest {
     @Nested
     class RemoveFromTest {
         private final Set<Dependency> input = new HashSet<>(asList(
-                DependencyBuilder.dependencyWith("foo", "bar", "1"),
-                DependencyBuilder.dependencyWith("localhost", "my-api", "2"),
-                DependencyBuilder.dependencyWith("localhost", "my-impl", "3")));
+                DependencyBuilder.newBuilder()
+                        .withGroupId("foo")
+                        .withArtifactId("bar")
+                        .withVersion("1")
+                        .build(),
+                DependencyBuilder.newBuilder()
+                        .withGroupId("localhost")
+                        .withArtifactId("my-api")
+                        .withVersion("2")
+                        .build(),
+                DependencyBuilder.newBuilder()
+                        .withGroupId("localhost")
+                        .withArtifactId("my-impl")
+                        .withVersion("3")
+                        .build()));
 
         @Test
         void removesExcludedDepsWithExactMatch() {
@@ -98,9 +110,21 @@ void removesMultiplePatterns() {
     @Nested
     class RetainingInTest {
         private final Set<Dependency> input = new HashSet<>(asList(
-                DependencyBuilder.dependencyWith("foo", "bar", "1"),
-                DependencyBuilder.dependencyWith("localhost", "my-api", "2"),
-                DependencyBuilder.dependencyWith("localhost", "my-impl", "3")));
+                DependencyBuilder.newBuilder()
+                        .withGroupId("foo")
+                        .withArtifactId("bar")
+                        .withVersion("1")
+                        .build(),
+                DependencyBuilder.newBuilder()
+                        .withGroupId("localhost")
+                        .withArtifactId("my-api")
+                        .withVersion("2")
+                        .build(),
+                DependencyBuilder.newBuilder()
+                        .withGroupId("localhost")
+                        .withArtifactId("my-impl")
+                        .withVersion("3")
+                        .build()));
 
         @Test
         void retainsOnlyDepsWithExactMatch() {