Skip to content

Commit

Permalink
[MENFORCER-359] RequirePluginVersions fails when versions are inherited
Browse files Browse the repository at this point in the history
  • Loading branch information
@rfscholte
rfscholte committed Jul 17, 2021
1 parent e10ed74 commit ca73329
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 145 deletions.
116 changes: 11 additions & 105 deletions enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequirePluginVersions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -168,9 +167,6 @@ public class RequirePluginVersions
/** The local. */
private ArtifactRepository local;

/** The remote repositories. */
private List<ArtifactRepository> remoteRepositories;

/** The log. */
private Log log;

Expand Down Expand Up @@ -202,7 +198,6 @@ public void execute( EnforcerRuleHelper helper )
factory = helper.getComponent( ArtifactFactory.class );
resolver = helper.getComponent( ArtifactResolver.class );
local = (ArtifactRepository) helper.evaluate( "${localRepository}" );
remoteRepositories = project.getRemoteArtifactRepositories();

utils = new EnforcerRuleUtils( helper );

Expand Down Expand Up @@ -234,7 +229,7 @@ public void execute( EnforcerRuleHelper helper )
for ( PluginWrapper pluginWrapper : pluginWrappers )
{
log.debug( "pluginWrappers: " + pluginWrapper.getGroupId() + ":" + pluginWrapper.getArtifactId() + ":"
+ pluginWrapper.getVersion() + " source:" + pluginWrapper.getSource() );
+ pluginWrapper.getVersion() + " source: " + pluginWrapper.getSource() );
}
// now look for the versions that aren't valid and add to a list.
List<Plugin> failures = new ArrayList<Plugin>();
Expand Down Expand Up @@ -721,17 +716,6 @@ private Set<Plugin> getAllPlugins( MavenProject project, Lifecycle lifecycle )
}
}

List<String> mojos = findOptionalMojosForLifecycle( project, lifecycle );
for ( String value : mojos )
{
String tokens[] = value.split( ":" );

Plugin plugin = new Plugin();
plugin.setGroupId( tokens[0] );
plugin.setArtifactId( tokens[1] );
plugins.add( plugin );
}

plugins.addAll( project.getBuildPlugins() );

return plugins;
Expand Down Expand Up @@ -854,51 +838,6 @@ private Map<String, String> findMappingsForLifecycle( MavenProject project, Life
return mappings;
}

/**
* Find optional mojos for lifecycle.
*
* @param project the project
* @param lifecycle the lifecycle
* @return the list
* @throws LifecycleExecutionException the lifecycle execution exception
* @throws PluginNotFoundException the plugin not found exception
*/
private List<String> findOptionalMojosForLifecycle( MavenProject project, Lifecycle lifecycle )
throws LifecycleExecutionException, PluginNotFoundException
{
String packaging = project.getPackaging();
List<String> optionalMojos = null;

LifecycleMapping m = (LifecycleMapping) findExtension( project, LifecycleMapping.ROLE, packaging,
session.getSettings(), session.getLocalRepository() );

if ( m != null )
{
optionalMojos = m.getOptionalMojos( lifecycle.getId() );
}

if ( optionalMojos == null )
{
try
{
m = helper.getComponent( LifecycleMapping.class, packaging );
optionalMojos = m.getOptionalMojos( lifecycle.getId() );
}
catch ( ComponentLookupException e )
{
log.debug( "Error looking up lifecycle mapping to retrieve optional mojos. Lifecycle ID: "
+ lifecycle.getId() + ". Error: " + e.getMessage(), e );
}
}

if ( optionalMojos == null )
{
optionalMojos = Collections.emptyList();
}

return optionalMojos;
}

