A few improvements

gnodet · gnodet · commit f1b7bf238763 · 2023-08-28T15:35:43.000+02:00
* change ModelParser to return an Optional&lt;Source&gt; instead of Path
* add a ModelParser@locateAndParse method
* move located pom existence check into a ModelLocator#locateExistingPom
This abstract the "pom.xml" file notion a bit more
diff --git a/api/maven-api-core/src/main/java/org/apache/maven/api/services/PathSource.java b/api/maven-api-core/src/main/java/org/apache/maven/api/services/PathSource.java
@@ -0,0 +1,54 @@
+/*
+ * 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.api.services;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Objects;
+
+import org.apache.maven.api.annotations.Nonnull;
+
+public class PathSource implements Source {
+
+    private final Path path;
+
+    public PathSource(@Nonnull Path path) {
+        this.path = Objects.requireNonNull(path);
+    }
+
+    @Nonnull
+    @Override
+    public Path getPath() {
+        return path;
+    }
+
+    @Nonnull
+    @Override
+    public InputStream getInputStream() throws IOException {
+        return Files.newInputStream(path);
+    }
+
+    @Nonnull
+    @Override
+    public String getLocation() {
+        return path.toString();
+    }
+}
diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/spi/ModelParser.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/spi/ModelParser.java
@@ -20,6 +20,7 @@
 
 import java.nio.file.Path;
 import java.util.Map;
+import java.util.Optional;
 
 import org.apache.maven.api.annotations.Experimental;
 import org.apache.maven.api.annotations.Nonnull;
@@ -40,16 +41,31 @@ public interface ModelParser {
      * @param dir the directory to locate the pom for, never {@code null}
      * @return the located pom or <code>null</code> if none was found by this parser
      */
-    @Nullable
-    Path locatePom(@Nonnull Path dir);
+    @Nonnull
+    Optional<Source> locate(@Nonnull Path dir);
 
     /**
      * Parse the model.
      *
      * @param source the source to parse, never {@code null}
+     * @param options possible parsing options, may be {@code null}
      * @return the parsed {@link Model}, never {@code null}
      * @throws ModelParserException if the model cannot be parsed
      */
     @Nonnull
-    Model parse(@Nonnull Source source, Map<String, ?> options) throws ModelParserException;
+    Model parse(@Nonnull Source source, @Nullable Map<String, ?> options) throws ModelParserException;
+
+    /**
+     * Locate and parse the model in the specified directory.
+     * This is equivalent to {@code locate(dir).map(s -> parse(s, options))}.
+     *
+     * @param dir the directory to locate the pom for, never {@code null}
+     * @param options possible parsing options, may be {@code null}
+     * @return an optional parsed {@link Model} or {@code null} if none could be found
+     * @throws ModelParserException if the located model cannot be parsd
+     */
+    default Optional<Model> locateAndParse(@Nonnull Path dir, @Nullable Map<String, ?> options)
+            throws ModelParserException {
+        return locate(dir).map(s -> parse(s, options));
+    }
 }
diff --git a/maven-core/src/main/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformer.java b/maven-core/src/main/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformer.java
@@ -39,10 +39,10 @@
 import java.util.function.BiConsumer;
 
 import org.apache.maven.api.model.Model;
+import org.apache.maven.api.services.Source;
 import org.apache.maven.api.spi.ModelParser;
 import org.apache.maven.feature.Features;
 import org.apache.maven.model.building.DefaultBuildPomXMLFilterFactory;
-import org.apache.maven.model.building.FileSource;
 import org.apache.maven.model.building.TransformerContext;
 import org.apache.maven.model.transform.RawToConsumerPomXMLFilterFactory;
 import org.apache.maven.model.transform.stax.XmlUtils;
