Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MigrateHibernateConstraintsToJavax does not honor switch to Jakarta in Spring Boot 2.2 #437

Open
Philzen opened this issue Oct 2, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@Philzen
Copy link

Philzen commented Oct 2, 2023

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 😉

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.

@Philzen Philzen added the bug Something isn't working label Oct 2, 2023
@timtebeek
Copy link
Contributor

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

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.boot2.MigrateHibernateConstraintsToJavax
displayName: Use `javax.validation.constraints`
description: Use `javax.validation.constraints` instead of the deprecated `org.hibernate.validator.constraints` in Spring Boot 2.0 or higher.
recipeList:
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: org.hibernate.validator.constraints.NotBlank
newFullyQualifiedTypeName: javax.validation.constraints.NotBlank
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: org.hibernate.validator.constraints.NotEmpty
newFullyQualifiedTypeName: javax.validation.constraints.NotEmpty
- org.openrewrite.java.dependencies.AddDependency:
groupId: javax.validation
artifactId: validation-api
version: 2.x
onlyIfUsing: javax.validation.constraints.*

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
Copy link
Author

Philzen commented Oct 22, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

2 participants