From c6f84844ea3c6eab81bdf9bcbbb24f736def47f0 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 30 Jun 2026 19:45:00 +0200 Subject: [PATCH] Fix UpgradeDependencyVersion not upgrading parent dependencyManagement after ChangeDependency in multi-module projects Two distinct multi-module bugs caused the chained `ChangeDependency` -> `UpgradeDependencyVersion` scenario from #8145 to fail: 1. `UpdateMavenModel` discarded a parent POM's own valid model update when re-resolving one of its aggregated child modules threw. After `ChangeDependency` renames a managed dependency in the parent, the child module still references the old (now unmanaged) coordinates and cannot be resolved mid-recipe. The resulting `MavenDownloadingExceptions` caused the entire parent update to be thrown away, leaving the parent's resolved model stale, so the following `UpgradeDependencyVersion` could not find the managed dependency to upgrade. Module re-resolution is now best-effort: a failing module keeps its previous resolution (it is refreshed when its own source file is updated) and no longer invalidates the parent's update. 2. `ChangeDependencyGroupIdAndArtifactId` added a spurious explicit `` to a child dependency whose version is managed by a local parent when the dependency uses `provided` (or `test`) scope. The managed-dependency check used `Scope.isInClasspathOf`, which models transitive classpath visibility and returns `false` for `provided`/`provided` and `test`/`test`. Version management applies whenever coordinates match regardless of scope, so an identical scope now correctly counts as managed. --- .../ChangeDependencyGroupIdAndArtifactId.java | 10 +- .../openrewrite/maven/UpdateMavenModel.java | 15 +-- ...ngeDependencyGroupIdAndArtifactIdTest.java | 95 ++++++++++++++++ .../maven/UpgradeDependencyVersionTest.java | 102 ++++++++++++++++++ 4 files changed, 209 insertions(+), 13 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java index 0d036878a7e..d8128bfa5e1 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java @@ -427,12 +427,18 @@ private boolean isDependencyManaged(Scope scope, String groupId, String artifact MavenResolutionResult result = getResolutionResult(); for (ResolvedManagedDependency managedDependency : result.getPom().getDependencyManagement()) { if (groupId.equals(managedDependency.getGroupId()) && artifactId.equals(managedDependency.getArtifactId())) { - return scope.isInClasspathOf(managedDependency.getScope()); + return scopeIsManagedBy(scope, managedDependency.getScope()); } } return false; } + // A managed version applies whenever coordinates match regardless of scope, so an identical scope + // qualifies; isInClasspathOf alone wrongly returns false for provided/provided and test/test. + private boolean scopeIsManagedBy(Scope dependencyScope, @Nullable Scope managedScope) { + return managedScope == null || dependencyScope == managedScope || dependencyScope.isInClasspathOf(managedScope); + } + private boolean canAffectManagedDependency(MavenResolutionResult result, Scope scope, String groupId, String artifactId) { // We're only going to be able to effect managed dependencies that are either direct or are brought in as direct via a local parent // `ChangeManagedDependencyGroupIdAndArtifactId` cannot manipulate BOM imported managed dependencies nor direct dependencies from remote parents @@ -440,7 +446,7 @@ private boolean canAffectManagedDependency(MavenResolutionResult result, Scope s for (ManagedDependency requestedManagedDependency : requestedPom.getDependencyManagement()) { if (matchesGlob(requestedManagedDependency.getGroupId(), groupId) && matchesGlob(requestedManagedDependency.getArtifactId(), artifactId)) { if (requestedManagedDependency instanceof ManagedDependency.Defined) { - return scope.isInClasspathOf(Scope.fromName(((ManagedDependency.Defined) requestedManagedDependency).getScope())); + return scopeIsManagedBy(scope, Scope.fromName(((ManagedDependency.Defined) requestedManagedDependency).getScope())); } } } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java index ea5c0403c27..c868b24cab5 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java @@ -28,7 +28,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.atomic.AtomicReference; import static java.util.Collections.emptyList; import static java.util.stream.Collectors.toList; @@ -199,28 +198,22 @@ private MavenResolutionResult updateResult(ExecutionContext ctx, MavenResolution MavenPomDownloader downloader = new MavenPomDownloader(projectPoms, ctx, getResolutionResult().getMavenSettings(), getResolutionResult().getActiveProfiles()); - AtomicReference exceptions = new AtomicReference<>(); try { ResolvedPom resolved = resolutionResult.getPom().resolve(ctx, downloader); - MavenResolutionResult mrr = resolutionResult + return resolutionResult .withPom(resolved) + // Re-resolve modules best-effort: a module that is transiently unresolvable mid-recipe keeps + // its previous resolution rather than discarding this pom's own valid update. .withModules(ListUtils.map(resolutionResult.getModules(), module -> { try { return updateResult(ctx, module, projectPoms); } catch (MavenDownloadingExceptions e) { - exceptions.set(MavenDownloadingExceptions.append(exceptions.get(), e)); return module; } })) .resolveDependencies(downloader, ctx); - if (exceptions.get() != null) { - throw exceptions.get(); - } - return mrr; - } catch (MavenDownloadingExceptions e) { - throw MavenDownloadingExceptions.append(exceptions.get(), e); } catch (MavenDownloadingException e) { - throw MavenDownloadingExceptions.append(exceptions.get(), e); + throw MavenDownloadingExceptions.append(null, e); } } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java index f6860e59396..d26b10172ee 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java @@ -1501,6 +1501,101 @@ void unmanagedToManagedExternalizedDepMgmt() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/8145") + @Test + void providedDependencyManagedByLocalParentDoesNotGetExplicitVersion() { + rewriteRun( + spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( + "javax.servlet", "javax.servlet-api", + "jakarta.servlet", "jakarta.servlet-api", + "5.0.x", null)), + mavenProject("parent", + pomXml( + """ + + com.example + parent + 1.0-SNAPSHOT + pom + + child + + + + + javax.servlet + javax.servlet-api + 4.0.0 + provided + + + + + """, + """ + + com.example + parent + 1.0-SNAPSHOT + pom + + child + + + + + jakarta.servlet + jakarta.servlet-api + 5.0.0 + provided + + + + + """ + ), + mavenProject("child", + pomXml( + """ + + + com.example + parent + 1.0-SNAPSHOT + + child + + + javax.servlet + javax.servlet-api + provided + + + + """, + """ + + + com.example + parent + 1.0-SNAPSHOT + + child + + + jakarta.servlet + jakarta.servlet-api + provided + + + + """ + ) + ) + ) + ); + } + @Test void latestPatch() { rewriteRun( diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java index 420c6791d56..8a3b722007e 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -385,6 +385,108 @@ void upgradeVersionWithGroupIdAndArtifactIdDefinedAsProperty() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/8145") + @Test + void changeDependencyThenUpgradeManagedVersionInParentOfMultiModule() { + rewriteRun( + spec -> spec.recipes( + // Phase 1: rename javax -> jakarta (EE9 migration) + new ChangeDependencyGroupIdAndArtifactId( + "javax.servlet", "javax.servlet-api", + "jakarta.servlet", "jakarta.servlet-api", + "5.0.x", null), + // Phase 2: bump jakarta EE9 -> EE10, which must upgrade the parent's managed version + new UpgradeDependencyVersion( + "jakarta.servlet", "jakarta.servlet-api", + "6.0.x", null, null, null) + ), + mavenProject("parent", + pomXml( + """ + + com.example + parent + 1.0-SNAPSHOT + pom + + child + + + + + javax.servlet + javax.servlet-api + 4.0.0 + provided + + + + + """, + """ + + com.example + parent + 1.0-SNAPSHOT + pom + + child + + + + + jakarta.servlet + jakarta.servlet-api + 6.0.0 + provided + + + + + """ + ), + mavenProject("child", + pomXml( + """ + + + com.example + parent + 1.0-SNAPSHOT + + child + + + javax.servlet + javax.servlet-api + provided + + + + """, + """ + + + com.example + parent + 1.0-SNAPSHOT + + child + + + jakarta.servlet + jakarta.servlet-api + provided + + + + """ + ) + ) + ) + ); + } + @Test void upgradeVersionSuccessively() { rewriteRun(