[MNG-8294] Consistency checks when loading parent (#1784)

Author: gnodet

api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelBuilderRequest.java

@@ -52,6 +52,10 @@ enum RequestType {
          * The request is for building a model from a POM file in a project on the filesystem.
          */
         BUILD_POM,
+        /**
+         * The request is for building the consumer POM.
+         */
+        CONSUMER_POM,
         /**
          * The request is for building a model from a parent POM file from a downloaded artifact.
          */

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

@@ -1744,20 +1744,16 @@
           <name>relativePath</name>
           <version>4.0.0+</version>
           <description>
-            The relative path of the parent {@code pom.xml} file within the checkout.
-            If not specified, it defaults to {@code ../pom.xml}.
+            The relative path of the parent subproject POM file or directory within the checkout.
+            If not specified, it defaults to {@code ..}, i.e. the parent directory.
             Maven looks for the parent POM first in this location on
-            the filesystem, then the local repository, and lastly in the remote repo.
-            {@code relativePath} allows you to select a different location,
-            for example when your structure is flat, or deeper without an intermediate parent POM.
-            However, the group ID, artifact ID and version are still required,
-            and must match the file in the location given, or it will revert to the repository for the POM.
-            This feature is only for enhancing the development in a local checkout of that project.
-            Set the value to an empty string in case you want to disable the feature and always resolve
-            the parent POM from the repositories.
+            the filesystem if explicitly provided, then in the reactor if groupId and artifactId are provided,
+            then in the default parent directory, then the local repository, and lastly in the remote repo.
+            However, if the both relative path and the group ID / artifact ID are provided,
+            they must match the file in the location given.
+            Specify either the {@code relativePath} or the {@code groupId}/{@code artifactId}, not both.
           </description>
           <type>String</type>
-          <defaultValue>..</defaultValue>
         </field>
       </fields>
       <codeSegments>

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java

@@ -326,6 +326,24 @@ public void validateFileModel(
                         "is either LATEST or RELEASE (both of them are being deprecated)",
                         parent);
             }
+
+            if (parent.getRelativePath() != null
+                    && !parent.getRelativePath().isEmpty()
+                    && (parent.getGroupId() != null && !parent.getGroupId().isEmpty()
+                            || parent.getArtifactId() != null
+                                    && !parent.getArtifactId().isEmpty())
+                    && validationLevel >= ModelValidator.VALIDATION_LEVEL_MAVEN_4_0
+                    && VALID_MODEL_VERSIONS.contains(m.getModelVersion())
+                    && !Objects.equals(m.getModelVersion(), ModelBuilder.MODEL_VERSION_4_0_0)) {
+                addViolation(
+                        problems,
+                        Severity.WARNING,
+                        Version.BASE,
+                        "parent.relativePath",
+                        null,
+                        "only specify relativePath or groupId/artifactId in modelVersion 4.1.0",
+                        parent);
+            }
         }
 
         if (validationLevel == ModelValidator.VALIDATION_LEVEL_MINIMAL) {

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelValidator.java

@@ -21,10 +21,14 @@
 import javax.inject.Inject;
 import javax.inject.Named;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+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.Objects;
 import java.util.stream.Collectors;
 
 import org.apache.maven.api.SessionData;
@@ -35,117 +39,32 @@
 import org.apache.maven.api.model.ModelBase;
 import org.apache.maven.api.model.Profile;
 import org.apache.maven.api.model.Repository;
-import org.apache.maven.api.services.Interpolator;
 import org.apache.maven.api.services.ModelBuilder;
 import org.apache.maven.api.services.ModelBuilderException;
 import org.apache.maven.api.services.ModelBuilderRequest;
 import org.apache.maven.api.services.ModelBuilderResult;
-import org.apache.maven.api.services.ModelProblemCollector;
 import org.apache.maven.api.services.ModelSource;
-import org.apache.maven.api.services.SuperPomProvider;
-import org.apache.maven.api.services.model.DependencyManagementImporter;
-import org.apache.maven.api.services.model.DependencyManagementInjector;
-import org.apache.maven.api.services.model.InheritanceAssembler;
+import org.apache.maven.api.services.Source;
 import org.apache.maven.api.services.model.LifecycleBindingsInjector;
-import org.apache.maven.api.services.model.ModelCacheFactory;
-import org.apache.maven.api.services.model.ModelInterpolator;
-import org.apache.maven.api.services.model.ModelNormalizer;
-import org.apache.maven.api.services.model.ModelPathTranslator;
-import org.apache.maven.api.services.model.ModelProcessor;
-import org.apache.maven.api.services.model.ModelResolver;
-import org.apache.maven.api.services.model.ModelUrlNormalizer;
-import org.apache.maven.api.services.model.ModelValidator;
-import org.apache.maven.api.services.model.ModelVersionParser;
-import org.apache.maven.api.services.model.PluginConfigurationExpander;
-import org.apache.maven.api.services.model.PluginManagementInjector;
-import org.apache.maven.api.services.model.ProfileActivationContext;
-import org.apache.maven.api.services.model.ProfileInjector;
-import org.apache.maven.api.services.model.ProfileSelector;
-import org.apache.maven.api.spi.ModelTransformer;
 import org.apache.maven.internal.impl.InternalSession;
-import org.apache.maven.internal.impl.model.DefaultModelBuilder;
-import org.apache.maven.internal.impl.model.DefaultProfileSelector;
-import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolator;
 import org.apache.maven.model.v4.MavenModelVersion;
 import org.apache.maven.project.MavenProject;
 import org.eclipse.aether.RepositorySystemSession;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 @Named
 class DefaultConsumerPomBuilder implements ConsumerPomBuilder {
     private static final String BOM_PACKAGING = "bom";
 
     public static final String POM_PACKAGING = "pom";
 
-    private final ProfileInjector profileInjector;
-    private final InheritanceAssembler inheritanceAssembler;
-    private final DependencyManagementImporter dependencyManagementImporter;
-    private final DependencyManagementInjector dependencyManagementInjector;
     private final LifecycleBindingsInjector lifecycleBindingsInjector;
-    private final ModelInterpolator modelInterpolator;
-    private final ModelNormalizer modelNormalizer;
-    private final ModelPathTranslator modelPathTranslator;
-    private final ModelProcessor modelProcessor;
-    private final ModelUrlNormalizer modelUrlNormalizer;
-    private final ModelValidator modelValidator;
-    private final PluginConfigurationExpander pluginConfigurationExpander;
-    private final PluginManagementInjector pluginManagementInjector;
-    private final SuperPomProvider superPomProvider;
-    private final ModelVersionParser versionParser;
-    private final ProfileActivationFilePathInterpolator profileActivationFilePathInterpolator;
-    private final List<ModelTransformer> transformers;
-    private final ModelCacheFactory modelCacheFactory;
-    private final ModelResolver modelResolver;
-    private final Interpolator interpolator;
 
     @Inject
     @SuppressWarnings("checkstyle:ParameterNumber")
-    DefaultConsumerPomBuilder(
-            ProfileInjector profileInjector,
-            InheritanceAssembler inheritanceAssembler,
-            DependencyManagementImporter dependencyManagementImporter,
-            DependencyManagementInjector dependencyManagementInjector,
-            LifecycleBindingsInjector lifecycleBindingsInjector,
-            ModelInterpolator modelInterpolator,
-            ModelNormalizer modelNormalizer,
-            ModelPathTranslator modelPathTranslator,
-            ModelProcessor modelProcessor,
-            ModelUrlNormalizer modelUrlNormalizer,
-            ModelValidator modelValidator,
-            PluginConfigurationExpander pluginConfigurationExpander,
-            PluginManagementInjector pluginManagementInjector,
-            SuperPomProvider superPomProvider,
-            ModelVersionParser versionParser,
-            ProfileActivationFilePathInterpolator profileActivationFilePathInterpolator,
-            List<ModelTransformer> transformers,
-            ModelCacheFactory modelCacheFactory,
-            ModelResolver modelResolver,
-            Interpolator interpolator) {
-        this.profileInjector = profileInjector;
-        this.inheritanceAssembler = inheritanceAssembler;
-        this.dependencyManagementImporter = dependencyManagementImporter;
-        this.dependencyManagementInjector = dependencyManagementInjector;
+    DefaultConsumerPomBuilder(LifecycleBindingsInjector lifecycleBindingsInjector) {
         this.lifecycleBindingsInjector = lifecycleBindingsInjector;
-        this.modelInterpolator = modelInterpolator;
-        this.modelNormalizer = modelNormalizer;
-        this.modelPathTranslator = modelPathTranslator;
-        this.modelProcessor = modelProcessor;
-        this.modelUrlNormalizer = modelUrlNormalizer;
-        this.modelValidator = modelValidator;
-        this.pluginConfigurationExpander = pluginConfigurationExpander;
-        this.pluginManagementInjector = pluginManagementInjector;
-        this.superPomProvider = superPomProvider;
-        this.versionParser = versionParser;
-        this.profileActivationFilePathInterpolator = profileActivationFilePathInterpolator;
-        this.transformers = transformers;
-        this.modelCacheFactory = modelCacheFactory;
-        this.modelResolver = modelResolver;
-        this.interpolator = interpolator;
     }
 
-    private final Logger logger = LoggerFactory.getLogger(getClass());
-
     @Override
     public Model build(RepositorySystemSession session, MavenProject project, Path src) throws ModelBuilderException {
         Model model = project.getModel().getDelegate();
@@ -174,49 +93,18 @@ protected Model buildNonPom(RepositorySystemSession session, MavenProject projec
 
     private ModelBuilderResult buildModel(RepositorySystemSession session, MavenProject project, Path src)
             throws ModelBuilderException {
-        ProfileSelector customSelector = new DefaultProfileSelector() {
-            @Override
-            public List<Profile> getActiveProfiles(
-                    Collection<Profile> profiles, ProfileActivationContext context, ModelProblemCollector problems) {
-                return new ArrayList<>();
-            }
-        };
-        // TODO: the custom selector should be used as a flag on the request
-        DefaultModelBuilder modelBuilder = new DefaultModelBuilder(
-                modelProcessor,
-                modelValidator,
-                modelNormalizer,
-                modelInterpolator,
-                modelPathTranslator,
-                modelUrlNormalizer,
-                superPomProvider,
-                inheritanceAssembler,
-                customSelector,
-                profileInjector,
-                pluginManagementInjector,
-                dependencyManagementInjector,
-                dependencyManagementImporter,
-                pluginConfigurationExpander,
-                profileActivationFilePathInterpolator,
-                versionParser,
-                transformers,
-                modelCacheFactory,
-                modelResolver,
-                interpolator);
         InternalSession iSession = InternalSession.from(session);
         ModelBuilderRequest.ModelBuilderRequestBuilder request = ModelBuilderRequest.builder();
-        request.requestType(ModelBuilderRequest.RequestType.BUILD_POM);
+        request.requestType(ModelBuilderRequest.RequestType.CONSUMER_POM);
         request.session(iSession);
-        request.source(ModelSource.fromPath(src));
+        // in order to resolve parents, we need to fake being at the correct location
+        request.source(new PomConsumerModelSource(project.getModel().getPomPath(), src));
         request.locationTracking(false);
         request.systemProperties(session.getSystemProperties());
         request.userProperties(session.getUserProperties());
         request.lifecycleBindingsInjector(lifecycleBindingsInjector::injectLifecycleBindings);
         ModelBuilder.ModelBuilderSession mbSession =
                 iSession.getData().get(SessionData.key(ModelBuilder.ModelBuilderSession.class));
-        if (mbSession == null) {
-            mbSession = modelBuilder.newSession();
-        }
         return mbSession.build(request.build());
     }
 
@@ -320,4 +208,63 @@ private static List<Repository> pruneRepositories(List<Repository> repositories)
                 .filter(r -> !org.apache.maven.api.Repository.CENTRAL_ID.equals(r.getId()))
                 .collect(Collectors.toList());
     }
+
+    static class PomConsumerModelSource implements ModelSource {
+        final Path path;
+        final Path src;
+
+        PomConsumerModelSource(Path path, Path src) {
+            this.path = path;
+            this.src = src;
+        }
+
+        @Override
+        public Path getPath() {
+            return path;
+        }
+
+        @Override
+        public InputStream openStream() throws IOException {
+            return Files.newInputStream(src);
+        }
+
+        @Override
+        public String getLocation() {
+            return src.toString();
+        }
+
+        @Override
+        public Source resolve(String relative) {
+            return ModelSource.fromPath(path.resolve(relative));
+        }
+
+        @Override
+        public ModelSource resolve(ModelLocator locator, String relative) {
+            String norm = relative.replace('\\', File.separatorChar).replace('/', File.separatorChar);
+            Path path = getPath().getParent().resolve(norm);
+            Path relatedPom = locator.locateExistingPom(path);
+            if (relatedPom != null) {
+                return ModelSource.fromPath(relatedPom);
+            }
+            return null;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            return this == o
+                    || o.getClass() == getClass()
+                            && Objects.equals(path, ((PomConsumerModelSource) o).path)
+                            && Objects.equals(src, ((PomConsumerModelSource) o).src);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(path, src);
+        }
+
+        @Override
+        public String toString() {
+            return "PomConsumerModelSource[" + "path=" + path + ']';
+        }
+    }
 }

maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java

@@ -74,6 +74,7 @@
 import org.apache.maven.bridge.MavenRepositorySystem;
 import org.apache.maven.internal.impl.InternalSession;
 import org.apache.maven.internal.impl.resolver.ArtifactDescriptorUtils;
+import org.apache.maven.model.building.DefaultModelProblem;
 import org.apache.maven.model.building.FileModelSource;
 import org.apache.maven.model.building.ModelBuildingRequest;
 import org.apache.maven.model.building.ModelSource2;
@@ -499,6 +500,11 @@ private List<ProjectBuildingResult> build(File pomFile, boolean recursive) {
             List<ProjectBuildingResult> results = new ArrayList<>();
             List<ModelBuilderResult> allModels = results(result).toList();
             for (ModelBuilderResult r : allModels) {
+                List<ModelProblem> problems = new ArrayList<>(r.getProblems());
+                results(r)
+                        .filter(c -> c != r)
+                        .flatMap(c -> c.getProblems().stream())
+                        .forEach(problems::remove);
                 if (r.getEffectiveModel() != null) {
                     File pom = r.getSource().getPath().toFile();
                     MavenProject project =
@@ -510,18 +516,17 @@ private List<ProjectBuildingResult> build(File pomFile, boolean recursive) {
                     project.setExecutionRoot(pom.equals(pomFile));
                     initProject(project, r);
                     project.setCollectedProjects(results(r)
-                            .filter(cr -> cr != r)
+                            .filter(cr -> cr != r && cr.getEffectiveModel() != null)
                             .map(cr -> projectIndex.get(cr.getEffectiveModel().getId()))
                             .collect(Collectors.toList()));
 
                     DependencyResolutionResult resolutionResult = null;
                     if (request.isResolveDependencies()) {
                         resolutionResult = resolveDependencies(project);
                     }
-                    results.add(
-                            new DefaultProjectBuildingResult(project, convert(result.getProblems()), resolutionResult));
+                    results.add(new DefaultProjectBuildingResult(project, convert(problems), resolutionResult));
                 } else {
-                    results.add(new DefaultProjectBuildingResult(null, convert(result.getProblems()), null));
+                    results.add(new DefaultProjectBuildingResult(null, convert(problems), null));
                 }
             }
             return results;
@@ -535,20 +540,21 @@ private List<org.apache.maven.model.building.ModelProblem> convert(List<ModelPro
             if (problems == null) {
                 return null;
             }
-            return problems.stream()
-                    .map(p -> (org.apache.maven.model.building.ModelProblem)
-                            new org.apache.maven.model.building.DefaultModelProblem(
-                                    p.getMessage(),
-                                    org.apache.maven.model.building.ModelProblem.Severity.valueOf(
-                                            p.getSeverity().name()),
-                                    org.apache.maven.model.building.ModelProblem.Version.valueOf(
-                                            p.getVersion().name()),
-                                    p.getSource(),
-                                    p.getLineNumber(),
-                                    p.getColumnNumber(),
-                                    p.getModelId(),
-                                    p.getException()))
-                    .toList();
+            return problems.stream().map(p -> convert(p)).toList();
+        }
+
+        private static org.apache.maven.model.building.ModelProblem convert(ModelProblem p) {
+            return new DefaultModelProblem(
+                    p.getMessage(),
+                    org.apache.maven.model.building.ModelProblem.Severity.valueOf(
+                            p.getSeverity().name()),
+                    org.apache.maven.model.building.ModelProblem.Version.valueOf(
+                            p.getVersion().name()),
+                    p.getSource(),
+                    p.getLineNumber(),
+                    p.getColumnNumber(),
+                    p.getModelId(),
+                    p.getException());
         }
 
         @SuppressWarnings({"checkstyle:methodlength", "deprecation"})