[MNG-1891] Fixed plugin ordering in profile injection AND model inheritance, to be consistent and to preserve as much ordering information as possible, to make plugin ordering more predictable. Also added several unit tests to express the problem(s) and verify the solutions. Ordering is in javadoc comments, and should be added to the plugin-configuration documentation on the site.

git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@425919 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
John Dennis Casey 2006-07-27 00:36:31 +00:00
parent 45d97d04d3
commit 5d50ceac1f
4 changed files with 322 additions and 50 deletions

View File

@ -57,6 +57,19 @@ import java.util.HashMap;
public final class ModelUtils
{
/**
* This should be the resulting ordering of plugins after merging:
*
* Given:
*
* parent: X -> A -> B -> D -> E
* child: Y -> A -> C -> D -> F
*
* Result:
*
* X -> Y -> A -> B -> C -> D -> E -> F
*/
public static void mergePluginLists( PluginContainer childContainer, PluginContainer parentContainer,
boolean handleAsInheritance )
{
@ -66,16 +79,33 @@ public final class ModelUtils
return;
}
List mergedPlugins = new ArrayList();
List parentPlugins = parentContainer.getPlugins();
if ( parentPlugins != null && !parentPlugins.isEmpty() )
{
Map assembledPlugins = new TreeMap();
parentPlugins = new ArrayList( parentPlugins );
// If we're processing this merge as an inheritance, we have to build up a list of
// plugins that were considered for inheritance.
if ( handleAsInheritance )
{
for ( Iterator it = parentPlugins.iterator(); it.hasNext(); )
{
Plugin plugin = (Plugin) it.next();
String inherited = plugin.getInherited();
if ( inherited != null && !Boolean.valueOf( inherited ).booleanValue() )
{
it.remove();
}
}
}
List assembledPlugins = new ArrayList();
Map childPlugins = childContainer.getPluginsAsMap();
for ( Iterator it = parentPlugins.iterator(); it.hasNext(); )
{
Plugin parentPlugin = (Plugin) it.next();
@ -90,16 +120,16 @@ public final class ModelUtils
if ( !handleAsInheritance || parentInherited == null ||
Boolean.valueOf( parentInherited ).booleanValue() )
{
Plugin assembledPlugin = parentPlugin;
Plugin childPlugin = (Plugin) childPlugins.get( parentPlugin.getKey() );
if ( childPlugin != null )
if ( childPlugin != null && !assembledPlugins.contains( childPlugin ) )
{
assembledPlugin = childPlugin;
Plugin assembledPlugin = childPlugin;
mergePluginDefinitions( childPlugin, parentPlugin, handleAsInheritance );
// fix for MNG-2221 (assembly cache was not being populated for later reference):
assembledPlugins.add( assembledPlugin );
}
// if we're processing this as an inheritance-based merge, and
@ -107,32 +137,80 @@ public final class ModelUtils
// clear the inherited flag in the merge result.
if ( handleAsInheritance && parentInherited == null )
{
assembledPlugin.unsetInheritanceApplied();
parentPlugin.unsetInheritanceApplied();
}
mergedPlugins.add(assembledPlugin);
// fix for MNG-2221 (assembly cache was not being populated for later reference):
assembledPlugins.put( assembledPlugin.getKey(), assembledPlugin );
}
// very important to use the parentPlugins List, rather than parentContainer.getPlugins()
// since this list is a local one, and may have been modified during processing.
List results = ModelUtils.orderAfterMerge( assembledPlugins, parentPlugins,
childContainer.getPlugins() );
childContainer.setPlugins( results );
childContainer.flushPluginMap();
}
for ( Iterator it = childPlugins.values().iterator(); it.hasNext(); )
{
Plugin childPlugin = (Plugin) it.next();
if ( !assembledPlugins.containsKey( childPlugin.getKey() ) )
{
mergedPlugins.add(childPlugin);
}
}
childContainer.setPlugins(mergedPlugins);
childContainer.flushPluginMap();
}
}
public static List orderAfterMerge( List merged, List highPrioritySource, List lowPrioritySource )
{
List results = new ArrayList();
if ( !merged.isEmpty() )
{
results.addAll( merged );
}
List missingFromResults = new ArrayList();
List sources = new ArrayList();
sources.add( highPrioritySource );
sources.add( lowPrioritySource );
for ( Iterator sourceIterator = sources.iterator(); sourceIterator.hasNext(); )
{
List source = (List) sourceIterator.next();
for ( Iterator it = source.iterator(); it.hasNext(); )
{
Object item = it.next();
if ( results.contains( item ) )
{
if ( !missingFromResults.isEmpty() )
{
int idx = results.indexOf( item );
if ( idx < 0 )
{
idx = 0;
}
results.addAll( idx, missingFromResults );
missingFromResults.clear();
}
}
else
{
missingFromResults.add( item );
}
}
if ( !missingFromResults.isEmpty() )
{
results.addAll( missingFromResults );
missingFromResults.clear();
}
}
return results;
}
public static void mergeReportPluginLists( Reporting child, Reporting parent, boolean handleAsInheritance )
{
if ( child == null || parent == null )

View File

@ -136,7 +136,19 @@ public class DefaultProfileInjector
}
}
private void injectPlugins( PluginContainer profileContainer, PluginContainer modelContainer )
/**
* This should be the resulting ordering of plugins after injection:
*
* Given:
*
* model: X -> A -> B -> D -> E
* profile: Y -> A -> C -> D -> F
*
* Result:
*
* X -> Y -> A -> B -> C -> D -> E -> F
*/
protected void injectPlugins( PluginContainer profileContainer, PluginContainer modelContainer )
{
if ( profileContainer == null || modelContainer == null )
{
@ -152,7 +164,7 @@ public class DefaultProfileInjector
}
else if ( profileContainer.getPlugins() != null )
{
Map mergedPlugins = new TreeMap();
List mergedPlugins = new ArrayList();
Map profilePlugins = profileContainer.getPluginsAsMap();
@ -160,31 +172,21 @@ public class DefaultProfileInjector
{
Plugin modelPlugin = (Plugin) it.next();
Plugin mergedPlugin = modelPlugin;
Plugin profilePlugin = (Plugin) profilePlugins.get( modelPlugin.getKey() );
if ( profilePlugin != null )
if ( profilePlugin != null && !mergedPlugins.contains( profilePlugin ) )
{
mergedPlugin = modelPlugin;
Plugin mergedPlugin = modelPlugin;
injectPluginDefinition( profilePlugin, modelPlugin );
}
mergedPlugins.put( mergedPlugin.getKey(), mergedPlugin );
}
for ( Iterator it = profilePlugins.values().iterator(); it.hasNext(); )
{
Plugin profilePlugin = (Plugin) it.next();
if ( !mergedPlugins.containsKey( profilePlugin.getKey() ) )
{
mergedPlugins.put( profilePlugin.getKey(), profilePlugin );
mergedPlugins.add( mergedPlugin );
}
}
modelContainer.setPlugins( new ArrayList( mergedPlugins.values() ) );
List results = ModelUtils.orderAfterMerge( mergedPlugins, modelPlugins, profileContainer.getPlugins() );
modelContainer.setPlugins( results );
modelContainer.flushPluginMap();
}

View File

@ -7,9 +7,12 @@ import org.apache.maven.model.Plugin;
import org.apache.maven.model.PluginContainer;
import org.apache.maven.model.PluginExecution;
import org.apache.maven.model.Dependency;
import org.codehaus.plexus.util.xml.Xpp3Dom;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
/*
* Copyright 2001-2005 The Apache Software Foundation.
@ -30,6 +33,111 @@ import java.util.List;
public class ModelUtilsTest
extends TestCase
{
public void testShouldNotInheritPluginWithInheritanceSetToFalse()
{
PluginContainer parent = new PluginContainer();
Plugin parentPlugin = createPlugin( "group", "artifact", "1.0", Collections.EMPTY_MAP );
parentPlugin.setInherited( "false" );
parent.addPlugin( parentPlugin );
PluginContainer child = new PluginContainer();
child.addPlugin( createPlugin( "group3", "artifact3", "1.0", Collections.EMPTY_MAP ) );
ModelUtils.mergePluginLists( child, parent, true );
List results = child.getPlugins();
assertEquals( 1, results.size() );
Plugin result1 = (Plugin) results.get( 0 );
assertEquals( "group3", result1.getGroupId() );
assertEquals( "artifact3", result1.getArtifactId() );
}
/**
* Test that this is the resulting ordering of plugins after merging:
*
* Given:
*
* parent: X -> A -> B -> D -> E
* child: Y -> A -> C -> D -> F
*
* Result:
*
* X -> Y -> A -> B -> C -> D -> E -> F
*/
public void testShouldPreserveChildOrderingOfPluginsAfterParentMerge()
{
PluginContainer parent = new PluginContainer();
parent.addPlugin( createPlugin( "group", "artifact", "1.0", Collections.EMPTY_MAP ) );
parent.addPlugin( createPlugin( "group2", "artifact2", "1.0", Collections.singletonMap( "key", "value" ) ) );
PluginContainer child = new PluginContainer();
child.addPlugin( createPlugin( "group3", "artifact3", "1.0", Collections.EMPTY_MAP ) );
child.addPlugin( createPlugin( "group2", "artifact2", "1.0", Collections.singletonMap( "key2", "value2" ) ) );
ModelUtils.mergePluginLists( child, parent, true );
List results = child.getPlugins();
assertEquals( 3, results.size() );
Plugin result1 = (Plugin) results.get( 0 );
assertEquals( "group", result1.getGroupId() );
assertEquals( "artifact", result1.getArtifactId() );
Plugin result2 = (Plugin) results.get( 1 );
assertEquals( "group3", result2.getGroupId() );
assertEquals( "artifact3", result2.getArtifactId() );
Plugin result3 = (Plugin) results.get( 2 );
assertEquals( "group2", result3.getGroupId() );
assertEquals( "artifact2", result3.getArtifactId() );
Xpp3Dom result3Config = (Xpp3Dom) result3.getConfiguration();
assertNotNull( result3Config );
assertEquals( "value", result3Config.getChild( "key" ).getValue() );
assertEquals( "value2", result3Config.getChild( "key2" ).getValue() );
}
private Plugin createPlugin( String groupId, String artifactId, String version, Map configuration )
{
Plugin plugin = new Plugin();
plugin.setGroupId( groupId );
plugin.setArtifactId( artifactId );
plugin.setVersion( version );
Xpp3Dom config = new Xpp3Dom( "configuration" );
if( configuration != null )
{
for ( Iterator it = configuration.entrySet().iterator(); it.hasNext(); )
{
Map.Entry entry = (Map.Entry) it.next();
Xpp3Dom param = new Xpp3Dom( String.valueOf( entry.getKey() ) );
param.setValue( String.valueOf( entry.getValue() ) );
config.addChild( param );
}
}
plugin.setConfiguration( config );
return plugin;
}
public void testShouldInheritOnePluginWithExecution()
{
Plugin parent = new Plugin();

View File

@ -1,22 +1,106 @@
package org.apache.maven.project.injection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import junit.framework.TestCase;
import org.apache.maven.model.Build;
import org.apache.maven.model.BuildBase;
import org.apache.maven.model.Model;
import org.apache.maven.model.Plugin;
import org.apache.maven.model.PluginContainer;
import org.apache.maven.model.PluginExecution;
import org.apache.maven.model.Profile;
import org.apache.maven.model.Repository;
import org.codehaus.plexus.util.xml.Xpp3Dom;
import java.util.List;
public class DefaultProfileInjectorTest
extends TestCase
{
/**
* Test that this is the resulting ordering of plugins after merging:
*
* Given:
*
* model: X -> A -> B -> D -> E
* profile: Y -> A -> C -> D -> F
*
* Result:
*
* X -> Y -> A -> B -> C -> D -> E -> F
*/
public void testShouldPreserveOrderingOfPluginsAfterProfileMerge()
{
PluginContainer profile = new PluginContainer();
profile.addPlugin( createPlugin( "group", "artifact", "1.0", Collections.EMPTY_MAP ) );
profile.addPlugin( createPlugin( "group2", "artifact2", "1.0", Collections.singletonMap( "key", "value" ) ) );
PluginContainer model = new PluginContainer();
model.addPlugin( createPlugin( "group3", "artifact3", "1.0", Collections.EMPTY_MAP ) );
model.addPlugin( createPlugin( "group2", "artifact2", "1.0", Collections.singletonMap( "key2", "value2" ) ) );
new DefaultProfileInjector().injectPlugins( profile, model );
List results = model.getPlugins();
assertEquals( 3, results.size() );
Plugin result1 = (Plugin) results.get( 0 );
assertEquals( "group3", result1.getGroupId() );
assertEquals( "artifact3", result1.getArtifactId() );
Plugin result2 = (Plugin) results.get( 1 );
assertEquals( "group", result2.getGroupId() );
assertEquals( "artifact", result2.getArtifactId() );
Plugin result3 = (Plugin) results.get( 2 );
assertEquals( "group2", result3.getGroupId() );
assertEquals( "artifact2", result3.getArtifactId() );
Xpp3Dom result3Config = (Xpp3Dom) result3.getConfiguration();
assertNotNull( result3Config );
assertEquals( "value", result3Config.getChild( "key" ).getValue() );
assertEquals( "value2", result3Config.getChild( "key2" ).getValue() );
}
private Plugin createPlugin( String groupId, String artifactId, String version, Map configuration )
{
Plugin plugin = new Plugin();
plugin.setGroupId( groupId );
plugin.setArtifactId( artifactId );
plugin.setVersion( version );
Xpp3Dom config = new Xpp3Dom( "configuration" );
if( configuration != null )
{
for ( Iterator it = configuration.entrySet().iterator(); it.hasNext(); )
{
Map.Entry entry = (Map.Entry) it.next();
Xpp3Dom param = new Xpp3Dom( String.valueOf( entry.getKey() ) );
param.setValue( String.valueOf( entry.getValue() ) );
config.addChild( param );
}
}
plugin.setConfiguration( config );
return plugin;
}
public void testProfilePluginConfigurationShouldOverrideCollidingModelPluginConfiguration()
{
Plugin mPlugin = new Plugin();