-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Add optional module for Jakarta Transactions impl of Jakarta EE 9 Spec #35810
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
base: master
Are you sure you want to change the base?
Conversation
11d1fc3
to
7ec4950
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces an optional Jakarta Transactions implementation for Jakarta EE 9 to support XA in ShardingSphere, updates native test dependencies, and unifies JDBC driver version management.
- Add new
shardingsphere-transaction-xa-jakarta
modules (SPI, provider, core) alongside existing XA core. - Update
pom.xml
andtest/native/pom.xml
to bump MS SQL, Oracle JDBC versions and declare Jakarta BOM properties. - Extend tests for Atomikos/Narayana providers and core JTA integration under Jakarta spec.
Reviewed Changes
Copilot reviewed 86 out of 86 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
test/native/pom.xml | Added Jakarta EE 9 BOM properties and switched XA core exclusions. |
pom.xml | Bumped <mssql.version> , added <ojdbc8.version> and test dependency. |
kernel/transaction/type/xa-jakarta | Introduced Jakarta EE 9–based XA modules (spi, provider, core). |
kernel/transaction/type/xa-jakarta/core/src/main/java/.../DataSourceSwapper.java | Implemented data source→XADataSource property mapping. |
kernel/transaction/type/xa-jakarta/spi/src/main/java/.../SingleXAResource.java | Wrapped XAResource with exception mapping stub. |
kernel/transaction/type/xa-jakarta/provider/* | Added Atomikos and Narayana transaction manager providers/tests. |
Comments suppressed due to low confidence (1)
kernel/transaction/type/xa-jakarta/provider/atomikos/src/test/java/org/apache/shardingsphere/transaction/xa/jakarta/atomikos/manager/AtomikosTransactionManagerProviderTest.java:78
- [nitpick] The test method name
assertEnListResource
has inconsistent camelCase; consider renaming toassertEnlistResource
for readability.
void assertEnListResource() throws SystemException, RollbackException {
...g/apache/shardingsphere/transaction/xa/jakarta/jta/datasource/swapper/DataSourceSwapper.java
Outdated
Show resolved
Hide resolved
...spi/src/main/java/org/apache/shardingsphere/transaction/xa/jakarta/spi/SingleXAResource.java
Outdated
Show resolved
Hide resolved
...spi/src/main/java/org/apache/shardingsphere/transaction/xa/jakarta/spi/SingleXAResource.java
Outdated
Show resolved
Hide resolved
f849c5e
to
749c2ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Cannot use
org.eclipse.transformer:transformer-maven-plugin:0.5.0
becauseorg.eclipse.transformer:transformer-maven-plugin:0.5.0
's handling conflicts with thejar-in-test-phase
ofmaven-jar-plugin
introduced in graalvm/native-build-tools#727 .
- This PR needs further review, as starting with GraalVM Native Build Tools 0.11.0, support for running integration tests using the
maven-failsafe-plugin
is available. See https://github.com/graalvm/native-build-tools/blob/master/docs/src/docs/asciidoc/changelog.adoc .
fcff5b2
to
040af70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Cannot use
org.eclipse.transformer:transformer-maven-plugin:0.5.0
becauseorg.eclipse.transformer:transformer-maven-plugin:0.5.0
's handling conflicts with thejar-in-test-phase
ofmaven-jar-plugin
introduced in graalvm/native-build-tools#727 .
- This PR needs further review, as starting with GraalVM Native Build Tools 0.11.0, support for running integration tests using the
maven-failsafe-plugin
is available. See https://github.com/graalvm/native-build-tools/blob/master/docs/src/docs/asciidoc/changelog.adoc .
- All the prerequisite issues have been solved based on #36050 .
Fixes #26041 .
Changes proposed in this pull request:
shardingsphere-transaction-xa-jakarta
basically reuses all the definitions ofshardingsphere-transaction-xa
, but switchesjavax.transaction
java package tojakarta.transaction
java package, and uses a different set of third-party dependencies.org.apache.shardingsphere.transaction.api.TransactionType#XA
is still used, becauseorg.apache.shardingsphere:shardingsphere-transaction-xa-jakarta-core
andorg.apache.shardingsphere:shardingsphere-transaction-xa-jakarta-core
cannot be used in the same Maven module at the same time, as the two modules use different versions of the same set of third-party dependencies. Going forward,ShardingSphere Proxy
andShardingSphere Proxy Native
should be updated to use the Jakarta Transactions implementation of Jakarta EE 9 Spec. Of course I know that there are new major impacts in General upgrade to Jakarta EE 11 APIs spring-projects/spring-framework#34011 on the Spring Boot 4 milestone. But we should always take it one step at a time, right?com.oracle.database.jdbc:ojdbc8
.com.microsoft.sqlserver:mssql-jdbc:12.10.1.jre8
. For Improve GraalVM Reachability Metadata and corresponding nativeTest related unit tests #29052 .12.10.1.jre8
. I can't understand whycom.microsoft.sqlserver:mssql-jdbc:6.1.7.jre8-preview
would depend on Jakarta EE 8's JAXB API.🫠Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e
.