Skip to content
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.

Sign up for GitHub

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

Add missing recurring dependencies to ResolvedDependency #3635

Closed
wants to merge 1 commit into from
Closed

Add missing recurring dependencies to ResolvedDependency #3635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class ResolvedDependency implements Serializable {
* Direct dependencies only that survived conflict resolution and exclusion.
*/
@NonFinal
@EqualsAndHashCode.Exclude // dependencies can be circular
List<ResolvedDependency> dependencies;

List<License> licenses;
Expand All @@ -74,6 +75,7 @@ public List<GroupArtifact> getEffectiveExclusions() {

/**
* Only used by dependency resolution to avoid unnecessary empty list allocations for leaf dependencies.
*
* @param dependencies A dependency list
*/
void unsafeSetDependencies(List<ResolvedDependency> dependencies) {
Expand Down
34 changes: 22 additions & 12 deletions rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,19 +582,28 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
.getResolutionListener()
.clear();
return resolveDependencies(scope, requirements, downloader, ctx);
} else if (contains(dependencies, ga, d.getClassifier())) {
// we've already resolved this previously and the requirement didn't change,
// so just skip and continue on
continue;
}
}

if ((d.getGav().getGroupId() != null && d.getGav().getGroupId().startsWith("${") && d.getGav().getGroupId().endsWith("}")) ||
(d.getGav().getArtifactId().startsWith("${") && d.getGav().getArtifactId().endsWith("}")) ||
(d.getGav().getVersion() != null && d.getGav().getVersion().startsWith("${") && d.getGav().getVersion().endsWith("}"))) {
(d.getGav().getArtifactId().startsWith("${") && d.getGav().getArtifactId().endsWith("}")) ||
(d.getGav().getVersion() != null && d.getGav().getVersion().startsWith("${") && d.getGav().getVersion().endsWith("}"))) {
throw new MavenDownloadingException("Could not resolve property", null, d.getGav());
}

ResolvedDependency resolved = find(dependencies, ga, d.getClassifier());
if (resolved != null) {
// build link between the including dependency and this one
ResolvedDependency includedBy = dd.getDependent();
if (includedBy != null) {
if (includedBy.getDependencies().isEmpty()) {
includedBy.unsafeSetDependencies(new ArrayList<>());
}
includedBy.getDependencies().add(resolved);
}
continue;
}

Pom dPom = downloader.download(d.getGav(), null, dd.definedIn, getRepositories());

MavenPomCache cache = MavenExecutionContextView.view(ctx).getPomCache();
Expand All @@ -606,7 +615,7 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
cache.putResolvedDependencyPom(dPom.getGav(), resolvedPom);
}