@@ -196,13 +196,19 @@ BiConsumer<Path, Path> transformer(RepositorySystemSession session) {
      * The actual transformation: visible for testing.
      */
     void transform(Path pomFile, Path destPomFile, TransformerContext context) throws IOException, XMLStreamException {
-        ModelParser parser = modelParsers.values().stream()
-                .filter(p -> pomFile.equals(p.locatePom(pomFile.getParent())))
-                .findAny()
-                .orElse(null);
-        if (parser != null) {
+        ModelParser parser = null;
+        Source source = null;
+        for (ModelParser p : modelParsers.values()) {
+            source = p.locate(pomFile).orElse(null);
+            if (source != null) {
+                parser = p;
+                break;
+            }
+        }
+
+        if (source != null) {
             try (OutputStream output = Files.newOutputStream(destPomFile)) {
-                Model model = parser.parse(new FileSource(pomFile), null);
+                Model model = parser.parse(source, null);
                 // transform
                 model = model.withRoot(false).withModules(null);
                 if (model.getParent() != null) {
diff --git a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
@@ -497,15 +497,11 @@ private boolean build(
 
                 module = module.replace('\\', File.separatorChar).replace('/', File.separatorChar);
 
-                File moduleFile = new File(basedir, module);
+                File moduleFile = modelProcessor.locateExistingPom(new File(basedir, module));
 
-                if (moduleFile.isDirectory()) {
-                    moduleFile = modelProcessor.locatePom(moduleFile);
-                }
-
-                if (!moduleFile.isFile()) {
+                if (moduleFile == null) {
                     ModelProblem problem = new DefaultModelProblem(
-                            "Child module " + moduleFile + " of " + pomFile + " does not exist",
+                            "Child module " + module + " of " + pomFile + " does not exist",
                             ModelProblem.Severity.ERROR,
                             ModelProblem.Version.BASE,
                             model,
diff --git a/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java b/maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java
@@ -97,15 +97,16 @@ public List<MavenProject> collectProjects(MavenExecutionRequest request) throws
     }
 
     private File getMultiModuleProjectPomFile(MavenExecutionRequest request) {
-        if (request.getPom().getParentFile().equals(request.getMultiModuleProjectDirectory())) {
+        File multiModuleProjectDirectory = request.getMultiModuleProjectDirectory();
+        if (request.getPom().getParentFile().equals(multiModuleProjectDirectory)) {
             return request.getPom();
         } else {
-            File multiModuleProjectPom = modelLocator.locatePom(request.getMultiModuleProjectDirectory());
-            if (!multiModuleProjectPom.exists()) {
+            File multiModuleProjectPom = modelLocator.locateExistingPom(multiModuleProjectDirectory);
+            if (multiModuleProjectPom == null) {
                 LOGGER.info(
                         "Maven detected that the requested POM file is part of a multi-module project, "
                                 + "but could not find a pom.xml file in the multi-module root directory '{}'.",
-                        request.getMultiModuleProjectDirectory());
+                        multiModuleProjectDirectory);
                 LOGGER.info("The reactor is limited to all projects under: "
                         + request.getPom().getParent());
                 return request.getPom();
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
@@ -1354,18 +1354,11 @@ private File determinePom(final CommandLine commandLine, final String workingDir
             current = resolveFile(new File(alternatePomFile), workingDirectory);
         }
 
-        File pom;
-        if (current.isDirectory() && modelProcessor != null) {
-            pom = modelProcessor.locatePom(current);
+        if (modelProcessor != null) {
+            return modelProcessor.locateExistingPom(current);
         } else {
-            pom = current;
+            return current.isFile() ? current : null;
         }
-
-        if (pom.isFile()) {
-            return pom;
-        }
-
-        return null;
     }
 
     // Visible for testing
diff --git a/maven-model-builder/pom.xml b/maven-model-builder/pom.xml
@@ -142,6 +142,7 @@ under the License.
               <exclude>org.apache.maven.model.interpolation.StringSearchModelInterpolator</exclude>
               <exclude>org.apache.maven.model.interpolation.StringVisitorModelInterpolator</exclude>
               <exclude>org.apache.maven.model.io.DefaultModelReader#DefaultModelReader():CONSTRUCTOR_REMOVED</exclude>
+              <exclude>org.apache.maven.model.locator.ModelLocator#locateExistingPom(java.io.File):METHOD_NEW_DEFAULT</exclude>
               <exclude>org.apache.maven.model.management.DefaultDependencyManagementInjector$ManagementModelMerger</exclude>
               <exclude>org.apache.maven.model.management.DefaultPluginManagementInjector$ManagementModelMerger</exclude>
               <exclude>org.apache.maven.model.merge.MavenModelMerger</exclude>
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
@@ -1888,7 +1888,8 @@ public TransformerContext initialize(ModelBuildingRequest request, ModelProblemC
             return new TransformerContext() {
                 @Override
                 public Path locate(Path path) {
-                    return modelProcessor.locatePom(path.toFile()).toPath();
+                    File file = modelProcessor.locateExistingPom(path.toFile());
+                    return file != null ? file.toPath() : null;
                 }
 
                 @Override
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProcessor.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProcessor.java
@@ -26,14 +26,13 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Collectors;
+import java.util.Optional;
 
 import org.apache.maven.api.model.Model;
 import org.apache.maven.api.spi.ModelParser;
@@ -95,13 +94,36 @@ public Path locatePom(Path projectDirectory) {
         // Note that the ModelProcessor#locatePom never returns null
         // while the ModelParser#locatePom needs to return an existing path!
         Path pom = modelParsers.stream()
-                .map(m -> m.locatePom(projectDirectory))
+                .map(m -> m.locate(projectDirectory)
+                        .map(org.apache.maven.api.services.Source::getPath)
+                        .orElse(null))
                 .filter(Objects::nonNull)
-                .filter(Files::exists)
                 .findFirst()
                 .orElseGet(
                         () -> modelLocator.locatePom(projectDirectory.toFile()).toPath());
-        if (!pom.getParent().equals(projectDirectory)) {
+        if (!pom.equals(projectDirectory) && !pom.getParent().equals(projectDirectory)) {
+            throw new IllegalArgumentException("The POM found does not belong to the given directory: " + pom);
+        }
+        return pom;
+    }
+
+    public File locateExistingPom(File projectDirectory) {
+        Path path = locateExistingPom(projectDirectory.toPath());
+        return path != null ? path.toFile() : null;
+    }
+
+    public Path locateExistingPom(Path projectDirectory) {
+        // Note that the ModelProcessor#locatePom never returns null
+        // while the ModelParser#locatePom needs to return an existing path!
+        Path pom = modelParsers.stream()
+                .map(m -> m.locate(projectDirectory).map(s -> s.getPath()).orElse(null))
+                .filter(Objects::nonNull)
+                .findFirst()
+                .orElseGet(() -> {
+                    File f = modelLocator.locateExistingPom(projectDirectory.toFile());
+                    return f != null ? f.toPath() : null;
+                });
+        if (pom != null && !pom.equals(projectDirectory) && !pom.getParent().equals(projectDirectory)) {
             throw new IllegalArgumentException("The POM found does not belong to the given directory: " + pom);
         }
         return pom;
@@ -118,15 +140,17 @@ protected org.apache.maven.api.model.Model read(
         }
         if (pomFile != null) {
             Path projectDirectory = pomFile.getParent();
-            Path path = pomFile;
-            List<ModelParser> parsers = modelParsers.stream()
-                    .filter(p -> path.equals(p.locatePom(projectDirectory)))
-                    .collect(Collectors.toList());
+            //            Path path = pomFile;
+            //            List<ModelParser> parsers = modelParsers.stream()
+            //                    .filter(p -> path.equals(p.locatePom(projectDirectory)))
+            //                    .collect(Collectors.toList());
             List<ModelParserException> exceptions = new ArrayList<>();
-            for (ModelParser parser : parsers) {
+            for (ModelParser parser : modelParsers) {
                 try {
-                    Model model = parser.parse(new FileSource(pomFile), options);
-                    return model.withPomFile(pomFile);
+                    Optional<Model> model = parser.locateAndParse(projectDirectory, options);
+                    if (model.isPresent()) {
+                        return model.get().withPomFile(pomFile);
+                    }
                 } catch (ModelParserException e) {
                     exceptions.add(e);
                 }
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContext.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContext.java
@@ -18,6 +18,7 @@
  */
 package org.apache.maven.model.building;
 
+import java.io.File;
 import java.nio.file.Path;
 import java.util.Map;
 import java.util.Objects;
@@ -101,7 +102,8 @@ public Model getRawModel(String groupId, String artifactId) {
 
     @Override
     public Path locate(Path path) {
-        return modelLocator.locatePom(path.toFile()).toPath();
+        File file = modelLocator.locateExistingPom(path.toFile());
+        return file != null ? file.toPath() : null;
     }
 
     static class GAKey {
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/FileModelSource.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/FileModelSource.java
@@ -55,13 +55,11 @@ public File getPomFile() {
     public ModelSource3 getRelatedSource(ModelLocator locator, String relPath) {
         relPath = relPath.replace('\\', File.separatorChar).replace('/', File.separatorChar);
 
-        File relatedPom = new File(getFile().getParentFile(), relPath);
+        File path = new File(getFile().getParentFile(), relPath);
 
-        if (relatedPom.isDirectory() && locator != null) {
-            relatedPom = locator.locatePom(relatedPom);
-        }
+        File relatedPom = locator.locateExistingPom(path);
 
-        if (relatedPom.isFile() && relatedPom.canRead()) {
+        if (relatedPom != null) {
             return new FileModelSource(relatedPom.toPath().normalize().toFile());
         }
 
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/locator/DefaultModelLocator.java b/maven-model-builder/src/main/java/org/apache/maven/model/locator/DefaultModelLocator.java
@@ -34,6 +34,21 @@ public class DefaultModelLocator implements ModelLocator {
 
     @Override
     public File locatePom(File projectDirectory) {
-        return new File(projectDirectory, "pom.xml");
+        return projectDirectory != null ? projectDirectory : new File(".");
+    }
+
+    @Override
+    public File locateExistingPom(File project) {
+        if (project == null || project.isDirectory()) {
+            project = locatePom(project);
+        }
+        if (project.isDirectory()) {
+            File pom = new File(project, "pom.xml");
+            return pom.isFile() ? pom : null;
+        } else if (project.isFile()) {
+            return project;
+        } else {
+            return null;
+        }
     }
 }
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/locator/ModelLocator.java b/maven-model-builder/src/main/java/org/apache/maven/model/locator/ModelLocator.java
@@ -38,4 +38,14 @@ public interface ModelLocator {
      * @return The path to the (possibly non-existent) POM file, never {@code null}.
      */
     File locatePom(File projectDirectory);
+
+    /**
+     * Returns the file containing the pom or null if a pom can not be found at the given file or in the given directory.
+     */
+    default File locateExistingPom(File project) {
+        if (project == null || project.isDirectory()) {
+            project = locatePom(project);
+        }
+        return project.isFile() ? project : null;
+    }
 }
diff --git a/maven-model-transform/src/main/java/org/apache/maven/model/transform/ParentXMLFilter.java b/maven-model-transform/src/main/java/org/apache/maven/model/transform/ParentXMLFilter.java
@@ -20,7 +20,6 @@
 
 import javax.xml.stream.XMLStreamReader;
 
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.List;
@@ -134,12 +133,9 @@ protected void process(List<Event> buffer) {
     }
 
     protected Optional<RelativeProject> resolveRelativePath(Path relativePath, String groupId, String artifactId) {
-        Path pomPath = projectPath.resolve(relativePath).normalize();
-        if (Files.isDirectory(pomPath) && modelLocator != null) {
-            pomPath = modelLocator.apply(pomPath);
-        }
+        Path pomPath = modelLocator.apply(projectPath.resolve(relativePath).normalize());
 
-        if (pomPath == null || !Files.isRegularFile(pomPath)) {
+        if (pomPath == null) {
             return Optional.empty();
         }
 

Original file line number	Diff line number	Diff line change
`@@ -1888,7 +1888,8 @@ public TransformerContext initialize(ModelBuildingRequest request, ModelProblemC`
`1888`	`1888`	`return new TransformerContext() {`
`1889`	`1889`	`@Override`
`1890`	`1890`	`public Path locate(Path path) {`
`1891`		`- return modelProcessor.locatePom(path.toFile()).toPath();`
	`1891`	`+ File file = modelProcessor.locateExistingPom(path.toFile());`
	`1892`	`+ return file != null ? file.toPath() : null;`
`1892`	`1893`	`}`
`1893`	`1894`
`1894`	`1895`	`@Override`