From 818feb563ca713c7a421dbf39862dc8232a711af Mon Sep 17 00:00:00 2001 From: Andrzej Jarmoniuk Date: Fri, 14 Oct 2022 19:50:19 +0200 Subject: [PATCH] #367: Include parent projects in property resolution --- .../codehaus/mojo/versions/api/PomHelper.java | 178 ++++++++++++------ .../DisplayPropertyUpdatesMojoTest.java | 88 +++++++++ .../mojo/versions/utils/TestUtils.java | 33 ++++ .../issue-367/child/pom.xml | 20 ++ .../issue-367/pom.xml | 11 ++ 5 files changed, 270 insertions(+), 60 deletions(-) create mode 100644 src/test/java/org/codehaus/mojo/versions/DisplayPropertyUpdatesMojoTest.java create mode 100644 src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/child/pom.xml create mode 100644 src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/pom.xml diff --git a/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java b/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java index 8c66929c67..affc232d27 100644 --- a/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java +++ b/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java @@ -22,11 +22,13 @@ import javax.xml.stream.XMLStreamException; import javax.xml.stream.events.XMLEvent; +import java.io.BufferedReader; import java.io.File; -import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStreamReader; import java.io.Reader; import java.io.StringReader; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -90,7 +92,7 @@ public class PomHelper * @throws IOException if the file is not found or if the file does not parse. */ public static Model getRawModel( MavenProject project ) - throws IOException + throws IOException { return getRawModel( project.getFile() ); } @@ -103,15 +105,12 @@ public static Model getRawModel( MavenProject project ) * @throws IOException if the file is not found or if the file does not parse. */ public static Model getRawModel( File moduleProjectFile ) - throws IOException + throws IOException { - try ( FileInputStream input = new FileInputStream( moduleProjectFile ) ) + try ( Reader reader = new BufferedReader( new InputStreamReader( Files.newInputStream( moduleProjectFile + .toPath() ) ) ) ) { - return new MavenXpp3Reader().read( input ); - } - catch ( XmlPullParserException e ) - { - throw new IOException( e.getMessage(), e ); + return getRawModel( reader ); } } @@ -123,11 +122,27 @@ public static Model getRawModel( File moduleProjectFile ) * @throws IOException if the file is not found or if the file does not parse. */ public static Model getRawModel( ModifiedPomXMLEventReader modifiedPomXMLEventReader ) - throws IOException + throws IOException { - try ( StringReader stringReader = new StringReader( modifiedPomXMLEventReader.asStringBuilder().toString() ) ) + try ( Reader reader = new StringReader( modifiedPomXMLEventReader.asStringBuilder().toString() ) ) { - return new MavenXpp3Reader().read( stringReader ); + return getRawModel( reader ); + } + } + + /** + * Gets the current raw model before any interpolation what-so-ever. + * + * @param reader The {@link Reader} to get the raw model for. + * @return The raw model. + * @throws IOException if the file is not found or if the file does not parse. + */ + public static Model getRawModel( Reader reader ) + throws IOException + { + try + { + return new MavenXpp3Reader().read( reader ); } catch ( XmlPullParserException e ) { @@ -967,6 +982,37 @@ else if ( matchScopeRegex.matcher( path ).matches() ) return madeReplacement; } + /** + * Traverses the project tree upwards, adding the raw models of every project it encounters to the map + * + * @param project maven project of the child for which the models need to be gathered + * @return gathered map of raw models per project + */ + private static Map getRawModelWithParents( MavenProject project ) throws IOException + { + // constructs a tree sorted from children to parents + Map models = new TreeMap<>( ( p1, p2 ) -> + { + for ( MavenProject p = p1; p != null; p = p.getParent() ) + { + if ( p == p2 ) // meaning p2 is an ancestor to p1 or p1 == p2 + { + return p == p1 + ? 0 + : -1; // p1 is the child + } + } + return 1; + } ); + for ( MavenProject p = project; p != null; p = p.getParent() ) + { + models.put( p, p.getFile() != null + ? getRawModel( p ) + : p.getOriginalModel() ); + } + return models; + } + /** * Examines the project to find any properties which are associated with versions of artifacts in the project. * @@ -978,11 +1024,12 @@ else if ( matchScopeRegex.matcher( path ).matches() ) * @since 1.0-alpha-3 */ public static PropertyVersionsBuilder[] getPropertyVersionsBuilders( VersionsHelper helper, MavenProject project ) - throws ExpressionEvaluationException, IOException + throws ExpressionEvaluationException, IOException { ExpressionEvaluator expressionEvaluator = helper.getExpressionEvaluator( project ); - Model projectModel = getRawModel( project ); - Map result = new TreeMap<>(); + Map reactorModels = getRawModelWithParents( project ); + + Map propertiesMap = new TreeMap<>(); Set activeProfiles = new TreeSet<>(); for ( Profile profile : project.getActiveProfiles() ) @@ -991,58 +1038,72 @@ public static PropertyVersionsBuilder[] getPropertyVersionsBuilders( VersionsHel } // add any properties from profiles first (as they override properties from the project - for ( Profile profile : projectModel.getProfiles() ) + for ( Iterator it = reactorModels.values().stream() + .flatMap( model -> model.getProfiles().stream() ) + .filter( profile -> activeProfiles.contains( profile.getId() ) ) + .iterator(); it.hasNext(); ) { - if ( !activeProfiles.contains( profile.getId() ) ) - { - continue; - } - addProperties( helper, result, profile.getId(), profile.getProperties() ); - if ( profile.getDependencyManagement() != null ) - { - addDependencyAssocations( helper, expressionEvaluator, result, - profile.getDependencyManagement().getDependencies(), false ); - } - addDependencyAssocations( helper, expressionEvaluator, result, profile.getDependencies(), false ); - if ( profile.getBuild() != null ) + Profile profile = it.next(); + try { - if ( profile.getBuild().getPluginManagement() != null ) + addProperties( helper, propertiesMap, profile.getId(), profile.getProperties() ); + if ( profile.getDependencyManagement() != null ) { - addPluginAssociations( helper, expressionEvaluator, result, - profile.getBuild().getPluginManagement().getPlugins() ); + addDependencyAssocations( helper, expressionEvaluator, propertiesMap, + profile.getDependencyManagement().getDependencies(), false ); + } + addDependencyAssocations( helper, expressionEvaluator, propertiesMap, + profile.getDependencies(), + false ); + if ( profile.getBuild() != null ) + { + if ( profile.getBuild().getPluginManagement() != null ) + { + addPluginAssociations( helper, expressionEvaluator, propertiesMap, + profile.getBuild().getPluginManagement().getPlugins() ); + } + addPluginAssociations( helper, expressionEvaluator, propertiesMap, + profile.getBuild().getPlugins() ); + } + if ( profile.getReporting() != null ) + { + addReportPluginAssociations( helper, expressionEvaluator, propertiesMap, + profile.getReporting().getPlugins() ); } - addPluginAssociations( helper, expressionEvaluator, result, profile.getBuild().getPlugins() ); } - if ( profile.getReporting() != null ) + catch ( ExpressionEvaluationException e ) { - addReportPluginAssociations( helper, expressionEvaluator, result, profile.getReporting().getPlugins() ); + throw new RuntimeException( e ); } } // second, we add all the properties in the pom - addProperties( helper, result, null, projectModel.getProperties() ); - Model model = projectModel; - MavenProject currentPrj = project; - while ( currentPrj != null ) + reactorModels.values().forEach( model -> addProperties( helper, propertiesMap, null, model.getProperties() ) ); + + + for ( MavenProject currentPrj = project; currentPrj != null; currentPrj = currentPrj.getParent() ) { + Model model = reactorModels.get( currentPrj ); + if ( model.getDependencyManagement() != null ) { - addDependencyAssocations( helper, expressionEvaluator, result, - model.getDependencyManagement().getDependencies(), false ); + addDependencyAssocations( helper, expressionEvaluator, propertiesMap, + model.getDependencyManagement().getDependencies(), false ); } - addDependencyAssocations( helper, expressionEvaluator, result, model.getDependencies(), false ); + addDependencyAssocations( helper, expressionEvaluator, propertiesMap, model.getDependencies(), false ); if ( model.getBuild() != null ) { if ( model.getBuild().getPluginManagement() != null ) { - addPluginAssociations( helper, expressionEvaluator, result, - model.getBuild().getPluginManagement().getPlugins() ); + addPluginAssociations( helper, expressionEvaluator, propertiesMap, + model.getBuild().getPluginManagement().getPlugins() ); } - addPluginAssociations( helper, expressionEvaluator, result, model.getBuild().getPlugins() ); + addPluginAssociations( helper, expressionEvaluator, propertiesMap, model.getBuild().getPlugins() ); } if ( model.getReporting() != null ) { - addReportPluginAssociations( helper, expressionEvaluator, result, model.getReporting().getPlugins() ); + addReportPluginAssociations( helper, expressionEvaluator, propertiesMap, + model.getReporting().getPlugins() ); } // third, we add any associations from the active profiles @@ -1054,36 +1115,33 @@ public static PropertyVersionsBuilder[] getPropertyVersionsBuilders( VersionsHel } if ( profile.getDependencyManagement() != null ) { - addDependencyAssocations( helper, expressionEvaluator, result, - profile.getDependencyManagement().getDependencies(), false ); + addDependencyAssocations( helper, expressionEvaluator, propertiesMap, + profile.getDependencyManagement().getDependencies(), false ); } - addDependencyAssocations( helper, expressionEvaluator, result, profile.getDependencies(), false ); + addDependencyAssocations( helper, expressionEvaluator, propertiesMap, profile.getDependencies(), + false ); if ( profile.getBuild() != null ) { if ( profile.getBuild().getPluginManagement() != null ) { - addPluginAssociations( helper, expressionEvaluator, result, - profile.getBuild().getPluginManagement().getPlugins() ); + addPluginAssociations( helper, expressionEvaluator, propertiesMap, + profile.getBuild().getPluginManagement().getPlugins() ); } - addPluginAssociations( helper, expressionEvaluator, result, profile.getBuild().getPlugins() ); + addPluginAssociations( helper, expressionEvaluator, propertiesMap, + profile.getBuild().getPlugins() ); } if ( profile.getReporting() != null ) { - addReportPluginAssociations( helper, expressionEvaluator, result, - profile.getReporting().getPlugins() ); + addReportPluginAssociations( helper, expressionEvaluator, propertiesMap, + profile.getReporting().getPlugins() ); } } - currentPrj = currentPrj.getParent(); - if ( currentPrj != null ) - { - model = currentPrj.getOriginalModel(); - } } // finally, remove any properties without associations - purgeProperties( result ); + purgeProperties( propertiesMap ); - return result.values().toArray( new PropertyVersionsBuilder[0] ); + return propertiesMap.values().toArray( new PropertyVersionsBuilder[0] ); } /** diff --git a/src/test/java/org/codehaus/mojo/versions/DisplayPropertyUpdatesMojoTest.java b/src/test/java/org/codehaus/mojo/versions/DisplayPropertyUpdatesMojoTest.java new file mode 100644 index 0000000000..be01de3bdf --- /dev/null +++ b/src/test/java/org/codehaus/mojo/versions/DisplayPropertyUpdatesMojoTest.java @@ -0,0 +1,88 @@ +package org.codehaus.mojo.versions; +/* + * 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. + */ + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashMap; + +import org.apache.maven.plugin.testing.AbstractMojoTestCase; +import org.apache.maven.plugin.testing.MojoRule; +import org.codehaus.mojo.versions.utils.MockUtils; +import org.codehaus.mojo.versions.utils.TestUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import static org.apache.commons.codec.CharEncoding.UTF_8; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.matchesPattern; + +/** + * Unit tests for {@link DisplayPropertyUpdatesMojo} + */ +public class DisplayPropertyUpdatesMojoTest extends AbstractMojoTestCase +{ + @Rule + public MojoRule mojoRule = new MojoRule( this ); + + private Path tempDir; + + @Before + public void setUp() throws Exception + { + super.setUp(); + tempDir = TestUtils.createTempDir( "display-property-updates" ); + } + + @After + public void tearDown() throws Exception + { + try + { + TestUtils.tearDownTempDir( tempDir ); + } + finally + { + super.tearDown(); + } + } + + @Test + public void testPropertiesFromParent() throws Exception + { + Path tempFile = Files.createTempFile( tempDir, "output", "" ); + + TestUtils.copyDir( Paths.get( "src/test/resources/org/codehaus/mojo/display-property-updates/issue-367" ), + tempDir ); + DisplayPropertyUpdatesMojo mojo = + (DisplayPropertyUpdatesMojo) mojoRule.lookupConfiguredMojo( tempDir.resolve( "child" ).toFile(), + "display-property-updates" ); + mojo.outputEncoding = UTF_8; + mojo.outputFile = tempFile.toFile(); + mojo.setPluginContext( new HashMap<>() ); + mojo.artifactMetadataSource = MockUtils.mockArtifactMetadataSource(); + mojo.execute(); + + assertThat( String.join( "", Files.readAllLines( tempFile ) ), + matchesPattern( ".*\\$\\{ver} \\.* 1\\.0\\.0 -> 2\\.0\\.0.*" ) ); + } +} diff --git a/src/test/java/org/codehaus/mojo/versions/utils/TestUtils.java b/src/test/java/org/codehaus/mojo/versions/utils/TestUtils.java index 6e45c91b97..950a9d184d 100644 --- a/src/test/java/org/codehaus/mojo/versions/utils/TestUtils.java +++ b/src/test/java/org/codehaus/mojo/versions/utils/TestUtils.java @@ -26,6 +26,7 @@ import java.nio.file.attribute.BasicFileAttributes; import static java.nio.file.FileVisitResult.CONTINUE; +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static org.apache.commons.text.CaseUtils.toCamelCase; /** @@ -35,6 +36,7 @@ public class TestUtils { /** * Creates a temporary directory with the given name + * * @param name name of the directory to create * @return {@linkplain Path} object pointing to the directory * @throws IOException should the I/O operation fail @@ -46,6 +48,7 @@ public static Path createTempDir( String name ) throws IOException /** * Deletes the given directory together with all its contents + * * @param dir directory to delete * @throws IOException should an I/O operation fail */ @@ -71,4 +74,34 @@ public FileVisitResult postVisitDirectory( Path dir, IOException exc ) throws IO } ); } } + + /** + * Copies the {@code src} directory to {@code dst} recursively, + * creating the missing directories if necessary + * + * @param src source directory path + * @param dst destination directory path + * @throws IOException should an I/O error occur + */ + public static void copyDir( Path src, Path dst ) throws IOException + { + Files.walkFileTree( src, new SimpleFileVisitor() + { + @Override + public FileVisitResult preVisitDirectory( Path dir, BasicFileAttributes attrs ) + throws IOException + { + Files.createDirectories( dst.resolve( src.relativize( dir ) ) ); + return CONTINUE; + } + + @Override + public FileVisitResult visitFile( Path file, BasicFileAttributes attrs ) + throws IOException + { + Files.copy( file, dst.resolve( src.relativize( file ) ), REPLACE_EXISTING ); + return CONTINUE; + } + } ); + } } diff --git a/src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/child/pom.xml b/src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/child/pom.xml new file mode 100644 index 0000000000..270daa5a5d --- /dev/null +++ b/src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/child/pom.xml @@ -0,0 +1,20 @@ + + 4.0.0 + + default-group + parent + 1.0.0 + + child + + + + + default-group + artifactA + ${ver} + + + + + diff --git a/src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/pom.xml b/src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/pom.xml new file mode 100644 index 0000000000..3a342f4087 --- /dev/null +++ b/src/test/resources/org/codehaus/mojo/display-property-updates/issue-367/pom.xml @@ -0,0 +1,11 @@ + + 4.0.0 + default-group + parent + 1.0.0 + pom + + + 1.0.0 + +