From 5401fb4c6402040ebde690ef3503467ca656e516 Mon Sep 17 00:00:00 2001 From: John Dennis Casey Date: Fri, 1 Jun 2007 20:35:44 +0000 Subject: [PATCH] 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 --- .../injection/DefaultProfileInjector.java | 28 ++-- .../injection/DefaultProfileInjectorTest.java | 145 +++++++++++++----- 2 files changed, 122 insertions(+), 51 deletions(-) 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 e8a41c3e24..44e7f2ce75 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 @@ -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 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 ); } 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 470727c282..0bb0aee858 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 @@ -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() ); + + } }