[MNG-7862] The ModelLocator should always be used when locating pom.xml (#1217)

api/maven-api-model/src/main/mdo/maven.mdo

@@ -1802,7 +1802,7 @@
             ]]>
           </description>
           <type>String</type>
-          <defaultValue>../pom.xml</defaultValue>
+          <defaultValue>..</defaultValue>
         </field>
       </fields>
       <codeSegments>

maven-core/src/main/java/org/apache/maven/project/ReactorModelPool.java

@@ -18,7 +18,6 @@
  */
 package org.apache.maven.project;
 
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.HashMap;
@@ -60,23 +59,6 @@ public Model get(String groupId, String artifactId, String version) {
                 .orElse(null);
     }
 
-    /**
-     * Find model by path, useful when location the parent by relativePath
-     *
-     * @param path
-     * @return the matching model or {@code null}
-     * @since 4.0.0
-     */
-    public Model get(Path path) {
-        final Path pomFile;
-        if (Files.isDirectory(path)) {
-            pomFile = path.resolve("pom.xml");
-        } else {
-            pomFile = path;
-        }
-        return modelsByPath.get(pomFile);
-    }
-
     private String getVersion(Model model) {
         String version = model.getVersion();
         if (version == null && model.getParent() != null) {

maven-core/src/test/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformerTest.java

@@ -74,12 +74,17 @@ public String getUserProperty(String key) {
 
         @Override
         public Model getRawModel(String groupId, String artifactId) throws IllegalStateException {
-            return null;
+            throw new UnsupportedOperationException();
         }
 
         @Override
         public Model getRawModel(Path p) {
-            return null;
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public Path locate(Path path) {
+            throw new UnsupportedOperationException();
         }
     }
 }

maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java

@@ -1349,23 +1349,20 @@ private File determinePom(final CommandLine commandLine, final String workingDir
             alternatePomFile = commandLine.getOptionValue(CLIManager.ALTERNATE_POM_FILE);
         }
 
+        File current = baseDirectory;
         if (alternatePomFile != null) {
-            File pom = resolveFile(new File(alternatePomFile), workingDirectory);
+            current = resolveFile(new File(alternatePomFile), workingDirectory);
-            if (pom.isDirectory()) {
+        }
-                if (modelProcessor != null) {
-                    pom = modelProcessor.locatePom(pom);
-                } else {
-                    pom = new File(pom, "pom.xml");
-                }
-            }
 
+        File pom;
+        if (current.isDirectory() && modelProcessor != null) {
+            pom = modelProcessor.locatePom(current);
+        } else {
+            pom = current;
+        }
+
+        if (pom.isFile()) {
             return pom;
-        } else if (modelProcessor != null) {
-            File pom = modelProcessor.locatePom(baseDirectory);
-
-            if (pom.isFile()) {
-                return pom;
-            }
         }
 
         return null;

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java

@@ -51,6 +51,11 @@ protected Function<Path, Optional<RelativeProject>> getRelativePathMapper() {
         return p -> Optional.ofNullable(context.getRawModel(p)).map(DefaultBuildPomXMLFilterFactory::toRelativeProject);
     }
 
+    @Override
+    protected Function<Path, Path> getModelLocator() {
+        return context::locate;
+    }
+
     @Override
     protected BiFunction<String, String, String> getDependencyKeyToVersionMapper() {
         return (g, a) -> Optional.ofNullable(context.getRawModel(g, a))

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

@@ -1363,7 +1363,7 @@ private ModelData readParentLocally(
             DefaultModelProblemCollector problems)
             throws ModelBuildingException {
         final Parent parent = childModel.getParent();
-        final ModelSource candidateSource;
+        final ModelSource2 candidateSource;
         final Model candidateModel;
         final WorkspaceModelResolver resolver = request.getWorkspaceModelResolver();
         if (resolver == null) {
@@ -1480,7 +1480,7 @@ private boolean rawChildVersionReferencesParent(String rawChildModelVersion) {
                 || rawChildModelVersion.equals("${project.parent.version}");
     }
 
-    private ModelSource getParentPomFile(Model childModel, Source source) {
+    private ModelSource2 getParentPomFile(Model childModel, Source source) {
         if (!(source instanceof ModelSource2)) {
             return null;
         }
@@ -1491,7 +1491,11 @@ private ModelSource getParentPomFile(Model childModel, Source source) {
             return null;
         }
 
-        return ((ModelSource2) source).getRelatedSource(parentPath);
+        if (source instanceof ModelSource3) {
+            return ((ModelSource3) source).getRelatedSource(modelProcessor, parentPath);
+        } else {
+            return ((ModelSource2) source).getRelatedSource(parentPath);
+        }
     }
 
     private ModelData readParentExternally(
@@ -1866,7 +1870,7 @@ protected boolean hasFatalErrors(ModelProblemCollectorExt problems) {
      * @since 4.0.0
      */
     private class DefaultTransformerContextBuilder implements TransformerContextBuilder {
-        private final DefaultTransformerContext context = new DefaultTransformerContext();
+        private final DefaultTransformerContext context = new DefaultTransformerContext(modelProcessor);
 
         private final Map<DefaultTransformerContext.GAKey, Set<Source>> mappedSources = new ConcurrentHashMap<>(64);
 
@@ -1882,6 +1886,11 @@ public TransformerContext initialize(ModelBuildingRequest request, ModelProblemC
             // We must assume the TransformerContext was created using this.newTransformerContextBuilder()
             DefaultModelProblemCollector problems = (DefaultModelProblemCollector) collector;
             return new TransformerContext() {
+                @Override
+                public Path locate(Path path) {
+                    return modelProcessor.locatePom(path.toFile()).toPath();
+                }
+
                 @Override
                 public String getUserProperty(String key) {
                     return context.userProperties.computeIfAbsent(

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContext.java

@@ -25,6 +25,7 @@
 import java.util.function.Supplier;
 
 import org.apache.maven.model.Model;
+import org.apache.maven.model.locator.ModelLocator;
 
 /**
  *
@@ -32,6 +33,8 @@
  * @since 4.0.0
  */
 class DefaultTransformerContext implements TransformerContext {
+    final ModelLocator modelLocator;
+
     final Map<String, String> userProperties = new ConcurrentHashMap<>();
 
     final Map<Path, Holder> modelByPath = new ConcurrentHashMap<>();
@@ -77,6 +80,10 @@ public Model computeIfAbsent(Supplier<Model> supplier) {
         }
     }
 
+    DefaultTransformerContext(ModelLocator modelLocator) {
+        this.modelLocator = modelLocator;
+    }
+
     @Override
     public String getUserProperty(String key) {
         return userProperties.get(key);
@@ -92,6 +99,11 @@ public Model getRawModel(String groupId, String artifactId) {
         return Holder.deref(modelByGA.get(new GAKey(groupId, artifactId)));
     }
 
+    @Override
+    public Path locate(Path path) {
+        return modelLocator.locatePom(path.toFile()).toPath();
+    }
+
     static class GAKey {
         private final String groupId;
         private final String artifactId;

maven-model-builder/src/main/java/org/apache/maven/model/building/FileModelSource.java

@@ -22,13 +22,14 @@
 import java.net.URI;
 
 import org.apache.maven.building.FileSource;
+import org.apache.maven.model.locator.ModelLocator;
 
 /**
  * Wraps an ordinary {@link File} as a model source.
  *
  * @author Benjamin Bentmann
  */
-public class FileModelSource extends FileSource implements ModelSource2 {
+public class FileModelSource extends FileSource implements ModelSource3 {
 
     /**
      * Creates a new model source backed by the specified file.
@@ -51,18 +52,17 @@ public File getPomFile() {
     }
 
     @Override
-    public ModelSource2 getRelatedSource(String relPath) {
+    public ModelSource3 getRelatedSource(ModelLocator locator, String relPath) {
         relPath = relPath.replace('\\', File.separatorChar).replace('/', File.separatorChar);
 
         File relatedPom = new File(getFile().getParentFile(), relPath);
 
-        if (relatedPom.isDirectory()) {
+        if (relatedPom.isDirectory() && locator != null) {
-            // TODO figure out how to reuse ModelLocator.locatePom(File) here
+            relatedPom = locator.locatePom(relatedPom);
-            relatedPom = new File(relatedPom, "pom.xml");
         }
 
         if (relatedPom.isFile() && relatedPom.canRead()) {
-            return new FileModelSource(new File(relatedPom.toURI().normalize()));
+            return new FileModelSource(relatedPom.toPath().normalize().toFile());
         }
 
         return null;

maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSource3.java

@@ -0,0 +1,56 @@
+/*
+ * 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.
+ */
+package org.apache.maven.model.building;
+
+import org.apache.maven.model.locator.ModelLocator;
+
+/**
+ * Enhancement to the {@link ModelSource2} to support locating POM files using the {@link ModelLocator}
+ * when pointing to a directory.
+ */
+public interface ModelSource3 extends ModelSource2 {
+    /**
+     * Returns model source identified by a path relative to this model source POM. Implementation <strong>MUST</strong>
+     * accept <code>relPath</code> parameter values that
+     * <ul>
+     * <li>use either / or \ file path separator</li>
+     * <li>have .. parent directory references</li>
+     * <li>point either at file or directory</li>
+     * </ul>
+     * If the given path points at a directory, the provided {@code ModelLocator} will be used
+     * to find the POM file, else if no locator is provided, a file named 'pom.xml' needs to be
+     * used by the requested model source.
+     *
+     * @param locator locator used to locate the pom file
+     * @param relPath path of the requested model source relative to this model source POM
+     * @return related model source or <code>null</code> if no such model source
+     */
+    ModelSource3 getRelatedSource(ModelLocator locator, String relPath);
+
+    /**
+     * When using a ModelSource3, the method with a {@code ModelLocator} argument should
+     * be used instead.
+     *
+     * @deprecated use {@link #getRelatedSource(ModelLocator, String)} instead
+     */
+    @Deprecated
+    default ModelSource3 getRelatedSource(String relPath) {
+        return getRelatedSource(null, relPath);
+    }
+}

maven-model-builder/src/main/java/org/apache/maven/model/building/TransformerContext.java

@@ -59,4 +59,6 @@ public interface TransformerContext {
      * @throws IllegalStateException if multiple versions of the same GA are part of the reactor
      */
     Model getRawModel(String groupId, String artifactId);
+
+    Path locate(Path path);
 }

maven-model-transform/src/main/java/org/apache/maven/model/transform/BuildToRawPomXMLFilterFactory.java

@@ -56,7 +56,7 @@ public final XMLStreamReader get(XMLStreamReader orgParser, Path projectFile) {
         }
 
         if (getRelativePathMapper() != null) {
-            parser = new ParentXMLFilter(parser, getRelativePathMapper(), projectFile.getParent());
+            parser = new ParentXMLFilter(parser, getRelativePathMapper(), getModelLocator(), projectFile.getParent());
         }
 
         CiFriendlyXMLFilter ciFriendlyFilter = new CiFriendlyXMLFilter(parser, consume);
@@ -77,6 +77,10 @@ protected Function<Path, Optional<RelativeProject>> getRelativePathMapper() {
         return null;
     }
 
+    protected Function<Path, Path> getModelLocator() {
+        return null;
+    }
+
     protected BiFunction<String, String, String> getDependencyKeyToVersionMapper() {
         return null;
     }

maven-model-transform/src/main/java/org/apache/maven/model/transform/ParentXMLFilter.java

@@ -47,6 +47,8 @@ class ParentXMLFilter extends NodeBufferingParser {
 
     private final Function<Path, Optional<RelativeProject>> relativePathMapper;
 
+    private final Function<Path, Path> modelLocator;
+
     private final Path projectPath;
 
     private static final Pattern S_FILTER = Pattern.compile("\\s+");
@@ -55,9 +57,13 @@ class ParentXMLFilter extends NodeBufferingParser {
      * @param relativePathMapper
      */
     ParentXMLFilter(
-            XMLStreamReader delegate, Function<Path, Optional<RelativeProject>> relativePathMapper, Path projectPath) {
+            XMLStreamReader delegate,
+            Function<Path, Optional<RelativeProject>> relativePathMapper,
+            Function<Path, Path> modelLocator,
+            Path projectPath) {
         super(delegate, "parent");
         this.relativePathMapper = relativePathMapper;
+        this.modelLocator = modelLocator;
         this.projectPath = projectPath;
     }
 
@@ -93,7 +99,7 @@ protected void process(List<Event> buffer) {
             } else if (event.event == END_ELEMENT && "parent".equals(event.name)) {
                 Optional<RelativeProject> resolvedParent;
                 if (!hasVersion && (!hasRelativePath || relativePath != null)) {
-                    Path relPath = Paths.get(Objects.toString(relativePath, "../pom.xml"));
+                    Path relPath = Paths.get(Objects.toString(relativePath, ".."));
                     resolvedParent = resolveRelativePath(relPath, groupId, artifactId);
                 } else {
                     resolvedParent = Optional.empty();
@@ -128,9 +134,13 @@ protected void process(List<Event> buffer) {
     }
 
     protected Optional<RelativeProject> resolveRelativePath(Path relativePath, String groupId, String artifactId) {
-        Path pomPath = projectPath.resolve(relativePath);
+        Path pomPath = projectPath.resolve(relativePath).normalize();
-        if (Files.isDirectory(pomPath)) {
+        if (Files.isDirectory(pomPath) && modelLocator != null) {
-            pomPath = pomPath.resolve("pom.xml");
+            pomPath = modelLocator.apply(pomPath);
+        }
+
+        if (pomPath == null || !Files.isRegularFile(pomPath)) {
+            return Optional.empty();
         }
 
         Optional<RelativeProject> mappedProject = relativePathMapper.apply(pomPath.normalize());

maven-model-transform/src/test/java/org/apache/maven/model/transform/ParentXMLFilterTest.java

@@ -20,6 +20,8 @@
 
 import javax.xml.stream.XMLStreamReader;
 
+import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Optional;
@@ -32,10 +34,24 @@
 
 class ParentXMLFilterTest extends AbstractXMLFilterTests {
     private Function<XMLStreamReader, XMLStreamReader> filterCreator;
+    private Path projectPath;
 
     @BeforeEach
-    void reset() {
+    void reset() throws IOException {
         filterCreator = null;
+        projectPath = Paths.get("target/test-classes/" + getClass().getSimpleName() + "/child");
+        Files.createDirectories(projectPath);
+        if (!Files.isRegularFile(projectPath.resolve("../pom.xml"))) {
+            Files.createFile(projectPath.resolve("../pom.xml"));
+        }
+        if (!Files.isRegularFile(projectPath.resolve("pom.xml"))) {
+            Files.createFile(projectPath.resolve("pom.xml"));
+        }
+        Path relPath = projectPath.resolve("RELATIVEPATH");
+        Files.createDirectories(relPath);
+        if (!Files.isRegularFile(relPath.resolve("pom.xml"))) {
+            Files.createFile(relPath.resolve("pom.xml"));
+        }
     }
 
     @Override
@@ -47,14 +63,13 @@ protected XMLStreamReader getFilter(XMLStreamReader parser) {
 
     protected XMLStreamReader createFilter(XMLStreamReader parser) {
         return createFilter(
-                parser,
+                parser, x -> Optional.of(new RelativeProject("GROUPID", "ARTIFACTID", "1.0.0")), projectPath);
-                x -> Optional.of(new RelativeProject("GROUPID", "ARTIFACTID", "1.0.0")),
-                Paths.get("pom.xml").toAbsolutePath());
     }
 
     protected XMLStreamReader createFilter(
             XMLStreamReader parser, Function<Path, Optional<RelativeProject>> pathMapper, Path projectPath) {
-        return new ParentXMLFilter(new FastForwardFilter(parser), pathMapper, projectPath);
+        Function<Path, Path> locator = p -> p.resolve("pom.xml");
+        return new ParentXMLFilter(new FastForwardFilter(parser), pathMapper, locator, projectPath);
     }
 
     @Test