ResolvedDependency resolved = new ResolvedDependency(dPom.getRepository(),
resolved = new ResolvedDependency(dPom.getRepository(),
resolvedPom.getGav(), dd.getDependency(), emptyList(),
resolvedPom.getRequested().getLicenses(),
resolvedPom.getValue(dd.getDependency().getType()),
Expand Down Expand Up @@ -646,7 +655,7 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
if (d.getExclusions() != null) {
for (GroupArtifact exclusion : d.getExclusions()) {
if (matchesGlob(getValue(d2.getGroupId()), getValue(exclusion.getGroupId())) &&
matchesGlob(getValue(d2.getArtifactId()), getValue(exclusion.getArtifactId()))) {
matchesGlob(getValue(d2.getArtifactId()), getValue(exclusion.getArtifactId()))) {
if (resolved.getEffectiveExclusions().isEmpty()) {
resolved.unsafeSetEffectiveExclusions(new ArrayList<>());
}
Expand Down Expand Up @@ -677,14 +686,15 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
return dependencies;
}

private boolean contains(List<ResolvedDependency> dependencies, GroupArtifact ga, @Nullable String classifier) {
@Nullable
private ResolvedDependency find(List<ResolvedDependency> dependencies, GroupArtifact ga, @Nullable String classifier) {
for (ResolvedDependency it : dependencies) {
if (it.getGroupId().equals(ga.getGroupId()) && it.getArtifactId().equals(ga.getArtifactId()) &&
(Objects.equals(classifier, it.getClassifier()))) {
return true;
(Objects.equals(classifier, it.getClassifier()))) {
return it;
}
}
return false;
return null;
}

private Scope getDependencyScope(Dependency d2, ResolvedPom containingPom) {
Expand Down
44 changes: 40 additions & 4 deletions rewrite-maven/src/test/java/org/openrewrite/maven/MavenDependencyFailuresTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@
import org.openrewrite.*;
import org.openrewrite.marker.Markup;
import org.openrewrite.maven.cache.InMemoryMavenPomCache;
import org.openrewrite.maven.internal.MavenPomDownloader;
import org.openrewrite.maven.table.MavenMetadataFailures;
import org.openrewrite.maven.tree.MavenRepository;
import org.openrewrite.maven.tree.*;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.xml.tree.Xml;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Optional;
import java.util.*;
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import static org.openrewrite.maven.Assertions.pomXml;
Expand All @@ -48,7 +49,7 @@ void unresolvableMavenMetadata() {
.cycles(1)
.expectedCyclesThatMakeChanges(1)
.dataTable(MavenMetadataFailures.Row.class, failures ->
assertThat(failures.stream().map(MavenMetadataFailures.Row::getMavenRepositoryUri).distinct()).containsExactlyInAnyOrder("https://repo.maven.apache.org/maven2")),
assertThat(failures.stream().map(MavenMetadataFailures.Row::getMavenRepositoryUri).distinct()).containsExactlyInAnyOrder("https://repo.maven.apache.org/maven2")),
pomXml(
"""
<project>
Expand Down Expand Up @@ -279,6 +280,41 @@ void unreachableRepository() {
);
}

@Test
void circularDependency(@TempDir Path tempDir) {
rewriteRun(
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
<version>1</version>
<dependencies>
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-core</artifactId>
<version>1.58.0</version>
</dependency>
</dependencies>
</project>
""",
spec -> spec.afterRecipe(after -> {
Optional<MavenResolutionResult> resolutionResult = after.getMarkers().findFirst(MavenResolutionResult.class);
assertThat(resolutionResult).isPresent();

ExecutionContext ctx = new InMemoryExecutionContext(Throwable::printStackTrace);
MavenPomDownloader pomDownloader = new MavenPomDownloader(Collections.emptyMap(), ctx);
Map<String, ResolvedDependency> resolvedDependencies = resolutionResult.get().getPom().resolveDependencies(Scope.Runtime, pomDownloader, ctx).stream()
.collect(Collectors.toMap(d -> d.getGroupId() + ':' + d.getArtifactId() + ':' + d.getVersion(), d -> d));
assertThat(resolvedDependencies.get("io.grpc:grpc-core:1.58.0").getDependencies())
.anySatisfy(d -> assertThat(d.getArtifactId()).isEqualTo("grpc-util"));
assertThat(resolvedDependencies.get("io.grpc:grpc-util:1.58.0").getDependencies())
.anySatisfy(d -> assertThat(d.getArtifactId()).isEqualTo("grpc-core"));
})
)
);
}

private Recipe updateModel() {
return toRecipe(() -> new MavenIsoVisitor<>() {
@Override
Expand Down
17 changes: 17 additions & 0 deletions rewrite-maven/src/test/java/org/openrewrite/maven/utilities/PrintMavenAsDotTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,26 @@ void dot() {
3 -> 6 [taillabel="Test"];
3 -> 7 [taillabel="Test"];
3 -> 8 [taillabel="Test"];
8 -> 6 [taillabel="Test"];
2 -> 4 [taillabel="Test"];
4 -> 6 [taillabel="Test"];
4 -> 3 [taillabel="Test"];
3 -> 6 [taillabel="Test"];
3 -> 7 [taillabel="Test"];
3 -> 8 [taillabel="Test"];
8 -> 6 [taillabel="Test"];
2 -> 5 [taillabel="Test"];
5 -> 6 [taillabel="Test"];
5 -> 9 [taillabel="Test"];
9 -> 6 [taillabel="Test"];
9 -> 7 [taillabel="Test"];
9 -> 8 [taillabel="Test"];
8 -> 6 [taillabel="Test"];
5 -> 3 [taillabel="Test"];
3 -> 6 [taillabel="Test"];
3 -> 7 [taillabel="Test"];
3 -> 8 [taillabel="Test"];
8 -> 6 [taillabel="Test"];
})~~>--><project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
Expand Down
Loading