From f2593b97ef7af89d981c90cb3c918fa44a07edff Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 23 Aug 2023 21:25:14 +0200 Subject: [PATCH] [MNG-7862] The ModelLocator should always be used when locating pom.xml (#1217) --- api/maven-api-model/src/main/mdo/maven.mdo | 2 +- .../maven/project/ReactorModelPool.java | 18 ------ .../ConsumerPomArtifactTransformerTest.java | 9 ++- .../java/org/apache/maven/cli/MavenCli.java | 25 ++++----- .../DefaultBuildPomXMLFilterFactory.java | 5 ++ .../model/building/DefaultModelBuilder.java | 17 ++++-- .../building/DefaultTransformerContext.java | 12 ++++ .../maven/model/building/FileModelSource.java | 12 ++-- .../maven/model/building/ModelSource3.java | 56 +++++++++++++++++++ .../model/building/TransformerContext.java | 2 + .../BuildToRawPomXMLFilterFactory.java | 6 +- .../model/transform/ParentXMLFilter.java | 20 +++++-- .../model/transform/ParentXMLFilterTest.java | 25 +++++++-- 13 files changed, 153 insertions(+), 56 deletions(-) create mode 100644 maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSource3.java diff --git a/api/maven-api-model/src/main/mdo/maven.mdo b/api/maven-api-model/src/main/mdo/maven.mdo index db55049c25..550da862be 100644 --- a/api/maven-api-model/src/main/mdo/maven.mdo +++ b/api/maven-api-model/src/main/mdo/maven.mdo @@ -1802,7 +1802,7 @@ ]]> String - ../pom.xml + .. diff --git a/maven-core/src/main/java/org/apache/maven/project/ReactorModelPool.java b/maven-core/src/main/java/org/apache/maven/project/ReactorModelPool.java index 305ec24ca3..b318057bae 100644 --- a/maven-core/src/main/java/org/apache/maven/project/ReactorModelPool.java +++ b/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) { diff --git a/maven-core/src/test/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformerTest.java b/maven-core/src/test/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformerTest.java index 27c4d15e6c..36b0774ef5 100644 --- a/maven-core/src/test/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformerTest.java +++ b/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(); } } } 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 index d7127f45d2..847bfcec06 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java +++ b/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); - if (pom.isDirectory()) { - if (modelProcessor != null) { - pom = modelProcessor.locatePom(pom); - } else { - pom = new File(pom, "pom.xml"); - } - } + current = resolveFile(new File(alternatePomFile), workingDirectory); + } + 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; diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java index eb211f5e40..7170173e3d 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java @@ -51,6 +51,11 @@ protected Function> getRelativePathMapper() { return p -> Optional.ofNullable(context.getRawModel(p)).map(DefaultBuildPomXMLFilterFactory::toRelativeProject); } + @Override + protected Function getModelLocator() { + return context::locate; + } + @Override protected BiFunction getDependencyKeyToVersionMapper() { return (g, a) -> Optional.ofNullable(context.getRawModel(g, a)) 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 index 9f78ee2301..fd5d2ffabb 100644 --- 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 @@ -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> 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( 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 index 6346a7d3d5..c4f77dd0b6 100644 --- 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 @@ -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 userProperties = new ConcurrentHashMap<>(); final Map modelByPath = new ConcurrentHashMap<>(); @@ -77,6 +80,10 @@ public Model computeIfAbsent(Supplier 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; 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 index 36284ef65a..92a059bbe6 100644 --- 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 @@ -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()) { - // TODO figure out how to reuse ModelLocator.locatePom(File) here - relatedPom = new File(relatedPom, "pom.xml"); + if (relatedPom.isDirectory() && locator != null) { + relatedPom = locator.locatePom(relatedPom); } if (relatedPom.isFile() && relatedPom.canRead()) { - return new FileModelSource(new File(relatedPom.toURI().normalize())); + return new FileModelSource(relatedPom.toPath().normalize().toFile()); } return null; diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSource3.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSource3.java new file mode 100644 index 0000000000..89fed34178 --- /dev/null +++ b/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 MUST + * accept relPath parameter values that + *
    + *
  • use either / or \ file path separator
    • + *
  • have .. parent directory references
    • + *
  • point either at file or directory
    • + *
+ * 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 null 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); + } +} diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/TransformerContext.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/TransformerContext.java index 9441523fe4..8fd5599afa 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/TransformerContext.java +++ b/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); } diff --git a/maven-model-transform/src/main/java/org/apache/maven/model/transform/BuildToRawPomXMLFilterFactory.java b/maven-model-transform/src/main/java/org/apache/maven/model/transform/BuildToRawPomXMLFilterFactory.java index 219fd89b8e..e8ec8b68ab 100644 --- a/maven-model-transform/src/main/java/org/apache/maven/model/transform/BuildToRawPomXMLFilterFactory.java +++ b/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> getRelativePathMapper() { return null; } + protected Function getModelLocator() { + return null; + } + protected BiFunction getDependencyKeyToVersionMapper() { return 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 index d483405f99..1f791e473f 100644 --- 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 @@ -47,6 +47,8 @@ class ParentXMLFilter extends NodeBufferingParser { private final Function> relativePathMapper; + private final Function 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> relativePathMapper, Path projectPath) { + XMLStreamReader delegate, + Function> relativePathMapper, + Function modelLocator, + Path projectPath) { super(delegate, "parent"); this.relativePathMapper = relativePathMapper; + this.modelLocator = modelLocator; this.projectPath = projectPath; } @@ -93,7 +99,7 @@ protected void process(List buffer) { } else if (event.event == END_ELEMENT && "parent".equals(event.name)) { Optional 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 buffer) { } protected Optional resolveRelativePath(Path relativePath, String groupId, String artifactId) { - Path pomPath = projectPath.resolve(relativePath); - if (Files.isDirectory(pomPath)) { - pomPath = pomPath.resolve("pom.xml"); + Path pomPath = projectPath.resolve(relativePath).normalize(); + if (Files.isDirectory(pomPath) && modelLocator != null) { + pomPath = modelLocator.apply(pomPath); + } + + if (pomPath == null || !Files.isRegularFile(pomPath)) { + return Optional.empty(); } Optional mappedProject = relativePathMapper.apply(pomPath.normalize()); diff --git a/maven-model-transform/src/test/java/org/apache/maven/model/transform/ParentXMLFilterTest.java b/maven-model-transform/src/test/java/org/apache/maven/model/transform/ParentXMLFilterTest.java index d27d604bed..73071eb059 100644 --- a/maven-model-transform/src/test/java/org/apache/maven/model/transform/ParentXMLFilterTest.java +++ b/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 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, - x -> Optional.of(new RelativeProject("GROUPID", "ARTIFACTID", "1.0.0")), - Paths.get("pom.xml").toAbsolutePath()); + parser, x -> Optional.of(new RelativeProject("GROUPID", "ARTIFACTID", "1.0.0")), projectPath); } protected XMLStreamReader createFilter( XMLStreamReader parser, Function> pathMapper, Path projectPath) { - return new ParentXMLFilter(new FastForwardFilter(parser), pathMapper, projectPath); + Function locator = p -> p.resolve("pom.xml"); + return new ParentXMLFilter(new FastForwardFilter(parser), pathMapper, locator, projectPath); } @Test