Resolves mojohaus#505: getReactorModels using correct module paths wh…

…en the module name includes pom.xml

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

src/it/it-set-issue-505/moduleA/moduleB/pom.xml

@@ -0,0 +1,13 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>default-group</groupId>
+        <artifactId>moduleA</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>moduleB</artifactId>
+
+</project>

src/it/it-set-issue-505/moduleA/pom.xml

@@ -0,0 +1,18 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>default-group</groupId>
+        <artifactId>default-artifact</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>moduleA</artifactId>
+    <packaging>pom</packaging>
+
+    <modules>
+        <module>moduleB/pom.xml</module>
+    </modules>
+
+</project>

src/it/it-set-issue-505/pom.xml

@@ -0,0 +1,13 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>default-group</groupId>
+    <artifactId>default-artifact</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>pom</packaging>
+
+    <modules>
+        <module>moduleA/pom.xml</module>
+    </modules>
+
+</project>

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' )

src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/moduleB/pom.xml

@@ -0,0 +1,13 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>default-group</groupId>
+        <artifactId>moduleA</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>moduleB</artifactId>
+
+</project>

src/test/resources/org/codehaus/mojo/versions/api/issue-505/moduleA/pom.xml

@@ -0,0 +1,18 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>default-group</groupId>
+        <artifactId>default-artifact</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>moduleA</artifactId>
+    <packaging>pom</packaging>
+
+    <modules>
+        <module>moduleB/pom.xml</module>
+    </modules>
+
+</project>

src/test/resources/org/codehaus/mojo/versions/api/issue-505/pom.xml

@@ -0,0 +1,27 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>default-group</groupId>
+    <artifactId>default-artifact</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>pom</packaging>
+
+    <modules>
+        <module>moduleA/pom.xml</module>
+    </modules>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>versions-maven-plugin</artifactId>
+                <goals>
+                    <goal>set</goal>
+                </goals>
+                <configuration>
+                    <newVersion>TEST</newVersion>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+</project>

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 <code>null</code> if no parent is specified.
-     * @throws XMLStreamException if something went wrong.
-     */
-    public static Artifact getProjectParent( final ModifiedPomXMLEventReader pom, VersionsHelper helper )
-        throws XMLStreamException
-    {
-        Stack<String> 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<String>
         }
     }
 
-    /**
-     * 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<String> 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,46 @@ public static Map<String, Model> getReactorModels( MavenProject project, Log log
     private static Map<String, Model> getReactorModels( String path, Model model, MavenProject project, Log logger )
         throws IOException
     {
-        if ( path.length() > 0 && !path.endsWith( "/" ) )
-        {
-            path += '/';
-        }
         Map<String, Model> result = new LinkedHashMap<>();
         Map<String, Model> childResults = new LinkedHashMap<>();
-
-        File baseDir = path.length() > 0 ? new File( project.getBasedir(), path ) : project.getBasedir();
-
         Set<String> 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 ->
+                {
+                    // this is the unlikely case represented in issue-505
+                    int pomXmlIndex = moduleName.indexOf( "/pom.xml" );
+                    return pomXmlIndex != -1
+                            ? moduleName.substring( 0, pomXmlIndex )
+                            : moduleName;
+                } )
+                .forEach( moduleName ->
+                {
+                    String modulePath = ( !path.isEmpty() && !path.endsWith( "/" )
+                            ?  path + "/"
+                            : path )
+                            + moduleName;
+                    File moduleProjectFile = new File( baseDir, moduleName + "/pom.xml" );
 
-            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( moduleProjectFile );
+                        result.put( modulePath, moduleModel );
+                        childResults.putAll( getReactorModels( modulePath, moduleModel, project, logger ) );
+                    }
+                    catch ( IOException e )
+                    {
+                        logger.debug( "Could not parse " + moduleProjectFile.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;
     }

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( "<super-parent><parent><child>value</child></parent></super-parent>" ) );
     }
+
+    @Test
+    public void testIssue505ChildModules() throws Exception
+    {
+        MavenProject project = mojoRule.readMavenProject(
+                new File( "src/test/resources/org/codehaus/mojo/versions/api/issue-505" ) );
+        Map<String, Model> reactorModels = PomHelper.getReactorModels( project, new SystemStreamLog() );
+        assertThat( reactorModels.keySet(), hasSize( 3 ) );
+    }
 }