From 1e379c1d012188a2c2f073812b0eadbf916a7a43 Mon Sep 17 00:00:00 2001 From: Tomo Suzuki Date: Wed, 7 Apr 2021 11:30:06 -0400 Subject: [PATCH 1/4] Refactoring: AnnotatedClassPath to replace ListMultimap --- .../DashboardUnavailableArtifactTest.java | 6 +- .../opensource/dashboard/FreemarkerTest.java | 4 +- .../classpath/AnnotatedClassPath.java | 62 +++++++++++++++ .../classpath/ClassPathBuilder.java | 19 +---- .../opensource/classpath/ClassPathResult.java | 26 ++----- .../classpath/DependencyMediation.java | 35 +++++++++ .../classpath/MavenDependencyMediation.java | 39 +++++++--- .../classpath/ClassPathResultTest.java | 51 +++++++------ .../MavenDependencyMediationTest.java | 75 +++++++++++++++++++ .../enforcer/LinkageCheckerRule.java | 9 +-- .../dependencies/gradle/LinkageCheckTask.java | 8 +- .../linkagemonitor/LinkageMonitorTest.java | 4 +- 12 files changed, 252 insertions(+), 86 deletions(-) create mode 100644 dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java create mode 100644 dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java create mode 100644 dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java diff --git a/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/DashboardUnavailableArtifactTest.java b/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/DashboardUnavailableArtifactTest.java index 270583ca72..1d8f8998bf 100644 --- a/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/DashboardUnavailableArtifactTest.java +++ b/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/DashboardUnavailableArtifactTest.java @@ -16,6 +16,7 @@ package com.google.cloud.tools.opensource.dashboard; +import com.google.cloud.tools.opensource.classpath.AnnotatedClassPath; import com.google.cloud.tools.opensource.classpath.ClassPathResult; import com.google.cloud.tools.opensource.dependencies.Artifacts; import com.google.cloud.tools.opensource.dependencies.Bom; @@ -23,7 +24,6 @@ import com.google.cloud.tools.opensource.dependencies.DependencyGraphBuilder; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.LinkedListMultimap; import com.google.common.io.MoreFiles; import com.google.common.io.RecursiveDeleteOption; import com.google.common.truth.Truth; @@ -91,7 +91,7 @@ public void testDashboardForRepositoryException() throws TemplateException { outputDirectory, cache, ImmutableMap.of(), - new ClassPathResult(LinkedListMultimap.create(), ImmutableList.of()), + new ClassPathResult(new AnnotatedClassPath(), ImmutableList.of()), bom); Assert.assertEquals( @@ -139,7 +139,7 @@ public void testDashboardWithRepositoryException() table, ImmutableList.of(), ImmutableMap.of(), - new ClassPathResult(LinkedListMultimap.create(), ImmutableList.of()), + new ClassPathResult(new AnnotatedClassPath(), ImmutableList.of()), bom); Path generatedHtml = outputDirectory.resolve("artifact_details.html"); diff --git a/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/FreemarkerTest.java b/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/FreemarkerTest.java index fa25b28d91..a499fa0552 100644 --- a/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/FreemarkerTest.java +++ b/dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/FreemarkerTest.java @@ -16,6 +16,7 @@ package com.google.cloud.tools.opensource.dashboard; +import com.google.cloud.tools.opensource.classpath.AnnotatedClassPath; import com.google.cloud.tools.opensource.classpath.ClassFile; import com.google.cloud.tools.opensource.classpath.ClassNotFoundProblem; import com.google.cloud.tools.opensource.classpath.ClassPathEntry; @@ -27,7 +28,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.LinkedListMultimap; import com.google.common.io.MoreFiles; import com.google.common.io.RecursiveDeleteOption; import com.google.common.truth.Truth; @@ -107,7 +107,7 @@ public void testCountFailures() throws IOException, TemplateException, ParsingEx table, globalDependencies, symbolProblemTable, - new ClassPathResult(LinkedListMultimap.create(), ImmutableList.of()), + new ClassPathResult(new AnnotatedClassPath(), ImmutableList.of()), new Bom("mock:artifact:1.6.7", null)); Path dashboardHtml = outputDirectory.resolve("index.html"); diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java new file mode 100644 index 0000000000..09ddf248fe --- /dev/null +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java @@ -0,0 +1,62 @@ +/* + * Copyright 2021 Google LLC. + * + * Licensed 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 com.google.cloud.tools.opensource.classpath; + +import com.google.cloud.tools.opensource.dependencies.DependencyPath; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.LinkedListMultimap; +import com.google.common.collect.ListMultimap; + +/** + * A class path that consists of a list of artifacts, each of which is annotated with dependency + * paths. The dependency paths tells where an artifact appears in the dependency graph. Note that an + * artifact may appear in the graph multiple times. + */ +public class AnnotatedClassPath { + + /** + * An ordered map from class path elements to one or more Maven dependency paths. + * + *

The keys of the returned map represent Maven artifacts in the resolved class path, including + * transitive dependencies. The return value of {@link LinkedListMultimap#keySet()} preserves key + * iteration order. + * + *

The values of the returned map for a key (class path entry) represent the different Maven + * dependency paths from {@code artifacts} to the Maven artifact. + */ + private LinkedListMultimap classPathEntryToDependencyPaths = + LinkedListMultimap.create(); + + public void put(ClassPathEntry classPathEntry, DependencyPath dependencyPath) { + classPathEntryToDependencyPaths.put(classPathEntry, dependencyPath); + } + + ImmutableList getClassPath() { + // LinkedListMultimap.keySet preserves the iteration order + return ImmutableList.copyOf(classPathEntryToDependencyPaths.keySet()); + } + + ImmutableList dependencyPathOf(ClassPathEntry classPathEntry) { + return ImmutableList.copyOf(classPathEntryToDependencyPaths.get(classPathEntry)); + } + + public static AnnotatedClassPath fromMultimap(ListMultimap classPathEntryToDependencyPaths) { + AnnotatedClassPath annotatedClassPath = new AnnotatedClassPath(); + annotatedClassPath.classPathEntryToDependencyPaths.putAll(classPathEntryToDependencyPaths); + return annotatedClassPath; + } +} diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathBuilder.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathBuilder.java index cd39a85953..27d64ef218 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathBuilder.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathBuilder.java @@ -18,8 +18,6 @@ import com.google.cloud.tools.opensource.dependencies.DependencyGraph; import com.google.cloud.tools.opensource.dependencies.DependencyGraphBuilder; -import com.google.cloud.tools.opensource.dependencies.DependencyPath; -import com.google.common.collect.LinkedListMultimap; import java.util.List; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.graph.Dependency; @@ -82,22 +80,11 @@ ClassPathResult resolveWithMaven(Artifact rootArtifact) { } private ClassPathResult mediate(DependencyGraph result) { - // TODO should DependencyGraphResult have a mediate() method that returns a ClassPathResult? - // To remove duplicates on (groupId:artifactId) for dependency mediation MavenDependencyMediation mediation = new MavenDependencyMediation(); - LinkedListMultimap multimap = LinkedListMultimap.create(); - List dependencyPaths = result.list(); - for (DependencyPath dependencyPath : dependencyPaths) { - Artifact artifact = dependencyPath.getLeaf(); - mediation.put(dependencyPath); - if (mediation.selects(artifact)) { - // We include multiple dependency paths to the first version of an artifact we see, - // but not paths to other versions of that artifact. - multimap.put(new ClassPathEntry(artifact), dependencyPath); - } - } - return new ClassPathResult(multimap, result.getUnresolvedArtifacts()); + AnnotatedClassPath classPathAnnotatedWithDependencyPath = mediation.mediate(result); + return new ClassPathResult( + classPathAnnotatedWithDependencyPath, result.getUnresolvedArtifacts()); } } diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java index 6f6cd06395..183b966818 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java @@ -22,10 +22,7 @@ import com.google.cloud.tools.opensource.dependencies.DependencyPath; import com.google.cloud.tools.opensource.dependencies.UnresolvableArtifactProblem; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.LinkedListMultimap; -import com.google.common.collect.ListMultimap; import java.io.IOException; import org.eclipse.aether.artifact.Artifact; @@ -34,25 +31,14 @@ public final class ClassPathResult { private final ImmutableList classPath; - /** - * An ordered map from class path elements to one or more Maven dependency paths. - * - *

The keys of the returned map represent Maven artifacts in the resolved class path, - * including transitive dependencies. The return value of {@link LinkedListMultimap#keySet()} - * preserves key iteration order. - * - *

The values of the returned map for a key (class path entry) represent the different Maven - * dependency paths from {@code artifacts} to the Maven artifact. - */ - private final ImmutableListMultimap dependencyPaths; + private final AnnotatedClassPath annotatedClassPath; private final ImmutableList artifactProblems; public ClassPathResult( - ListMultimap dependencyPaths, - Iterable artifactProblems) { - this.dependencyPaths = ImmutableListMultimap.copyOf(dependencyPaths); - this.classPath = ImmutableList.copyOf(dependencyPaths.keySet()); + AnnotatedClassPath dependencyPaths, Iterable artifactProblems) { + this.annotatedClassPath = dependencyPaths; + this.classPath = dependencyPaths.getClassPath(); this.artifactProblems = ImmutableList.copyOf(artifactProblems); } @@ -66,7 +52,7 @@ public ImmutableList getClassPath() { * an empty list if the entry is not in the class path. */ public ImmutableList getDependencyPaths(ClassPathEntry entry) { - return dependencyPaths.get(entry); + return annotatedClassPath.dependencyPathOf(entry); } /** Returns problems encountered while constructing the dependency graph. */ @@ -101,7 +87,7 @@ public String formatDependencyPaths(Iterable entries) { public ImmutableSet getClassPathEntries(String coordinates) { ImmutableSet.Builder builder = ImmutableSet.builder(); for (ClassPathEntry entry : classPath) { - for (DependencyPath dependencyPath : dependencyPaths.get(entry)) { + for (DependencyPath dependencyPath : annotatedClassPath.dependencyPathOf(entry)) { if (dependencyPath.size() > 1) { Artifact artifact = dependencyPath.get(1); if (Artifacts.toCoordinates(artifact).equals(coordinates)) { diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java new file mode 100644 index 0000000000..e5d5d63a1f --- /dev/null +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java @@ -0,0 +1,35 @@ +/* + * Copyright 2021 Google LLC. + * + * Licensed 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 com.google.cloud.tools.opensource.classpath; + +import com.google.cloud.tools.opensource.dependencies.DependencyGraph; + +/** + * Algorithm to select artifacts when there are multiple versions for the same groupId and + * artifactId in a dependency graph. + */ +public interface DependencyMediation { + /** + * Returns the map associating the list of class path entries with their dependency paths, after + * performing dependency mediation. This means that the returned list of class path entries have + * no duplicate artifacts in terms of the groupId and artifactId combination. + * + * @param dependencyGraph dependency graph that may have duplicate artifacts that have the same + * groupId and artifactId combination + */ + AnnotatedClassPath mediate(DependencyGraph dependencyGraph); +} diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediation.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediation.java index 216dca1b18..00dccfdcbf 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediation.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediation.java @@ -16,30 +16,27 @@ package com.google.cloud.tools.opensource.classpath; +import com.google.cloud.tools.opensource.dependencies.Artifacts; +import com.google.cloud.tools.opensource.dependencies.DependencyGraph; +import com.google.cloud.tools.opensource.dependencies.DependencyPath; import java.io.File; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import org.eclipse.aether.artifact.Artifact; -import com.google.cloud.tools.opensource.dependencies.Artifacts; -import com.google.cloud.tools.opensource.dependencies.DependencyPath; -/** - * Retain only the first version of a groupId:artifactId encountered. - */ -class MavenDependencyMediation { - +/** Retain only the first version of a groupId:artifactId encountered. */ +class MavenDependencyMediation implements DependencyMediation { + private List artifacts = new ArrayList<>(); - /** - * Returns true iff dependency mediation selects this artifact. - */ + /** Returns true iff dependency mediation selects this artifact. */ // TODO might be a problem if there's a classifier - boolean selects(Artifact artifact) { + private boolean selects(Artifact artifact) { return artifacts.contains(artifact); } - void put(DependencyPath dependencyPath) { + private void put(DependencyPath dependencyPath) { Artifact artifact = dependencyPath.getLeaf(); File file = artifact.getFile(); if (file == null) { @@ -61,4 +58,22 @@ private void put(Artifact artifact) { artifacts.add(artifact); } + @Override + public AnnotatedClassPath mediate(DependencyGraph dependencyGraph) { + + AnnotatedClassPath annotatedClassPath = new AnnotatedClassPath(); + List dependencyPaths = dependencyGraph.list(); + for (DependencyPath dependencyPath : dependencyPaths) { + // DependencyPaths have items in level-order; nearest items come first. + Artifact artifact = dependencyPath.getLeaf(); + put(dependencyPath); + if (selects(artifact)) { + // We include multiple dependency paths to the first version of an artifact we see, + // but not paths to other versions of that artifact. + annotatedClassPath.put(new ClassPathEntry(artifact), dependencyPath); + } + } + + return annotatedClassPath; + } } \ No newline at end of file diff --git a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ClassPathResultTest.java b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ClassPathResultTest.java index 2a540bd2f2..acd740b1fd 100644 --- a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ClassPathResultTest.java +++ b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ClassPathResultTest.java @@ -53,10 +53,11 @@ public class ClassPathResultTest { @Test public void testFormatDependencyPaths_onePath() { - ImmutableListMultimap tree = - ImmutableListMultimap.of(jarA, dependencyPath_A, jarB, dependencyPath_B); + AnnotatedClassPath annotatedClassPath = + AnnotatedClassPath.fromMultimap( + ImmutableListMultimap.of(jarA, dependencyPath_A, jarB, dependencyPath_B)); - ClassPathResult classPathResult = new ClassPathResult(tree, ImmutableSet.of()); + ClassPathResult classPathResult = new ClassPathResult(annotatedClassPath, ImmutableSet.of()); String actual = classPathResult.formatDependencyPaths(ImmutableList.of(jarA)); @@ -65,10 +66,11 @@ public void testFormatDependencyPaths_onePath() { @Test public void testFormatDependencyPaths_path_A_B() { - ImmutableListMultimap tree = - ImmutableListMultimap.of(jarA, dependencyPath_A, jarB, dependencyPath_B); + AnnotatedClassPath annotatedClassPath = + AnnotatedClassPath.fromMultimap( + ImmutableListMultimap.of(jarA, dependencyPath_A, jarB, dependencyPath_B)); - ClassPathResult classPathResult = new ClassPathResult(tree, ImmutableSet.of()); + ClassPathResult classPathResult = new ClassPathResult(annotatedClassPath, ImmutableSet.of()); String actual = classPathResult.formatDependencyPaths(ImmutableList.of(jarA, jarB)); @@ -82,10 +84,11 @@ public void testFormatDependencyPaths_path_A_B() { @Test public void testFormatDependencyPaths_twoPathsForA() { - ImmutableListMultimap tree = - ImmutableListMultimap.of(jarA, dependencyPath_A, jarA, dependencyPath_B_A); + AnnotatedClassPath annotatedClassPath = + AnnotatedClassPath.fromMultimap( + ImmutableListMultimap.of(jarA, dependencyPath_A, jarA, dependencyPath_B_A)); - ClassPathResult classPathResult = new ClassPathResult(tree, ImmutableSet.of()); + ClassPathResult classPathResult = new ClassPathResult(annotatedClassPath, ImmutableSet.of()); String actual = classPathResult.formatDependencyPaths(ImmutableList.of(jarA)); @@ -96,11 +99,12 @@ public void testFormatDependencyPaths_twoPathsForA() { @Test public void testFormatDependencyPaths_threePathsForA() { - ImmutableListMultimap tree = - ImmutableListMultimap.of( - jarA, dependencyPath_A, jarA, dependencyPath_B_A, jarA, dependencyPath_A_B_A); + AnnotatedClassPath annotatedClassPath = + AnnotatedClassPath.fromMultimap( + ImmutableListMultimap.of( + jarA, dependencyPath_A, jarA, dependencyPath_B_A, jarA, dependencyPath_A_B_A)); - ClassPathResult classPathResult = new ClassPathResult(tree, ImmutableSet.of()); + ClassPathResult classPathResult = new ClassPathResult(annotatedClassPath, ImmutableSet.of()); String actual = classPathResult.formatDependencyPaths(ImmutableList.of(jarA)); @@ -113,10 +117,10 @@ public void testFormatDependencyPaths_threePathsForA() { @Test public void testFormatDependencyPaths_irrelevantJar() { - ImmutableListMultimap tree = - ImmutableListMultimap.of(jarA, dependencyPath_A); + AnnotatedClassPath annotatedClassPath = + AnnotatedClassPath.fromMultimap(ImmutableListMultimap.of(jarA, dependencyPath_A)); - ClassPathResult classPathResult = new ClassPathResult(tree, ImmutableSet.of()); + ClassPathResult classPathResult = new ClassPathResult(annotatedClassPath, ImmutableSet.of()); try { classPathResult.formatDependencyPaths(ImmutableList.of(jarB)); @@ -128,13 +132,14 @@ public void testFormatDependencyPaths_irrelevantJar() { @Test public void testGetClassPathEntries() { - ImmutableListMultimap tree = - ImmutableListMultimap.of( - jarA, dependencyPath_A, - jarB, dependencyPath_B, - jarA, dependencyPath_A_B_A); - - ClassPathResult result = new ClassPathResult(tree, ImmutableSet.of()); + AnnotatedClassPath annotatedClassPath = + AnnotatedClassPath.fromMultimap( + ImmutableListMultimap.of( + jarA, dependencyPath_A, + jarB, dependencyPath_B, + jarA, dependencyPath_A_B_A)); + + ClassPathResult result = new ClassPathResult(annotatedClassPath, ImmutableSet.of()); ImmutableSet classPathEntries = result.getClassPathEntries("com.google:a:1"); assertEquals(1, classPathEntries.size()); diff --git a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java new file mode 100644 index 0000000000..262187e4c3 --- /dev/null +++ b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java @@ -0,0 +1,75 @@ +/* + * Copyright 2021 Google LLC. + * + * Licensed 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 com.google.cloud.tools.opensource.classpath; + +import static org.junit.Assert.*; + +import com.google.cloud.tools.opensource.dependencies.DependencyGraph; +import com.google.cloud.tools.opensource.dependencies.DependencyPath; +import com.google.common.truth.Correspondence; +import com.google.common.truth.Truth; +import java.io.File; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.graph.Dependency; +import org.junit.Test; + +public class MavenDependencyMediationTest { + + Artifact artifactA1 = new DefaultArtifact("g:a:1.0.0").setFile(new File("a-1.0.0.jar")); + Artifact artifactA2 = new DefaultArtifact("g:a:2.0.0").setFile(new File("a-2.0.0.jar")); + Artifact artifactB1 = new DefaultArtifact("g:b:1.0.0").setFile(new File("b-1.0.0.jar")); + MavenDependencyMediation mediation = new MavenDependencyMediation(); + + Correspondence CLASS_PATH_ENTRY_TO_ARTIFACT_ = + Correspondence.transforming(ClassPathEntry::getArtifact, "has an artifact of"); + + @Test + public void testMediation_noDuplicates() { + + DependencyGraph graph = new DependencyGraph(null); + // The old version comes first in the graph.list + graph.addPath(new DependencyPath(null).append(new Dependency(artifactA1, "compile"))); + graph.addPath(new DependencyPath(null).append(new Dependency(artifactA2, "compile"))); + // The duplicate shouldn't appear in the class path + graph.addPath(new DependencyPath(null).append(new Dependency(artifactA1, "compile"))); + AnnotatedClassPath result = mediation.mediate(graph); + + Truth.assertThat(result.getClassPath()).hasSize(1); + + Truth.assertThat(result.getClassPath()) + .comparingElementsUsing(CLASS_PATH_ENTRY_TO_ARTIFACT_) + .containsExactly(artifactA1); + } + + @Test + public void testMediation_oneArtifactForEachVersionlessCoordinates() { + + DependencyGraph graph = new DependencyGraph(null); + // The old version comes first in the graph.list + graph.addPath(new DependencyPath(null).append(new Dependency(artifactA1, "compile"))); + graph.addPath(new DependencyPath(null).append(new Dependency(artifactA2, "compile"))); + // The duplicate shouldn't appear in the class path + graph.addPath(new DependencyPath(null).append(new Dependency(artifactB1, "compile"))); + AnnotatedClassPath result = mediation.mediate(graph); + + Truth.assertThat(result.getClassPath()) + .comparingElementsUsing(CLASS_PATH_ENTRY_TO_ARTIFACT_) + .containsExactly(artifactA1, artifactB1) + .inOrder(); + } +} diff --git a/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java b/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java index 2b3f14d8a6..c3eca527e7 100644 --- a/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java +++ b/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java @@ -20,6 +20,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static org.apache.maven.enforcer.rule.api.EnforcerLevel.WARN; +import com.google.cloud.tools.opensource.classpath.AnnotatedClassPath; import com.google.cloud.tools.opensource.classpath.ClassPathBuilder; import com.google.cloud.tools.opensource.classpath.ClassPathEntry; import com.google.cloud.tools.opensource.classpath.ClassPathResult; @@ -37,7 +38,6 @@ import com.google.cloud.tools.opensource.dependencies.UnresolvableArtifactProblem; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSet; import java.io.File; import java.io.IOException; @@ -341,8 +341,7 @@ private static ClassPathResult buildClassPathResult(DependencyResolutionResult r unresolvedDependencies.stream().map(Dependency::getArtifact).collect(toImmutableSet()); DependencyGraph dependencyGraph = DependencyGraph.from(root); - ImmutableListMultimap.Builder builder = - ImmutableListMultimap.builder(); + AnnotatedClassPath annotatedClassPath = new AnnotatedClassPath(); ImmutableList.Builder problems = ImmutableList.builder(); for (DependencyPath path : dependencyGraph.list()) { Artifact artifact = path.getLeaf(); @@ -350,10 +349,10 @@ private static ClassPathResult buildClassPathResult(DependencyResolutionResult r if (unresolvedArtifacts.contains(artifact)) { problems.add(new UnresolvableArtifactProblem(artifact)); } else { - builder.put(new ClassPathEntry(artifact), path); + annotatedClassPath.put(new ClassPathEntry(artifact), path); } } - return new ClassPathResult(builder.build(), problems.build()); + return new ClassPathResult(annotatedClassPath, problems.build()); } /** Builds a class path for {@code bomProject}. */ diff --git a/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java b/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java index 25a55afa48..14834b1f2b 100644 --- a/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java +++ b/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java @@ -18,6 +18,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; +import com.google.cloud.tools.opensource.classpath.AnnotatedClassPath; import com.google.cloud.tools.opensource.classpath.ClassFile; import com.google.cloud.tools.opensource.classpath.ClassPathBuilder; import com.google.cloud.tools.opensource.classpath.ClassPathEntry; @@ -326,13 +327,12 @@ private static DependencyGraph createDependencyGraph(ResolvedConfiguration confi private static ClassPathResult createClassPathResult(ResolvedConfiguration configuration) { DependencyGraph dependencyGraph = createDependencyGraph(configuration); - ImmutableListMultimap.Builder builder = - ImmutableListMultimap.builder(); + AnnotatedClassPath annotatedClassPath = new AnnotatedClassPath(); for (DependencyPath path : dependencyGraph.list()) { Artifact artifact = path.getLeaf(); - builder.put(new ClassPathEntry(artifact), path); + annotatedClassPath.put(new ClassPathEntry(artifact), path); } - return new ClassPathResult(builder.build(), ImmutableList.of()); + return new ClassPathResult(annotatedClassPath, ImmutableList.of()); } } diff --git a/linkage-monitor/src/test/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitorTest.java b/linkage-monitor/src/test/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitorTest.java index f377b6cbc4..3a162037af 100644 --- a/linkage-monitor/src/test/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitorTest.java +++ b/linkage-monitor/src/test/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitorTest.java @@ -25,6 +25,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.cloud.tools.opensource.classpath.AnnotatedClassPath; import com.google.cloud.tools.opensource.classpath.ClassFile; import com.google.cloud.tools.opensource.classpath.ClassNotFoundProblem; import com.google.cloud.tools.opensource.classpath.ClassPathEntry; @@ -170,7 +171,8 @@ public void generateMessageForNewError() { snapshotProblems, baselineProblems, new ClassPathResult( - ImmutableListMultimap.of(jarA, pathToA, jarB, pathToB), + AnnotatedClassPath.fromMultimap( + ImmutableListMultimap.of(jarA, pathToA, jarB, pathToB)), ImmutableList.of())); assertEquals( "Newly introduced problem:\n" From 30eaee2d6e4801c62d523c5704d29ea8f9604b67 Mon Sep 17 00:00:00 2001 From: Tomo Suzuki Date: Wed, 7 Apr 2021 11:48:52 -0400 Subject: [PATCH 2/4] format --- .../classpath/AnnotatedClassPath.java | 18 ++++++++++++++---- .../classpath/DependencyMediation.java | 6 +++--- .../MavenDependencyMediationTest.java | 8 +++----- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java index 09ddf248fe..39100505bc 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java @@ -23,8 +23,9 @@ /** * A class path that consists of a list of artifacts, each of which is annotated with dependency - * paths. The dependency paths tells where an artifact appears in the dependency graph. Note that an - * artifact may appear in the graph multiple times. + * paths. The dependency paths tell where an artifact appears in the dependency graph. Note that the + * same artifact (having the same groupId, artifactId, and version) may appear in a graph multiple + * times. */ public class AnnotatedClassPath { @@ -35,12 +36,16 @@ public class AnnotatedClassPath { * transitive dependencies. The return value of {@link LinkedListMultimap#keySet()} preserves key * iteration order. * - *

The values of the returned map for a key (class path entry) represent the different Maven + *

The values of the returned map for a key (class path entry) represent the different * dependency paths from {@code artifacts} to the Maven artifact. */ private LinkedListMultimap classPathEntryToDependencyPaths = LinkedListMultimap.create(); + /** + * Adds {@code classPathEntry} with {@code dependencyPath}. The dependency path represents a path + * from the root of the dependency graph to the artifact. + */ public void put(ClassPathEntry classPathEntry, DependencyPath dependencyPath) { classPathEntryToDependencyPaths.put(classPathEntry, dependencyPath); } @@ -54,7 +59,12 @@ ImmutableList dependencyPathOf(ClassPathEntry classPathEntry) { return ImmutableList.copyOf(classPathEntryToDependencyPaths.get(classPathEntry)); } - public static AnnotatedClassPath fromMultimap(ListMultimap classPathEntryToDependencyPaths) { + /** + * Returns an {@link AnnotatedClassPath} that has class path entry and dependency paths + * relationship represented in {@code classPathEntryToDependencyPaths}. + */ + public static AnnotatedClassPath fromMultimap( + ListMultimap classPathEntryToDependencyPaths) { AnnotatedClassPath annotatedClassPath = new AnnotatedClassPath(); annotatedClassPath.classPathEntryToDependencyPaths.putAll(classPathEntryToDependencyPaths); return annotatedClassPath; diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java index e5d5d63a1f..e6ce00d453 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java @@ -24,9 +24,9 @@ */ public interface DependencyMediation { /** - * Returns the map associating the list of class path entries with their dependency paths, after - * performing dependency mediation. This means that the returned list of class path entries have - * no duplicate artifacts in terms of the groupId and artifactId combination. + * Returns {@link AnnotatedClassPath} after performing dependency mediation. This means that the + * returned list of class path entries has no duplicate artifacts in terms of the groupId and + * artifactId combination. * * @param dependencyGraph dependency graph that may have duplicate artifacts that have the same * groupId and artifactId combination diff --git a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java index 262187e4c3..fce5e0d8c0 100644 --- a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java +++ b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/MavenDependencyMediationTest.java @@ -16,8 +16,6 @@ package com.google.cloud.tools.opensource.classpath; -import static org.junit.Assert.*; - import com.google.cloud.tools.opensource.dependencies.DependencyGraph; import com.google.cloud.tools.opensource.dependencies.DependencyPath; import com.google.common.truth.Correspondence; @@ -35,7 +33,7 @@ public class MavenDependencyMediationTest { Artifact artifactB1 = new DefaultArtifact("g:b:1.0.0").setFile(new File("b-1.0.0.jar")); MavenDependencyMediation mediation = new MavenDependencyMediation(); - Correspondence CLASS_PATH_ENTRY_TO_ARTIFACT_ = + Correspondence CLASS_PATH_ENTRY_TO_ARTIFACT = Correspondence.transforming(ClassPathEntry::getArtifact, "has an artifact of"); @Test @@ -52,7 +50,7 @@ public void testMediation_noDuplicates() { Truth.assertThat(result.getClassPath()).hasSize(1); Truth.assertThat(result.getClassPath()) - .comparingElementsUsing(CLASS_PATH_ENTRY_TO_ARTIFACT_) + .comparingElementsUsing(CLASS_PATH_ENTRY_TO_ARTIFACT) .containsExactly(artifactA1); } @@ -68,7 +66,7 @@ public void testMediation_oneArtifactForEachVersionlessCoordinates() { AnnotatedClassPath result = mediation.mediate(graph); Truth.assertThat(result.getClassPath()) - .comparingElementsUsing(CLASS_PATH_ENTRY_TO_ARTIFACT_) + .comparingElementsUsing(CLASS_PATH_ENTRY_TO_ARTIFACT) .containsExactly(artifactA1, artifactB1) .inOrder(); } From fbce53149ea632755db16ef9040b86848e6a9ebe Mon Sep 17 00:00:00 2001 From: Tomo Suzuki Date: Wed, 7 Apr 2021 12:23:28 -0400 Subject: [PATCH 3/4] Clarified classifier or extension --- .../cloud/tools/opensource/classpath/DependencyMediation.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java index e6ce00d453..0475c2bd83 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java @@ -21,6 +21,8 @@ /** * Algorithm to select artifacts when there are multiple versions for the same groupId and * artifactId in a dependency graph. + * + *

The algorithm does not take classifier or extension into account.

*/ public interface DependencyMediation { /** From fbd568a4c67a5e00b4006ce82cc22d37ced79f38 Mon Sep 17 00:00:00 2001 From: Tomo Suzuki Date: Wed, 7 Apr 2021 20:27:58 -0400 Subject: [PATCH 4/4] Applied review --- .../cloud/tools/opensource/classpath/AnnotatedClassPath.java | 4 +++- .../cloud/tools/opensource/classpath/ClassPathResult.java | 4 ++-- .../cloud/tools/opensource/classpath/DependencyMediation.java | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java index 39100505bc..f651001284 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/AnnotatedClassPath.java @@ -17,6 +17,7 @@ package com.google.cloud.tools.opensource.classpath; import com.google.cloud.tools.opensource.dependencies.DependencyPath; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; @@ -55,7 +56,7 @@ ImmutableList getClassPath() { return ImmutableList.copyOf(classPathEntryToDependencyPaths.keySet()); } - ImmutableList dependencyPathOf(ClassPathEntry classPathEntry) { + ImmutableList pathsTo(ClassPathEntry classPathEntry) { return ImmutableList.copyOf(classPathEntryToDependencyPaths.get(classPathEntry)); } @@ -63,6 +64,7 @@ ImmutableList dependencyPathOf(ClassPathEntry classPathEntry) { * Returns an {@link AnnotatedClassPath} that has class path entry and dependency paths * relationship represented in {@code classPathEntryToDependencyPaths}. */ + @VisibleForTesting public static AnnotatedClassPath fromMultimap( ListMultimap classPathEntryToDependencyPaths) { AnnotatedClassPath annotatedClassPath = new AnnotatedClassPath(); diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java index 183b966818..77fdd68e26 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/ClassPathResult.java @@ -52,7 +52,7 @@ public ImmutableList getClassPath() { * an empty list if the entry is not in the class path. */ public ImmutableList getDependencyPaths(ClassPathEntry entry) { - return annotatedClassPath.dependencyPathOf(entry); + return annotatedClassPath.pathsTo(entry); } /** Returns problems encountered while constructing the dependency graph. */ @@ -87,7 +87,7 @@ public String formatDependencyPaths(Iterable entries) { public ImmutableSet getClassPathEntries(String coordinates) { ImmutableSet.Builder builder = ImmutableSet.builder(); for (ClassPathEntry entry : classPath) { - for (DependencyPath dependencyPath : annotatedClassPath.dependencyPathOf(entry)) { + for (DependencyPath dependencyPath : annotatedClassPath.pathsTo(entry)) { if (dependencyPath.size() > 1) { Artifact artifact = dependencyPath.get(1); if (Artifacts.toCoordinates(artifact).equals(coordinates)) { diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java index 0475c2bd83..46ee2582b1 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/DependencyMediation.java @@ -22,7 +22,7 @@ * Algorithm to select artifacts when there are multiple versions for the same groupId and * artifactId in a dependency graph. * - *

The algorithm does not take classifier or extension into account.

+ *

The algorithm does not take classifier or extension into account. */ public interface DependencyMediation { /**