Skip to content

Upgrade existing jakarta.annotation-api to 2.0.x for EE9#729

Merged
timtebeek merged 2 commits into
openrewrite:mainfrom
evie-lau:jakartaAnnotationsUpgrade
May 19, 2025
Merged

Upgrade existing jakarta.annotation-api to 2.0.x for EE9#729
timtebeek merged 2 commits into
openrewrite:mainfrom
evie-lau:jakartaAnnotationsUpgrade

Conversation

@evie-lau

@evie-lau evie-lau commented May 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What's changed?

In the Jakarta EE 9 recipe list, add an extra recipe step to ensure jakarta.annotation-api is upgraded to the correct version for EE9 in case an older version was used.

What's your motivation?

QUESTION: Is it even a valid case to run AddCommonAnnotationsDependencies first, before running JavaxMigrationToJakarta (Jakarta EE 9 migration)? Should the EE 9 migration supercede the former recipe?

If AddCommonAnnotationsDependencies is run beforehand, it updates the original javax.annotation-api to use jakarta.annotation-api:1.3.x. The EE9 recipe then fails to upgrade to 2.0.x because the original javax.annotation-api no longer exists.

When this recipe runs afterwards, it should update the version if the jakarta version of the artifact already existed.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite May 16, 2025
github-actions[bot]
github-actions Bot reviewed May 16, 2025
View reviewed changes

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions could not be made:

  • src/main/resources/META-INF/rewrite/examples.yml
    • lines 3282-3281
    • lines 3328-3346

@evie-lau evie-lau marked this pull request as draft May 16, 2025 20:01
@evie-lau

evie-lau commented May 16, 2025

Copy link
Copy Markdown
Contributor Author

I noticed this fails this test

rewrite-migrate-java/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxToJakartaTest.java

Lines 577 to 613 in 5adb91d

void doNothingIfNotFoundTransitiveDependency() {
rewriteRun(
spec -> spec.parser(JavaParser.fromJavaVersion().dependsOn(javaxServlet)),
mavenProject(
"Sample",
//language=java
srcMainJava(
java(
"""
import jakarta.servlet.A;
public class TestApplication {
}
"""
)
),
//language=xml
pomXml(
"""
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.sample</groupId>
<artifactId>sample</artifactId>
<version>1.0.0</version>
<dependencies>
<dependency>
<groupId>jakarta.annotation</groupId>
<artifactId>jakarta.annotation-api</artifactId>
<version>1.3.5</version>
</dependency>
</dependencies>
</project>
"""
)
)
);
}

I'm trying to figure out if this change makes sense, or whether the tests need to be updated.

@timtebeek timtebeek added the enhancement New feature or request label May 17, 2025
@timtebeek timtebeek marked this pull request as ready for review May 17, 2025 13:43
github-actions[bot]
github-actions Bot reviewed May 17, 2025
View reviewed changes

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions could not be made:

  • src/main/resources/META-INF/rewrite/examples.yml
    • lines 997-1007
    • lines 1036-1042

@timtebeek

timtebeek commented May 17, 2025

Copy link
Copy Markdown
Member

Thanks for the call out; fine to change that dependency version; I think it might just have been missed.

As for your other question: not sure; I'd expect folks to run Java8toJava11, and through that AddCommonAnnotationsDependencies before they run JavaxMigrationToJakarta, but it's hard to say at times.

You're right to point out that the scanning onlyIfUsing sees the "old" state before any recipes are applied, and that should be taken into account when composing recipes and their relative run order. If you think we should make any adjustments let us know.

Fine to merge this PR already from my side, but wasn't sure if there's more you'd like to work through before a merge.

@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite May 17, 2025
@evie-lau evie-lau mentioned this pull request May 17, 2025
Closed
@evie-lau

evie-lau commented May 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I think this might be good to go. It's also now in line with what pretty much all of the other EE9 recipes do, where they all run ChangeDependency followed by UpgradeDependencyVersion to the same version specified.

@timtebeek timtebeek merged commit f4603f7 into openrewrite:main May 19, 2025
1 of 2 checks passed
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite May 19, 2025
@evie-lau evie-lau deleted the jakartaAnnotationsUpgrade branch May 19, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@timtebeek timtebeek timtebeek approved these changes

Assignees

@evie-lau evie-lau

Labels

enhancement New feature or request

Projects

OpenRewrite
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@evie-lau @timtebeek