From 1a4148bd1e24b5f37a638ac887374f32da7df95d Mon Sep 17 00:00:00 2001 From: Andrzej Jarmoniuk Date: Sun, 9 Apr 2023 14:09:03 +0200 Subject: [PATCH] #299: allowAnyUpdates should be ignored with a warning message if any of: allowMajorUpdates, allowMinorUpdates, allowIncrementalUpdates is set to false --- .../DisplayDependencyUpdatesMojo.java | 32 +-- .../DisplayDependencyUpdatesMojoTest.java | 212 +++++++++--------- .../versions/utils/CloseableTempFile.java | 46 ++++ 3 files changed, 163 insertions(+), 127 deletions(-) create mode 100644 versions-test/src/main/java/org/codehaus/mojo/versions/utils/CloseableTempFile.java diff --git a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java index 8b86b19c0f..c4154f86ff 100644 --- a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java +++ b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojo.java @@ -21,13 +21,7 @@ import javax.inject.Inject; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; -import java.util.TreeSet; +import java.util.*; import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.artifact.versioning.ArtifactVersion; @@ -54,9 +48,7 @@ import static org.apache.commons.lang3.StringUtils.countMatches; import static org.codehaus.mojo.versions.api.Segment.MAJOR; import static org.codehaus.mojo.versions.filtering.DependencyFilter.filterDependencies; -import static org.codehaus.mojo.versions.utils.MavenProjectUtils.extractDependenciesFromDependencyManagement; -import static org.codehaus.mojo.versions.utils.MavenProjectUtils.extractDependenciesFromPlugins; -import static org.codehaus.mojo.versions.utils.MavenProjectUtils.extractPluginDependenciesFromPluginsInPluginManagement; +import static org.codehaus.mojo.versions.utils.MavenProjectUtils.*; /** * Displays all dependencies that have newer versions available. @@ -246,11 +238,14 @@ public class DisplayDependencyUpdatesMojo extends AbstractVersionsDisplayMojo { private boolean allowIncrementalUpdates = true; /** - * Whether to allow any version change to be allowed. This keeps - * compatibility with previous versions of the plugin. - * If you set this to false you can control changes in version - * number by {@link #allowMajorUpdates}, {@link #allowMinorUpdates} or - * {@link #allowIncrementalUpdates}. + *

