Author: jdcasey
Date: Wed Jul 26 17:36:31 2006
New Revision: 425919
URL: http://svn.apache.org/viewvc?rev=425919&view=rev
Log:
[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.
Modified:
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java
Modified:
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java
URL:
http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java?rev=425919&r1=425918&r2=425919&view=diff
==============================================================================
---
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java
(original)
+++
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java
Wed Jul 26 17:36:31 2006
@@ -57,6 +57,19 @@
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 @@
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 @@
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,30 +137,78 @@
// 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(); )
+ 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(); )
{
- Plugin childPlugin = (Plugin) it.next();
-
- if ( !assembledPlugins.containsKey( childPlugin.getKey() ) )
+ 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
{
- mergedPlugins.add(childPlugin);
+ missingFromResults.add( item );
}
}
-
- childContainer.setPlugins(mergedPlugins);
-
- childContainer.flushPluginMap();
+
+ if ( !missingFromResults.isEmpty() )
+ {
+ results.addAll( missingFromResults );
+
+ missingFromResults.clear();
+ }
}
+
+ return results;
}
public static void mergeReportPluginLists( Reporting child, Reporting
parent, boolean handleAsInheritance )
Modified:
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java
URL:
http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java?rev=425919&r1=425918&r2=425919&view=diff
==============================================================================
---
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java
(original)
+++
maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java
Wed Jul 26 17:36:31 2006
@@ -136,7 +136,19 @@
}
}
- 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 @@
}
else if ( profileContainer.getPlugins() != null )
{
- Map mergedPlugins = new TreeMap();
+ List mergedPlugins = new ArrayList();
Map profilePlugins = profileContainer.getPluginsAsMap();
@@ -160,31 +172,21 @@
{
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();
}
Modified:
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java
URL:
http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java?rev=425919&r1=425918&r2=425919&view=diff
==============================================================================
---
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java
(original)
+++
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java
Wed Jul 26 17:36:31 2006
@@ -7,9 +7,12 @@
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 @@
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();
Modified:
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java
URL:
http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java?rev=425919&r1=425918&r2=425919&view=diff
==============================================================================
---
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java
(original)
+++
maven/components/trunk/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java
Wed Jul 26 17:36:31 2006
@@ -1,21 +1,105 @@
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()
{
