From ffef9771a1ea8b0cc05c7155c36bf0f4537f5107 Mon Sep 17 00:00:00 2001 From: dimitris Date: Mon, 14 Oct 2024 11:23:52 +0200 Subject: [PATCH 1/7] bug-474: added relevant test, in some cases the provided requested value contains the version, I added an extra check getting the value directly from the pom --- .../maven/UpgradeDependencyVersion.java | 68 +++++++++++++---- .../maven/UpgradeDependencyVersionTest.java | 76 +++++++++++++++++++ 2 files changed, 128 insertions(+), 16 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 2fa42d3482a..46b7d95c940 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -70,15 +70,15 @@ public class UpgradeDependencyVersion extends ScanningRecipe value = tag.getValue(); if (!value.isPresent() || !value.get().equals(pomProperty.propertyValue)) { doAfterVisit(new ChangeTagValueVisitor<>(tag, pomProperty.propertyValue)); @@ -301,9 +301,9 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD ResolvedManagedDependency dm = findManagedDependency(t); // if a managed dependency is expressed as a property, change the property value if (dm != null && - dm.getRequested().getVersion() != null && - dm.getRequested().getVersion().startsWith("${") && - !implicitlyDefinedVersionProperties.contains(dm.getRequested().getVersion())) { + dm.getRequested().getVersion() != null && + dm.getRequested().getVersion().startsWith("${") && + !implicitlyDefinedVersionProperties.contains(dm.getRequested().getVersion())) { doAfterVisit(new ChangePropertyValue(dm.getRequested().getVersion().substring(2, dm.getRequested().getVersion().length() - 1), newerVersion, overrideManagedVersion, false).getVisitor()); @@ -329,9 +329,9 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD String artifactId = managedDependency.getArtifactId(); String version = managedDependency.getVersion(); if (version != null && - !accumulator.projectArtifacts.contains(new GroupArtifact(groupId, artifactId)) && - matchesGlob(groupId, UpgradeDependencyVersion.this.groupId) && - matchesGlob(artifactId, UpgradeDependencyVersion.this.artifactId)) { + !accumulator.projectArtifacts.contains(new GroupArtifact(groupId, artifactId)) && + matchesGlob(groupId, UpgradeDependencyVersion.this.groupId) && + matchesGlob(artifactId, UpgradeDependencyVersion.this.artifactId)) { return upgradeVersion(ctx, t, managedDependency.getRequested().getVersion(), groupId, artifactId, version); } } else { @@ -342,7 +342,7 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD if (!accumulator.projectArtifacts.contains(new GroupArtifact(group, artifactId))) { ResolvedGroupArtifactVersion bom = dm.getBomGav(); if (Objects.equals(group, bom.getGroupId()) && - Objects.equals(artifactId, bom.getArtifactId())) { + Objects.equals(artifactId, bom.getArtifactId())) { return upgradeVersion(ctx, t, requireNonNull(dm.getRequestedBom()).getVersion(), bom.getGroupId(), bom.getArtifactId(), bom.getVersion()); } } @@ -382,12 +382,17 @@ private String resolveVersion(String version) { public @Nullable TreeVisitor upgradeVersion(ExecutionContext ctx, Xml.Tag tag, @Nullable String requestedVersion, String groupId, String artifactId, String version2) throws MavenDownloadingException { String newerVersion = findNewerVersion(groupId, artifactId, version2, ctx); + String requestedGroupId = getRequestedGroupIdForArtifact(artifactId); if (newerVersion == null) { return null; } else if (requestedVersion != null && requestedVersion.startsWith("${")) { //noinspection unchecked return (TreeVisitor) new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false) .getVisitor(); + } else if (requestedVersion != null && requestedGroupId != null && requestedGroupId.startsWith("${")) { + //noinspection unchecked + return (TreeVisitor) new ChangePropertyValue(requestedGroupId.substring(2, requestedGroupId.length() - 1), newerVersion, overrideManagedVersion, false) + .getVisitor(); } else { Xml.Tag childVersionTag = tag.getChild("version").orElse(null); if (childVersionTag != null) { @@ -402,6 +407,37 @@ private String resolveVersion(String version) { return MavenDependency.findNewerVersion(groupId, artifactId, version, getResolutionResult(), metadataFailures, versionComparator, ctx); } + + /** + * Gets the requested group id for the provided artifact id from the pom + * + * @return The groupd if for the requested artifact id or null + */ + @Nullable + private String getRequestedGroupIdForArtifact(String artifactId) { + return getResolutionResult().getPom().getRequested().getDependencyManagement().stream() + .map(d -> { + if (d instanceof ManagedDependency.Imported) { + ManagedDependency.Imported imported = (ManagedDependency.Imported) d; + return imported.getGav(); + } else if (d instanceof ManagedDependency.Defined) { + ManagedDependency.Defined defined = (ManagedDependency.Defined) d; + return defined.getGav(); + } else { + return null; + } + }) + .filter(gav -> { + if (gav != null) { + return artifactId.equals(gav.getArtifactId()); + } + return false; + }) + .findFirst() + .map(GroupArtifactVersion::getVersion) + .orElse(null); + + } }; } 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 63b2e70bc43..5cd42c3400b 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -38,6 +38,82 @@ class UpgradeDependencyVersionTest implements RewriteTest { + @Test + @Issue("https://github.com/openrewrite/rewrite-spring/issues/474") + void upgradePropertyAdditionalCheck() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("org.springframework", "*", "5.2.x", "", + false, null)).expectedCyclesThatMakeChanges(2), + mavenProject("project", + //language=xml + pomXml( + """ + + 4.0.0 + com.openrewrite.example + example + 0.0.1-SNAPSHOT + example + Demo project for Spring Boot + + 2.2.13.RELEASE + 5.2.22.RELEASE + + + + + org.springframework.boot + spring-boot-dependencies + ${spring.boot.version} + pom + import + + + org.springframework + spring-jms + ${spring.jms.version} + + + + + """, + """ + + 4.0.0 + com.openrewrite.example + example + 0.0.1-SNAPSHOT + example + Demo project for Spring Boot + + 2.2.13.RELEASE + 5.2.25.RELEASE + + + + + org.springframework.boot + spring-boot-dependencies + ${spring.boot.version} + pom + import + + + org.springframework + spring-jms + ${spring.jms.version} + + + + + """ + ) + ) + ); + } + @Test void doNotOverrideImplicitProperty() { rewriteRun( From 5468e68807e2ed0ee6417e89bc219e931414ff70 Mon Sep 17 00:00:00 2001 From: dimitris Date: Mon, 14 Oct 2024 11:30:07 +0200 Subject: [PATCH 2/7] bug-474: remove formatting changes. --- .../maven/UpgradeDependencyVersion.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 46b7d95c940..e0298c3cdf9 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -70,15 +70,15 @@ public class UpgradeDependencyVersion extends ScanningRecipe value = tag.getValue(); if (!value.isPresent() || !value.get().equals(pomProperty.propertyValue)) { doAfterVisit(new ChangeTagValueVisitor<>(tag, pomProperty.propertyValue)); @@ -329,9 +329,9 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD String artifactId = managedDependency.getArtifactId(); String version = managedDependency.getVersion(); if (version != null && - !accumulator.projectArtifacts.contains(new GroupArtifact(groupId, artifactId)) && - matchesGlob(groupId, UpgradeDependencyVersion.this.groupId) && - matchesGlob(artifactId, UpgradeDependencyVersion.this.artifactId)) { + !accumulator.projectArtifacts.contains(new GroupArtifact(groupId, artifactId)) && + matchesGlob(groupId, UpgradeDependencyVersion.this.groupId) && + matchesGlob(artifactId, UpgradeDependencyVersion.this.artifactId)) { return upgradeVersion(ctx, t, managedDependency.getRequested().getVersion(), groupId, artifactId, version); } } else { From d3f33821e1567d2f12bf5c586372f0efbfbc7cd1 Mon Sep 17 00:00:00 2001 From: dimitris Date: Mon, 14 Oct 2024 11:31:02 +0200 Subject: [PATCH 3/7] bug-474: remove formatting changes. --- .../org/openrewrite/maven/UpgradeDependencyVersion.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index e0298c3cdf9..773fa549a57 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -301,9 +301,9 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD ResolvedManagedDependency dm = findManagedDependency(t); // if a managed dependency is expressed as a property, change the property value if (dm != null && - dm.getRequested().getVersion() != null && - dm.getRequested().getVersion().startsWith("${") && - !implicitlyDefinedVersionProperties.contains(dm.getRequested().getVersion())) { + dm.getRequested().getVersion() != null && + dm.getRequested().getVersion().startsWith("${") && + !implicitlyDefinedVersionProperties.contains(dm.getRequested().getVersion())) { doAfterVisit(new ChangePropertyValue(dm.getRequested().getVersion().substring(2, dm.getRequested().getVersion().length() - 1), newerVersion, overrideManagedVersion, false).getVisitor()); @@ -342,7 +342,7 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD if (!accumulator.projectArtifacts.contains(new GroupArtifact(group, artifactId))) { ResolvedGroupArtifactVersion bom = dm.getBomGav(); if (Objects.equals(group, bom.getGroupId()) && - Objects.equals(artifactId, bom.getArtifactId())) { + Objects.equals(artifactId, bom.getArtifactId())) { return upgradeVersion(ctx, t, requireNonNull(dm.getRequestedBom()).getVersion(), bom.getGroupId(), bom.getArtifactId(), bom.getVersion()); } } From 1db63dbd06b8dcc59c12301fcb21c80da3d734f2 Mon Sep 17 00:00:00 2001 From: dimitris Date: Mon, 14 Oct 2024 12:34:11 +0200 Subject: [PATCH 4/7] bug-474: replace stream with for --- .../maven/UpgradeDependencyVersion.java | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 773fa549a57..138110027c3 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -415,28 +415,20 @@ private String resolveVersion(String version) { */ @Nullable private String getRequestedGroupIdForArtifact(String artifactId) { - return getResolutionResult().getPom().getRequested().getDependencyManagement().stream() - .map(d -> { - if (d instanceof ManagedDependency.Imported) { - ManagedDependency.Imported imported = (ManagedDependency.Imported) d; - return imported.getGav(); - } else if (d instanceof ManagedDependency.Defined) { - ManagedDependency.Defined defined = (ManagedDependency.Defined) d; - return defined.getGav(); - } else { - return null; - } - }) - .filter(gav -> { - if (gav != null) { - return artifactId.equals(gav.getArtifactId()); - } - return false; - }) - .findFirst() - .map(GroupArtifactVersion::getVersion) - .orElse(null); - + for (ManagedDependency managedDependency : getResolutionResult().getPom().getRequested().getDependencyManagement()) { + if (managedDependency instanceof ManagedDependency.Imported) { + ManagedDependency.Imported imported = (ManagedDependency.Imported) managedDependency; + if (artifactId.equals(imported.getGav().getArtifactId())) { + return imported.getGav().getVersion(); + } + } else if (managedDependency instanceof ManagedDependency.Defined) { + ManagedDependency.Defined defined = (ManagedDependency.Defined) managedDependency; + if (artifactId.equals(defined.getGav().getArtifactId())) { + return defined.getGav().getVersion(); + } + } + } + return null; } }; } From b65710b1c22d491179ff2ae0ab5b28536b1b7050 Mon Sep 17 00:00:00 2001 From: dimijdriven <146715394+dimijdriven@users.noreply.github.com> Date: Mon, 14 Oct 2024 13:06:34 +0200 Subject: [PATCH 5/7] Update rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/org/openrewrite/maven/UpgradeDependencyVersion.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 138110027c3..c716f2153d0 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -413,8 +413,7 @@ private String resolveVersion(String version) { * * @return The groupd if for the requested artifact id or null */ - @Nullable - private String getRequestedGroupIdForArtifact(String artifactId) { + private @Nullable String getRequestedGroupIdForArtifact(String artifactId) { for (ManagedDependency managedDependency : getResolutionResult().getPom().getRequested().getDependencyManagement()) { if (managedDependency instanceof ManagedDependency.Imported) { ManagedDependency.Imported imported = (ManagedDependency.Imported) managedDependency; From 27c7f2cca8689fd9b7d490b7e733a72ebf6359b9 Mon Sep 17 00:00:00 2001 From: dimijdriven <146715394+dimijdriven@users.noreply.github.com> Date: Tue, 15 Oct 2024 10:57:29 +0200 Subject: [PATCH 6/7] Update rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java Co-authored-by: Tim te Beek --- .../java/org/openrewrite/maven/UpgradeDependencyVersion.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index c716f2153d0..5b8740c29e9 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -408,11 +408,6 @@ private String resolveVersion(String version) { versionComparator, ctx); } - /** - * Gets the requested group id for the provided artifact id from the pom - * - * @return The groupd if for the requested artifact id or null - */ private @Nullable String getRequestedGroupIdForArtifact(String artifactId) { for (ManagedDependency managedDependency : getResolutionResult().getPom().getRequested().getDependencyManagement()) { if (managedDependency instanceof ManagedDependency.Imported) { From 822bc959a8a909f44b8f7f5349485a792e67a74b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 27 Oct 2024 13:58:41 +0100 Subject: [PATCH 7/7] Add new test at the end; drop allowing two cycles --- .../maven/UpgradeDependencyVersionTest.java | 153 +++++++++--------- 1 file changed, 77 insertions(+), 76 deletions(-) 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 5cd42c3400b..96beab44e59 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -38,82 +38,6 @@ class UpgradeDependencyVersionTest implements RewriteTest { - @Test - @Issue("https://github.com/openrewrite/rewrite-spring/issues/474") - void upgradePropertyAdditionalCheck() { - rewriteRun( - spec -> spec.recipe(new UpgradeDependencyVersion("org.springframework", "*", "5.2.x", "", - false, null)).expectedCyclesThatMakeChanges(2), - mavenProject("project", - //language=xml - pomXml( - """ - - 4.0.0 - com.openrewrite.example - example - 0.0.1-SNAPSHOT - example - Demo project for Spring Boot - - 2.2.13.RELEASE - 5.2.22.RELEASE - - - - - org.springframework.boot - spring-boot-dependencies - ${spring.boot.version} - pom - import - - - org.springframework - spring-jms - ${spring.jms.version} - - - - - """, - """ - - 4.0.0 - com.openrewrite.example - example - 0.0.1-SNAPSHOT - example - Demo project for Spring Boot - - 2.2.13.RELEASE - 5.2.25.RELEASE - - - - - org.springframework.boot - spring-boot-dependencies - ${spring.boot.version} - pom - import - - - org.springframework - spring-jms - ${spring.jms.version} - - - - - """ - ) - ) - ); - } - @Test void doNotOverrideImplicitProperty() { rewriteRun( @@ -1965,4 +1889,81 @@ void exactVersionMissingInMavenMetadata() { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-spring/issues/474") + void upgradePropertyAdditionalCheck() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("org.springframework", "*", "5.2.x", "", + false, null)), + mavenProject("project", + //language=xml + pomXml( + """ + + 4.0.0 + com.openrewrite.example + example + 0.0.1-SNAPSHOT + example + Demo project for Spring Boot + + 2.2.13.RELEASE + 5.2.22.RELEASE + + + + + org.springframework.boot + spring-boot-dependencies + ${spring.boot.version} + pom + import + + + org.springframework + spring-jms + ${spring.jms.version} + + + + + """, + """ + + 4.0.0 + com.openrewrite.example + example + 0.0.1-SNAPSHOT + example + Demo project for Spring Boot + + 2.2.13.RELEASE + 5.2.25.RELEASE + + + + + org.springframework.boot + spring-boot-dependencies + ${spring.boot.version} + pom + import + + + org.springframework + spring-jms + ${spring.jms.version} + + + + + """ + ) + ) + ); + } + }