Ignored -- largely replaced by {@linkplain #allowMajorUpdates}, + * {@linkplain #allowMinorUpdates} and {@linkplain #allowIncrementalUpdates}, + * which are equal to {@code true} by default.

+ * + *

Please note: Prior to version 2.16.0, leaving this parameter at its default + * value ({@code true}) would mean that the plugin would ignore + * {@linkplain #allowMajorUpdates}, {@linkplain #allowMinorUpdates}, and {@linkplain #allowIncrementalUpdates}, + * which confused many users.

* * @since 2.5 * @deprecated This will be removed with version 3.0.0 @@ -489,6 +484,11 @@ 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."); + } } /** @@ -506,7 +506,7 @@ static void validateGAVList(List gavList, int numSections, String argume } private Optional calculateUpdateScope() { - return allowAnyUpdates + return allowMajorUpdates && allowMinorUpdates && allowIncrementalUpdates ? empty() : of(SegmentUtils.determineUnchangedSegment( allowMajorUpdates, allowMinorUpdates, allowIncrementalUpdates, getLog()) diff --git a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojoTest.java b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojoTest.java index bb039104d3..c8367d0f57 100644 --- a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojoTest.java +++ b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/DisplayDependencyUpdatesMojoTest.java @@ -20,21 +20,19 @@ */ import java.io.File; -import java.io.IOException; import java.nio.file.Files; -import java.nio.file.Path; import java.util.Arrays; import java.util.HashMap; import java.util.List; import org.apache.maven.model.Model; import org.apache.maven.plugin.MojoExecutionException; -import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugin.testing.AbstractMojoTestCase; import org.apache.maven.plugin.testing.MojoRule; import org.apache.maven.project.MavenProject; import org.codehaus.mojo.versions.filtering.WildcardMatcher; import org.codehaus.mojo.versions.model.TestIgnoreVersions; +import org.codehaus.mojo.versions.utils.CloseableTempFile; import org.codehaus.mojo.versions.utils.DependencyBuilder; import org.hamcrest.Matchers; import org.junit.Rule; @@ -45,16 +43,9 @@ import static java.util.Collections.singletonList; import static org.codehaus.mojo.versions.model.TestIgnoreVersions.TYPE_REGEX; import static org.codehaus.mojo.versions.model.TestIgnoreVersions.matches; -import static org.codehaus.mojo.versions.utils.MockUtils.mockAetherRepositorySystem; -import static org.codehaus.mojo.versions.utils.MockUtils.mockMavenSession; -import static org.codehaus.mojo.versions.utils.MockUtils.mockRepositorySystem; +import static org.codehaus.mojo.versions.utils.MockUtils.*; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.anyOf; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.*; /** * Basic tests for {@linkplain DisplayDependencyUpdatesMojo}. @@ -83,11 +74,7 @@ public void testValidateGAVListFailed() { @Test public void testRuleSetPresentAndWorking() throws Exception { - File outputFile = null; - try { - outputFile = File.createTempFile("display-dependency-updates", ""); - assert outputFile.exists(); - + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { DisplayDependencyUpdatesMojo mojo = (DisplayDependencyUpdatesMojo) mojoRule.lookupConfiguredMojo( new File("target/test-classes/org/codehaus/mojo/display-dependency-updates/ruleset"), "display-dependency-updates"); @@ -108,7 +95,7 @@ public void testRuleSetPresentAndWorking() throws Exception { .withVersion(".+-M\\d+")))); // This is just an example of how to create it-style tests as unit tests; the advantage is easier debugging - mojo.outputFile = outputFile; + mojo.outputFile = tempFile.getPath().toFile(); mojo.aetherRepositorySystem = mockAetherRepositorySystem(new HashMap() { { put("dummy-api", new String[] {"1.0.0", "1.0.1", "1.1.0-M1", "1.2.0-SNAPSHOT"}); @@ -117,10 +104,8 @@ public void testRuleSetPresentAndWorking() throws Exception { assertThat(mojo.ruleSet.getIgnoreVersions(), Matchers.hasSize(3)); mojo.execute(); - List output = Files.readAllLines(outputFile.toPath(), UTF_8); + List output = Files.readAllLines(tempFile.getPath(), UTF_8); assertThat(output, not(hasItem(containsString("1.1.0-M1")))); - } finally { - assert outputFile == null || !outputFile.exists() || outputFile.delete(); } } @@ -145,12 +130,8 @@ private MavenProject createProject() { } @Test - public void testVersionsWithQualifiersNotConsideredAsMinorUpdates() - throws MojoExecutionException, MojoFailureException, IllegalAccessException, IOException { - Path tempPath = null; - try { - tempPath = Files.createTempFile("display-dependency-updates", ""); - final File tempFile = tempPath.toFile(); + public void testVersionsWithQualifiersNotConsideredAsMinorUpdates() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { new DisplayDependencyUpdatesMojo( mockRepositorySystem(), mockAetherRepositorySystem(new HashMap() { @@ -170,7 +151,7 @@ public void testVersionsWithQualifiersNotConsideredAsMinorUpdates() setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); setVariableValueToObject(this, "dependencyExcludes", emptyList()); this.allowSnapshots = true; - this.outputFile = tempFile; + this.outputFile = tempFile.getPath().toFile(); setPluginContext(new HashMap<>()); session = mockMavenSession(); @@ -178,25 +159,17 @@ public void testVersionsWithQualifiersNotConsideredAsMinorUpdates() }.execute(); assertThat( - String.join("", Files.readAllLines(tempPath)), + String.join("", Files.readAllLines(tempFile.getPath())), not(anyOf( containsString("2.0.0-SNAPSHOT"), containsString("2.0.0-beta"), containsString("2.0.0-rc1")))); - } finally { - if (tempPath != null && Files.exists(tempPath)) { - Files.delete(tempPath); - } } } @Test - public void testAllowMajorUpdatesFalse() - throws MojoExecutionException, MojoFailureException, IllegalAccessException, IOException { - Path tempPath = null; - try { - tempPath = Files.createTempFile("display-dependency-updates", ""); - final File tempFile = tempPath.toFile(); + public void testAllowMajorUpdatesFalse() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { new DisplayDependencyUpdatesMojo( mockRepositorySystem(), mockAetherRepositorySystem(new HashMap() { @@ -213,31 +186,23 @@ public void testAllowMajorUpdatesFalse() setVariableValueToObject(this, "processDependencies", true); setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); setVariableValueToObject(this, "dependencyExcludes", emptyList()); - this.outputFile = tempFile; + this.outputFile = tempFile.getPath().toFile(); setPluginContext(new HashMap<>()); session = mockMavenSession(); } }.execute(); - String output = String.join("", Files.readAllLines(tempPath)); + String output = String.join("", Files.readAllLines(tempFile.getPath())); assertThat(output, containsString("1.1.0")); assertThat(output, not(containsString("2.0.0"))); - } finally { - if (tempPath != null && Files.exists(tempPath)) { - Files.delete(tempPath); - } } } @Test - public void testAllowMinorUpdatesFalse() - throws MojoExecutionException, MojoFailureException, IllegalAccessException, IOException { - Path tempPath = null; - try { - tempPath = Files.createTempFile("display-dependency-updates", ""); - final File tempFile = tempPath.toFile(); + public void testAllowMinorUpdatesFalse() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { new DisplayDependencyUpdatesMojo( mockRepositorySystem(), mockAetherRepositorySystem(new HashMap() { @@ -254,32 +219,24 @@ public void testAllowMinorUpdatesFalse() setVariableValueToObject(this, "processDependencies", true); setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); setVariableValueToObject(this, "dependencyExcludes", emptyList()); - this.outputFile = tempFile; + this.outputFile = tempFile.getPath().toFile(); setPluginContext(new HashMap<>()); session = mockMavenSession(); } }.execute(); - String output = String.join("", Files.readAllLines(tempPath)); + String output = String.join("", Files.readAllLines(tempFile.getPath())); assertThat(output, containsString("1.0.1")); assertThat(output, not(containsString("1.1.0"))); assertThat(output, not(containsString("2.0.0"))); - } finally { - if (tempPath != null && Files.exists(tempPath)) { - Files.delete(tempPath); - } } } @Test - public void testAllowIncrementalUpdatesFalse() - throws MojoExecutionException, MojoFailureException, IllegalAccessException, IOException { - Path tempPath = null; - try { - tempPath = Files.createTempFile("display-dependency-updates", ""); - final File tempFile = tempPath.toFile(); + public void testAllowIncrementalUpdatesFalse() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { new DisplayDependencyUpdatesMojo( mockRepositorySystem(), mockAetherRepositorySystem(new HashMap() { @@ -296,33 +253,25 @@ public void testAllowIncrementalUpdatesFalse() setVariableValueToObject(this, "processDependencies", true); setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); setVariableValueToObject(this, "dependencyExcludes", emptyList()); - this.outputFile = tempFile; + this.outputFile = tempFile.getPath().toFile(); setPluginContext(new HashMap<>()); session = mockMavenSession(); } }.execute(); - String output = String.join("", Files.readAllLines(tempPath)); + String output = String.join("", Files.readAllLines(tempFile.getPath())); assertThat(output, containsString("1.0.0-1")); assertThat(output, not(containsString("1.0.1"))); assertThat(output, not(containsString("1.1.0"))); assertThat(output, not(containsString("2.0.0"))); - } finally { - if (tempPath != null && Files.exists(tempPath)) { - Files.delete(tempPath); - } } } @Test - public void testVersionsWithQualifiersNotConsideredAsIncrementalUpdates() - throws MojoExecutionException, MojoFailureException, IllegalAccessException, IOException { - Path tempPath = null; - try { - tempPath = Files.createTempFile("display-dependency-updates", ""); - final File tempFile = tempPath.toFile(); + public void testVersionsWithQualifiersNotConsideredAsIncrementalUpdates() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { new DisplayDependencyUpdatesMojo( mockRepositorySystem(), mockAetherRepositorySystem(new HashMap() { @@ -342,7 +291,7 @@ public void testVersionsWithQualifiersNotConsideredAsIncrementalUpdates() setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); setVariableValueToObject(this, "dependencyExcludes", emptyList()); this.allowSnapshots = true; - this.outputFile = tempFile; + this.outputFile = tempFile.getPath().toFile(); setPluginContext(new HashMap<>()); session = mockMavenSession(); @@ -350,25 +299,17 @@ public void testVersionsWithQualifiersNotConsideredAsIncrementalUpdates() }.execute(); assertThat( - String.join("", Files.readAllLines(tempPath)), + String.join("", Files.readAllLines(tempFile.getPath())), not(anyOf( containsString("1.9.0-SNAPSHOT"), containsString("1.9.0-beta"), containsString("1.9.0-rc1")))); - } finally { - if (tempPath != null && Files.exists(tempPath)) { - Files.delete(tempPath); - } } } @Test public void testDetermineUpdatedSegment() throws Exception { - File outputFile = null; - try { - outputFile = File.createTempFile("display-dependency-updates", ""); - assert outputFile.exists(); - + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { DisplayDependencyUpdatesMojo mojo = (DisplayDependencyUpdatesMojo) mojoRule.lookupConfiguredMojo( new File("target/test-classes/org/codehaus/mojo/display-dependency-updates/ruleset"), "display-dependency-updates"); @@ -390,7 +331,7 @@ public void testDetermineUpdatedSegment() throws Exception { .withVersion(".+-M\\d+")))); // This is just an example of how to create it-style tests as unit tests; the advantage is easier debugging - mojo.outputFile = outputFile; + mojo.outputFile = tempFile.getPath().toFile(); mojo.aetherRepositorySystem = mockAetherRepositorySystem(new HashMap() { { put("dummy-api", new String[] {"1.0.0", "1.0.1", "1.1.0-M1", "1.2.0-SNAPSHOT"}); @@ -399,26 +340,20 @@ public void testDetermineUpdatedSegment() throws Exception { assertThat(mojo.ruleSet.getIgnoreVersions(), Matchers.hasSize(3)); mojo.execute(); - List output = Files.readAllLines(outputFile.toPath(), UTF_8); + List output = Files.readAllLines(tempFile.getPath(), UTF_8); assertThat(output, not(hasItem(containsString("1.1.0-M1")))); - } finally { - assert outputFile == null || !outputFile.exists() || outputFile.delete(); } } @Test public void testVersionInterpolation() throws Exception { - File outputFile = null; - try { - outputFile = File.createTempFile("display-dependency-updates", ""); - assert outputFile.exists(); - + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { DisplayDependencyUpdatesMojo mojo = (DisplayDependencyUpdatesMojo) mojoRule.lookupConfiguredMojo( new File("target/test-classes/org/codehaus/mojo/display-dependency-updates/version-interpolation"), "display-dependency-updates"); // This is just an example of how to create it-style tests as unit tests; the advantage is easier debugging - mojo.outputFile = outputFile; + mojo.outputFile = tempFile.getPath().toFile(); mojo.aetherRepositorySystem = mockAetherRepositorySystem(new HashMap() { { put("dummy-api", new String[] {"2.0.1"}); @@ -426,20 +361,14 @@ public void testVersionInterpolation() throws Exception { }); setVariableValueToObject(mojo, "processDependencyManagementTransitive", false); mojo.execute(); - List output = Files.readAllLines(outputFile.toPath(), UTF_8); + List output = Files.readAllLines(tempFile.getPath(), UTF_8); assertThat(output, not(hasItem(containsString("mycomponent.version")))); - } finally { - assert outputFile == null || !outputFile.exists() || outputFile.delete(); } } @Test - public void testAllowSnapshots() - throws MojoExecutionException, MojoFailureException, IllegalAccessException, IOException { - Path tempPath = null; - try { - tempPath = Files.createTempFile("display-dependency-updates", ""); - File tempFile = tempPath.toFile(); + public void testAllowSnapshots() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { new DisplayDependencyUpdatesMojo( mockRepositorySystem(), mockAetherRepositorySystem(new HashMap() { @@ -455,20 +384,81 @@ public void testAllowSnapshots() setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); setVariableValueToObject(this, "dependencyExcludes", emptyList()); this.allowSnapshots = true; - this.outputFile = tempFile; + this.outputFile = tempFile.getPath().toFile(); setPluginContext(new HashMap<>()); session = mockMavenSession(); } }.execute(); - String output = String.join("", Files.readAllLines(tempPath)); + String output = String.join("", Files.readAllLines(tempFile.getPath())); assertThat(output, containsString("1.0.1-SNAPSHOT")); - } finally { - if (tempPath != null && Files.exists(tempPath)) { - Files.delete(tempPath); - } + } + } + + /** + * With {@code allowMajorUpdates}, {@code allowMinorUpdates}, or {@code allowIncrementalUpdates} all equal + * {@code true}, {@code allowAnyUpdates} should be respected. + */ + @Test + public void testAllowAnyUpdatesTrue() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { + new DisplayDependencyUpdatesMojo( + mockRepositorySystem(), + mockAetherRepositorySystem(new HashMap() { + { + put("default-dependency", new String[] {"2.0.0", "1.0.0"}); + } + }), + null, + null) { + { + setProject(createProject()); + setVariableValueToObject(this, "processDependencies", true); + setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); + setVariableValueToObject(this, "dependencyExcludes", emptyList()); + this.outputFile = tempFile.getPath().toFile(); + setPluginContext(new HashMap<>()); + session = mockMavenSession(); + } + }.execute(); + + String output = String.join("", Files.readAllLines(tempFile.getPath())); + assertThat(output, containsString("2.0.0")); + } + } + + /** + * Setting {@code allowMajorUpdates}, {@code allowMinorUpdates}, or {@code allowIncrementalUpdates} to {@code false} + * should also set {@code allowAnyUpdates} to {@code false} + */ + @Test + public void testAllowAnyUpdatesFalseIfOtherAreSet() throws Exception { + try (CloseableTempFile tempFile = new CloseableTempFile("display-dependency-updates")) { + new DisplayDependencyUpdatesMojo( + mockRepositorySystem(), + mockAetherRepositorySystem(new HashMap() { + { + put("default-dependency", new String[] {"2.0.0", "1.0.0"}); + } + }), + null, + null) { + { + setProject(createProject()); + setVariableValueToObject(this, "processDependencies", true); + setVariableValueToObject(this, "dependencyIncludes", singletonList(WildcardMatcher.WILDCARD)); + setVariableValueToObject(this, "dependencyExcludes", emptyList()); + this.outputFile = tempFile.getPath().toFile(); + setVariableValueToObject(this, "allowMajorUpdates", false); + setPluginContext(new HashMap<>()); + session = mockMavenSession(); + } + }.execute(); + + String output = String.join("", Files.readAllLines(tempFile.getPath())); + assertThat(output, not(containsString("2.0.0"))); } } } diff --git a/versions-test/src/main/java/org/codehaus/mojo/versions/utils/CloseableTempFile.java b/versions-test/src/main/java/org/codehaus/mojo/versions/utils/CloseableTempFile.java new file mode 100644 index 0000000000..609f8a788f --- /dev/null +++ b/versions-test/src/main/java/org/codehaus/mojo/versions/utils/CloseableTempFile.java @@ -0,0 +1,46 @@ +package org.codehaus.mojo.versions.utils; /* + * Copyright MojoHaus and Contributors + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +/** + * Closeable temporary file + */ +public class CloseableTempFile implements AutoCloseable { + private final Path path; + + /** + * @return path of the temporary file + */ + public Path getPath() { + return path; + } + + /** + * Creates a new temporary file with the given prefix + * @param prefix prefix of the temporary file + * @throws IOException thrown if file creation fails + */ + public CloseableTempFile(String prefix) throws IOException { + path = Files.createTempFile(prefix, ".tmp"); + } + + @Override + public void close() throws Exception { + Files.deleteIfExists(path); + } +}