From ff6fe8d0f0781ecc966c6d9c4c3d57884a41162a Mon Sep 17 00:00:00 2001 From: Andrzej Jarmoniuk Date: Wed, 2 Nov 2022 20:08:07 +0100 Subject: [PATCH] Resolves #505: getReactorModels using correct module paths when the module name includes pom.xml --- .../it/it-set-issue-505/invoker.properties | 2 + .../it-set-issue-505/moduleA/moduleB/pom.xml | 13 ++ .../src/it/it-set-issue-505/moduleA/pom.xml | 18 +++ .../src/it/it-set-issue-505/pom.xml | 13 ++ .../src/it/it-set-issue-505/verify.groovy | 3 + .../codehaus/mojo/versions/api/PomHelper.java | 139 ++++-------------- .../mojo/versions/api/PomHelperTest.java | 28 +++- .../api/issue-505/moduleA/moduleA.xml | 18 +++ .../api/issue-505/moduleA/moduleB/pom.xml | 13 ++ .../mojo/versions/api/issue-505/pom.xml | 27 ++++ 10 files changed, 156 insertions(+), 118 deletions(-) create mode 100644 versions-maven-plugin/src/it/it-set-issue-505/invoker.properties create mode 100644 versions-maven-plugin/src/it/it-set-issue-505/moduleA/moduleB/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-505/moduleA/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-505/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-505/verify.groovy create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleA.xml create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleB/pom.xml create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/pom.xml diff --git a/versions-maven-plugin/src/it/it-set-issue-505/invoker.properties b/versions-maven-plugin/src/it/it-set-issue-505/invoker.properties new file mode 100644 index 0000000000..f21410e6a3 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-505/invoker.properties @@ -0,0 +1,2 @@ +invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:set +invoker.mavenOpts = -DnewVersion=TEST diff --git a/versions-maven-plugin/src/it/it-set-issue-505/moduleA/moduleB/pom.xml b/versions-maven-plugin/src/it/it-set-issue-505/moduleA/moduleB/pom.xml new file mode 100644 index 0000000000..92023f8d5f --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-505/moduleA/moduleB/pom.xml @@ -0,0 +1,13 @@ + + 4.0.0 + + + default-group + moduleA + 1.0-SNAPSHOT + + + moduleB + + diff --git a/versions-maven-plugin/src/it/it-set-issue-505/moduleA/pom.xml b/versions-maven-plugin/src/it/it-set-issue-505/moduleA/pom.xml new file mode 100644 index 0000000000..123b5c733c --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-505/moduleA/pom.xml @@ -0,0 +1,18 @@ + + 4.0.0 + + + default-group + default-artifact + 1.0-SNAPSHOT + + + moduleA + pom + + + moduleB/pom.xml + + + diff --git a/versions-maven-plugin/src/it/it-set-issue-505/pom.xml b/versions-maven-plugin/src/it/it-set-issue-505/pom.xml new file mode 100644 index 0000000000..c2a9983323 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-505/pom.xml @@ -0,0 +1,13 @@ + + 4.0.0 + default-group + default-artifact + 1.0-SNAPSHOT + pom + + + moduleA/pom.xml + + + diff --git a/versions-maven-plugin/src/it/it-set-issue-505/verify.groovy b/versions-maven-plugin/src/it/it-set-issue-505/verify.groovy new file mode 100644 index 0000000000..4ebd952101 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-505/verify.groovy @@ -0,0 +1,3 @@ +assert new File( basedir, "pom.xml" ).text.contains( 'TEST' ) +assert new File( basedir, "moduleA/pom.xml" ).text.contains( 'TEST' ) +assert new File( basedir, "moduleA/moduleB/pom.xml" ).text.contains( 'TEST' ) diff --git a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java index 557f0eac0c..bf9b8e9591 100644 --- a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java +++ b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java @@ -46,7 +46,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.maven.artifact.Artifact; import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException; import org.apache.maven.artifact.versioning.VersionRange; @@ -500,67 +499,6 @@ public static boolean setProjectParentVersion( final ModifiedPomXMLEventReader p return madeReplacement; } - /** - * Gets the parent artifact from the pom. - * - * @param pom The pom. - * @param helper The helper (used to create the artifact). - * @return The parent artifact or null if no parent is specified. - * @throws XMLStreamException if something went wrong. - */ - public static Artifact getProjectParent( final ModifiedPomXMLEventReader pom, VersionsHelper helper ) - throws XMLStreamException - { - Stack stack = new Stack<>(); - String path = ""; - final Pattern matchScopeRegex = Pattern.compile( "/project/parent((/groupId)|(/artifactId)|(/version))" ); - String groupId = null; - String artifactId = null; - String version = null; - - pom.rewind(); - - while ( pom.hasNext() ) - { - XMLEvent event = pom.nextEvent(); - if ( event.isStartElement() ) - { - stack.push( path ); - final String elementName = event.asStartElement().getName().getLocalPart(); - path = path + "/" + elementName; - - if ( matchScopeRegex.matcher( path ).matches() ) - { - if ( "groupId".equals( elementName ) ) - { - groupId = pom.getElementText().trim(); - path = stack.pop(); - } - else if ( "artifactId".equals( elementName ) ) - { - artifactId = pom.getElementText().trim(); - path = stack.pop(); - } - else if ( "version".equals( elementName ) ) - { - version = pom.getElementText().trim(); - path = stack.pop(); - } - } - } - if ( event.isEndElement() ) - { - path = stack.pop(); - } - } - if ( groupId == null || artifactId == null || version == null ) - { - return null; - } - return helper.createDependencyArtifact( groupId, artifactId, version, "pom", - null, null ); - } - /** * Searches the pom re-defining the specified dependency to the specified version. * @@ -1432,18 +1370,6 @@ public static void debugModules( Log logger, String message, Collection } } - /** - * Modifies the collection of child modules removing those which cannot be found relative to the parent. - * - * @param logger The logger to log to. - * @param project the project. - * @param childModules the child modules. - */ - public static void removeMissingChildModules( Log logger, MavenProject project, Collection childModules ) - { - removeMissingChildModules( logger, project.getBasedir(), childModules ); - } - /** * Modifies the collection of child modules removing those which cannot be found relative to the parent. * @@ -1619,51 +1545,42 @@ public static Map getReactorModels( MavenProject project, Log log private static Map getReactorModels( String path, Model model, MavenProject project, Log logger ) throws IOException { - if ( path.length() > 0 && !path.endsWith( "/" ) ) - { - path += '/'; - } Map result = new LinkedHashMap<>(); Map childResults = new LinkedHashMap<>(); - - File baseDir = path.length() > 0 ? new File( project.getBasedir(), path ) : project.getBasedir(); - Set childModules = getAllChildModules( model, logger ); + File baseDir = path.length() > 0 + ? new File( project.getBasedir(), path ) + : project.getBasedir(); removeMissingChildModules( logger, baseDir, childModules ); - for ( String moduleName : childModules ) - { - String modulePath = path + moduleName; - - File moduleDir = new File( baseDir, moduleName ); - - File moduleProjectFile; + childModules.stream() + .map( moduleName -> new File( baseDir, moduleName ) ) + .filter( File::exists ) + .forEach( moduleFile -> + { + File pomFile = moduleFile.isDirectory() + ? new File( moduleFile, "/pom.xml" ) + : moduleFile; + String modulePath = ( !path.isEmpty() && !path.endsWith( "/" ) + ? path + "/" + : path ) + + pomFile.getParentFile().getName(); - if ( moduleDir.isDirectory() ) - { - moduleProjectFile = new File( moduleDir, "pom.xml" ); - } - else - { - // i don't think this should ever happen... but just in case - // the module references the file-name - moduleProjectFile = moduleDir; - } + try + { + // the aim of this goal is to fix problems when the project cannot be parsed by Maven, + // so we have to work with the raw model and not the interpolated parsed model from maven + Model moduleModel = getRawModel( pomFile ); + result.put( modulePath, moduleModel ); + childResults.putAll( getReactorModels( modulePath, moduleModel, project, logger ) ); + } + catch ( IOException e ) + { + logger.debug( "Could not parse " + pomFile.getPath(), e ); + } + } ); - try - { - // the aim of this goal is to fix problems when the project cannot be parsed by Maven - // so we have to work with the raw model and not the interpolated parsed model from maven - Model moduleModel = getRawModel( moduleProjectFile ); - result.put( modulePath, moduleModel ); - childResults.putAll( getReactorModels( modulePath, moduleModel, project, logger ) ); - } - catch ( IOException e ) - { - logger.debug( "Could not parse " + moduleProjectFile.getPath(), e ); - } - } result.putAll( childResults ); // more efficient update order if all children are added after siblings return result; } diff --git a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java index e447355b39..9e472005f3 100644 --- a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java +++ b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java @@ -7,34 +7,39 @@ import java.io.StringReader; import java.net.URL; import java.util.List; +import java.util.Map; import org.apache.maven.model.Dependency; import org.apache.maven.model.Model; import org.apache.maven.model.io.xpp3.MavenXpp3Reader; +import org.apache.maven.plugin.logging.SystemStreamLog; +import org.apache.maven.plugin.testing.AbstractMojoTestCase; +import org.apache.maven.plugin.testing.MojoRule; +import org.apache.maven.project.MavenProject; import org.codehaus.mojo.versions.rewriting.ModifiedPomXMLEventReader; import org.codehaus.stax2.XMLInputFactory2; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; import static org.codehaus.mojo.versions.utils.ModifiedPomXMLEventReaderUtils.matches; import static org.codehaus.stax2.XMLInputFactory2.P_PRESERVE_LOCATION; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertTrue; /** * Tests the methods of {@link PomHelper}. */ -public class PomHelperTest +public class PomHelperTest extends AbstractMojoTestCase { + @Rule + public MojoRule mojoRule = new MojoRule( this ); + private static final XMLInputFactory INPUT_FACTORY = XMLInputFactory2.newInstance(); @BeforeClass - public static void setUp() + public static void setUpClass() { INPUT_FACTORY.setProperty( P_PRESERVE_LOCATION, Boolean.TRUE ); } @@ -265,4 +270,13 @@ public void testSetProjectValueNewValueNonEmptyParent() throws XMLStreamExceptio assertThat( xmlEventReader, matches( "value" ) ); } + + @Test + public void testIssue505ChildModules() throws Exception + { + MavenProject project = mojoRule.readMavenProject( + new File( "src/test/resources/org/codehaus/mojo/versions/api/issue-505" ) ); + Map reactorModels = PomHelper.getReactorModels( project, new SystemStreamLog() ); + assertThat( reactorModels.keySet(), hasSize( 3 ) ); + } } diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleA.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleA.xml new file mode 100644 index 0000000000..123b5c733c --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleA.xml @@ -0,0 +1,18 @@ + + 4.0.0 + + + default-group + default-artifact + 1.0-SNAPSHOT + + + moduleA + pom + + + moduleB/pom.xml + + + diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleB/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleB/pom.xml new file mode 100644 index 0000000000..92023f8d5f --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleB/pom.xml @@ -0,0 +1,13 @@ + + 4.0.0 + + + default-group + moduleA + 1.0-SNAPSHOT + + + moduleB + + diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/pom.xml new file mode 100644 index 0000000000..4cb7712ea5 --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/versions/api/issue-505/pom.xml @@ -0,0 +1,27 @@ + + 4.0.0 + default-group + default-artifact + 1.0-SNAPSHOT + pom + + + moduleA/moduleA.xml + + + + + + org.codehaus.mojo + versions-maven-plugin + + set + + + TEST + + + + +