diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java index bcd0a191f8c3..822c0a6e3eb7 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java +++ b/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. - * + *

* 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. - * + *

* 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. - * + *

* 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.) - * + *

* 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. - * + *

* 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 modelParsers; + private final @Nullable List modelParsers; - @Inject public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable List 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 exceptions = new ArrayList<>(); if (pomFile != null) { - Path projectDirectory = pomFile.getParent(); - List exceptions = new ArrayList<>(); for (ModelParser parser : modelParsers) { try { - Optional 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 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); - } } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelProcessorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelProcessorTest.java new file mode 100644 index 000000000000..0a7cb82f720e --- /dev/null +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelProcessorTest.java @@ -0,0 +1,243 @@ +/* + * 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.impl.model; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Optional; + +import org.apache.maven.api.model.Model; +import org.apache.maven.api.services.Source; +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 org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class DefaultModelProcessorTest { + + @TempDir + Path tempDir; + + Path testProjectDir, testPomFile; + + @AfterEach + void cleanup() throws IOException { + if (testPomFile != null && Files.exists(testPomFile)) { + Files.deleteIfExists(testPomFile); + } + if (testProjectDir != null && Files.exists(testProjectDir)) { + Files.deleteIfExists(testProjectDir); + } + } + + @Test + void readWithValidParserShouldReturnModel() throws Exception { + ModelXmlFactory factory = mock(ModelXmlFactory.class); + ModelParser parser = mock(ModelParser.class); + XmlReaderRequest request = mock(XmlReaderRequest.class); + Model model = mock(Model.class); + Path path = Path.of("project/pom.xml"); + when(request.getPath()).thenReturn(path); + when(request.isStrict()).thenReturn(true); + when(model.withPomFile(path)).thenReturn(model); + when(parser.locateAndParse(any(), any())).thenReturn(Optional.of(model)); + Model result = new DefaultModelProcessor(factory, List.of(parser)).read(request); + assertNotNull(result); + assertEquals(model, result); + } + + @Test + void readNullPomPathShouldUseFactoryDirectly() throws Exception { + ModelXmlFactory factory = mock(ModelXmlFactory.class); + XmlReaderRequest request = mock(XmlReaderRequest.class); + Model model = mock(Model.class); + when(request.getPath()).thenReturn(null); + when(factory.read(request)).thenReturn(model); + Model result = new DefaultModelProcessor(factory, List.of()).read(request); + assertNotNull(result); + assertEquals(model, result); + } + + @Test + void locateExistingPomWithParsersShouldReturnFirstValid() { + Path expectedPom = Path.of("project/pom.xml"); + Source mockSource = mock(Source.class); + when(mockSource.getPath()).thenReturn(expectedPom); + ModelParser parser = mock(ModelParser.class); + when(parser.locate(any())).thenReturn(Optional.of(mockSource)); + assertEquals( + expectedPom, + new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of(parser)) + .locateExistingPom(Path.of("project"))); + } + + @Test + void locateExistingPomParserReturnsPathOutsideProjectShouldThrow() { + Source mockSource = mock(Source.class); + when(mockSource.getPath()).thenReturn(Path.of("other/pom.xml")); + ModelParser parser = mock(ModelParser.class); + when(parser.locate(any())).thenReturn(Optional.of(mockSource)); + assertThat(assertThrows(IllegalArgumentException.class, () -> new DefaultModelProcessor( + mock(ModelXmlFactory.class), List.of(parser)) + .locateExistingPom(Path.of("project"))) + .getMessage()) + .contains("does not belong to the given directory"); + } + + @Test + void locateExistingPomFallbackWithValidPomShouldReturnPom() throws Exception { + testProjectDir = Files.createTempDirectory(tempDir, "testproject"); + testPomFile = testProjectDir.resolve("pom.xml"); + Files.createFile(testPomFile); + assertEquals( + testPomFile, + new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of()).locateExistingPom(testProjectDir)); + } + + @Test + void locateExistingPomFallbackWithFileAsPathShouldReturnThatFile() throws Exception { + testPomFile = Files.createTempFile(tempDir, "pom", ".xml"); + assertEquals( + testPomFile, + new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of()).locateExistingPom(testPomFile)); + } + + @Test + void readWithParserThrowingExceptionShouldCollectException() { + ModelXmlFactory factory = mock(ModelXmlFactory.class); + ModelParser parser = mock(ModelParser.class); + XmlReaderRequest request = mock(XmlReaderRequest.class); + when(request.getPath()).thenReturn(Path.of("project/pom.xml")); + when(request.isStrict()).thenReturn(true); + when(parser.locateAndParse(any(), any())).thenThrow(new RuntimeException("Parser error")); + when(factory.read(request)).thenThrow(new RuntimeException("Factory error")); + RuntimeException ex = assertThrows( + RuntimeException.class, () -> new DefaultModelProcessor(factory, List.of(parser)).read(request)); + assertEquals("Factory error", ex.getMessage()); + assertEquals(1, ex.getSuppressed().length); + assertInstanceOf(ModelParserException.class, ex.getSuppressed()[0]); + assertEquals("Parser error", ex.getSuppressed()[0].getCause().getMessage()); + } + + @Test + void readWithFactoryThrowingExceptionShouldRethrowWithSuppressed() { + ModelXmlFactory factory = mock(ModelXmlFactory.class); + XmlReaderRequest request = mock(XmlReaderRequest.class); + when(request.getPath()).thenReturn(Path.of("project/pom.xml")); + when(factory.read(request)).thenThrow(new RuntimeException("Factory error")); + ModelParser parser = mock(ModelParser.class); + when(parser.locateAndParse(any(), any())).thenThrow(new RuntimeException("Parser error")); + RuntimeException ex = assertThrows( + RuntimeException.class, () -> new DefaultModelProcessor(factory, List.of(parser)).read(request)); + assertEquals("Factory error", ex.getMessage()); + assertEquals(1, ex.getSuppressed().length); + assertInstanceOf(ModelParserException.class, ex.getSuppressed()[0]); + assertEquals("Parser error", ex.getSuppressed()[0].getCause().getMessage()); + } + + @Test + void locateExistingPomWithDirectoryContainingPom() throws IOException { + testProjectDir = Files.createTempDirectory(tempDir, "project"); + testPomFile = testProjectDir.resolve("pom.xml"); + Files.createFile(testPomFile); + assertEquals( + testPomFile, + new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of()).locateExistingPom(testProjectDir)); + } + + @Test + void locateExistingPomWithDirectoryWithoutPom() throws IOException { + testProjectDir = Files.createTempDirectory(tempDir, "project"); + assertNull(new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of()).locateExistingPom(testProjectDir)); + } + + @Test + void locateExistingPomWithPomFile() throws IOException { + testPomFile = Files.createTempFile(tempDir, "pom", ".xml"); + assertEquals( + testPomFile, + new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of()).locateExistingPom(testPomFile)); + } + + @Test + void locateExistingPomWithNonExistentPath() { + assertNotNull(new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of()) + .locateExistingPom(tempDir.resolve("nonexistent"))); + } + + @Test + void locateExistingPomWithNullProjectAndNoPomInUserDirShouldReturnNull() { + System.setProperty("user.dir", tempDir.toString()); + assertNull(new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of()).locateExistingPom(null)); + } + + @Test + void locateExistingPomShouldAcceptPomInProjectDirectory() { + Path projectDir = Path.of("project"); + Path pomInDir = projectDir.resolve("pom.xml"); + Source mockSource = mock(Source.class); + when(mockSource.getPath()).thenReturn(pomInDir); + ModelParser parser = mock(ModelParser.class); + when(parser.locate(any())).thenReturn(Optional.of(mockSource)); + assertEquals( + pomInDir, + new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of(parser)).locateExistingPom(projectDir)); + } + + @Test + void locateExistingPomShouldAcceptPomAsProjectDirectory() { + Path pomFile = Path.of("pom.xml"); + Source mockSource = mock(Source.class); + when(mockSource.getPath()).thenReturn(pomFile); + ModelParser parser = mock(ModelParser.class); + when(parser.locate(any())).thenReturn(Optional.of(mockSource)); + assertEquals( + pomFile, + new DefaultModelProcessor(mock(ModelXmlFactory.class), List.of(parser)).locateExistingPom(pomFile)); + } + + @Test + void locateExistingPomShouldRejectPomInDifferentDirectory() { + Source mockSource = mock(Source.class); + when(mockSource.getPath()).thenReturn(Path.of("other/pom.xml")); + ModelParser parser = mock(ModelParser.class); + when(parser.locate(any())).thenReturn(Optional.of(mockSource)); + assertTrue(assertThrows(IllegalArgumentException.class, () -> new DefaultModelProcessor( + mock(ModelXmlFactory.class), List.of(parser)) + .locateExistingPom(Path.of("project"))) + .getMessage() + .contains("does not belong to the given directory")); + } +}