From 2f6ec159fe49c6ebc769da2b604f4d3af4037267 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 20 Sep 2023 20:48:50 +0200 Subject: [PATCH] [MNG-7615] Add a cycle detection to the transformer context --- .../ConsumerPomArtifactTransformerTest.java | 4 +- .../building/BuildModelSourceTransformer.java | 11 +- .../model/building/DefaultModelBuilder.java | 137 +------------- .../building/DefaultTransformerContext.java | 4 +- .../DefaultTransformerContextBuilder.java | 178 ++++++++++++++++++ .../model/building/TransformerContext.java | 14 +- .../BuildModelSourceTransformerTest.java | 4 +- 7 files changed, 201 insertions(+), 151 deletions(-) create mode 100644 maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContextBuilder.java 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 0c177964dd..0d5ca973c3 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 @@ -88,12 +88,12 @@ public String getUserProperty(String key) { } @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(); } diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java index f0a8cb79da..cc1b90c362 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java @@ -71,7 +71,7 @@ void handleParent(Path pomFile, TransformerContext context, Model model) { String path = Optional.ofNullable(parent.getRelativePath()).orElse(".."); if (version == null && !path.isEmpty()) { Optional 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 @@ void handleCiFriendlyVersion(TransformerContext context, Model model) { 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 @@ protected String replaceCiFriendlyVersion(TransformerContext context, String ver } protected Optional resolveRelativePath( - Path projectPath, TransformerContext context, Path relativePath, String groupId, String artifactId) { - Path pomPath = projectPath.resolve(relativePath).normalize(); + Path pomFile, TransformerContext context, Path relativePath, String groupId, String artifactId) { + Path pomPath = pomFile.resolveSibling(relativePath).normalize(); if (Files.isDirectory(pomPath)) { pomPath = context.locate(pomPath); } @@ -134,7 +135,7 @@ protected Optional resolveRelativePath( return Optional.empty(); } - Optional mappedProject = Optional.ofNullable(context.getRawModel(pomPath.normalize())) + Optional mappedProject = Optional.ofNullable(context.getRawModel(pomFile, pomPath.normalize())) .map(BuildModelSourceTransformer::toRelativeProject); if (mappedProject.isPresent()) { 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 f3399b748d..77cb05f89e 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 @@ -25,21 +25,16 @@ 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 DefaultModelBuilder setReportingConverter(ReportingConverter reportingCon @Override public DefaultTransformerContextBuilder newTransformerContextBuilder() { - return new DefaultTransformerContextBuilder(); + return new DefaultTransformerContextBuilder(this); } @Override @@ -1021,7 +1016,7 @@ private Model readFileModel(ModelBuildingRequest request, DefaultModelProblemCol 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 @@ private org.apache.maven.api.model.Model doReadFileModel( 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 @@ private Model readRawModel(ModelBuildingRequest request, DefaultModelProblemColl return cachedData != null ? cachedData.getModel() : null; } - private String getGroupId(Model model) { + String getGroupId(Model model) { return getGroupId(model.getDelegate()); } @@ -1906,127 +1901,7 @@ protected boolean hasFatalErrors(ModelProblemCollectorExt problems) { } } - /** - * 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 - */ - private class DefaultTransformerContextBuilder implements TransformerContextBuilder { - private final DefaultTransformerContext context = new DefaultTransformerContext(modelProcessor); - - private final Map> 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 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); - } + ModelProcessor getModelProcessor() { + return modelProcessor; } } 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 c0ac5c38c3..462278445f 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 @@ -95,12 +95,12 @@ public String getUserProperty(String key) { } @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))); } diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContextBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultTransformerContextBuilder.java new file mode 100644 index 0000000000..a2dce0e491 --- /dev/null +++ b/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> 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 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); + } +} 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 2f9f26f2d0..7eb4fea6e1 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 @@ -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); } diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/BuildModelSourceTransformerTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/BuildModelSourceTransformerTest.java index 2d9072a243..7784cb4cce 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/building/BuildModelSourceTransformerTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/building/BuildModelSourceTransformerTest.java @@ -60,7 +60,7 @@ void testParent() { .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 @@ void testReactorDependencies() { .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")