[MNG-4341] [regression] Plugins are not executed in POM order

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

maven-core/src/main/java/org/apache/maven/model/plugin/DefaultLifecycleBindingsInjector.java

@@ -22,6 +22,7 @@ package org.apache.maven.model.plugin;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -102,7 +103,7 @@ public class DefaultLifecycleBindingsInjector
             {
                 List<Plugin> tgt = target.getPlugins();
 
-                Map<Object, Plugin> merged = new LinkedHashMap<Object, Plugin>( ( src.size() + tgt.size() ) * 2 );
+                Map<Object, Plugin> merged = new LinkedHashMap<Object, Plugin>( src.size() * 2 );
 
                 for ( Iterator<Plugin> it = src.iterator(); it.hasNext(); )
                 {
@@ -113,6 +114,10 @@ public class DefaultLifecycleBindingsInjector
 
                 Map<Object, Plugin> unmanaged = new LinkedHashMap<Object, Plugin>( merged );
 
+                Map<Object, List<Plugin>> predecessors = new HashMap<Object, List<Plugin>>();
+
+                List<Plugin> pending = new ArrayList<Plugin>( tgt.size() );
+
                 for ( Iterator<Plugin> it = tgt.iterator(); it.hasNext(); )
                 {
                     Plugin element = it.next();
@@ -122,8 +127,18 @@ public class DefaultLifecycleBindingsInjector
                     {
                         mergePlugin( element, existing, sourceDominant, context );
                         unmanaged.remove( key );
+                        merged.put( key, element );
+
+                        if ( !pending.isEmpty() )
+                        {
+                            predecessors.put( key, pending );
+                            pending = new ArrayList<Plugin>();
+                        }
+                    }
+                    else
+                    {
+                        pending.add( element );
                     }
-                    merged.put( key, element );
                 }
 
                 if ( !unmanaged.isEmpty() )
@@ -146,7 +161,23 @@ public class DefaultLifecycleBindingsInjector
                     }
                 }
 
-                target.setPlugins( new ArrayList<Plugin>( merged.values() ) );
+                List<Plugin> result = new ArrayList<Plugin>( src.size() + tgt.size() );
+
+                for ( Map.Entry<Object, Plugin> entry : merged.entrySet() )
+                {
+                    List<Plugin> pre = predecessors.get( entry.getKey() );
+
+                    if ( pre != null )
+                    {
+                        result.addAll( pre );
+                    }
+
+                    result.add( entry.getValue() );
+                }
+
+                result.addAll( pending );
+
+                target.setPlugins( result );
             }
         }
     }

maven-core/src/test/java/org/apache/maven/project/PomConstructionTest.java

@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.Properties;
 
 import org.apache.maven.artifact.repository.layout.DefaultRepositoryLayout;
+import org.apache.maven.model.Plugin;
 import org.apache.maven.model.PluginExecution;
 import org.apache.maven.model.building.ModelBuildingRequest;
 import org.apache.maven.project.harness.PomTestWrapper;
@@ -1675,6 +1676,30 @@ public class PomConstructionTest
         assertEquals( "test-2", executions.get( 3 ).getId() );
     }
 
+    public void testPluginDeclarationsRetainPomOrderAfterInjectionOfDefaultPlugins()
+        throws Exception
+    {
+        PomTestWrapper pom = buildPom( "plugin-exec-order-with-lifecycle" );
+        List<Plugin> plugins = (List<Plugin>) pom.getValue( "build/plugins" );
+        int resourcesPlugin = -1;
+        int customPlugin = -1;
+        for ( int i = 0; i < plugins.size(); i++ )
+        {
+            Plugin plugin = plugins.get( i );
+            if ( "maven-resources-plugin".equals( plugin.getArtifactId() ) )
+            {
+                assertTrue( resourcesPlugin < 0 );
+                resourcesPlugin = i;
+            }
+            else if ( "maven-it-plugin-log-file".equals( plugin.getArtifactId() ) )
+            {
+                assertTrue( customPlugin < 0 );
+                customPlugin = i;
+            }
+        }
+        assertTrue( plugins.toString(), customPlugin == resourcesPlugin - 1 );
+    }
+
     private void assertPathSuffixEquals( String expected, Object actual )
     {
         String a = actual.toString();

maven-core/src/test/resources-project-builder/plugin-exec-order-with-lifecycle/pom.xml

@@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<project>
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>org.apache.maven.its.mng4341</groupId>
+  <artifactId>test</artifactId>
+  <version>0.1</version>
+  <!-- NOTE: The upper-case packaging name is intentional and triggers a special mode in the EmptyLifecycleExecutor -->
+  <packaging>JAR</packaging>
+
+  <name>Maven Integration Test :: MNG-4341</name> 
+  <description>
+    Test that plugins bound to the same phase get executed in POM order even if one of the plugins participates
+    in the default lifecycle bindings for the project's packaging.
+  </description>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.its.plugins</groupId>
+        <artifactId>maven-it-plugin-log-file</artifactId>
+        <version>2.1-SNAPSHOT</version>
+      </plugin>
+      <plugin>
+        <!-- NOTE: It's essential that this plugin is also referenced by the default lifecycle bindings -->
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-resources-plugin</artifactId>
+        <version>0.1-stub-SNAPSHOT</version>
+      </plugin>
+    </plugins>
+  </build>
+</project>