diff --git a/integration-tests/src/test/java/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT.java b/integration-tests/src/test/java/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT.java index 43a42cc..a4975ac 100644 --- a/integration-tests/src/test/java/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT.java +++ b/integration-tests/src/test/java/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT.java @@ -136,7 +136,7 @@ void override_property_test_case(MavenExecutionResult result) { Model model = result.getMavenProjectResult().getModel(); DependencyModel dependencyModel = new DependencyModel(model); - // verify version property has been overriden + // verify version property has been overridden assertThat(model.getProperties().getProperty("undertow.version")) .usingComparator(VersionMatcher.COMPARATOR).isEqualTo("2.2.5.Final-Overridden"); // dependency still referencing the property @@ -170,4 +170,27 @@ void override_dependency_test_case(MavenExecutionResult result) { assertThat(o.get().getVersion()).isEqualTo("2.2.5.Final-Overridden"); }); } + + + /** + * Tests injecting properties defined in external parent pom. + */ + @MavenGoal("${project.groupId}:wildfly-channel-maven-plugin:${project.version}:upgrade") + @SystemProperty(value = "manifestFile", content = "manifest.yaml") + @MavenTest + void external_properties_test_case(MavenExecutionResult result) { + assertThat(result).isSuccessful(); + + Model model = result.getMavenProjectResult().getModel(); + DependencyModel dependencyModel = new DependencyModel(model); + + // verify version property has been overridden + assertThat(model.getProperties().get("version.clean.plugin")).isEqualTo("3.3.2"); + // verify dependency still referencing the property + assertThat(dependencyModel.getDependency("org.apache.maven.plugins", "maven-clean-plugin", "pom", null)) + .satisfies(o -> { + assertThat(o).isPresent(); + assertThat(o.get().getVersion()).isEqualTo("${version.clean.plugin}"); + }); + } } diff --git a/integration-tests/src/test/resources-its/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/external_properties_test_case/manifest.yaml b/integration-tests/src/test/resources-its/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/external_properties_test_case/manifest.yaml new file mode 100644 index 0000000..0bccf15 --- /dev/null +++ b/integration-tests/src/test/resources-its/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/external_properties_test_case/manifest.yaml @@ -0,0 +1,6 @@ +schemaVersion: 1.0.0 +name: Sample Manifest +streams: + - groupId: org.apache.maven.plugins + artifactId: maven-clean-plugin + version: "3.3.2" diff --git a/integration-tests/src/test/resources-its/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/external_properties_test_case/pom.xml b/integration-tests/src/test/resources-its/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/external_properties_test_case/pom.xml new file mode 100644 index 0000000..1ad0335 --- /dev/null +++ b/integration-tests/src/test/resources-its/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/external_properties_test_case/pom.xml @@ -0,0 +1,34 @@ + + + 4.0.0 + + + org.jboss + jboss-parent + 40 + + + org.wildfly + test-project + 1.0.0-SNAPSHOT + pom + + + 11 + 11 + UTF-8 + + + + + + org.apache.maven.plugins + maven-clean-plugin + ${version.clean.plugin} + pom + + + + diff --git a/plugin/src/main/java/org/wildfly/channelplugin/UpgradeComponentsMojo.java b/plugin/src/main/java/org/wildfly/channelplugin/UpgradeComponentsMojo.java index 7ce7476..eca9ddf 100644 --- a/plugin/src/main/java/org/wildfly/channelplugin/UpgradeComponentsMojo.java +++ b/plugin/src/main/java/org/wildfly/channelplugin/UpgradeComponentsMojo.java @@ -196,6 +196,14 @@ public class UpgradeComponentsMojo extends AbstractMojo { @Parameter(property = "ignoreTestDependencies", defaultValue = "true") boolean ignoreTestDependencies; + /** + * When a dependency is defined with version string referencing a property, and that property is defined in a parent + * pom outside the project, the property would be injected into a pom where the dependency is defined, if this + * parameter is set to true (default). + */ + @Parameter(property = "injectExternalProperties", defaultValue = "true") + boolean injectExternalProperties; + @Inject DependencyGraphBuilder dependencyGraphBuilder; @@ -333,6 +341,9 @@ private void processModule(Project pmeProject, PomManipulator manipulator) String newVersion = upgrade.getRight(); Dependency locatedDependency = upgrade.getLeft(); + @SuppressWarnings("UnnecessaryLocalVariable") + Dependency d = locatedDependency; + if (overriddenDependencies.contains(locatedDependency)) { // if there was a hard version override, the dependency is not processed again continue; @@ -348,27 +359,32 @@ private void processModule(Project pmeProject, PomManipulator manipulator) } Pair projectProperty = followProperties(pmeProject, versionPropertyName); + if (projectProperty == null) { + Pair externalProperty = resolveExternalProperty(mavenProject, versionPropertyName); + if (externalProperty != null) { + projectProperty = Pair.of(null, externalProperty.getLeft()); + } + } if (projectProperty == null) { - Dependency d = locatedDependency; ChannelPluginLogger.LOGGER.errorf( - "Unable to upgrade %s:%s:%s to '%s', can't locate property '%s' in POM file %s", + "Unable to upgrade %s:%s:%s to '%s', can't locate property '%s' in the project", d.getGroupId(), d.getArtifactId(), d.getVersion(), newVersion, - versionPropertyName, pmeProject.getPom().getPath()); + versionPropertyName); continue; } Project targetProject = projectProperty.getLeft(); String targetPropertyName = projectProperty.getRight(); + if (isIgnoredProperty(targetPropertyName)) { - getLog().info(String.format("Ignoring property '%s' (ignored prefix)", targetPropertyName)); + getLog().info(String.format("Ignoring property '%s'", targetPropertyName)); continue; } if (upgradedProperties.containsKey(projectProperty)) { if (!upgradedProperties.get(projectProperty).equals(newVersion)) { // property has already been changed to different value - Dependency d = locatedDependency; String propertyName = projectProperty.getRight(); String currentPropertyValue = upgradedProperties.get(projectProperty); if (inlineVersionOnConflict) { @@ -391,10 +407,25 @@ private void processModule(Project pmeProject, PomManipulator manipulator) // get manipulator for the module where the target property is located upgradedProperties.put(projectProperty, newVersion); - PomManipulator targetManipulator = manipulators.get( - Pair.of(targetProject.getGroupId(), targetProject.getArtifactId())); - targetManipulator.overrideProperty(targetPropertyName, newVersion); - } else { // dependency version is inlined in version element, can be directly overriden + + if (targetProject != null) { + // property has been located in some project module + // => override the located property in the module where it has been located + PomManipulator targetManipulator = manipulators.get( + Pair.of(targetProject.getGroupId(), targetProject.getArtifactId())); + targetManipulator.overrideProperty(targetPropertyName, newVersion); + } else if (injectExternalProperties) { + // property has been located in external parent pom + // => inject the property into current module + PomManipulator targetManipulator = manipulators.get( + Pair.of(pmeProject.getGroupId(), pmeProject.getArtifactId())); + targetManipulator.injectProperty(targetPropertyName, newVersion); + } else { + getLog().warn(String.format("Can't upgrade %s:%s:%s to %s, property %s is not defined in the " + + "scope of the project (consider enabling the injectExternalProperties parameter).", + d.getGroupId(), d.getArtifactId(), d.getVersion(), newVersion, targetPropertyName)); + } + } else { // dependency version is inlined in version element, can be directly overwritten manipulator.overrideDependencyVersion(toArtifactRef(locatedDependency), newVersion); } } @@ -566,13 +597,22 @@ private List> findDependenciesToUpgrade( continue; } if (artifactRef.getVersionString() == null) { - getLog().warn("Resolved dependency has null version: " + artifactRef); + // this is not expected to happen + getLog().error("Resolved dependency has null version: " + artifactRef); continue; } if (VersionUtils.isProperty(artifactRef.getVersionString())) { - // didn't manage to resolve dependency version - getLog().warn("Resolved dependency has version with property: " + artifactRef); - continue; + // hack: PME doesn't seem to resolve properties from external parent poms + Pair externalProperty = resolveExternalProperty(mavenProject, + VersionUtils.extractPropertyName(artifactRef.getVersionString())); + if (externalProperty != null) { + artifactRef = new SimpleArtifactRef(artifactRef.getGroupId(), artifactRef.getArtifactId(), + externalProperty.getRight(), artifactRef.getType(), artifactRef.getClassifier()); + } else { + // didn't manage to resolve dependency version, this is not expected to happen + getLog().error("Resolved dependency has version with property: " + artifactRef); + continue; + } } if ("test".equals(dependency.getScope()) && ignoreTestDependencies) { getLog().info("Skipping dependency (ignored scope): " @@ -612,10 +652,10 @@ private void initChannel() throws MojoExecutionException { throw new MojoExecutionException("Exactly one of [channelFile, channelGAV, manifestFile, manifestGAV] has to be given."); } - if ((StringUtils.isNotBlank(manifestFile) || StringUtils.isNotBlank(manifestGAV)) && remoteRepositories.isEmpty()) { - // Do not enforce this for now, repositories are also read from project pom.xml currently. - //throw new MojoExecutionException("The remoteRepositories property is mandatory when manifest is given."); - } + // Do not enforce this for now, repositories are also read from project pom.xml currently. + /*if ((StringUtils.isNotBlank(manifestFile) || StringUtils.isNotBlank(manifestGAV)) && remoteRepositories.isEmpty()) { + throw new MojoExecutionException("The remoteRepositories property is mandatory when manifest is given."); + }*/ try { if (StringUtils.isNotBlank(channelFile)) { @@ -838,4 +878,32 @@ static Pair followProperties(Project pmeProject, String propert } } + /** + * Resolves a property from external parent pom. If given property references another property, this method + * tries to traverse the property chain. + * + * @return pair [property, value], where the property is the last property name in the traversal chain + */ + static Pair resolveExternalProperty(MavenProject mavenProject, String propertyName) { + if (mavenProject == null) { + return null; + } + Properties properties = mavenProject.getModel().getProperties(); + if (!properties.containsKey(propertyName)) { + return resolveExternalProperty(mavenProject.getParent(), propertyName); + } else { + // property is defined in this module + String propertyValue = (String) properties.get(propertyName); + if (VersionUtils.isProperty(propertyValue)) { + // the property value is also a property reference -> follow the chain + String newPropertyName = VersionUtils.extractPropertyName(propertyValue); + Pair targetProperty = resolveExternalProperty(mavenProject, newPropertyName); + if (targetProperty != null) { + return targetProperty; + } + } + return Pair.of(propertyName, propertyValue); + } + } + } diff --git a/plugin/src/main/java/org/wildfly/channelplugin/manipulation/PomManipulator.java b/plugin/src/main/java/org/wildfly/channelplugin/manipulation/PomManipulator.java index af6fe8f..431ece9 100644 --- a/plugin/src/main/java/org/wildfly/channelplugin/manipulation/PomManipulator.java +++ b/plugin/src/main/java/org/wildfly/channelplugin/manipulation/PomManipulator.java @@ -95,6 +95,10 @@ public void injectRepository(String id, String url) throws XMLStreamException { } } + public void injectProperty(String key, String version) throws XMLStreamException { + injectProperty(eventReader, key, version); + } + /** * Writes the updated POM file. */ @@ -116,7 +120,7 @@ private void assertOpen() { } /** - * This method attempts to inject new depenendency into at the end of the dependencyManagement section. + * This method attempts to inject new dependency into at the end of the dependencyManagement section. *

* The dependencyManagement section must be already present in the POM. */ @@ -124,7 +128,7 @@ static void injectManagedDependency(ModifiedPomXMLEventReader eventReader, Artif Collection exclusions) throws XMLStreamException { eventReader.rewind(); - Stack stack = new Stack(); + Stack stack = new Stack<>(); String path = ""; while (eventReader.hasNext()) { @@ -148,6 +152,38 @@ static void injectManagedDependency(ModifiedPomXMLEventReader eventReader, Artif } } + /** + * This method attempts to inject new property at the end of the properties section. + *

+ * The properties section must be already present in the POM. + */ + static void injectProperty(ModifiedPomXMLEventReader eventReader, String key, String version) throws XMLStreamException { + eventReader.rewind(); + + Stack stack = new Stack<>(); + String path = ""; + + while (eventReader.hasNext()) { + XMLEvent event = eventReader.nextEvent(); + if (event.isStartElement()) { + stack.push(path); + path = path + "/" + event.asStartElement().getName().getLocalPart(); + } else if (event.isEndElement()) { + if (event.asEndElement().getName().getLocalPart().equals("properties") + && path.equals("/project/properties")) { + eventReader.mark(0); + eventReader.replaceMark(0, String.format(" <%s>%s\n", key, version, key) + + " " + ); + eventReader.clearMark(0); + break; + } + + path = stack.pop(); + } + } + } + private static String composeDependencyElementString(ArtifactRef artifact, Collection exclusions) { StringBuilder sb = new StringBuilder(); sb.append(" \n"); diff --git a/plugin/src/test/java/org/wildfly/channelplugin/UpgradeComponentsMojoTestCase.java b/plugin/src/test/java/org/wildfly/channelplugin/UpgradeComponentsMojoTestCase.java index d95368e..9304b98 100644 --- a/plugin/src/test/java/org/wildfly/channelplugin/UpgradeComponentsMojoTestCase.java +++ b/plugin/src/test/java/org/wildfly/channelplugin/UpgradeComponentsMojoTestCase.java @@ -3,6 +3,7 @@ import java.util.Properties; import org.apache.maven.model.Model; +import org.apache.maven.project.MavenProject; import org.commonjava.maven.ext.common.model.Project; import org.junit.jupiter.api.Test; @@ -12,33 +13,72 @@ public class UpgradeComponentsMojoTestCase { @Test public void testFollowProperties() throws Exception { - Properties properties = new Properties(); - properties.put("version.a", "1.0"); - properties.put("version.b", "${version.c}"); - properties.put("version.c", "${version.d}"); - properties.put("version.d", "2.0"); - - Model model = new Model(); + final Model model = new Model(); model.setVersion("version"); - model.setProperties(properties); - Project project = new Project(model); + model.setProperties(sampleProperties()); + final Project project = new Project(model); assertThat(UpgradeComponentsMojo.followProperties(project, "version.a")).satisfies(pair -> { assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isSameAs(project); assertThat(pair.getRight()).isEqualTo("version.a"); }); assertThat(UpgradeComponentsMojo.followProperties(project, "version.b")).satisfies(pair -> { assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isSameAs(project); assertThat(pair.getRight()).isEqualTo("version.d"); }); assertThat(UpgradeComponentsMojo.followProperties(project, "version.c")).satisfies(pair -> { assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isSameAs(project); assertThat(pair.getRight()).isEqualTo("version.d"); }); assertThat(UpgradeComponentsMojo.followProperties(project, "version.d")).satisfies(pair -> { assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isSameAs(project); assertThat(pair.getRight()).isEqualTo("version.d"); }); } + @Test + public void testResolveExternalProperty() { + final Model parentModel = new Model(); + parentModel.setVersion("version"); + parentModel.setProperties(sampleProperties()); + final MavenProject parentProject = new MavenProject(parentModel); + + final MavenProject project = new MavenProject(); + project.setParent(parentProject); + + assertThat(UpgradeComponentsMojo.resolveExternalProperty(project, "version.a")).satisfies(pair -> { + assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isEqualTo("version.a"); + assertThat(pair.getRight()).isEqualTo("1.0"); + }); + assertThat(UpgradeComponentsMojo.resolveExternalProperty(project, "version.b")).satisfies(pair -> { + assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isEqualTo("version.d"); + assertThat(pair.getRight()).isEqualTo("2.0"); + }); + assertThat(UpgradeComponentsMojo.resolveExternalProperty(project, "version.c")).satisfies(pair -> { + assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isEqualTo("version.d"); + assertThat(pair.getRight()).isEqualTo("2.0"); + }); + assertThat(UpgradeComponentsMojo.resolveExternalProperty(project, "version.d")).satisfies(pair -> { + assertThat(pair).isNotNull(); + assertThat(pair.getLeft()).isEqualTo("version.d"); + assertThat(pair.getRight()).isEqualTo("2.0"); + }); + } + + private Properties sampleProperties() { + Properties properties = new Properties(); + properties.put("version.a", "1.0"); + properties.put("version.b", "${version.c}"); + properties.put("version.c", "${version.d}"); + properties.put("version.d", "2.0"); + return properties; + } + } diff --git a/plugin/src/test/java/org/wildfly/channelplugin/manipulation/PomManipulatorTestCase.java b/plugin/src/test/java/org/wildfly/channelplugin/manipulation/PomManipulatorTestCase.java index 3a8b308..0e510f8 100644 --- a/plugin/src/test/java/org/wildfly/channelplugin/manipulation/PomManipulatorTestCase.java +++ b/plugin/src/test/java/org/wildfly/channelplugin/manipulation/PomManipulatorTestCase.java @@ -12,6 +12,7 @@ import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; +import org.apache.maven.model.Model; import org.assertj.core.api.Assertions; import org.codehaus.mojo.versions.api.PomHelper; import org.codehaus.mojo.versions.rewriting.ModifiedPomXMLEventReader; @@ -33,6 +34,7 @@ public class PomManipulatorTestCase { @BeforeEach public void before() throws XMLStreamException, URISyntaxException, IOException { URL pomUrl = getClass().getResource("pom.xml"); + Assertions.assertThat(pomUrl).isNotNull(); content = PomHelper.readXmlFile(new File(pomUrl.toURI())); XMLInputFactory inputFactory = XMLInputFactory2.newInstance(); @@ -41,8 +43,7 @@ public void before() throws XMLStreamException, URISyntaxException, IOException } @Test - public void testInsertManagedDependency() - throws IOException, XMLStreamException, ManipulationException { + public void testInsertManagedDependency() throws IOException, XMLStreamException, ManipulationException { ArtifactRef dep = new SimpleArtifactRef("org.aesh", "aesh", "2.4.0", "jar", null); DependencyModel model = readDependencyModel(); @@ -59,13 +60,28 @@ public void testInsertManagedDependency() }); } - private DependencyModel readDependencyModel() throws IOException, ManipulationException { + @Test + public void testInsertProperty() throws IOException, XMLStreamException, ManipulationException { + Model model = readModel(); + Assertions.assertThat(model.getProperties().contains("prop")).isFalse(); + + PomManipulator.injectProperty(eventReader, "prop", "value"); + + model = readModel(); + Assertions.assertThat(model.getProperties().getProperty("prop")).isEqualTo("value"); + } + + private Model readModel() throws IOException, ManipulationException { Path pomFile = Files.createTempFile("pom", "xml"); Files.write(pomFile, content.toString().getBytes()); PomIO pomIO = new PomIO(); List projects = pomIO.parseProject(pomFile.toFile()); Assertions.assertThat(projects.size()).isEqualTo(1); - return new DependencyModel(projects.get(0).getModel()); + return projects.get(0).getModel(); + } + + private DependencyModel readDependencyModel() throws IOException, ManipulationException { + return new DependencyModel(readModel()); } }