Skip to content

[1431] Upgrade codebase to Java 17 language features #1421

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 15, 2025
Merged

[1431] Upgrade codebase to Java 17 language features #1421

merged 4 commits into from
Sep 15, 2025
+1,017 −876

Conversation

groldan
Copy link
Member

@groldan groldan commented Jul 17, 2025 •
edited
Loading

Fixes #1431

@groldan groldan changed the title Apply the Upgrade to java17 OpenRewrite recipe Apply the "Upgrade to java 17" OpenRewrite recipe Jul 17, 2025
@groldan groldan marked this pull request as ready for review July 17, 2025 16:26
@pmauduit
Copy link

Tested after rebasing onto main, and got no error so far issuing a mvn clean verify:

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for geowebcache 1.28-SNAPSHOT:
[INFO] 
[INFO] geowebcache ........................................ SUCCESS [  6.817 s]
[INFO] gwc-core ........................................... SUCCESS [04:27 min]
[INFO] gwc-georss ......................................... SUCCESS [ 18.129 s]
[INFO] gwc-gmaps .......................................... SUCCESS [  6.727 s]
[INFO] gwc-kml ............................................ SUCCESS [  5.880 s]
[INFO] gwc-rest ........................................... SUCCESS [ 56.930 s]
[INFO] gwc-tms ............................................ SUCCESS [ 10.119 s]
[INFO] gwc-ve ............................................. SUCCESS [  6.085 s]
[INFO] gwc-wms ............................................ SUCCESS [01:57 min]
[INFO] gwc-wmts ........................................... SUCCESS [ 21.895 s]
[INFO] Disk Quota management module ....................... SUCCESS [  3.722 s]
[INFO] Disk Quota management module - core service ........ SUCCESS [ 13.673 s]
[INFO] Disk Quota management module - BerkeleyDB backend .. SUCCESS [  9.315 s]
[INFO] Disk Quota management module - JDBC backends ....... SUCCESS [ 27.763 s]
[INFO] ArcGIS Server tile cache support ................... SUCCESS [ 37.962 s]
[INFO] MBTiles cache support .............................. SUCCESS [04:10 min]
[INFO] gwc-distributed .................................... SUCCESS [ 16.269 s]
[INFO] AWS S3 Storage ..................................... SUCCESS [ 29.782 s]
[INFO] SQLite Storage ..................................... SUCCESS [01:53 min]
[INFO] Swift Storage ...................................... SUCCESS [ 23.145 s]
[INFO] Web module ......................................... SUCCESS [ 39.197 s]
[INFO] gwc-azure-blob ..................................... SUCCESS [ 11.736 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  18:17 min
[INFO] Finished at: 2025-08-27T15:23:13+02:00
[INFO] ------------------------------------------------------------------------

I can see this message when building though:

OpenJDK 64-Bit Server VM warning: Ignoring option --illegal-access=warn; support was removed in 17.0

And wondering if we could get rid of the option from the pom file:

@aaime
Copy link
Member

aaime commented Aug 27, 2025

I'd say yes, we are building the main branch with java 17 +, so it's ignored anyways no?

pmauduit
pmauduit approved these changes Aug 27, 2025
View reviewed changes
Copy link

@pmauduit pmauduit left a comment

Choose a reason for hiding this comment

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

Just one potentially useless "string".formatted() caught my eyes, apart from that LGTM.
I'd have expected more code rewrite patterns in the diff, but discovered some things I was not aware of already.

@pmauduit
Copy link

so it's ignored anyways no?

yes, it did not harm my compilation at least

@groldan groldan force-pushed the geoserver3/java17_openrewrite branch from bf649f9 to a53ecfa Compare September 14, 2025 01:16
@groldan groldan changed the title Apply the "Upgrade to java 17" OpenRewrite recipe [1431] Upgrade codebase to Java 17 language features Sep 14, 2025
@groldan groldan force-pushed the geoserver3/java17_openrewrite branch from a53ecfa to 0d7f8b7 Compare September 14, 2025 02:37
@aaime
Copy link
Member

aaime commented Sep 15, 2025

Does not compile, updating to the latest main will fix it, in particular this commit:

commit 665371576b8e134a9355f4ca49952839f897834c (HEAD -> main, gwc/main)
Author: Jody Garnett <[email protected]>
Date:   Sun Sep 14 20:38:12 2025 -0700

    Use GeoTools ImageWorker to avoid having to care about specific ImageN Operator

@groldan
Initial apply of OpenReWrite recipe
e973009 
```
mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
 -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-migrate-java:RELEASE \
 -Drewrite.activeRecipes=org.openrewrite.java.migrate.UpgradeToJava17 \
 -Drewrite.exportDatatables=true
```
@groldan
lint
8ab504d
@groldan
Remove unused dependency jakarta.annotation:jakarta.annotation-api ad…
0df6a25 
…ded by the recipe
@groldan
Fix new PMD errors introduced by the OpenRewrite recipe
5b0b35f
@groldan groldan force-pushed the geoserver3/java17_openrewrite branch from 0d7f8b7 to 5b0b35f Compare September 15, 2025 14:20
@groldan
Copy link
Member Author

groldan commented Sep 15, 2025

@aaime thanks, all working after rebasing

@aaime
Copy link
Member

aaime commented Sep 15, 2025

Had a look at the PR, everything looks fine to me, good to merge IMHO.

@groldan groldan merged commit 9854bbf into GeoWebCache:main Sep 15, 2025
9 checks passed
@groldan groldan deleted the geoserver3/java17_openrewrite branch September 15, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@pmauduit pmauduit pmauduit approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade codebase to Java 17 language features
3 participants
@groldan @pmauduit @aaime