[MNG-7615] Add a cycle detection to the transformer context

maven-core/src/test/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformerTest.java

@@ -88,12 +88,12 @@ class ConsumerPomArtifactTransformerTest {
         }
 
         @Override
-        public Model getRawModel(String groupId, String artifactId) throws IllegalStateException {
+        public Model getRawModel(Path from, String groupId, String artifactId) throws IllegalStateException {
             throw new UnsupportedOperationException();
         }
 
         @Override
-        public Model getRawModel(Path p) {
+        public Model getRawModel(Path from, Path p) {
             throw new UnsupportedOperationException();
         }
 

maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java

@@ -71,7 +71,7 @@ class BuildModelSourceTransformer implements ModelSourceTransformer {
             String path = Optional.ofNullable(parent.getRelativePath()).orElse("..");
             if (version == null && !path.isEmpty()) {
                 Optional<RelativeProject> resolvedParent = resolveRelativePath(
-                        pomFile.getParent(), context, Paths.get(path), parent.getGroupId(), parent.getArtifactId());
+                        pomFile, context, Paths.get(path), parent.getGroupId(), parent.getArtifactId());
                 resolvedParent.ifPresent(relativeProject -> parent.setVersion(relativeProject.getVersion()));
             }
         }
@@ -99,7 +99,8 @@ class BuildModelSourceTransformer implements ModelSourceTransformer {
     void handleReactorDependencies(TransformerContext context, Model model) {
         for (Dependency dep : model.getDependencies()) {
             if (dep.getVersion() == null) {
-                Model depModel = context.getRawModel(dep.getGroupId(), dep.getArtifactId());
+                Model depModel =
+                        context.getRawModel(model.getDelegate().getPomFile(), dep.getGroupId(), dep.getArtifactId());
                 if (depModel != null) {
                     String v = depModel.getVersion();
                     if (v == null && depModel.getParent() != null) {
@@ -124,8 +125,8 @@ class BuildModelSourceTransformer implements ModelSourceTransformer {
     }
 
     protected Optional<RelativeProject> resolveRelativePath(
-            Path projectPath, TransformerContext context, Path relativePath, String groupId, String artifactId) {
+            Path pomFile, TransformerContext context, Path relativePath, String groupId, String artifactId) {
-        Path pomPath = projectPath.resolve(relativePath).normalize();
+        Path pomPath = pomFile.resolveSibling(relativePath).normalize();
         if (Files.isDirectory(pomPath)) {
             pomPath = context.locate(pomPath);
         }
@@ -134,7 +135,7 @@ class BuildModelSourceTransformer implements ModelSourceTransformer {
             return Optional.empty();
         }
 
-        Optional<RelativeProject> mappedProject = Optional.ofNullable(context.getRawModel(pomPath.normalize()))
+        Optional<RelativeProject> mappedProject = Optional.ofNullable(context.getRawModel(pomFile, pomPath.normalize()))
                 .map(BuildModelSourceTransformer::toRelativeProject);
 
         if (mappedProject.isPresent()) {

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

@@ -25,21 +25,16 @@ import javax.inject.Singleton;
 import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Field;
-import java.nio.file.Files;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
-import java.util.Set;
 import java.util.concurrent.Callable;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ForkJoinTask;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -660,7 +655,7 @@ public class DefaultModelBuilder implements ModelBuilder {
 
     @Override
     public DefaultTransformerContextBuilder newTransformerContextBuilder() {
-        return new DefaultTransformerContextBuilder();
+        return new DefaultTransformerContextBuilder(this);
     }
 
     @Override
@@ -1021,7 +1016,7 @@ public class DefaultModelBuilder implements ModelBuilder {
             if (request.getTransformerContextBuilder() instanceof DefaultTransformerContextBuilder) {
                 DefaultTransformerContextBuilder contextBuilder =
                         (DefaultTransformerContextBuilder) request.getTransformerContextBuilder();
-                contextBuilder.putSource(getGroupId(model), model.getArtifactId(), modelSource);
+                contextBuilder.putSource(getGroupId(model), model.getArtifactId(), (FileModelSource) modelSource);
             }
         }
 
@@ -1128,7 +1123,7 @@ public class DefaultModelBuilder implements ModelBuilder {
         return model;
     }
 
-    private Model readRawModel(ModelBuildingRequest request, DefaultModelProblemCollector problems)
+    Model readRawModel(ModelBuildingRequest request, DefaultModelProblemCollector problems)
             throws ModelBuildingException {
         ModelSource modelSource = request.getModelSource();
 
@@ -1169,7 +1164,7 @@ public class DefaultModelBuilder implements ModelBuilder {
         return cachedData != null ? cachedData.getModel() : null;
     }
 
-    private String getGroupId(Model model) {
+    String getGroupId(Model model) {
         return getGroupId(model.getDelegate());
     }
 
@@ -1906,127 +1901,7 @@ public class DefaultModelBuilder implements ModelBuilder {
         }
     }
 
-    /**
+    ModelProcessor getModelProcessor() {
-     * Builds up the transformer context.
+        return modelProcessor;
-     * After the buildplan is ready, the build()-method returns the immutable context useful during distribution.
-     * This is an inner class, as it must be able to call readRawModel()
-     *
-     * @since 4.0.0
-     */
-    private class DefaultTransformerContextBuilder implements TransformerContextBuilder {
-        private final DefaultTransformerContext context = new DefaultTransformerContext(modelProcessor);
-
-        private final Map<DefaultTransformerContext.GAKey, Set<Source>> mappedSources = new ConcurrentHashMap<>(64);
-
-        /**
-         * If an interface could be extracted, DefaultModelProblemCollector should be ModelProblemCollectorExt
-         *
-         * @param request
-         * @param collector
-         * @return
-         */
-        @Override
-        public TransformerContext initialize(ModelBuildingRequest request, ModelProblemCollector collector) {
-            // We must assume the TransformerContext was created using this.newTransformerContextBuilder()
-            DefaultModelProblemCollector problems = (DefaultModelProblemCollector) collector;
-            return new TransformerContext() {
-                @Override
-                public Path locate(Path path) {
-                    File file = modelProcessor.locateExistingPom(path.toFile());
-                    return file != null ? file.toPath() : null;
-                }
-
-                @Override
-                public String getUserProperty(String key) {
-                    return context.userProperties.computeIfAbsent(
-                            key, k -> request.getUserProperties().getProperty(key));
-                }
-
-                @Override
-                public Model getRawModel(String gId, String aId) {
-                    return context.modelByGA
-                            .computeIfAbsent(
-                                    new DefaultTransformerContext.GAKey(gId, aId),
-                                    k -> new DefaultTransformerContext.Holder())
-                            .computeIfAbsent(() -> findRawModel(gId, aId));
-                }
-
-                @Override
-                public Model getRawModel(Path path) {
-                    return context.modelByPath
-                            .computeIfAbsent(path, k -> new DefaultTransformerContext.Holder())
-                            .computeIfAbsent(() -> findRawModel(path));
-                }
-
-                private Model findRawModel(String groupId, String artifactId) {
-                    Source source = getSource(groupId, artifactId);
-                    if (source != null) {
-                        try {
-                            ModelBuildingRequest gaBuildingRequest =
-                                    new DefaultModelBuildingRequest(request).setModelSource((ModelSource) source);
-                            Model model = readRawModel(gaBuildingRequest, problems);
-                            if (source instanceof FileModelSource) {
-                                Path path = ((FileModelSource) source).getFile().toPath();
-                                context.modelByPath
-                                        .computeIfAbsent(path, k -> new DefaultTransformerContext.Holder())
-                                        .computeIfAbsent(() -> model);
-                            }
-                            return model;
-                        } catch (ModelBuildingException e) {
-                            // gathered with problem collector
-                        }
-                    }
-                    return null;
-                }
-
-                private Model findRawModel(Path p) {
-                    if (!Files.isRegularFile(p)) {
-                        throw new IllegalArgumentException("Not a regular file: " + p);
-                    }
-
-                    DefaultModelBuildingRequest req = new DefaultModelBuildingRequest(request)
-                            .setPomFile(p.toFile())
-                            .setModelSource(new FileModelSource(p.toFile()));
-
-                    try {
-                        Model model = readRawModel(req, problems);
-                        DefaultTransformerContext.GAKey key =
-                                new DefaultTransformerContext.GAKey(getGroupId(model), model.getArtifactId());
-                        context.modelByGA
-                                .computeIfAbsent(key, k -> new DefaultTransformerContext.Holder())
-                                .computeIfAbsent(() -> model);
-                        return model;
-                    } catch (ModelBuildingException e) {
-                        // gathered with problem collector
-                    }
-                    return null;
-                }
-            };
-        }
-
-        @Override
-        public TransformerContext build() {
-            return context;
-        }
-
-        public Source getSource(String groupId, String artifactId) {
-            Set<Source> sources = mappedSources.get(new DefaultTransformerContext.GAKey(groupId, artifactId));
-            if (sources == null) {
-                return null;
-            }
-            return sources.stream()
-                    .reduce((a, b) -> {
-                        throw new IllegalStateException(String.format(
-                                "No unique Source for %s:%s: %s and %s",
-                                groupId, artifactId, a.getLocation(), b.getLocation()));
-                    })
-                    .orElse(null);
-        }
-
-        public void putSource(String groupId, String artifactId, Source source) {
-            mappedSources
-                    .computeIfAbsent(new DefaultTransformerContext.GAKey(groupId, artifactId), k -> new HashSet<>())
-                    .add(source);
-        }
     }
 }

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContext.java

@@ -95,12 +95,12 @@ class DefaultTransformerContext implements TransformerContext {
     }
 
     @Override
-    public Model getRawModel(Path p) {
+    public Model getRawModel(Path from, Path p) {
         return Holder.deref(modelByPath.get(p));
     }
 
     @Override
-    public Model getRawModel(String groupId, String artifactId) {
+    public Model getRawModel(Path from, String groupId, String artifactId) {
         return Holder.deref(modelByGA.get(new GAKey(groupId, artifactId)));
     }
 

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContextBuilder.java

@@ -0,0 +1,178 @@
+/*
+ * 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 java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.maven.model.Model;
+import org.apache.maven.model.building.DefaultTransformerContext.GAKey;
+import org.apache.maven.model.building.DefaultTransformerContext.Holder;
+import org.codehaus.plexus.util.dag.CycleDetectedException;
+import org.codehaus.plexus.util.dag.DAG;
+
+/**
+ * Builds up the transformer context.
+ * After the buildplan is ready, the build()-method returns the immutable context useful during distribution.
+ * This is an inner class, as it must be able to call readRawModel()
+ *
+ * @since 4.0.0
+ */
+class DefaultTransformerContextBuilder implements TransformerContextBuilder {
+    private final DAG dag = new DAG();
+    private final DefaultModelBuilder defaultModelBuilder;
+    private final DefaultTransformerContext context;
+
+    private final Map<String, Set<FileModelSource>> mappedSources = new ConcurrentHashMap<>(64);
+
+    DefaultTransformerContextBuilder(DefaultModelBuilder defaultModelBuilder) {
+        this.defaultModelBuilder = defaultModelBuilder;
+        this.context = new DefaultTransformerContext(defaultModelBuilder.getModelProcessor());
+    }
+
+    /**
+     * If an interface could be extracted, DefaultModelProblemCollector should be ModelProblemCollectorExt
+     */
+    @Override
+    public TransformerContext initialize(ModelBuildingRequest request, ModelProblemCollector collector) {
+        // We must assume the TransformerContext was created using this.newTransformerContextBuilder()
+        DefaultModelProblemCollector problems = (DefaultModelProblemCollector) collector;
+        return new TransformerContext() {
+            @Override
+            public Path locate(Path path) {
+                return context.locate(path);
+            }
+
+            @Override
+            public String getUserProperty(String key) {
+                return context.userProperties.computeIfAbsent(
+                        key, k -> request.getUserProperties().getProperty(key));
+            }
+
+            @Override
+            public Model getRawModel(Path from, String gId, String aId) {
+                Model model = findRawModel(from, gId, aId);
+                if (model != null) {
+                    context.modelByGA.put(new GAKey(gId, aId), new Holder(model));
+                    context.modelByPath.put(model.getPomFile().toPath(), new Holder(model));
+                }
+                return model;
+            }
+
+            @Override
+            public Model getRawModel(Path from, Path path) {
+                Model model = findRawModel(from, path);
+                if (model != null) {
+                    String groupId = defaultModelBuilder.getGroupId(model);
+                    context.modelByGA.put(
+                            new DefaultTransformerContext.GAKey(groupId, model.getArtifactId()), new Holder(model));
+                    context.modelByPath.put(path, new Holder(model));
+                }
+                return model;
+            }
+
+            private Model findRawModel(Path from, String groupId, String artifactId) {
+                FileModelSource source = getSource(groupId, artifactId);
+                if (source != null) {
+                    if (!addEdge(from, source.getFile().toPath(), problems)) {
+                        return null;
+                    }
+                    try {
+                        ModelBuildingRequest gaBuildingRequest =
+                                new DefaultModelBuildingRequest(request).setModelSource(source);
+                        return defaultModelBuilder.readRawModel(gaBuildingRequest, problems);
+                    } catch (ModelBuildingException e) {
+                        // gathered with problem collector
+                    }
+                }
+                return null;
+            }
+
+            private Model findRawModel(Path from, Path p) {
+                if (!Files.isRegularFile(p)) {
+                    throw new IllegalArgumentException("Not a regular file: " + p);
+                }
+
+                if (!addEdge(from, p, problems)) {
+                    return null;
+                }
+
+                DefaultModelBuildingRequest req = new DefaultModelBuildingRequest(request)
+                        .setPomFile(p.toFile())
+                        .setModelSource(new FileModelSource(p.toFile()));
+
+                try {
+                    return defaultModelBuilder.readRawModel(req, problems);
+                } catch (ModelBuildingException e) {
+                    // gathered with problem collector
+                }
+                return null;
+            }
+        };
+    }
+
+    private boolean addEdge(Path from, Path p, DefaultModelProblemCollector problems) {
+        try {
+            synchronized (dag) {
+                dag.addEdge(from.toString(), p.toString());
+            }
+            return true;
+        } catch (CycleDetectedException e) {
+            problems.add(new DefaultModelProblem(
+                    "Cycle detected between models at " + from + " and " + p,
+                    ModelProblem.Severity.FATAL,
+                    null,
+                    null,
+                    0,
+                    0,
+                    null,
+                    e));
+            return false;
+        }
+    }
+
+    @Override
+    public TransformerContext build() {
+        return context;
+    }
+
+    public FileModelSource getSource(String groupId, String artifactId) {
+        Set<FileModelSource> sources = mappedSources.get(groupId + ":" + artifactId);
+        if (sources == null) {
+            return null;
+        }
+        return sources.stream()
+                .reduce((a, b) -> {
+                    throw new IllegalStateException(String.format(
+                            "No unique Source for %s:%s: %s and %s",
+                            groupId, artifactId, a.getLocation(), b.getLocation()));
+                })
+                .orElse(null);
+    }
+
+    public void putSource(String groupId, String artifactId, FileModelSource source) {
+        mappedSources
+                .computeIfAbsent(groupId + ":" + artifactId, k -> new HashSet<>())
+                .add(source);
+    }
+}

maven-model-builder/src/main/java/org/apache/maven/model/building/TransformerContext.java

@@ -36,35 +36,31 @@ public interface TransformerContext {
 
     /**
      * Get the value of the Maven user property.
-     *
-     * @param key
-     * @return
      */
     String getUserProperty(String key);
 
     /**
      * Get the model based on the path when resolving the parent based on relativePath.
      *
+     * @param from    the requiring model
      * @param pomFile the path to the pomFile
      * @return the model, otherwise {@code null}
      */
-    Model getRawModel(Path pomFile);
+    Model getRawModel(Path from, Path pomFile);
 
     /**
      * Get the model from the reactor based on the groupId and artifactId when resolving reactor dependencies.
      *
-     * @param groupId the groupId
+     * @param from    the requiring model
+     * @param groupId    the groupId
      * @param artifactId the artifactId
      * @return the model, otherwise {@code null}
      * @throws IllegalStateException if multiple versions of the same GA are part of the reactor
      */
-    Model getRawModel(String groupId, String artifactId);
+    Model getRawModel(Path from, String groupId, String artifactId);
 
     /**
      * Locate the POM file inside the given directory.
-     *
-     * @param path
-     * @return
      */
     Path locate(Path path);
 }

maven-model-builder/src/test/java/org/apache/maven/model/building/BuildModelSourceTransformerTest.java

@@ -60,7 +60,7 @@ public class BuildModelSourceTransformerTest {
                 .artifactId("ARTIFACTID")
                 .version("1.0-SNAPSHOT")
                 .build());
-        Mockito.when(context.getRawModel(root.resolve("pom.xml"))).thenReturn(parent);
+        Mockito.when(context.getRawModel(pomFile, root.resolve("pom.xml"))).thenReturn(parent);
         Mockito.when(context.locate(root)).thenReturn(root.resolve("pom.xml"));
         Model initial = new Model(org.apache.maven.api.model.Model.newBuilder()
                 .parent(org.apache.maven.api.model.Parent.newBuilder()
@@ -86,7 +86,7 @@ public class BuildModelSourceTransformerTest {
                 .artifactId("ARTIFACTID")
                 .version("1.0-SNAPSHOT")
                 .build());
-        Mockito.when(context.getRawModel("GROUPID", "ARTIFACTID")).thenReturn(dep);
+        Mockito.when(context.getRawModel(pomFile, "GROUPID", "ARTIFACTID")).thenReturn(dep);
         Model initial = new Model(org.apache.maven.api.model.Model.newBuilder()
                 .dependencies(Collections.singleton(org.apache.maven.api.model.Dependency.newBuilder()
                         .groupId("GROUPID")