diff --git a/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java b/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java index 7e6de40f6a..b2bafb218b 100644 --- a/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java +++ b/maven-project/src/main/java/org/apache/maven/project/ModelUtils.java @@ -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 ) diff --git a/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java b/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java index dfe265c39f..97333a82c0 100644 --- a/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java +++ b/maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java @@ -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(); } diff --git a/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java b/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java index eb4525196e..5da6fcbdc8 100644 --- a/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java +++ b/maven-project/src/test/java/org/apache/maven/project/ModelUtilsTest.java @@ -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(); diff --git a/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java b/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java index a35bdd6407..301c38aaf4 100644 --- a/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java +++ b/maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java @@ -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();