/**
* Find extension.
*
Expand Down Expand Up @@ -998,45 +937,18 @@ private PluginDescriptor verifyPlugin( Plugin plugin, MavenProject project, Sett
protected List<PluginWrapper> getAllPluginEntries( MavenProject project )
throws ArtifactResolutionException, ArtifactNotFoundException, IOException, XmlPullParserException
{
List<Model> models = new ArrayList<>();

List<MavenProject> sortedProjects = session.getProjectDependencyGraph().getSortedProjects();

if ( !sortedProjects.isEmpty() && sortedProjects.get( 0 ).getParent() != null )
{
getOriginalModelFromAllParents( models, sortedProjects );
}

for ( MavenProject mavenProject : sortedProjects )
{
models.add( mavenProject.getOriginalModel() );
}

List<PluginWrapper> plugins = new ArrayList<>();
// now find all the plugin entries, either in
// build.plugins or build.pluginManagement.plugins, profiles.plugins and reporting
for ( Model model : models )
{
getPlugins( plugins, model );
getReportingPlugins( plugins, model );
getPluginManagementPlugins( plugins, model );

addPluginsInProfiles( plugins, model );
}
getPlugins( plugins, project.getModel() );
getReportingPlugins( plugins, project.getModel() );
getPluginManagementPlugins( plugins, project.getModel() );
addPluginsInProfiles( plugins, project.getModel() );

return plugins;
}

private void getOriginalModelFromAllParents( List<Model> models, List<MavenProject> sortedProjects )
{
MavenProject parent = sortedProjects.get( 0 ).getParent();
do
{
models.add( parent.getOriginalModel() );
parent = parent.getParent();
}
while ( parent != null );
}

private void addPluginsInProfiles( List<PluginWrapper> plugins, Model model )
{
Expand All @@ -1054,8 +966,7 @@ private void getProfilePluginManagementPlugins( List<PluginWrapper> plugins, Mod
try
{
List<Plugin> modelPlugins = profile.getBuild().getPluginManagement().getPlugins();
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ), model.getId() + "profile["
+ profile.getId() + "].build.pluginManagement.plugins" ) );
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ) ) );
}
catch ( NullPointerException e )
{
Expand All @@ -1069,8 +980,7 @@ private void getProfileReportingPlugins( List<PluginWrapper> plugins, Model mode
{
List<ReportPlugin> modelReportPlugins = profile.getReporting().getPlugins();
// add the reporting plugins
plugins.addAll( PluginWrapper.addAll( utils.resolveReportPlugins( modelReportPlugins ), model.getId()
+ "profile[" + profile.getId() + "].reporting.plugins" ) );
plugins.addAll( PluginWrapper.addAll( utils.resolveReportPlugins( modelReportPlugins ) ) );
}
catch ( NullPointerException e )
{
Expand All @@ -1083,8 +993,7 @@ private void getProfilePlugins( List<PluginWrapper> plugins, Model model, Profil
try
{
List<Plugin> modelPlugins = profile.getBuild().getPlugins();
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ), model.getId()
+ ".profiles.profile[" + profile.getId() + "].build.plugins" ) );
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ) ) );
}
catch ( NullPointerException e )
{
Expand All @@ -1097,8 +1006,7 @@ private void getPlugins( List<PluginWrapper> plugins, Model model )
try
{
List<Plugin> modelPlugins = model.getBuild().getPlugins();
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ),
model.getId() + ".build.plugins" ) );
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ) ) );
}
catch ( NullPointerException e )
{
Expand All @@ -1111,8 +1019,7 @@ private void getPluginManagementPlugins( List<PluginWrapper> plugins, Model mode
try
{
List<Plugin> modelPlugins = model.getBuild().getPluginManagement().getPlugins();
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ),
model.getId() + ".build.pluginManagement.plugins" ) );
plugins.addAll( PluginWrapper.addAll( utils.resolvePlugins( modelPlugins ) ) );
}
catch ( NullPointerException e )
{
Expand All @@ -1126,8 +1033,7 @@ private void getReportingPlugins( List<PluginWrapper> plugins, Model model )
{
List<ReportPlugin> modelReportPlugins = model.getReporting().getPlugins();
// add the reporting plugins
plugins.addAll( PluginWrapper.addAll( utils.resolveReportPlugins( modelReportPlugins ),
model.getId() + ".reporting" ) );
plugins.addAll( PluginWrapper.addAll( utils.resolveReportPlugins( modelReportPlugins ) ) );
}
catch ( NullPointerException e )
{
Expand Down
68 changes: 30 additions & 38 deletions enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/PluginWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.ArrayList;
import java.util.List;

import org.apache.maven.model.InputLocation;
import org.apache.maven.model.InputLocationTracker;
import org.apache.maven.model.Plugin;
import org.apache.maven.model.ReportPlugin;

Expand All @@ -31,15 +33,15 @@
*/
public class PluginWrapper
{
private String groupId;
private final String groupId;

private String artifactId;
private final String artifactId;

private String version;
private final String version;

private String source;
private final InputLocationTracker locationTracker;

public static List<PluginWrapper> addAll( List<?> plugins, String source )
public static List<PluginWrapper> addAll( List<?> plugins )
{
List<PluginWrapper> results = null;

Expand All @@ -50,13 +52,13 @@ public static List<PluginWrapper> addAll( List<?> plugins, String source )
{
if ( o instanceof Plugin )
{
results.add( new PluginWrapper( (Plugin) o, source ) );
results.add( new PluginWrapper( (Plugin) o ) );
}
else
{
if ( o instanceof ReportPlugin )
{
results.add( new PluginWrapper( (ReportPlugin) o, source ) );
results.add( new PluginWrapper( (ReportPlugin) o ) );
}
}

Expand All @@ -65,59 +67,49 @@ public static List<PluginWrapper> addAll( List<?> plugins, String source )
return results;
}

public PluginWrapper( Plugin plugin, String source )
private PluginWrapper( Plugin plugin )
{
setGroupId( plugin.getGroupId() );
setArtifactId( plugin.getArtifactId() );
setVersion( plugin.getVersion() );
setSource( source );
this.groupId = plugin.getGroupId();
this.artifactId = plugin.getArtifactId();
this.version = plugin.getVersion();
this.locationTracker = plugin;
}

public PluginWrapper( ReportPlugin plugin, String source )
private PluginWrapper( ReportPlugin plugin )
{
setGroupId( plugin.getGroupId() );
setArtifactId( plugin.getArtifactId() );
setVersion( plugin.getVersion() );
setSource( source );
this.groupId = plugin.getGroupId();
this.artifactId = plugin.getArtifactId();
this.version = plugin.getVersion();
this.locationTracker = plugin;
}

public String getGroupId()
{
return groupId;
}

public void setGroupId( String groupId )
{
this.groupId = groupId;
}

public String getArtifactId()
{
return artifactId;
}

public void setArtifactId( String artifactId )
{
this.artifactId = artifactId;
}

public String getVersion()
{
return version;
}

public void setVersion( String version )
{
this.version = version;
}

public String getSource()
{
return source;
}

public void setSource( String source )
{
this.source = source;
InputLocation inputLocation = locationTracker.getLocation( "version" );

if ( inputLocation == null )
{
// most likely super-pom or default-lifecycle-bindings in Maven 3.6.0 or before (MNG-6593 / MNG-6600)
return "unknown";
}
else
{
return inputLocation.getSource().getLocation();
}
}
}
4 changes: 2 additions & 2 deletions ...rcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequirePluginVersions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public void testHasVersionSpecified()
plugins.add( EnforcerTestUtils.newPlugin( "group", "g-artifact", "1.0-12345678.123456-1" ) );


List<PluginWrapper> pluginWrappers = PluginWrapper.addAll( plugins, "unit" );
List<PluginWrapper> pluginWrappers = PluginWrapper.addAll( plugins );

RequirePluginVersions rule = new RequirePluginVersions();
rule.setBanLatest( false );
Expand Down Expand Up @@ -145,7 +145,7 @@ public void testHasVersionSpecifiedWithProperties()
plugins.add( EnforcerTestUtils.newPlugin( "group", "e-artifact", "${}" ) );
plugins.add( EnforcerTestUtils.newPlugin( "group", "f-artifact", "${ }" ) );

List<PluginWrapper> pluginWrappers = PluginWrapper.addAll( plugins, "unit" );
List<PluginWrapper> pluginWrappers = PluginWrapper.addAll( plugins );

RequirePluginVersions rule = new RequirePluginVersions();
rule.setBanLatest( false );
Expand Down
Loading

0 comments on commit ca73329

Please sign in to comment.