From b7c4611626985189a47facd8885ab69678989bfd Mon Sep 17 00:00:00 2001 From: Gordon Daugherty Date: Thu, 20 Jun 2024 12:52:31 -0500 Subject: [PATCH 1/5] Added tests per Slack conversation with Tim te Beek --- ...pgradeTransitiveDependencyVersionTest.java | 273 ++++++++++++++++++ 1 file changed, 273 insertions(+) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java index 811feaa8267..a537d9fc53c 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java @@ -22,6 +22,7 @@ import static org.openrewrite.maven.Assertions.pomXml; +// Bonus: If there's a way to modify these assertions to do an XML-based comparison instead of a string compare please point that out. These are unnecessarily brittle as-is due to caring about whitespace. class UpgradeTransitiveDependencyVersionTest implements RewriteTest { @Override @@ -98,4 +99,276 @@ void leavesDirectDependencyUntouched() { """) ); } + + /* + Demonstrates that this recipe currently takes no "upgrade dependency" action when it observes that a BOM manages the dependency version and that BOM appears to already set it to the desired version number. + The problem with this seems to be that it is looking at the BOM in isolation and doesn't realize that a maven property set in the master POM is overriding a property value in the BOM + and thus causing Maven to use the version number from the parent POM's property value instead of the one seen in the BOM. + + See the XML comments in the "before" XML below. + */ + @Test + void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { + rewriteRun( + spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("commons-codec", "commons-codec", "1.15", null, + null, null, null, null, null, null)) +/* .executionContext( + MavenExecutionContextView + .view(new InMemoryExecutionContext()) + .setRepositories(List.of( + MavenRepository.builder().id("jenkins").uri(myOrganizationsInternalMavenRepoURL).build() + )) + + )*/, + pomXml( + """ + + + 4.0.0 + + + com.ijson.common + ijson-parent-pom + 1.0.8 + + + com.mycompany.app + my-app + 1.0.0 + + + + + + org.springframework.boot + spring-boot-dependencies + 2.5.15 + pom + import + + + + + + + + org.apache.httpcomponents + httpclient + 4.5.6 + + + + + """, + """ + + + 4.0.0 + + + com.ijson.common + ijson-parent-pom + 1.0.8 + + + com.mycompany.app + my-app + 1.0.0 + + + + + commons-codec + commons-codec + 1.15 + + + + org.springframework.boot + spring-boot-dependencies + 2.5.15 + pom + import + + + + + + + + org.apache.httpcomponents + httpclient + 4.5.6 + + + + + """ + ) + ); + } + + // Demonstrates that transitive dependency versions can be upgraded when their version is managed by a master POM. + @Test + void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { + rewriteRun( + spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("org.apache.commons", "commons-lang3", "3.14.0", null, + null, null, null, null, null, null)) + /*.executionContext( + MavenExecutionContextView + .view(new InMemoryExecutionContext()) + .setRepositories(List.of( + MavenRepository.builder().id("jenkins").uri(myOrganizationsInternalMavenRepoURL).build() + )) + + )*/, + pomXml( + """ + + + 4.0.0 + + + org.apache.logging.log4j + log4j + 2.13.3 + + + com.mycompany.app + my-app + + + + + org.apache.commons + commons-text + 1.12.0 + + + + """, + """ + + + 4.0.0 + + + org.apache.logging.log4j + log4j + 2.13.3 + + + com.mycompany.app + my-app + + + + org.apache.commons + commons-lang3 + 3.14.0 + + + + + + + + org.apache.commons + commons-text + 1.12.0 + + + + """ + ) + ); + } + + // Demonstrates that transitive dependency versions can be upgraded when their version is managed by a BOM specified in the dependencyManagement section. + @Test + void upgradeTransitiveDependencyVersion_WorksForBillOfMaterialsManagedDeps() { + rewriteRun( + spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("org.springframework.boot", "spring-boot-actuator", "2.7.0", null, + null, null, null, null, null, null)) + /*.executionContext( + MavenExecutionContextView + .view(new InMemoryExecutionContext()) + .setRepositories(List.of( + MavenRepository.builder().id("jenkins").uri(myOrganizationsInternalMavenRepoURL).build() + )) + + )*/, + pomXml( + """ + + + 4.0.0 + + com.mycompany.app + my-app + 1.0.0 + + + + + org.springframework.boot + spring-boot-dependencies + 2.5.15 + pom + import + + + + + + + + org.springframework.boot + spring-boot-starter-actuator + 2.7.0 + + + + """, + """ + + + 4.0.0 + + com.mycompany.app + my-app + 1.0.0 + + + + + org.springframework.boot + spring-boot-actuator + 2.7.0 + + + org.springframework.boot + spring-boot-dependencies + 2.5.15 + pom + import + + + + + + + + org.springframework.boot + spring-boot-starter-actuator + 2.7.0 + + + + """ + ) + ); + } } From eb192223124eca8ad36cada2b896365c7031b6e1 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jul 2024 18:15:36 +0200 Subject: [PATCH 2/5] Apply formatter to minimize automated review comments --- ...pgradeTransitiveDependencyVersionTest.java | 187 +++++++++--------- 1 file changed, 94 insertions(+), 93 deletions(-) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java index a537d9fc53c..7f2aa18bda1 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java @@ -36,45 +36,45 @@ public void defaults(RecipeSpec spec) { void singleProject() { rewriteRun( pomXml( - """ - - 4.0.0 - org.openrewrite - core - 0.1.0-SNAPSHOT - - - org.openrewrite - rewrite-java - 7.0.0 - - - - """, """ - - 4.0.0 - org.openrewrite - core - 0.1.0-SNAPSHOT - - - - com.fasterxml.jackson.core - jackson-core - 2.12.5 - - - - - - org.openrewrite - rewrite-java - 7.0.0 - - - - """) + + 4.0.0 + org.openrewrite + core + 0.1.0-SNAPSHOT + + + org.openrewrite + rewrite-java + 7.0.0 + + + + """, + """ + + 4.0.0 + org.openrewrite + core + 0.1.0-SNAPSHOT + + + + com.fasterxml.jackson.core + jackson-core + 2.12.5 + + + + + + org.openrewrite + rewrite-java + 7.0.0 + + + + """) ); } @@ -82,21 +82,22 @@ void singleProject() { void leavesDirectDependencyUntouched() { rewriteRun( pomXml( - """ - - 4.0.0 - org.openrewrite - core - 0.1.0-SNAPSHOT - - - com.fasterxml.jackson.core - jackson-core - 2.12.0 - - - - """) + """ + + 4.0.0 + org.openrewrite + core + 0.1.0-SNAPSHOT + + + com.fasterxml.jackson.core + jackson-core + 2.12.0 + + + + """ + ) ); } @@ -111,7 +112,7 @@ void leavesDirectDependencyUntouched() { void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { rewriteRun( spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("commons-codec", "commons-codec", "1.15", null, - null, null, null, null, null, null)) + null, null, null, null, null, null)) /* .executionContext( MavenExecutionContextView .view(new InMemoryExecutionContext()) @@ -125,17 +126,17 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { 4.0.0 - + com.ijson.common ijson-parent-pom 1.0.8 - + com.mycompany.app my-app 1.0.0 - + @@ -148,7 +149,7 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { - + @@ -164,17 +165,17 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { 4.0.0 - + com.ijson.common ijson-parent-pom 1.0.8 - + com.mycompany.app my-app 1.0.0 - + @@ -192,7 +193,7 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { - + @@ -213,7 +214,7 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { rewriteRun( spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("org.apache.commons", "commons-lang3", "3.14.0", null, - null, null, null, null, null, null)) + null, null, null, null, null, null)) /*.executionContext( MavenExecutionContextView .view(new InMemoryExecutionContext()) @@ -269,7 +270,7 @@ void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { - + @@ -289,7 +290,7 @@ void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { void upgradeTransitiveDependencyVersion_WorksForBillOfMaterialsManagedDeps() { rewriteRun( spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("org.springframework.boot", "spring-boot-actuator", "2.7.0", null, - null, null, null, null, null, null)) + null, null, null, null, null, null)) /*.executionContext( MavenExecutionContextView .view(new InMemoryExecutionContext()) @@ -300,37 +301,37 @@ void upgradeTransitiveDependencyVersion_WorksForBillOfMaterialsManagedDeps() { )*/, pomXml( """ - + - 4.0.0 + 4.0.0 - com.mycompany.app - my-app - 1.0.0 + com.mycompany.app + my-app + 1.0.0 - - - - org.springframework.boot - spring-boot-dependencies - 2.5.15 - pom - import - - - - - - - - org.springframework.boot - spring-boot-starter-actuator - 2.7.0 - - - - """, + + + + org.springframework.boot + spring-boot-dependencies + 2.5.15 + pom + import + + + + + + + + org.springframework.boot + spring-boot-starter-actuator + 2.7.0 + + + + """, """ From 1328b9b04ddd8999485eef2401b8e997801fd976 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jul 2024 19:00:32 +0200 Subject: [PATCH 3/5] Drop tabs from indentation --- ...pgradeTransitiveDependencyVersionTest.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java index 7f2aa18bda1..20d9393b6fe 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java @@ -230,21 +230,21 @@ void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { 4.0.0 - org.apache.logging.log4j - log4j - 2.13.3 + org.apache.logging.log4j + log4j + 2.13.3 com.mycompany.app my-app - - - org.apache.commons - commons-text - 1.12.0 - + + + org.apache.commons + commons-text + 1.12.0 + """, @@ -272,12 +272,12 @@ void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { - - - org.apache.commons - commons-text - 1.12.0 - + + + org.apache.commons + commons-text + 1.12.0 + """ From 06f4a2c9e3fcc905113967c75a9ddfb29bef033c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jul 2024 19:11:58 +0200 Subject: [PATCH 4/5] Update rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- ...pgradeTransitiveDependencyVersionTest.java | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java index 20d9393b6fe..f243157682c 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java @@ -301,36 +301,36 @@ void upgradeTransitiveDependencyVersion_WorksForBillOfMaterialsManagedDeps() { )*/, pomXml( """ - - - 4.0.0 - - com.mycompany.app - my-app - 1.0.0 - - - - - org.springframework.boot - spring-boot-dependencies - 2.5.15 - pom - import - - - - - - - - org.springframework.boot - spring-boot-starter-actuator - 2.7.0 - - - + + + 4.0.0 + + com.mycompany.app + my-app + 1.0.0 + + + + + org.springframework.boot + spring-boot-dependencies + 2.5.15 + pom + import + + + + + + + + org.springframework.boot + spring-boot-starter-actuator + 2.7.0 + + + """, """ Date: Fri, 26 Jul 2024 16:33:27 +0200 Subject: [PATCH 5/5] Drop commented out code blocks; link test to issue --- ...pgradeTransitiveDependencyVersionTest.java | 37 ++++--------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java index f243157682c..f523aebbfc0 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeTransitiveDependencyVersionTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -74,7 +75,8 @@ void singleProject() { - """) + """ + ) ); } @@ -109,18 +111,11 @@ void leavesDirectDependencyUntouched() { See the XML comments in the "before" XML below. */ @Test + @Issue("https://github.com/openrewrite/rewrite/pull/4274") void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { rewriteRun( spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("commons-codec", "commons-codec", "1.15", null, - null, null, null, null, null, null)) -/* .executionContext( - MavenExecutionContextView - .view(new InMemoryExecutionContext()) - .setRepositories(List.of( - MavenRepository.builder().id("jenkins").uri(myOrganizationsInternalMavenRepoURL).build() - )) - - )*/, + null, null, null, null, null, null)), pomXml( """ @@ -159,7 +154,6 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { - """, """ @@ -203,7 +197,6 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { - """ ) ); @@ -214,15 +207,7 @@ void upgradeTransitiveDependencyVersion_NewIssue_NotYetFixed() { void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { rewriteRun( spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("org.apache.commons", "commons-lang3", "3.14.0", null, - null, null, null, null, null, null)) - /*.executionContext( - MavenExecutionContextView - .view(new InMemoryExecutionContext()) - .setRepositories(List.of( - MavenRepository.builder().id("jenkins").uri(myOrganizationsInternalMavenRepoURL).build() - )) - - )*/, + null, null, null, null, null, null)), pomXml( """ @@ -290,15 +275,7 @@ void upgradeTransitiveDependencyVersion_WorksForMasterPomManagedDeps() { void upgradeTransitiveDependencyVersion_WorksForBillOfMaterialsManagedDeps() { rewriteRun( spec -> spec.recipe(new UpgradeTransitiveDependencyVersion("org.springframework.boot", "spring-boot-actuator", "2.7.0", null, - null, null, null, null, null, null)) - /*.executionContext( - MavenExecutionContextView - .view(new InMemoryExecutionContext()) - .setRepositories(List.of( - MavenRepository.builder().id("jenkins").uri(myOrganizationsInternalMavenRepoURL).build() - )) - - )*/, + null, null, null, null, null, null)), pomXml( """