From d421b45b9644a5a75f053662cc2bb2bc0f5a5221 Mon Sep 17 00:00:00 2001 From: Andrzej Jarmoniuk Date: Fri, 21 Oct 2022 20:15:45 +0200 Subject: [PATCH] Resolves #776: onlyUpgradable change the filter to versions where the current version is not the latest one --- .../versions/DependencyUpdatesReportMojo.java | 20 +- .../versions/PluginUpdatesReportMojo.java | 6 +- .../versions/api/AbstractVersionDetails.java | 6 + .../versions/api/DefaultVersionsHelper.java | 204 +++++------------- .../mojo/versions/api/VersionDetails.java | 8 + .../DependencyUpdatesReportMojoTest.java | 20 +- .../versions/PluginUpdatesReportMojoTest.java | 49 ++++- 7 files changed, 143 insertions(+), 170 deletions(-) diff --git a/src/main/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojo.java b/src/main/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojo.java index 6cda7845a4..c831013008 100644 --- a/src/main/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojo.java +++ b/src/main/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojo.java @@ -25,15 +25,18 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.stream.Collectors; import org.apache.maven.artifact.manager.WagonManager; import org.apache.maven.artifact.metadata.ArtifactMetadataRetrievalException; import org.apache.maven.artifact.metadata.ArtifactMetadataSource; import org.apache.maven.artifact.resolver.ArtifactResolver; +import org.apache.maven.artifact.versioning.ArtifactVersion; import org.apache.maven.doxia.sink.Sink; import org.apache.maven.model.Dependency; import org.apache.maven.plugins.annotations.Mojo; @@ -202,8 +205,21 @@ && getProject().getOriginalModel().getDependencyManagement().getDependencies() ! if ( onlyUpgradable ) { - dependencyUpdates = filter( dependencyUpdates, e -> e.getVersions().length > 1 ); - dependencyManagementUpdates = filter( dependencyManagementUpdates, e -> e.getVersions().length > 1 ); + dependencyUpdates = filter( dependencyUpdates, e -> e.getVersions().length > 0 ); + dependencyManagementUpdates = filter( dependencyManagementUpdates, e -> e.getVersions().length > 0 ); + } + + if ( getLog().isDebugEnabled() ) + { + getLog().debug( "Dependency versions:" ); + dependencyUpdates.forEach( ( key, value ) -> getLog().debug( key.toString() + ": " + + Arrays.stream( value.getVersions() ).map( ArtifactVersion::toString ) + .collect( Collectors.joining( ", " ) ) ) ); + + getLog().debug( "Dependency management versions:" ); + dependencyManagementUpdates.forEach( ( key, value ) -> getLog().debug( key.toString() + ": " + + Arrays.stream( value.getVersions() ).map( ArtifactVersion::toString ) + .collect( Collectors.joining( ", " ) ) ) ); } for ( String format : formats ) diff --git a/src/main/java/org/codehaus/mojo/versions/PluginUpdatesReportMojo.java b/src/main/java/org/codehaus/mojo/versions/PluginUpdatesReportMojo.java index 1b47c0f7c4..05b9bdbfe2 100644 --- a/src/main/java/org/codehaus/mojo/versions/PluginUpdatesReportMojo.java +++ b/src/main/java/org/codehaus/mojo/versions/PluginUpdatesReportMojo.java @@ -163,10 +163,8 @@ protected void doGenerateReport( Locale locale, Sink sink ) throws MavenReportEx if ( onlyUpgradable ) { - pluginUpdates = - filter( pluginUpdates, plugin -> plugin.getVersions().length > 1 ); - pluginManagementUpdates = filter( pluginManagementUpdates, - plugin -> plugin.getVersions().length > 1 ); + pluginUpdates = filter( pluginUpdates, p -> p.getVersions().length > 0 ); + pluginManagementUpdates = filter( pluginManagementUpdates, p -> p.getVersions().length > 0 ); } PluginUpdatesModel model = new PluginUpdatesModel( pluginUpdates, pluginManagementUpdates ); diff --git a/src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java b/src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java index ebcd62b7cc..ed8134ba89 100644 --- a/src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java +++ b/src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java @@ -339,6 +339,12 @@ public final ArtifactVersion[] getAllUpdates( Optional updateScope, boo return null; } + @Override + public final ArtifactVersion[] getAllUpdates() + { + return getAllUpdates( (VersionRange) null, isIncludeSnapshots() ); + } + @Override public final ArtifactVersion[] getAllUpdates( VersionRange versionRange ) { diff --git a/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java b/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java index 947333b60b..ed959429d8 100644 --- a/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java +++ b/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java @@ -38,7 +38,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; -import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -46,6 +45,8 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; import org.apache.maven.artifact.Artifact; import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.artifact.manager.WagonManager; @@ -668,109 +669,95 @@ public ArtifactVersion createArtifactVersion( String version ) return new DefaultArtifactVersion( version ); } + /** + * Returns a map of all possible updates per dependency. The lookup is done in parallel using + * {@code LOOKUP_PARALLEL_THREADS} threads. + * + * @param dependencies The set of {@link Dependency} instances to look up. + * @param usePluginRepositories Search the plugin repositories. + * @return map containing the ArtifactVersions object per dependency + * @throws ArtifactMetadataRetrievalException if the lookup does not succeed + */ @Override public Map lookupDependenciesUpdates( Set dependencies, boolean usePluginRepositories ) throws ArtifactMetadataRetrievalException { - // Create the request for details collection for parallel lookup... - final List> requestsForDetails = - new ArrayList<>( dependencies.size() ); - for ( final Dependency dependency : dependencies ) - { - requestsForDetails.add( new DependencyLookup( dependency, usePluginRepositories ) ); - } - - final Map dependencyUpdates = new TreeMap<>( DependencyComparator.INSTANCE ); - - // Lookup details in parallel... - final ExecutorService executor = Executors.newFixedThreadPool( LOOKUP_PARALLEL_THREADS ); + ExecutorService executor = Executors.newFixedThreadPool( LOOKUP_PARALLEL_THREADS ); try { - final List> responseForDetails = - executor.invokeAll( requestsForDetails ); - - // Construct the final results... - for ( final Future details : responseForDetails ) + Map dependencyUpdates = new TreeMap<>( DependencyComparator.INSTANCE ); + List>> futures = dependencies.stream() + .map( dependency -> executor.submit( () -> new ImmutablePair<> + ( dependency, lookupDependencyUpdates( dependency, usePluginRepositories ) ) ) ) + .collect( Collectors.toList() ); + for ( Future> details : futures ) { - final DependencyArtifactVersions dav = details.get(); - dependencyUpdates.put( dav.getDependency(), dav.getArtifactVersions() ); + Pair pair = details.get(); + dependencyUpdates.put( pair.getKey(), pair.getValue() ); } + + return dependencyUpdates; } catch ( ExecutionException | InterruptedException ie ) { throw new ArtifactMetadataRetrievalException( "Unable to acquire metadata for dependencies " + dependencies - + ": " + ie.getMessage(), ie, null ); + + ": " + ie.getMessage(), ie, null ); } finally { - executor.shutdownNow(); + executor.shutdown(); } - return dependencyUpdates; } @Override public ArtifactVersions lookupDependencyUpdates( Dependency dependency, boolean usePluginRepositories ) throws ArtifactMetadataRetrievalException { - getLog().debug( "Checking " - + ArtifactUtils.versionlessKey( dependency.getGroupId(), dependency.getArtifactId() ) - + " for updates newer than " + dependency.getVersion() ); - - return lookupArtifactVersions( createDependencyArtifact( dependency ), usePluginRepositories ); + ArtifactVersions allVersions = lookupArtifactVersions( createDependencyArtifact( dependency ), + usePluginRepositories ); + return new ArtifactVersions( allVersions.getArtifact(), Arrays.stream( allVersions.getAllUpdates() ) + .collect( Collectors.toList() ), allVersions.getVersionComparator() ); } @Override public Map lookupPluginsUpdates( Set plugins, boolean allowSnapshots ) throws ArtifactMetadataRetrievalException { - // Create the request for details collection for parallel lookup... - List> requestsForDetails = new ArrayList<>( plugins.size() ); - for ( final Plugin plugin : plugins ) - { - requestsForDetails.add( new PluginLookup( plugin, allowSnapshots ) ); - } - - Map pluginUpdates = new TreeMap<>( PluginComparator.INSTANCE ); - - // Lookup details in parallel... ExecutorService executor = Executors.newFixedThreadPool( LOOKUP_PARALLEL_THREADS ); try { - final List> responseForDetails = - executor.invokeAll( requestsForDetails ); - - // Construct the final results... - for ( final Future details : responseForDetails ) + Map pluginUpdates = new TreeMap<>( PluginComparator.INSTANCE ); + List>> futures = plugins.stream() + .map( p -> executor.submit( () -> new ImmutablePair<> + ( p, lookupPluginUpdates( p, allowSnapshots ) ) ) ) + .collect( Collectors.toList() ); + for ( Future> details : futures ) { - final PluginPluginUpdatesDetails pud = details.get(); - pluginUpdates.put( pud.getPlugin(), pud.getPluginUpdatesDetails() ); + Pair pair = details.get(); + pluginUpdates.put( pair.getKey(), pair.getValue() ); } + + return pluginUpdates; } catch ( ExecutionException | InterruptedException ie ) { throw new ArtifactMetadataRetrievalException( "Unable to acquire metadata for plugins " + plugins + ": " - + ie.getMessage(), ie, null ); + + ie.getMessage(), ie, null ); } finally { - executor.shutdownNow(); + executor.shutdown(); } - return pluginUpdates; } @Override public PluginUpdatesDetails lookupPluginUpdates( Plugin plugin, boolean allowSnapshots ) throws ArtifactMetadataRetrievalException { - String version = plugin.getVersion(); - version = version == null ? "LATEST" : version; - getLog().debug( "Checking " + ArtifactUtils.versionlessKey( plugin.getGroupId(), plugin.getArtifactId() ) - + " for updates newer than " + version ); - - final ArtifactVersions pluginArtifactVersions = - lookupArtifactVersions( createPluginArtifact( plugin.getGroupId(), plugin.getArtifactId(), version ), - true ); + String version = plugin.getVersion() != null + ? plugin.getVersion() + : "LATEST"; Set pluginDependencies = new TreeSet<>( DependencyComparator.INSTANCE ); if ( plugin.getDependencies() != null ) @@ -780,7 +767,13 @@ public PluginUpdatesDetails lookupPluginUpdates( Plugin plugin, boolean allowSna Map pluginDependencyDetails = lookupDependenciesUpdates( pluginDependencies, false ); - return new PluginUpdatesDetails( pluginArtifactVersions, pluginDependencyDetails, allowSnapshots ); + ArtifactVersions allVersions = + lookupArtifactVersions( createPluginArtifact( plugin.getGroupId(), plugin.getArtifactId(), version ), + true ); + ArtifactVersions updatedVersions = new ArtifactVersions( allVersions.getArtifact(), + Arrays.stream( allVersions.getAllUpdates() ).collect( Collectors.toList() ), + allVersions.getVersionComparator() ); + return new PluginUpdatesDetails( updatedVersions, pluginDependencyDetails, allowSnapshots ); } @Override @@ -832,8 +825,8 @@ public Map getVersionPropertiesMap( MavenProject pro } } - List includePropertiesList = getSplittedProperties( includeProperties ); - List excludePropertiesList = getSplittedProperties( excludeProperties ); + List includePropertiesList = getSplitProperties( includeProperties ); + List excludePropertiesList = getSplitProperties( excludeProperties ); getLog().debug( "Searching for properties associated with builders" ); Iterator i = properties.values().iterator(); @@ -902,7 +895,7 @@ else if ( !excludePropertiesList.isEmpty() && excludePropertiesList.contains( pr return propertyVersions; } - private List getSplittedProperties( String commaSeparatedProperties ) + private List getSplitProperties( String commaSeparatedProperties ) { List propertiesList = Collections.emptyList(); if ( StringUtils.isNotEmpty( commaSeparatedProperties ) ) @@ -913,97 +906,6 @@ private List getSplittedProperties( String commaSeparatedProperties ) return propertiesList; } - // This is a data container to hold the result of a Dependency lookup to its ArtifactVersions. - private static class DependencyArtifactVersions - { - private final Dependency dependency; - - private final ArtifactVersions artifactVersions; - - DependencyArtifactVersions( final Dependency dependency, final ArtifactVersions artifactVersions ) - { - this.dependency = dependency; - this.artifactVersions = artifactVersions; - } - - public Dependency getDependency() - { - return dependency; - } - - public ArtifactVersions getArtifactVersions() - { - return artifactVersions; - } - } - - // This is a data container to hold the result of a Dependency lookup to its ArtifactVersions. - private static class PluginPluginUpdatesDetails - { - private final Plugin plugin; - - private final PluginUpdatesDetails pluginUpdatesDetails; - - PluginPluginUpdatesDetails( final Plugin plugin, final PluginUpdatesDetails pluginUpdatesDetails ) - { - this.plugin = plugin; - this.pluginUpdatesDetails = pluginUpdatesDetails; - } - - public Plugin getPlugin() - { - return plugin; - } - - public PluginUpdatesDetails getPluginUpdatesDetails() - { - return pluginUpdatesDetails; - } - } - - // This Callable wraps lookupDependencyUpdates so that it can be run in parallel. - private class DependencyLookup - implements Callable - { - private final Dependency dependency; - - private final boolean usePluginRepositories; - - DependencyLookup( final Dependency dependency, final boolean usePluginRepositories ) - { - this.dependency = dependency; - this.usePluginRepositories = usePluginRepositories; - } - - public DependencyArtifactVersions call() - throws Exception - { - return new DependencyArtifactVersions( dependency, - lookupDependencyUpdates( dependency, usePluginRepositories ) ); - } - } - - // This Callable wraps lookupPluginUpdates so that it can be run in parallel. - private class PluginLookup - implements Callable - { - private final Plugin plugin; - - private final boolean allowSnapshots; - - PluginLookup( final Plugin plugin, final Boolean allowSnapshots ) - { - this.plugin = plugin; - this.allowSnapshots = allowSnapshots; - } - - public PluginPluginUpdatesDetails call() - throws Exception - { - return new PluginPluginUpdatesDetails( plugin, lookupPluginUpdates( plugin, allowSnapshots ) ); - } - } - /** * Builder class for {@linkplain DefaultVersionsHelper} */ diff --git a/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java b/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java index bd414b40fc..265b4fcfaa 100644 --- a/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java +++ b/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java @@ -348,6 +348,14 @@ ArtifactVersion getNewestUpdate( Optional updateScope, boolean includeS ArtifactVersion[] getAllUpdates( Optional updateScope, boolean includeSnapshots ) throws InvalidSegmentException; + /** + * Returns the all versions newer than the specified current version + * + * @return the all versions after currentVersion + * @since 2.13.0 + */ + ArtifactVersion[] getAllUpdates(); + /** * Returns the all versions newer than the specified current version, but within the specified update scope. * diff --git a/src/test/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojoTest.java b/src/test/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojoTest.java index d14013bdc6..3b31c751f4 100644 --- a/src/test/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojoTest.java +++ b/src/test/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojoTest.java @@ -149,6 +149,12 @@ public TestDependencyUpdatesReportMojo withIgnoredVersions( return this; } + public TestDependencyUpdatesReportMojo withAllowSnapshots( boolean allowSnapshots ) + { + this.allowSnapshots = allowSnapshots; + return this; + } + private static RepositorySystem mockRepositorySystem() { RepositorySystem repositorySystem = mock( RepositorySystem.class ); @@ -183,13 +189,21 @@ public void testOnlyUpgradableDependencies() throws IOException, MavenReportExce SinkFactory sinkFactory = new Xhtml5SinkFactory(); new TestDependencyUpdatesReportMojo() .withOnlyUpgradable( true ) + .withArtifactMetadataSource( mockArtifactMetadataSource( new HashMap() + {{ + put( "artifactA", new String[] { "1.0.0", "2.0.0" } ); + put( "artifactB", new String[] { "1.0.0" } ); + put( "artifactC", new String[] { "1.0.0", "2.0.0" } ); + }} ) ) .withDependencies( - dependencyOf( "artifactA" ), dependencyOf( "artifactB" ), - dependencyOf( "artifactC" ) ) + dependencyOf( "artifactA", "1.0.0" ), + dependencyOf( "artifactB", "1.0.0" ), + dependencyOf( "artifactC", "2.0.0" ) ) .generate( sinkFactory.createSink( os ), sinkFactory, Locale.getDefault() ); String output = os.toString(); - assertThat( output, allOf( containsString( "artifactA" ), containsString( "artifactB" ) ) ); + assertThat( output, containsString( "artifactA" ) ); + assertThat( output, not( containsString( "artifactB" ) ) ); assertThat( output, not( containsString( "artifactC" ) ) ); } diff --git a/src/test/java/org/codehaus/mojo/versions/PluginUpdatesReportMojoTest.java b/src/test/java/org/codehaus/mojo/versions/PluginUpdatesReportMojoTest.java index 25f2ea781a..46053944ce 100644 --- a/src/test/java/org/codehaus/mojo/versions/PluginUpdatesReportMojoTest.java +++ b/src/test/java/org/codehaus/mojo/versions/PluginUpdatesReportMojoTest.java @@ -24,10 +24,12 @@ import java.io.OutputStream; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.Locale; import java.util.Set; import org.apache.maven.artifact.DefaultArtifact; +import org.apache.maven.artifact.metadata.ArtifactMetadataSource; import org.apache.maven.doxia.module.xhtml5.Xhtml5SinkFactory; import org.apache.maven.doxia.sink.SinkFactory; import org.apache.maven.model.Build; @@ -45,7 +47,6 @@ import static org.apache.maven.artifact.Artifact.SCOPE_RUNTIME; import static org.codehaus.mojo.versions.utils.MockUtils.mockArtifactMetadataSource; import static org.codehaus.mojo.versions.utils.MockUtils.mockI18N; -import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; @@ -84,6 +85,13 @@ public TestPluginUpdatesReportMojo withPlugins( Plugin... plugins ) return this; } + public TestPluginUpdatesReportMojo withArtifactMetadataSource( + ArtifactMetadataSource artifactMetadataSource ) + { + this.artifactMetadataSource = artifactMetadataSource; + return this; + } + public TestPluginUpdatesReportMojo withPluginManagement( Plugin... pluginManagement ) { project.getBuild().getPluginManagement().setPlugins( Arrays.asList( pluginManagement ) ); @@ -131,13 +139,18 @@ private static RepositorySystem mockRepositorySystem() } private static Plugin pluginOf( String artifactId ) + { + return pluginOf( artifactId, "1.0.0" ); + } + + private static Plugin pluginOf( String artifactId, String version ) { return new Plugin() { { setGroupId( "defaultGroup" ); setArtifactId( artifactId ); - setVersion( "1.0.0" ); + setVersion( version ); } }; } @@ -148,13 +161,21 @@ public void testOnlyUpgradablePlugins() throws IOException, MavenReportException OutputStream os = new ByteArrayOutputStream(); SinkFactory sinkFactory = new Xhtml5SinkFactory(); new TestPluginUpdatesReportMojo() - .withPlugins( pluginOf( "artifactA" ), pluginOf( "artifactB" ), - pluginOf( "artifactC" ) ) - .withOnlyUpgradable( true ) - .generate( sinkFactory.createSink( os ), sinkFactory, Locale.getDefault() ); + .withArtifactMetadataSource( mockArtifactMetadataSource( new HashMap() + {{ + put( "artifactA", new String[] { "1.0.0", "2.0.0" } ); + put( "artifactB", new String[] { "1.0.0" } ); + put( "artifactC", new String[] { "1.0.0", "2.0.0" } ); + }} ) ) + .withPlugins( pluginOf( "artifactA", "1.0.0" ), + pluginOf( "artifactB", "1.0.0" ), + pluginOf( "artifactC", "2.0.0" ) ) + .withOnlyUpgradable( true ) + .generate( sinkFactory.createSink( os ), sinkFactory, Locale.getDefault() ); String output = os.toString(); - assertThat( output, allOf( containsString( "artifactA" ), containsString( "artifactB" ) ) ); + assertThat( output, containsString( "artifactA" ) ); + assertThat( output, not( containsString( "artifactB" ) ) ); assertThat( output, not( containsString( "artifactC" ) ) ); } @@ -164,13 +185,21 @@ public void testOnlyUpgradableWithPluginManagement() throws IOException, MavenRe OutputStream os = new ByteArrayOutputStream(); SinkFactory sinkFactory = new Xhtml5SinkFactory(); new TestPluginUpdatesReportMojo() - .withPluginManagement( pluginOf( "artifactA" ), pluginOf( "artifactB" ), - pluginOf( "artifactC" ) ) + .withArtifactMetadataSource( mockArtifactMetadataSource( new HashMap() + {{ + put( "artifactA", new String[] { "1.0.0", "2.0.0" } ); + put( "artifactB", new String[] { "1.0.0" } ); + put( "artifactC", new String[] { "1.0.0", "2.0.0" } ); + }} ) ) + .withPluginManagement( pluginOf( "artifactA", "1.0.0" ), + pluginOf( "artifactB", "1.0.0" ), + pluginOf( "artifactC", "2.0.0" ) ) .withOnlyUpgradable( true ) .generate( sinkFactory.createSink( os ), sinkFactory, Locale.getDefault() ); String output = os.toString(); - assertThat( output, allOf( containsString( "artifactA" ), containsString( "artifactB" ) ) ); + assertThat( output, containsString( "artifactA" ) ); + assertThat( output, not( containsString( "artifactB" ) ) ); assertThat( output, not( containsString( "artifactC" ) ) ); }