Support Jersey 3 and Jakarta EE 9/10 #2919
Conversation
…arta10 to support Jersey 3 and Jakarta EE 9 and 10 respectively * feat: Add module servicetalk-data-protobuf-jersey3-jakarta9 and -jakarta10 to support Jersey 3 and Jakarta EE 9 and 10 respectively * feat: Add module servicetalk-http-router-jersey3-jakarta9 and -jakarta10 to support Jersey 3 and Jakarta EE 9 and 10 respectively * feat: Add module servicetalk-http-router-jersey3-internal-jakarta9 and -jakarta10 to support Jersey 3 and Jakarta EE 9 and 10 respectively * feat: Add module servicetalk-http-security-jersey3-jakarta9 and -jakarta10 to support Jersey 3 and Jakarta EE 9 and 10 respectively * feat: Update build.gradle in new modules to pull source from the original -jersey modules and change the imports * chore: Add Jersey and Jakarta EE dependency versions for both 9 and 10 * doc: Update related docs to describe how to use Jersey 3.0 and 3.1
idelpivnitskiy
requested review from
chrisvest,
bryce-anderson and
idelpivnitskiy
| // Dependencies must remain consistent between servicetalk-data-jackson-jerseyX modules | ||
| dependencies { | ||
| api platform(project(":servicetalk-dependencies")) | ||
| api platform("org.glassfish.jersey:jersey-bom:${actualJerseyVersion}") |
There was a problem hiding this comment.
Note for other reviewers: we are at the limit of our gradle-fu. Please focus on correctness, documentation, and build behavior. Optimization of the build scripts and removal of the duplication might be a good task for the follow-up, but not a goal of this PR.
|
@blake-bauman please rebase on top of the latest |
|
I think it would also be good to have two additional test modules (for 9 and 10, respectively) for smoke-testing that if you depend on the right combination of ServiceTalk modules and Jakarta EE 9 or 10, then you can actually make a server and a client and send a request through. |
|
@chrisvest I believe this is already tested bcz the script copies the whole |
|
I thought I had called that out in both the PR description and commit already:
|
|
@chrisvest confirm, I see tests running if I do |
| task copySourcesForJersey3(type: Copy) { | ||
| dependsOn tasks.cleanSources | ||
| from '../servicetalk-data-jackson-jersey/src' | ||
| into 'src' |
There was a problem hiding this comment.
It seems like these should go into the build/generated folder somewhere and we can avoid having all the noise with .gitignore and it's more explicit what they are.
That said, I'm strugging to figure out how to do that myself. Right now I have something like this but am not able to get the tests to depend on the fixtures. The changes below are by no means complete but illustrate what I was thinking.
diff --git a/servicetalk-http-router-jersey3-jakarta10/build.gradle b/servicetalk-http-router-jersey3-jakarta10/build.gradle
index 72aa3bf11..674a9c791 100644
--- a/servicetalk-http-router-jersey3-jakarta10/build.gradle
+++ b/servicetalk-http-router-jersey3-jakarta10/build.gradle
@@ -24,24 +24,14 @@ checkstyleTest {
enabled false
}
-checkstyleTestFixtures {
- enabled false
-}
-
spotbugsMain {
enabled false
}
-task cleanSources(type: Delete) {
- // Don't delete src/testFixtures/java/.gitkeep
- // gitkeep used to ensure that a src/textFixtures/java exists or Gradle processing will fail
- delete 'src/main', 'src/test', 'src/testFixtures/java/io'
-}
-
task copySourcesForJersey3(type: Copy) {
- dependsOn tasks.cleanSources
+// dependsOn tasks.cleanSources
from '../servicetalk-http-router-jersey/src'
- into 'src'
+ into "${getBuildDir()}/generated/sources/"
filter { line -> line.replaceAll('javax.ws.rs', 'jakarta.ws.rs') }
filter { line -> line.replaceAll('javax.inject', 'jakarta.inject') }
@@ -49,11 +39,31 @@ task copySourcesForJersey3(type: Copy) {
filter { line -> line.replaceAll('javax.annotation.Priority', 'jakarta.annotation.Priority') }
}
+
+
tasks.processResources.dependsOn(copySourcesForJersey3)
tasks.sourcesJar.dependsOn(copySourcesForJersey3)
tasks.compileJava.dependsOn(copySourcesForJersey3)
-tasks.processTestResources.dependsOn(copySourcesForJersey3)
-tasks.compileTestJava.dependsOn(copySourcesForJersey3)
+
+sourceSets {
+ main {
+ java {
+ srcDir "${getBuildDir()}/generated/sources/main"
+ }
+ }
+
+ testFixtures {
+ java {
+ srcDir "${getBuildDir()}/generated/sources/testFixtures"
+ }
+ }
+
+ test {
+ java {
+ srcDir "${getBuildDir()}/generated/sources/test"
+ }
+ }
+}
def actualJerseyVersion = "${jersey3VersionEE10}"
def actualJavaxActivationVersion = "${javaxActivationVersionEE10}"
diff --git a/servicetalk-http-router-jersey3-jakarta10/src/testFixtures/java/.gitkeep b/servicetalk-http-router-jersey3-jakarta10/src/testFixtures/java/.gitkeep
deleted file mode 100644
index e69de29bb..000000000
There was a problem hiding this comment.
As noted here (#2919 (comment)), let's consider any optimizations for gradle files for a follow-up. Otherwise, we will miss the upcoming release.
There was a problem hiding this comment.
It seems like it would fall into the 'build behavior' category but I don't have a problem punting that to a followup. 👍
|
Build failures: Even though Jersey 3.0 branch requires only JDK8 (https://github.com/eclipse-ee4j/jersey/blob/bf9e4d367c9e18ce0c29b5e5df70d73e5d590802/pom.xml#L2112), their transitive dependency hk2 decided to bump JDK requirements in a bug fix release from 3.0.4 to 3.0.5: https://github.com/eclipse-ee4j/glassfish-hk2/blob/3.0.5-RELEASE/pom.xml#L778 So, we will require JDK11 for all |
There was a problem hiding this comment.
All green now 🎉
Do you see anything else @bryce-anderson @chrisvest?
|
Many thanks @blake-bauman, |
Motivation
Jersey, in its latest releases has adopted both Jakarta EE 9 and 10 which makes a break from the legacy JEE API. This unfortunately makes any Jersey 3 application incompatible with any Jersey 2 application. Widely-used frameworks like Spring have also adopted Jakarta EE 10.
In order for ServiceTalk to support those applications, it needs to offer Jersey 3-compatible modules that ServiceTalk applications can consume.
Modifications
Result
Applications can use the relevant new Jersey 3 modules to be compatible with Jakarta EE 9 and 10.