Pull #2292: Modernize codebase with Java improvements - functionalize DefaultModelProcessor#read

Author: Vincent Potucek

impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java

@@ -29,37 +29,39 @@
 import java.util.Optional;
 
 import org.apache.maven.api.annotations.Nullable;
-import org.apache.maven.api.di.Inject;
 import org.apache.maven.api.di.Named;
 import org.apache.maven.api.di.Singleton;
 import org.apache.maven.api.model.Model;
+import org.apache.maven.api.services.Source;
 import org.apache.maven.api.services.model.ModelProcessor;
 import org.apache.maven.api.services.xml.ModelXmlFactory;
 import org.apache.maven.api.services.xml.XmlReaderRequest;
 import org.apache.maven.api.spi.ModelParser;
 import org.apache.maven.api.spi.ModelParserException;
 
+import static org.apache.maven.api.spi.ModelParser.STRICT;
+
 /**
  *
  * Note: uses @Typed to limit the types it is available for injection to just ModelProcessor.
- *
+ * <p>
  * This is because the ModelProcessor interface extends ModelLocator and ModelReader. If we
  * made this component available under all its interfaces then it could end up being injected
  * into itself leading to a stack overflow.
- *
+ * <p>
  * A side effect of using @Typed is that it translates to explicit bindings in the container.
  * So instead of binding the component under a 'wildcard' key it is now bound with an explicit
  * key. Since this is a default component; this will be a plain binding of ModelProcessor to
  * this implementation type; that is, no hint/name.
- *
+ * <p>
  * This leads to a second side effect in that any @Inject request for just ModelProcessor in
  * the same injector is immediately matched to this explicit binding, which means extensions
  * cannot override this binding. This is because the lookup is always short-circuited in this
  * specific situation (plain @Inject request, and plain explicit binding for the same type.)
- *
+ * <p>
  * The simplest solution is to use a custom @Named here so it isn't bound under the plain key.
  * This is only necessary for default components using @Typed that want to support overriding.
- *
+ * <p>
  * As a non-default component this now gets a negative priority relative to other implementations
  * of the same interface. Since we want to allow overriding this doesn't matter in this case.
  * (if it did we could add @Priority of 0 to match the priority given to default components.)
@@ -69,75 +71,74 @@
 public class DefaultModelProcessor implements ModelProcessor {
 
     private final ModelXmlFactory modelXmlFactory;
-    private final List<ModelParser> modelParsers;
+    private final @Nullable List<ModelParser> modelParsers;
 
-    @Inject
     public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable List<ModelParser> modelParsers) {
         this.modelXmlFactory = modelXmlFactory;
         this.modelParsers = modelParsers;
     }
 
+    /**
+     * @implNote The ModelProcessor#locatePom never returns null while the ModelParser#locatePom needs to return an existing path!
+     */
     @Override
     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(org.apache.maven.api.services.Source::getPath)
-                        .orElse(null))
-                .filter(Objects::nonNull)
+        return modelParsers.stream()
+                .map(parser -> parser.locate(projectDirectory))
+                .filter(Optional::isPresent)
+                .map(Optional::get)
+                .map(Source::getPath)
+                .peek(path -> throwIfWrongProjectDirLocation(path, projectDirectory))
                 .findFirst()
-                .orElseGet(() -> doLocateExistingPom(projectDirectory));
+                .orElseGet(() -> locateExistingPomWithUserDirDefault(projectDirectory));
+    }
+
+    private static void throwIfWrongProjectDirLocation(Path pom, Path projectDirectory) {
         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;
+    }
+
+    private Path locateExistingPomWithUserDirDefault(Path project) {
+        return locateExistingPomInDirOrFile(project != null ? project : Paths.get(System.getProperty("user.dir")));
+    }
+
+    private static Path locateExistingPomInDirOrFile(Path project) {
+        return Files.isDirectory(project) ? isRegularFileOrNull(project.resolve("pom.xml")) : project;
+    }
+
+    private static Path isRegularFileOrNull(Path pom) {
+        return Files.isRegularFile(pom) ? pom : null;
     }
 
     @Override
     public Model read(XmlReaderRequest request) throws IOException {
         Objects.requireNonNull(request, "source cannot be null");
-        Path pomFile = request.getPath();
+        return readPomWithParentInheritance(request, request.getPath());
+    }
+
+    private Model readPomWithParentInheritance(XmlReaderRequest request, Path pomFile) {
+        List<ModelParserException> exceptions = new ArrayList<>();
         if (pomFile != null) {
-            Path projectDirectory = pomFile.getParent();
-            List<ModelParserException> exceptions = new ArrayList<>();
             for (ModelParser parser : modelParsers) {
                 try {
-                    Optional<Model> model =
-                            parser.locateAndParse(projectDirectory, Map.of(ModelParser.STRICT, request.isStrict()));
-                    if (model.isPresent()) {
-                        return model.get().withPomFile(pomFile);
-                    }
-                } catch (ModelParserException e) {
-                    exceptions.add(e);
+                    return parser.locateAndParse(pomFile, Map.of(STRICT, request.isStrict()))
+                            .orElseThrow()
+                            .withPomFile(pomFile);
+                } catch (RuntimeException ex) {
+                    exceptions.add(new ModelParserException(ex));
                 }
             }
-            try {
-                return doRead(request);
-            } catch (IOException e) {
-                exceptions.forEach(e::addSuppressed);
-                throw e;
-            }
-        } else {
-            return doRead(request);
         }
+        return readPomWithSuppressedErrorRethrow(request, exceptions);
     }
 
-    private Path doLocateExistingPom(Path project) {
-        if (project == null) {
-            project = Paths.get(System.getProperty("user.dir"));
-        }
-        if (Files.isDirectory(project)) {
-            Path pom = project.resolve("pom.xml");
-            return Files.isRegularFile(pom) ? pom : null;
-        } else if (Files.isRegularFile(project)) {
-            return project;
-        } else {
-            return null;
+    private Model readPomWithSuppressedErrorRethrow(XmlReaderRequest request, List<ModelParserException> exceptions) {
+        try {
+            return modelXmlFactory.read(request);
+        } catch (RuntimeException ex) {
+            exceptions.forEach(ex::addSuppressed);
+            throw ex;
         }
     }
-
-    private Model doRead(XmlReaderRequest request) throws IOException {
-        return modelXmlFactory.read(request);
-    }
 }