-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactoring: MavenDependencyMediation with AnnotatedClassPath #2013
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
/* | ||
* 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.annotations.VisibleForTesting; | ||
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 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 { | ||
|
||
/** | ||
* An ordered map from class path elements to one or more Maven dependency paths. | ||
* | ||
* <p>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. | ||
* | ||
* <p>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<ClassPathEntry, DependencyPath> 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); | ||
} | ||
|
||
ImmutableList<ClassPathEntry> getClassPath() { | ||
// LinkedListMultimap.keySet preserves the iteration order | ||
return ImmutableList.copyOf(classPathEntryToDependencyPaths.keySet()); | ||
} | ||
|
||
ImmutableList<DependencyPath> pathsTo(ClassPathEntry classPathEntry) { | ||
return ImmutableList.copyOf(classPathEntryToDependencyPaths.get(classPathEntry)); | ||
} | ||
|
||
/** | ||
* Returns an {@link AnnotatedClassPath} that has class path entry and dependency paths | ||
* relationship represented in {@code classPathEntryToDependencyPaths}. | ||
*/ | ||
@VisibleForTesting | ||
public static AnnotatedClassPath fromMultimap( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to only be used by tests. Maybe non-public and/or visible for testing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
ListMultimap<ClassPathEntry, DependencyPath> classPathEntryToDependencyPaths) { | ||
AnnotatedClassPath annotatedClassPath = new AnnotatedClassPath(); | ||
annotatedClassPath.classPathEntryToDependencyPaths.putAll(classPathEntryToDependencyPaths); | ||
return annotatedClassPath; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<ClassPathEntry, DependencyPath> multimap = LinkedListMultimap.create(); | ||
List<DependencyPath> dependencyPaths = result.list(); | ||
for (DependencyPath dependencyPath : dependencyPaths) { | ||
Artifact artifact = dependencyPath.getLeaf(); | ||
mediation.put(dependencyPath); | ||
if (mediation.selects(artifact)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has moved to |
||
// 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()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<ClassPathEntry> classPath; | ||
|
||
/** | ||
* An ordered map from class path elements to one or more Maven dependency paths. | ||
* | ||
* <p>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. | ||
* | ||
* <p>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<ClassPathEntry, DependencyPath> dependencyPaths; | ||
private final AnnotatedClassPath annotatedClassPath; | ||
Comment on lines
-47
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The meaning of the variable with now clearer with more concrete type. |
||
|
||
private final ImmutableList<UnresolvableArtifactProblem> artifactProblems; | ||
|
||
public ClassPathResult( | ||
ListMultimap<ClassPathEntry, DependencyPath> dependencyPaths, | ||
Iterable<UnresolvableArtifactProblem> artifactProblems) { | ||
this.dependencyPaths = ImmutableListMultimap.copyOf(dependencyPaths); | ||
this.classPath = ImmutableList.copyOf(dependencyPaths.keySet()); | ||
AnnotatedClassPath dependencyPaths, Iterable<UnresolvableArtifactProblem> artifactProblems) { | ||
this.annotatedClassPath = dependencyPaths; | ||
this.classPath = dependencyPaths.getClassPath(); | ||
this.artifactProblems = ImmutableList.copyOf(artifactProblems); | ||
} | ||
|
||
|
@@ -66,7 +52,7 @@ public ImmutableList<ClassPathEntry> getClassPath() { | |
* an empty list if the entry is not in the class path. | ||
*/ | ||
public ImmutableList<DependencyPath> getDependencyPaths(ClassPathEntry entry) { | ||
return dependencyPaths.get(entry); | ||
return annotatedClassPath.pathsTo(entry); | ||
} | ||
|
||
/** Returns problems encountered while constructing the dependency graph. */ | ||
|
@@ -101,7 +87,7 @@ public String formatDependencyPaths(Iterable<ClassPathEntry> entries) { | |
public ImmutableSet<ClassPathEntry> getClassPathEntries(String coordinates) { | ||
ImmutableSet.Builder<ClassPathEntry> builder = ImmutableSet.builder(); | ||
for (ClassPathEntry entry : classPath) { | ||
for (DependencyPath dependencyPath : dependencyPaths.get(entry)) { | ||
for (DependencyPath dependencyPath : annotatedClassPath.pathsTo(entry)) { | ||
if (dependencyPath.size() > 1) { | ||
Artifact artifact = dependencyPath.get(1); | ||
if (Artifacts.toCoordinates(artifact).equals(coordinates)) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about classifiers and types? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarified in the next paragraph. This doesn't read them. |
||
* artifactId in a dependency graph. | ||
* | ||
* <p>The algorithm does not take classifier or extension into account. | ||
*/ | ||
public interface DependencyMediation { | ||
/** | ||
* 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 | ||
*/ | ||
AnnotatedClassPath mediate(DependencyGraph dependencyGraph); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This interface should work both Maven and Gradle. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<Artifact> 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(); | ||
Comment on lines
-38
to
40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method |
||
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<DependencyPath> 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; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing obscure LinkedListMultimap in favor of AnnotatedClassPath, which deliver meaning in its name.