Skip to content

Commit 3be17c8

Browse files
Jenson3210Jenson3210
authored
Fixed the scanners being executed in a loop that then causes all scans to be updated in a single run and the marker to be wrong
1 parent 944f23e commit 3be17c8

4 files changed

Lines changed: 126 additions & 11 deletions

File tree

‎rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -418,10 +418,26 @@ public boolean isAcceptable(SourceFile sf, ExecutionContext ctx) {
418418
if ((tree != t || updateLockFile.isAcceptable(sf, ctx)) && projectMarker.isPresent()) {
419419
GradleProject gradleProject = projectMarker.get();
420420
Map<GroupArtifact, Set<String>> configurationsPerGa = acc.getConfigurationPerGAPerModule().getOrDefault(getGradleProjectKey(gradleProject), emptyMap());
421-
for (Map.Entry<GroupArtifact, Object> newVersion : acc.getGaToNewVersion().entrySet()) {
422-
if (newVersion.getValue() instanceof String) {
423-
GroupArtifactVersion gav = new GroupArtifactVersion(newVersion.getKey().getGroupId(), newVersion.getKey().getArtifactId(), (String) newVersion.getValue());
424-
gradleProject = replaceVersion(gradleProject, ctx, gav, configurationsPerGa.getOrDefault(gav.asGroupArtifact(), emptySet()));
421+
if (acc.getGaToNewVersion().isEmpty()) {
422+
DependencyMatcher matcher = new DependencyMatcher(groupId, artifactId, null);
423+
DependencyVersionSelector versionSelector = new DependencyVersionSelector(metadataFailures, gradleProject, null);
424+
for (GroupArtifact groupArtifact : configurationsPerGa.keySet()) {
425+
if (!matcher.matches(groupArtifact.getGroupId(), groupArtifact.getArtifactId())) {
426+
continue;
427+
}
428+
try {
429+
String selectedVersion = versionSelector.select(groupArtifact, null, newVersion, versionPattern, ctx);
430+
GroupArtifactVersion gav = new GroupArtifactVersion(groupArtifact.getGroupId(), groupArtifact.getArtifactId(), selectedVersion);
431+
gradleProject = replaceVersion(gradleProject, ctx, gav, configurationsPerGa.getOrDefault(gav.asGroupArtifact(), emptySet()));
432+
} catch (MavenDownloadingException ignore) {}
433+
}
434+
435+
} else {
436+
for (Map.Entry<GroupArtifact, Object> newVersion : acc.getGaToNewVersion().entrySet()) {
437+
if (newVersion.getValue() instanceof String) {
438+
GroupArtifactVersion gav = new GroupArtifactVersion(newVersion.getKey().getGroupId(), newVersion.getKey().getArtifactId(), (String) newVersion.getValue());
439+
gradleProject = replaceVersion(gradleProject, ctx, gav, configurationsPerGa.getOrDefault(gav.asGroupArtifact(), emptySet()));
440+
}
425441
}
426442
}
427443
if (projectMarker.get() != gradleProject) {
@@ -886,6 +902,9 @@ public static GradleProject replaceVersion(GradleProject gp, ExecutionContext ct
886902
private static org.openrewrite.maven.tree.Dependency maybeUpdateDependency(
887903
org.openrewrite.maven.tree.Dependency dep,
888904
org.openrewrite.maven.tree.Dependency newDep) {
905+
if (Objects.equals(dep.getGroupId(), newDep.getGroupId()) && Objects.equals(dep.getArtifactId(), newDep.getArtifactId()) && Objects.equals(dep.getVersion(), newDep.getVersion())) {
906+
return dep;
907+
}
889908
if (Objects.equals(dep.getGroupId(), newDep.getGroupId()) && Objects.equals(dep.getArtifactId(), newDep.getArtifactId())) {
890909
return newDep;
891910
}
@@ -910,6 +929,9 @@ private static ResolvedDependency maybeUpdateResolvedDependency(
910929
if (traversalHistory.contains(dep)) {
911930
return dep;
912931
}
932+
if (Objects.equals(dep.getGroupId(), newDep.getGroupId()) && Objects.equals(dep.getArtifactId(), newDep.getArtifactId()) && Objects.equals(dep.getVersion(), newDep.getVersion())) {
933+
return dep;
934+
}
913935
if (Objects.equals(dep.getGroupId(), newDep.getGroupId()) && Objects.equals(dep.getArtifactId(), newDep.getArtifactId())) {
914936
return newDep;
915937
}

‎rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersion.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import java.util.concurrent.atomic.AtomicBoolean;
5757
import java.util.stream.Collectors;
5858

59+
import static java.util.Collections.emptyMap;
5960
import static java.util.Collections.singletonMap;
6061
import static java.util.Objects.requireNonNull;
6162
import static org.openrewrite.Preconditions.not;
@@ -337,6 +338,7 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
337338

338339
@Override
339340
public TreeVisitor<?, ExecutionContext> getVisitor(DependencyVersionState acc) {
341+
final DependencyMatcher dependencyMatcher = new DependencyMatcher(groupId, artifactId, null);
340342
return Preconditions.check(new FindGradleProject(FindGradleProject.SearchCriteria.Marker), new TreeVisitor<Tree, ExecutionContext>() {
341343
private final UpdateGradle updateGradle = new UpdateGradle(acc.getUpdatesPerProject());
342344
private final UpdateDependencyLock updateLockFile = new UpdateDependencyLock();
@@ -376,6 +378,9 @@ private GradleProject updatedModel(GradleProject gradleProject, Map<GroupArtifac
376378
.collect(Collectors.toSet());
377379
Set<GroupArtifactVersion> gavs = new LinkedHashSet<>();
378380
for (Map.Entry<GroupArtifact, Map<GradleDependencyConfiguration, String>> update : toUpdate.entrySet()) {
381+
if (!dependencyMatcher.matches(update.getKey().getGroupId(), update.getKey().getArtifactId())) {
382+
continue;
383+
}
379384
Map<GradleDependencyConfiguration, String> configs = update.getValue();
380385
String groupId = update.getKey().getGroupId();
381386
String artifactId = update.getKey().getArtifactId();
@@ -397,6 +402,7 @@ private GradleProject updatedModel(GradleProject gradleProject, Map<GroupArtifac
397402
private class UpdateGradle extends JavaVisitor<ExecutionContext> {
398403

399404
final Map<String, Map<GroupArtifact, Map<GradleDependencyConfiguration, String>>> updatesPerProject;
405+
final DependencyMatcher dependencyMatcher = new DependencyMatcher(groupId, artifactId, null);
400406

401407
@SuppressWarnings("NotNullFieldNotInitialized")
402408
GradleProject gradleProject;
@@ -407,7 +413,11 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) {
407413
JavaSourceFile cu = (JavaSourceFile) tree;
408414
gradleProject = cu.getMarkers().findFirst(GradleProject.class).orElseThrow(() -> new IllegalStateException("Unable to find GradleProject marker."));
409415

410-
if (!updatesPerProject.get(getGradleProjectKey(gradleProject)).isEmpty()) {
416+
Map<GroupArtifact, Map<GradleDependencyConfiguration, String>> projectRequiredUpdates = updatesPerProject.getOrDefault(getGradleProjectKey(gradleProject), emptyMap());
417+
if (!projectRequiredUpdates.isEmpty()) {
418+
if (projectRequiredUpdates.keySet().stream().noneMatch(ga -> dependencyMatcher.matches(ga.getGroupId(), ga.getArtifactId()))) {
419+
return cu;
420+
}
411421
cu = (JavaSourceFile) Preconditions.check(
412422
not(new JavaIsoVisitor<ExecutionContext>() {
413423
@Override
@@ -432,7 +442,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
432442
new AddConstraintsBlock(cu instanceof K.CompilationUnit)
433443
).visitNonNull(cu, ctx);
434444

435-
for (Map.Entry<GroupArtifact, Map<GradleDependencyConfiguration, String>> update : updatesPerProject.get(getGradleProjectKey(gradleProject)).entrySet()) {
445+
for (Map.Entry<GroupArtifact, Map<GradleDependencyConfiguration, String>> update : projectRequiredUpdates.entrySet()) {
446+
if (!dependencyMatcher.matches(update.getKey().getGroupId(), update.getKey().getArtifactId())) {
447+
continue;
448+
}
436449
Map<GradleDependencyConfiguration, String> configs = update.getValue();
437450
for (Map.Entry<GradleDependencyConfiguration, String> config : configs.entrySet()) {
438451
cu = (JavaSourceFile) new AddConstraint(cu instanceof K.CompilationUnit, config.getKey().getName(), new GroupArtifactVersion(update.getKey().getGroupId(),

‎rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeDependencyVersionTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,7 @@ void kotlinDslStringInterpolation() {
14401440
@Test
14411441
void lockFileGetsUpdated() {
14421442
rewriteRun(
1443-
spec -> spec.recipe(new UpgradeDependencyVersion("org.apache.tomcat.embed", "*", "latest.patch", null)),
1443+
spec -> spec.recipe(new UpgradeDependencyVersion("org.apache.tomcat.embed", "*", "10.0.27", null)),
14441444
//language=groovy
14451445
buildGradle(
14461446
"""
@@ -1483,7 +1483,7 @@ void lockFileGetsUpdated() {
14831483
@Test
14841484
void multimoduleProjectLockFile() {
14851485
rewriteRun(spec ->
1486-
spec.recipe(new UpgradeDependencyVersion("org.apache.tomcat.embed", "*", "latest.patch", null)),
1486+
spec.recipe(new UpgradeDependencyVersion("org.apache.tomcat.embed", "*", "10.0.27", null)),
14871487
settingsGradle(
14881488
"""
14891489
rootProject.name = 'my-project'
@@ -1716,7 +1716,7 @@ void jacksonBomGetsUpdatedInLockFile() {
17161716
@Test
17171717
void multiProject() {
17181718
rewriteRun(
1719-
spec -> spec.recipe(new UpgradeDependencyVersion("org.apache.tomcat.embed", "*", "latest.patch", null)),
1719+
spec -> spec.recipe(new UpgradeDependencyVersion("org.apache.tomcat.embed", "*", "10.0.27", null)),
17201720
buildGradle(
17211721
"""
17221722
plugins { id 'java' }

‎rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersionTest.java‎

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
*/
1616
package org.openrewrite.gradle;
1717

18+
import lombok.EqualsAndHashCode;
19+
import lombok.Value;
20+
import org.jspecify.annotations.Nullable;
1821
import org.junit.jupiter.api.Test;
19-
import org.openrewrite.DocumentExample;
20-
import org.openrewrite.Issue;
22+
import org.openrewrite.*;
2123
import org.openrewrite.test.RecipeSpec;
2224
import org.openrewrite.test.RewriteTest;
2325

@@ -917,4 +919,82 @@ void addConstraintUpdatesLockfile() {
917919
)
918920
);
919921
}
922+
923+
@Test
924+
void canHandleNullScannedAccumulator() {
925+
UpgradeTransitiveDependencyVersion updateClassgraph = new UpgradeTransitiveDependencyVersion("io.github.classgraph", "classgraph", "4.8.112", null, null, null);
926+
UpgradeTransitiveDependencyVersion updateJackson = new UpgradeTransitiveDependencyVersion("com.fasterxml*", "jackson-core", "2.12.5", null, "CVE-2024-BAD", null);
927+
rewriteRun(
928+
spec -> spec.recipe(new ScanningAccumulatedUpgradeRecipe(updateClassgraph, updateJackson)),
929+
buildGradle(
930+
"""
931+
plugins {
932+
id 'java'
933+
}
934+
repositories { mavenCentral() }
935+
936+
dependencies {
937+
implementation 'org.openrewrite:rewrite-java:7.0.0'
938+
}
939+
""",
940+
"""
941+
plugins {
942+
id 'java'
943+
}
944+
repositories { mavenCentral() }
945+
946+
dependencies {
947+
constraints {
948+
implementation('com.fasterxml.jackson.core:jackson-core:2.12.5') {
949+
because 'CVE-2024-BAD'
950+
}
951+
}
952+
953+
implementation 'org.openrewrite:rewrite-java:7.0.0'
954+
}
955+
"""
956+
)
957+
);
958+
}
959+
960+
@Value
961+
@EqualsAndHashCode(callSuper = false)
962+
public static class ScanningAccumulatedUpgradeRecipe extends ScanningRecipe<UpgradeTransitiveDependencyVersion.DependencyVersionState> {
963+
@Override
964+
public String getDisplayName() {
965+
return "Accumulation-scanned recipe";
966+
}
967+
968+
@Override
969+
public String getDescription() {
970+
return "Some recipes hava loop to determine all updates and add them to the scanner. This cycle/recipe only can update for the provided dependency.";
971+
}
972+
973+
private final UpgradeTransitiveDependencyVersion scanAlsoFor;
974+
private final UpgradeTransitiveDependencyVersion upgradeDependency;
975+
976+
@Override
977+
public UpgradeTransitiveDependencyVersion.DependencyVersionState getInitialValue(ExecutionContext ctx) {
978+
return new UpgradeTransitiveDependencyVersion.DependencyVersionState();
979+
}
980+
981+
@Override
982+
public TreeVisitor<?, ExecutionContext> getScanner(UpgradeTransitiveDependencyVersion.DependencyVersionState acc) {
983+
return new TreeVisitor<>() {
984+
@Override
985+
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
986+
if (tree instanceof SourceFile) {
987+
tree = scanAlsoFor.getScanner(acc).visit(tree, ctx);
988+
tree = upgradeDependency.getScanner(acc).visit(tree, ctx);
989+
}
990+
return tree;
991+
}
992+
};
993+
}
994+
995+
@Override
996+
public TreeVisitor<?, ExecutionContext> getVisitor(UpgradeTransitiveDependencyVersion.DependencyVersionState acc) {
997+
return upgradeDependency.getVisitor(acc);
998+
}
999+
}
9201000
}

0 commit comments

Comments
 (0)
⚡