MigrateHibernateConstraintsToJavax does not honor switch to Jakarta in Spring Boot 2.2

Philzen commented 1 year ago

What version of OpenRewrite are you using?

OpenRewrite 8.5.0

How are you running OpenRewrite?

From the CLI as per https://docs.openrewrite.org/recipes/java/spring/boot2/upgradespringboot_2_5

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
  -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-spring:RELEASE \
  -Drewrite.activeRecipes=org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_5

What did expect to see?

As for this particular project, i guess only the version of Spring Boot would need an update to 2.5.15.

What did happen?

It upgraded the version number and added

        <dependency>
            <groupId>jakarta.validation</groupId>
            <artifactId>jakarta.validation-api</artifactId>
            <!-- Version is managed by SpringBoot -->
        </dependency>
+        <dependency>
+            <groupId>javax.validation</groupId>
+            <artifactId>validation-api</artifactId>
+        </dependency>

to my POM – although it already contains the jakarta dependency which provides the javax.validation.* package just right before where it added the new entry.

This now introduces to different versions of basically the same package: grafik

Note that i now have jakarta.validation:jakarta.validation-api@2.0.2 and javax.validation:validation-api@2.0.1.Final in my libraries, which is not a good situation :wink:

The output of above CLI run was:

[INFO] --- rewrite:5.7.1:run (default-cli) @ AMQP ---
[INFO] Using active recipe(s) [org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_5]
[INFO] Using active styles(s) []
Downloading from central: https://repo.maven.apache.org/maven2/org/openrewrite/recipe/rewrite-spring/maven-metadata.xml
Downloaded from central: https://repo.maven.apache.org/maven2/org/openrewrite/recipe/rewrite-spring/maven-metadata.xml (2.4 kB at 49 kB/s)
[INFO] Validating active recipes...
[INFO] Project [amqp] Resolving Poms...
[INFO] Project [amqp] Parsing source files
[INFO] Running recipe(s)...
[WARNING] Changes have been made to pom.xml by:
[WARNING]     org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_5
[WARNING]         org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_4
[WARNING]             org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_3
[WARNING]                 org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_2
[WARNING]                     org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_1
[WARNING]                         org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_0
[WARNING]                             org.openrewrite.java.spring.boot2.MigrateHibernateConstraintsToJavax
[WARNING]                                 org.openrewrite.java.dependencies.AddDependency: {groupId=javax.validation, artifactId=validation-api, version=2.x, onlyIfUsing=javax.validation.constraints.*}
[WARNING]                                     org.openrewrite.maven.AddDependency: {groupId=javax.validation, artifactId=validation-api, version=2.x, onlyIfUsing=javax.validation.constraints.*}
[WARNING]         org.openrewrite.maven.UpgradeParentVersion: {groupId=org.springframework.boot, artifactId=spring-boot-starter-parent, newVersion=2.5.x}
[WARNING] Please review and commit the results.

and i believe MigrateHibernateConstraintsToJavax is the culprit here. This may or may not be able to be solved w/o a change to the UpgradeSpringBoot_2_2 recipe.

The change i'm thinking of was currently incubating on my list to report this as an improvement for the Spring Boot 2.2 recipe, as i had to do this manually so far – but now i realize there is a sub-recipe in the recipe list that conflicts with this (otherwise correct) change, so i'd even consider this a bug, as it introduces version conflicts.

After having run a couple of recipes starting with the 2.2 migration yesterday, i ran mvn dependency:tree and realized this diverge. So i manually added a commit in between my 2.2 and 2.3 migration commit to switch to Jakarta – which seems the most feasible thing to do from looking at https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.2-Release-Notes#jakarta-ee-dependencies

So far i have identified at least these two recipes that i believe could be included in the list for spring-boot-22 recipe for the projects i'm currently migrating to work as expected:

From looking at the release notes, https://docs.openrewrite.org/recipes/java/migrate/jakarta/javaxeltojakartael (3.) should also be included. Also, when i check the dependency tree in my project (at Spring Boot 2.2.x), https://docs.openrewrite.org/recipes/java/migrate/jakarta/javaxxmlbindmigrationtojakartaxmlbind (4.) should also be included in . There may be more, we'd need to check the all the spring boot components there.

However, as outlined in this bug report, in order for such an improvement to be feasible at all, MigrateHibernateConstraintsToJavax needs to honor the jakarta.validation first and leave it alone.

timtebeek commented 1 year ago

Thanks for the detailed report @Philzen ! I'll add some links to the recipes implementations, such that we can see where to go from there.

Firstly there's this recipe in rewrite-spring, as you've pointed out, that adds the non-Jakarta dependency https://github.com/openrewrite/rewrite-spring/blob/eef8ab3c613aa6cb41d0be60ca4cd6765c399b8d/src/main/resources/META-INF/rewrite/spring-boot-20.yml#L127-L142

Then there's this recipe that removes validation-api and adds in jakarta.validation-api in rewrite-migrate-java that's part of JavaxMigrationToJakarta, which we run as part of UpgradeSpringBoot_3_0. https://github.com/openrewrite/rewrite-migrate-java/blob/c3f924f62aec6c94b6f1dbbe0c54412633043541/src/main/resources/META-INF/rewrite/jakarta-ee-9.yml#L305-L331

That might explain why we haven't seen reports of this issue; when folks immediately upgrade to 3.0 they will end up with the right dependency on the classpath.

Philzen commented 1 year ago

While the original issue was only introducing an additional / duplicated dependency, i now found an example where it leaves the project in a non-compiling state after running the recipe.

Just ran upgradespringboot_2_7 to upgrade a project from 2.1.18 which uses com.sun.mail:javax.mail. As per the release notes for SB 2.2, this dependency must be upgraded to com.sun.mail:jakarta.mail, but the recipe did not do it.

Therefore i can now confirm my earlier assumptions that at the very least the SB 2.2 recipe should include jakarta/javaxmailtojakartamail and javaxvalidationmigrationtojakartavalidation.

openrewrite / rewrite-spring