OPEN - issue MNG-2784: Multiple executions of the same plugin at the same life cycle phase in a multi-module profile mixed up

http://jira.codehaus.org/browse/MNG-2784

NOT applying this patch, as there is a much simpler solution. The processing is currently in the correct order, so all we need to do is make sure the Map.values() method retains this order. Therefore, I changed the Map implementation for plugin executions to LinkedHashMap.

I've also added a test for this issue...

git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@543599 13f79535-47bb-0310-9956-ffa450edef68

maven-project/src/main/java/org/apache/maven/project/injection/DefaultProfileInjector.java

@@ -41,10 +41,10 @@ import org.codehaus.plexus.util.xml.Xpp3Dom;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.TreeMap;
 
 /**
  * Inject profile data into a Model, using the profile as the dominant data source, and
@@ -141,24 +141,24 @@ public class DefaultProfileInjector
 
     /**
      * This should be the resulting ordering of plugins after injection:
-     * 
+     *
      * Given:
-     * 
+     *
      *   model: X -> A -> B -> D -> E
      *   profile: Y -> A -> C -> D -> F
-     *  
+     *
-     * Result: 
+     * Result:
-     * 
+     *
      *   X -> Y -> A -> B -> C -> D -> E -> F
      */
     protected void injectPlugins( PluginContainer profileContainer, PluginContainer modelContainer )
     {
-        if ( profileContainer == null || modelContainer == null )
+        if ( ( profileContainer == null ) || ( modelContainer == null ) )
         {
             // nothing to do...
             return;
         }
-        
+
         List modelPlugins = modelContainer.getPlugins();
 
         if ( modelPlugins == null )
@@ -177,7 +177,7 @@ public class DefaultProfileInjector
 
                 Plugin profilePlugin = (Plugin) profilePlugins.get( modelPlugin.getKey() );
 
-                if ( profilePlugin != null && !mergedPlugins.contains( profilePlugin ) )
+                if ( ( profilePlugin != null ) && !mergedPlugins.contains( profilePlugin ) )
                 {
                     Plugin mergedPlugin = modelPlugin;
 
@@ -197,7 +197,7 @@ public class DefaultProfileInjector
 
     private void injectPluginDefinition( Plugin profilePlugin, Plugin modelPlugin )
     {
-        if ( profilePlugin == null || modelPlugin == null )
+        if ( ( profilePlugin == null ) || ( modelPlugin == null ) )
         {
             // nothing to do.
             return;
@@ -219,13 +219,13 @@ public class DefaultProfileInjector
         // from here to the end of the method is dealing with merging of the <executions/> section.
         List modelExecutions = modelPlugin.getExecutions();
 
-        if ( modelExecutions == null || modelExecutions.isEmpty() )
+        if ( ( modelExecutions == null ) || modelExecutions.isEmpty() )
         {
             modelPlugin.setExecutions( profilePlugin.getExecutions() );
         }
         else
         {
-            Map executions = new TreeMap();
+            Map executions = new LinkedHashMap();
 
             Map profileExecutions = profilePlugin.getExecutionsAsMap();
 
@@ -249,7 +249,7 @@ public class DefaultProfileInjector
 
                     List goals = new ArrayList();
 
-                    if ( modelGoals != null && !modelGoals.isEmpty() )
+                    if ( ( modelGoals != null ) && !modelGoals.isEmpty() )
                     {
                         goals.addAll( modelGoals );
                     }
@@ -331,7 +331,7 @@ public class DefaultProfileInjector
 
         List modelModules = model.getModules();
 
-        if ( modelModules != null && !modelModules.isEmpty() )
+        if ( ( modelModules != null ) && !modelModules.isEmpty() )
         {
             modules.addAll( modelModules );
         }

maven-project/src/test/java/org/apache/maven/project/injection/DefaultProfileInjectorTest.java

@@ -41,81 +41,81 @@ public class DefaultProfileInjectorTest
 
     /**
      * 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: 
+     * 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;
     }
 
@@ -174,7 +174,7 @@ public class DefaultProfileInjectorTest
         Xpp3Dom rChild = rConfig.getChild( "test" );
 
         assertEquals( "replacedValue", rChild.getValue() );
-        
+
         Xpp3Dom rChild2 = rConfig.getChild( "test2" );
 
         assertEquals( "value2", rChild2.getValue() );
@@ -212,7 +212,7 @@ public class DefaultProfileInjectorTest
 
         PluginExecution pExec = new PluginExecution();
         pExec.setId("profile-injected");
-        
+
         Xpp3Dom pConfigChild = new Xpp3Dom( "test" );
         pConfigChild.setValue( "replacedValue" );
 
@@ -222,7 +222,7 @@ public class DefaultProfileInjectorTest
         pExec.setConfiguration( pConfig );
 
         pPlugin.addExecution( pExec );
-        
+
         BuildBase pBuild = new BuildBase();
         pBuild.addPlugin( pPlugin );
 
@@ -235,26 +235,26 @@ public class DefaultProfileInjectorTest
 
         Build rBuild = model.getBuild();
         Plugin rPlugin = (Plugin) rBuild.getPlugins().get( 0 );
-        
+
         PluginExecution rExec = (PluginExecution) rPlugin.getExecutionsAsMap().get( "profile-injected" );
-        
+
         assertNotNull( rExec );
-        
+
         Xpp3Dom rExecConfig = (Xpp3Dom) rExec.getConfiguration();
 
         Xpp3Dom rChild = rExecConfig.getChild( "test" );
 
         assertEquals( "replacedValue", rChild.getValue() );
-        
+
         Xpp3Dom rConfig = (Xpp3Dom) rPlugin.getConfiguration();
-        
+
         assertNotNull( rConfig );
-        
+
         Xpp3Dom rChild2 = rConfig.getChild( "test2" );
 
         assertEquals( "value2", rChild2.getValue() );
     }
-    
+
     public void testProfileRepositoryShouldOverrideModelRepository()
     {
         Repository mRepository = new Repository();
@@ -299,4 +299,75 @@ public class DefaultProfileInjectorTest
         assertEquals( "module1", rModules.get( 0 ) );
     }
 
+    // NOTE: The execution-id's are important, because they are NOT in
+    // alphabetical order. The trunk version of Maven currently injects
+    // profiles into a TreeMap, then calls map.values(), which puts the
+    // executions in alphabetical order...the WRONG order.
+    public void testShouldPreserveOrderingOfProfileInjectedPluginExecutions()
+    {
+        Plugin profilePlugin = new Plugin();
+        profilePlugin.setGroupId( "group" );
+        profilePlugin.setArtifactId( "artifact" );
+        profilePlugin.setVersion( "version" );
+
+        PluginExecution exec1 = new PluginExecution();
+        exec1.setId( "z" );
+        profilePlugin.addExecution( exec1 );
+
+        PluginExecution exec2 = new PluginExecution();
+        exec2.setId( "y" );
+        profilePlugin.addExecution( exec2 );
+
+        BuildBase buildBase = new BuildBase();
+        buildBase.addPlugin( profilePlugin );
+
+        Profile profile = new Profile();
+        profile.setBuild( buildBase );
+
+        Plugin modelPlugin = new Plugin();
+        modelPlugin.setGroupId( "group" );
+        modelPlugin.setArtifactId( "artifact" );
+        modelPlugin.setVersion( "version" );
+
+        PluginExecution exec3 = new PluginExecution();
+        exec3.setId( "w" );
+        modelPlugin.addExecution( exec3 );
+
+        PluginExecution exec4 = new PluginExecution();
+        exec4.setId( "x" );
+        modelPlugin.addExecution( exec4 );
+
+        Build build = new Build();
+        build.addPlugin( modelPlugin );
+
+        Model model = new Model();
+        model.setBuild( build );
+
+        new DefaultProfileInjector().inject( profile, model );
+
+        List plugins = model.getBuild().getPlugins();
+        assertNotNull( plugins );
+        assertEquals( 1, plugins.size() );
+
+        Plugin plugin = (Plugin) plugins.get( 0 );
+
+        List executions = plugin.getExecutions();
+        assertNotNull( executions );
+        assertEquals( 4, executions.size() );
+
+        Iterator it = executions.iterator();
+
+        PluginExecution e = (PluginExecution) it.next();
+        assertEquals( "w", e.getId() );
+
+        e = (PluginExecution) it.next();
+        assertEquals( "x", e.getId() );
+
+        e = (PluginExecution) it.next();
+        assertEquals( "z", e.getId() );
+
+        e = (PluginExecution) it.next();
+        assertEquals( "y", e.getId() );
+
+    }
 }