From 0f18470576d1eb0649aa93c739b8dd39c9368716 Mon Sep 17 00:00:00 2001 From: Niels Basjes Date: Wed, 12 Apr 2023 12:26:47 +0200 Subject: [PATCH] [MNG-7750] Fix unwanted interpolation in plugins from profiles. (#1075) This is ONLY the reproduction of the problem I found. If you disable the activeProfile (in buildPom and remove the assert) then all checks pass. If you enable this profile (provided code) then in several places the ${project.basedir} has been interpolated and shows the path of the build and the assertions fail the build. --- https://issues.apache.org/jira/browse/MNG-7750 --- .../maven/project/PomConstructionTest.java | 196 ++++++++++++++++++ .../plugin-interpolation-build/pom.xml | 123 +++++++++++ .../plugin-interpolation-reporting/pom.xml | 123 +++++++++++ .../model/profile/DefaultProfileInjector.java | 8 +- 4 files changed, 446 insertions(+), 4 deletions(-) create mode 100644 maven-core/src/test/resources-project-builder/plugin-interpolation-build/pom.xml create mode 100644 maven-core/src/test/resources-project-builder/plugin-interpolation-reporting/pom.xml diff --git a/maven-core/src/test/java/org/apache/maven/project/PomConstructionTest.java b/maven-core/src/test/java/org/apache/maven/project/PomConstructionTest.java index e3c9f59f4556..28d747c10f9a 100644 --- a/maven-core/src/test/java/org/apache/maven/project/PomConstructionTest.java +++ b/maven-core/src/test/java/org/apache/maven/project/PomConstructionTest.java @@ -26,8 +26,12 @@ import java.util.Properties; import org.apache.maven.artifact.repository.layout.DefaultRepositoryLayout; +import org.apache.maven.model.Model; import org.apache.maven.model.Plugin; import org.apache.maven.model.PluginExecution; +import org.apache.maven.model.Profile; +import org.apache.maven.model.ReportPlugin; +import org.apache.maven.model.ReportSet; import org.apache.maven.model.building.ModelBuildingRequest; import org.apache.maven.project.harness.PomTestWrapper; import org.apache.maven.repository.RepositorySystem; @@ -38,6 +42,7 @@ import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory; import org.eclipse.aether.repository.LocalRepository; +import org.junit.Assert; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.lessThan; @@ -133,6 +138,197 @@ public void testProfilePropertiesInterpolation() throws Exception { assertEquals("PASSED", pom.getValue("properties[1]/property")); } + /*MNG-7750*/ + + private void checkBuildPluginWithArtifactId( + List plugins, String artifactId, String expectedId, String expectedConfig) { + Plugin plugin = plugins.stream() + .filter(p -> p.getArtifactId().equals(artifactId)) + .findFirst() + .orElse(null); + assertNotNull("Unable to find plugin with artifactId: " + artifactId, plugin); + List pluginExecutions = plugin.getExecutions(); + assertEquals("Wrong number of plugin executions for \"" + artifactId + "\"", 1, pluginExecutions.size()); + assertEquals( + "Wrong id for \"" + artifactId + "\"", + expectedId, + pluginExecutions.get(0).getId()); + + String config = pluginExecutions.get(0).getConfiguration().toString(); + assertTrue( + "Wrong config for \"" + artifactId + "\": (" + config + ") does not contain :" + expectedConfig, + config.contains(expectedConfig)); + } + + public void testBuildPluginInterpolation() throws Exception { + PomTestWrapper pom = buildPom("plugin-interpolation-build", "activeProfile"); + Model originalModel = pom.getMavenProject().getOriginalModel(); + + // ============================================= + assertEquals("||${project.basedir}||", originalModel.getProperties().get("prop-outside")); + + List outsidePlugins = originalModel.getBuild().getPlugins(); + Assert.assertEquals(1, outsidePlugins.size()); + + checkBuildPluginWithArtifactId( + outsidePlugins, + "plugin-all-profiles", + "Outside ||${project.basedir}||", + "Outside ||${project.basedir}||"); + + // ============================================= + Profile activeProfile = originalModel.getProfiles().stream() + .filter(profile -> profile.getId().equals("activeProfile")) + .findFirst() + .orElse(null); + assertNotNull("Unable to find the activeProfile", activeProfile); + + assertTrue( + "The activeProfile should be active in the maven project", + pom.getMavenProject().getActiveProfiles().contains(activeProfile)); + + assertEquals("||${project.basedir}||", activeProfile.getProperties().get("prop-active")); + + List activeProfilePlugins = activeProfile.getBuild().getPlugins(); + assertEquals("Number of active profile plugins", 2, activeProfilePlugins.size()); + + checkBuildPluginWithArtifactId( + activeProfilePlugins, + "plugin-all-profiles", + "Active all ||${project.basedir}||", + "Active all ||${project.basedir}||"); + + checkBuildPluginWithArtifactId( + activeProfilePlugins, + "only-active-profile", + "Active only ||${project.basedir}||", + "Active only ||${project.basedir}||"); + + // ============================================= + + Profile inactiveProfile = originalModel.getProfiles().stream() + .filter(profile -> profile.getId().equals("inactiveProfile")) + .findFirst() + .orElse(null); + assertNotNull("Unable to find the inactiveProfile", inactiveProfile); + + assertFalse( + "The inactiveProfile should NOT be active in the maven project", + pom.getMavenProject().getActiveProfiles().contains(inactiveProfile)); + + assertEquals("||${project.basedir}||", inactiveProfile.getProperties().get("prop-inactive")); + + List inactiveProfilePlugins = inactiveProfile.getBuild().getPlugins(); + assertEquals("Number of active profile plugins", 2, inactiveProfilePlugins.size()); + + checkBuildPluginWithArtifactId( + inactiveProfilePlugins, + "plugin-all-profiles", + "Inactive all ||${project.basedir}||", + "Inactive all ||${project.basedir}||"); + + checkBuildPluginWithArtifactId( + inactiveProfilePlugins, + "only-inactive-profile", + "Inactive only ||${project.basedir}||", + "Inactive only ||${project.basedir}||"); + } + + private void checkReportPluginWithArtifactId( + List plugins, String artifactId, String expectedId, String expectedConfig) { + ReportPlugin plugin = plugins.stream() + .filter(p -> p.getArtifactId().equals(artifactId)) + .findFirst() + .orElse(null); + assertNotNull("Unable to find plugin with artifactId: " + artifactId, plugin); + List pluginReportSets = plugin.getReportSets(); + assertEquals("Wrong number of plugin reportSets for \"" + artifactId + "\"", 1, pluginReportSets.size()); + assertEquals( + "Wrong id for \"" + artifactId + "\"", + expectedId, + pluginReportSets.get(0).getId()); + + String config = pluginReportSets.get(0).getConfiguration().toString(); + assertTrue( + "Wrong config for \"" + artifactId + "\": (" + config + ") does not contain :" + expectedConfig, + config.contains(expectedConfig)); + } + + public void testReportingPluginInterpolation() throws Exception { + PomTestWrapper pom = buildPom("plugin-interpolation-reporting", "activeProfile"); + Model originalModel = pom.getMavenProject().getOriginalModel(); + + // ============================================= + assertEquals("||${project.basedir}||", originalModel.getProperties().get("prop-outside")); + + List outsidePlugins = originalModel.getReporting().getPlugins(); + Assert.assertEquals(1, outsidePlugins.size()); + + checkReportPluginWithArtifactId( + outsidePlugins, + "plugin-all-profiles", + "Outside ||${project.basedir}||", + "Outside ||${project.basedir}||"); + + // ============================================= + Profile activeProfile = originalModel.getProfiles().stream() + .filter(profile -> profile.getId().equals("activeProfile")) + .findFirst() + .orElse(null); + assertNotNull("Unable to find the activeProfile", activeProfile); + + assertTrue( + "The activeProfile should be active in the maven project", + pom.getMavenProject().getActiveProfiles().contains(activeProfile)); + + assertEquals("||${project.basedir}||", activeProfile.getProperties().get("prop-active")); + + List activeProfilePlugins = activeProfile.getReporting().getPlugins(); + assertEquals("Number of active profile plugins", 2, activeProfilePlugins.size()); + + checkReportPluginWithArtifactId( + activeProfilePlugins, + "plugin-all-profiles", + "Active all ||${project.basedir}||", + "Active all ||${project.basedir}||"); + + checkReportPluginWithArtifactId( + activeProfilePlugins, + "only-active-profile", + "Active only ||${project.basedir}||", + "Active only ||${project.basedir}||"); + + // ============================================= + + Profile inactiveProfile = originalModel.getProfiles().stream() + .filter(profile -> profile.getId().equals("inactiveProfile")) + .findFirst() + .orElse(null); + assertNotNull("Unable to find the inactiveProfile", inactiveProfile); + + assertFalse( + "The inactiveProfile should NOT be active in the maven project", + pom.getMavenProject().getActiveProfiles().contains(inactiveProfile)); + + assertEquals("||${project.basedir}||", inactiveProfile.getProperties().get("prop-inactive")); + + List inactiveProfilePlugins = + inactiveProfile.getReporting().getPlugins(); + assertEquals("Number of active profile plugins", 2, inactiveProfilePlugins.size()); + + checkReportPluginWithArtifactId( + inactiveProfilePlugins, + "plugin-all-profiles", + "Inactive all ||${project.basedir}||", + "Inactive all ||${project.basedir}||"); + + checkReportPluginWithArtifactId( + inactiveProfilePlugins, + "only-inactive-profile", + "Inactive only ||${project.basedir}||", + "Inactive only ||${project.basedir}||"); + } + // Some better conventions for the test poms needs to be created and each of these tests // that represent a verification of a specification item needs to be a couple lines at most. // The expressions help a lot, but we need a clean to pick up a directory of POMs, automatically load diff --git a/maven-core/src/test/resources-project-builder/plugin-interpolation-build/pom.xml b/maven-core/src/test/resources-project-builder/plugin-interpolation-build/pom.xml new file mode 100644 index 000000000000..56718154a51b --- /dev/null +++ b/maven-core/src/test/resources-project-builder/plugin-interpolation-build/pom.xml @@ -0,0 +1,123 @@ + + + + + 4.0.0 + + org.apache.maven.its.build.plugins + test + 1.0 + + MNG-7750 + + Test build plugin and execution interpolation. + + + + ||${project.basedir}|| + + + + + + plugin-all-profiles + + + Outside ||${project.basedir}|| + + Outside ||${project.basedir}|| + + + + + + + + + + activeProfile + + ||${project.basedir}|| + + + + + plugin-all-profiles + + + Active all ||${project.basedir}|| + + Active all ||${project.basedir}|| + + + + + + only-active-profile + + + Active only ||${project.basedir}|| + + Active only ||${project.basedir}|| + + + + + + + + + + inactiveProfile + + ||${project.basedir}|| + + + + + plugin-all-profiles + + + Inactive all ||${project.basedir}|| + + Inactive all ||${project.basedir}|| + + + + + + only-inactive-profile + + + Inactive only ||${project.basedir}|| + + Inactive only ||${project.basedir}|| + + + + + + + + + + + diff --git a/maven-core/src/test/resources-project-builder/plugin-interpolation-reporting/pom.xml b/maven-core/src/test/resources-project-builder/plugin-interpolation-reporting/pom.xml new file mode 100644 index 000000000000..aeb6f7de2c94 --- /dev/null +++ b/maven-core/src/test/resources-project-builder/plugin-interpolation-reporting/pom.xml @@ -0,0 +1,123 @@ + + + + + 4.0.0 + + org.apache.maven.its.reporting.plugins + test + 1.0 + + MNG-7750 + + Test reporting plugin and reportSet interpolation. + + + + ||${project.basedir}|| + + + + + + plugin-all-profiles + + + Outside ||${project.basedir}|| + + Outside ||${project.basedir}|| + + + + + + + + + + activeProfile + + ||${project.basedir}|| + + + + + plugin-all-profiles + + + Active all ||${project.basedir}|| + + Active all ||${project.basedir}|| + + + + + + only-active-profile + + + Active only ||${project.basedir}|| + + Active only ||${project.basedir}|| + + + + + + + + + + inactiveProfile + + ||${project.basedir}|| + + + + + plugin-all-profiles + + + Inactive all ||${project.basedir}|| + + Inactive all ||${project.basedir}|| + + + + + + only-inactive-profile + + + Inactive only ||${project.basedir}|| + + Inactive only ||${project.basedir}|| + + + + + + + + + + + diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileInjector.java b/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileInjector.java index f34d69e3651d..3949ed8b4fde 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileInjector.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileInjector.java @@ -108,7 +108,7 @@ protected void mergePluginContainer_Plugins( pending = new ArrayList<>(); } } else { - pending.add(element); + pending.add(element.clone()); } } @@ -145,7 +145,7 @@ protected void mergePlugin_Executions( if (existing != null) { mergePluginExecution(existing, element, sourceDominant, context); } else { - merged.put(key, element); + merged.put(key, element.clone()); } } @@ -170,7 +170,7 @@ protected void mergeReporting_Plugins( Object key = getReportPluginKey(element); ReportPlugin existing = merged.get(key); if (existing == null) { - merged.put(key, element); + merged.put(key, element.clone()); } else { mergeReportPlugin(existing, element, sourceDominant, context); } @@ -199,7 +199,7 @@ protected void mergeReportPlugin_ReportSets( if (existing != null) { mergeReportSet(existing, element, sourceDominant, context); } else { - merged.put(key, element); + merged.put(key, element.clone()); } }