Fix Spring Data Cosmos not work JDK 16 #31417

saragluna · 2022-10-12T08:57:17Z

Description

Due to JEP 396: Strongly Encapsulate JDK Internals by Default, now if MappingContext try to reflect the UUID, java.time or java.lang classes, it will throw InaccessibleObjectException.

This PR will add a CosmsoSimpleTypes to mark UUID as a a simple type, which will skip the reflection.

@kushagraThapar made some changes to the ci/live pipeline, the Java versions with ci/live would be according to him:

spring data cosmos ->
  emulator CI -> spring -> 1.8 and 1.17
  live CI -> spring -> 1.8, 1.11 and 1.17
azure-cosmos ->
  emulator CI -> 1.8 and 1.17
  live CI -> 1.8, 1.11 and 1.17
unit tests -> 1.8, 1.11 and 1.17

All SDK Contribution checklist:

The pull request does not introduce [breaking changes]
CHANGELOG is updated for new features, bug fixes or other significant changes.
I have read the contribution guidelines.

General Guidelines and Best Practices

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

saragluna · 2022-10-12T09:01:35Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-12T09:02:04Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-12T09:17:34Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-12T09:17:46Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-12T09:58:21Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-12T09:58:35Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-12T14:20:19Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-12T14:20:32Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-12T14:49:02Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-12T14:49:18Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-12T15:23:34Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-12T15:23:50Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-12T15:30:11Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-12T15:30:20Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-13T01:43:11Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-13T01:43:23Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2022-10-13T02:14:09Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-13T02:14:22Z

Azure Pipelines successfully started running 1 pipeline(s).

kushagraThapar · 2022-10-18T21:45:52Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-18T21:46:06Z

Azure Pipelines successfully started running 1 pipeline(s).

kushagraThapar · 2022-10-18T23:53:19Z

sdk/cosmos/platform-matrix.json

@@ -33,8 +33,7 @@
 "ProfileFlag": "-Pe2e",
 "Agent": {
 "ubuntu": { "OSVmImage": "MMSUbuntu20.04", "Pool": "azsdk-pool-mms-ubuntu-2004-general" }
- },
- "JavaTestVersion": "1.11"
+ }


@alzimmermsft , I am wondering what other azure SDKs are testing against?
@saragluna - while supporting java version 16 or 17 is good to have, but we should still aim to test our cosmos Java SDKs with at least 8 and 11. (since most of our customers are on version 8 and 11).

The ci pipeline will run against Java 8 and Java 11, the live tests pipeline will use Java 17. I found this https://github.com/Azure/azure-sdk-for-java/blob/main/eng/pipelines/templates/stages/platform-matrix.json, but @alzimmermsft could help confirm it.

The problem is whether we should run the live tests against JDK8, JDK 11, and JDK 17.

Got it, thanks for the information @saragluna , let's confirm with @alzimmermsft
Also, I am seeing some issue on spring live tests - https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1927553&view=logs&j=caa02ea9-2389-5ab0-dd04-e2bf1c48642e&t=2c713f32-d936-5c8d-942a-fcedc5ff7533&l=43
will investigate it tomorrow and will hopefully unblock this PR and prepare for merge, thanks for the patience!

Also, looks like there are more errors related to some java modules I think - https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1927553&view=ms.vss-test-web.build-test-results-tab&runId=36095886&paneView=debug&resultId=100160
@saragluna can you please do a sanity check and see if these are also related to JDK 17 ? (Since these are in live CI tests).
also wondering, if we really need to update live CI for azure-cosmos java SDK? May be we should just limit the live CI to JDK 17 to just spring data cosmos if we need it for spring boot 3 support, thoughts?

I think the error is related to JDK 17 since it's using the reflection. Since spring-data-cosmos depends on azure-cosmos, so for spring-data-cosmos to support JDK 17, we should ensure that azure-cosmos supports it.

@kushagraThapar, @saragluna, a majority of the CI pipelines are primarily testing against Java 8 and Java 17 with some testing against Java 11, the live test pipelines are more of a full mix where Java 8, 11, and 17 are represented fairly equally.

Thanks @alzimmermsft , can you please help us with the appropriate Java version testing? Please feel free to make edits / updates to this PR to allow supporting and testing Java 8, 11 and 17.

Gladly, the current base matrix for CI for all com.azure SDKs is

Each OS tests against Java 8

One Linux run against Java 11 as a sanity check

Each OS tests against Java 17

Some of the runs are slightly specialized but that's the rough breakdown.

OS Java 8 Java 11 Java 17

Linux x x x

macOS x x

Windows x x

Java 11 may be reintroduced as an all OS run again in the future if needed.

kushagraThapar · 2022-10-19T23:06:25Z

@alzimmermsft I made some changes to the CI pipeline, can you please help review them?

benbp · 2022-10-20T01:06:15Z

This is great, I'm really happy to see the replace statements get removed.

kushagraThapar · 2022-10-20T01:09:15Z

This is great, I'm really happy to see the replace statements get removed.

Thanks @benbp , please review the PR from eng point of view.
I have made some changes on the cosmos live CIs as well, would like to run at least one live CI with 17 version and rest with 11 or 8.

alzimmermsft

The changes to Java versions ran look good to me

kushagraThapar · 2022-10-20T06:27:31Z

@benbp / @alzimmermsft looks like the lib/security folder is different on java 8 vs java 17. And hence the step that adds emulator certificate is failing.

This is the script - which has a fixed path for security folder inside java home ->

- powershell: |
              cd $env:java_home\jre\lib\security
              cp $env:temp\CosmosDbEmulatorCert.cer .
              keytool -keystore cacerts -importcert -noprompt -trustcacerts -alias CosmosDbEmulatorCert -file CosmosDbEmulatorCert.cer -storepass changeit
            displayName: 'Create Java TrustStore'

for cosmos-sdk-client.yml file. Any ideas on how to fix it? May be we can pass in the java version and check the security folder path based on that?

kushagraThapar · 2022-10-20T06:27:39Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-20T06:27:52Z

Azure Pipelines successfully started running 1 pipeline(s).

alzimmermsft · 2022-10-20T14:46:13Z

@benbp / @alzimmermsft looks like the lib/security folder is different on java 8 vs java 17. And hence the step that adds emulator certificate is failing.

This is the script - which has a fixed path for security folder inside java home ->
- powershell: |
              cd $env:java_home\jre\lib\security
              cp $env:temp\CosmosDbEmulatorCert.cer .
              keytool -keystore cacerts -importcert -noprompt -trustcacerts -alias CosmosDbEmulatorCert -file CosmosDbEmulatorCert.cer -storepass changeit
            displayName: 'Create Java TrustStore'
for cosmos-sdk-client.yml file. Any ideas on how to fix it? May be we can pass in the java version and check the security folder path based on that?

Yeah, this changed somewhere between Java 8 and Java 11. The way I would go about this is using:

if (Test-Path $env:JAVA_HOME\jre\lib\security) {
  cd $env:JAVA_HOME\jre\lib\security
} elseif (Test-Path $env:JAVA_HOME\security) {
  cd $env:JAVA_HOME\security
} else {
  Write-Error "JDK directory structure is unknown and unsupported. JAVA_HOME is '$env:JAVA_HOME'"
  exit 1
}

Test if the Java 8 path exists and use it, else test the Java 11+ path exists and use it, otherwise write a nice error of what went wrong.

Edit

It was brought to my attention that I forgot /lib in the check for Java 11+, it should be

elseif (Test-Path $eng:JAVA_HOME\lib\security) {
  cd $env:JAVA_HOME\lib\security
}

Edit by Kushagra Thapar

On offline debugging, we found out the typo, the below script refers to the correct one :)

- powershell: |
              if (Test-Path $env:JAVA_HOME\jre\lib\security) {
                cd $env:JAVA_HOME\jre\lib\security
              } elseif (Test-Path $env:JAVA_HOME\lib\security) {
                cd $env:JAVA_HOME\lib\security
              } else {
                Write-Error "JDK directory structure is unknown and unsupported. JAVA_HOME is '$env:JAVA_HOME'"
                exit 1
              }
              cp $env:temp\CosmosDbEmulatorCert.cer .
              keytool -keystore cacerts -importcert -noprompt -trustcacerts -alias CosmosDbEmulatorCert -file CosmosDbEmulatorCert.cer -storepass changeit
            displayName: 'Create Java TrustStore'

benbp · 2022-10-20T17:37:39Z

sdk/cosmos/integration-matrix.json

 {
 "matrix": {
 "Cosmos": {
- "Session_Integration": {
+ "Session_Integration_8": {
+ "ArmTemplateParameters": "@{ enableMultipleWriteLocations = $false; defaultConsistencyLevel = 'Session' }",
+ "ProfileFlag": "-P integration-test-azure",
+ "Pool": "azsdk-pool-mms-ubuntu-2004-general",
+ "OSVmImage": "MMSUbuntu2004",
+ "JavaTestVersion": "1.8"
+ },
+ "Session_Integration_11": {
 "ArmTemplateParameters": "@{ enableMultipleWriteLocations = $false; defaultConsistencyLevel = 'Session' }",
 "ProfileFlag": "-P integration-test-azure",
 "Pool": "azsdk-pool-mms-ubuntu-2004-general",
 "OSVmImage": "MMSUbuntu2004",
 "JavaTestVersion": "1.11"
+ },
+ "Session_Integration_17": {
+ "ArmTemplateParameters": "@{ enableMultipleWriteLocations = $false; defaultConsistencyLevel = 'Session' }",
+ "ProfileFlag": "-P integration-test-azure",
+ "Pool": "azsdk-pool-mms-ubuntu-2004-general",
+ "OSVmImage": "MMSUbuntu2004",
+ "JavaTestVersion": "1.17"
 }
 }
 }


This matrix will be easier to read and maintain if you de-duplicate the config entries:

{ "matrix": { "cosmos": { "Session_Integration": { "ArmTemplateParameters": "@{ enableMultipleWriteLocations = $false; defaultConsistencyLevel = 'Session' }", "ProfileFlag": "-P integration-test-azure", "Pool": "azsdk-pool-mms-ubuntu-2004-general", "OSVmImage": "MMSUbuntu2004" } }, "JavaTestVersion": ["1.8", "1.11", "1.17"] } }

benbp · 2022-10-20T17:38:58Z

One comment on optimizing the integration matrix, otherwise configs look good.

kushagraThapar · 2022-10-20T22:13:35Z

/azp run java - cosmos - tests

azure-pipelines · 2022-10-20T22:13:50Z

Azure Pipelines successfully started running 1 pipeline(s).

kushagraThapar

Thanks @saragluna for the fix.

kushagraThapar · 2022-10-21T04:40:08Z

/check-enforcer override

saragluna added 2 commits October 12, 2022 16:41

add simple type holder for UUID

cd0e63f

fix repository type

791c032

ghost added Cosmos azure-spring All azure-spring related issues labels Oct 12, 2022

saragluna force-pushed the xiada/fix-spring-cosmos-jdk-16 branch from cd92028 to be6141a Compare October 12, 2022 08:59

reenable JDK 17

953dfaa

saragluna force-pushed the xiada/fix-spring-cosmos-jdk-16 branch from be6141a to 953dfaa Compare October 12, 2022 09:15

saragluna force-pushed the xiada/fix-spring-cosmos-jdk-16 branch from 69796fc to bd4b2a2 Compare October 12, 2022 14:18

saragluna force-pushed the xiada/fix-spring-cosmos-jdk-16 branch from bd4b2a2 to 8045613 Compare October 12, 2022 14:48

saragluna force-pushed the xiada/fix-spring-cosmos-jdk-16 branch from 8045613 to 32c3a4c Compare October 12, 2022 15:23

saragluna force-pushed the xiada/fix-spring-cosmos-jdk-16 branch from 32c3a4c to dd3bbd9 Compare October 13, 2022 01:38

saragluna force-pushed the xiada/fix-spring-cosmos-jdk-16 branch from dd3bbd9 to 06db49f Compare October 13, 2022 02:09

kushagraThapar requested changes Oct 18, 2022

View reviewed changes

Testing with CI and fixed live tests for JDK 16

a3c63db

kushagraThapar requested review from weshaggard and benbp as code owners October 19, 2022 22:59

Fixed emulator matrix file

d3a2619

alzimmermsft approved these changes Oct 20, 2022

View reviewed changes

kushagraThapar added 3 commits October 20, 2022 09:28

Fixed emulator pipelines with java version checks

a0fa14d

Reverted eng common changes

25b1392

Fixed lib / security path

132e20a

benbp reviewed Oct 20, 2022

View reviewed changes

Fixed typos

33cc729

kushagraThapar approved these changes Oct 21, 2022

View reviewed changes

kushagraThapar enabled auto-merge (squash) October 21, 2022 04:39

kushagraThapar merged commit 6333ace into Azure:main Oct 21, 2022

saragluna deleted the xiada/fix-spring-cosmos-jdk-16 branch October 21, 2022 10:15

kushagraThapar mentioned this pull request Oct 21, 2022

Updated release versions to 4.38.1 for azure-cosmos and connectors #31677

Merged

Xiphoseer mentioned this pull request May 17, 2024

[azure-spring-data-cosmos] Turn java.math.Big{Decimal,Integer} into simple types #40239

Open

6 tasks

Fix Spring Data Cosmos not work JDK 16 #31417

Fix Spring Data Cosmos not work JDK 16 #31417

Conversation

saragluna commented Oct 12, 2022 • edited Loading

Description

All SDK Contribution checklist:

saragluna commented Oct 12, 2022

azure-pipelines bot commented Oct 12, 2022

saragluna commented Oct 12, 2022

azure-pipelines bot commented Oct 12, 2022

saragluna commented Oct 12, 2022

azure-pipelines bot commented Oct 12, 2022

saragluna commented Oct 12, 2022

azure-pipelines bot commented Oct 12, 2022

saragluna commented Oct 12, 2022

azure-pipelines bot commented Oct 12, 2022

saragluna commented Oct 12, 2022

azure-pipelines bot commented Oct 12, 2022

saragluna commented Oct 12, 2022

azure-pipelines bot commented Oct 12, 2022

saragluna commented Oct 13, 2022

azure-pipelines bot commented Oct 13, 2022

saragluna commented Oct 13, 2022

azure-pipelines bot commented Oct 13, 2022

kushagraThapar commented Oct 18, 2022

azure-pipelines bot commented Oct 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alzimmermsft Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

kushagraThapar commented Oct 19, 2022

benbp commented Oct 20, 2022

kushagraThapar commented Oct 20, 2022

alzimmermsft left a comment

Choose a reason for hiding this comment

kushagraThapar commented Oct 20, 2022

kushagraThapar commented Oct 20, 2022

azure-pipelines bot commented Oct 20, 2022

alzimmermsft commented Oct 20, 2022 • edited by kushagraThapar Loading

Choose a reason for hiding this comment

benbp commented Oct 20, 2022

kushagraThapar commented Oct 20, 2022

azure-pipelines bot commented Oct 20, 2022

kushagraThapar left a comment

Choose a reason for hiding this comment

kushagraThapar commented Oct 21, 2022

saragluna commented Oct 12, 2022 •

edited

Loading

alzimmermsft Oct 19, 2022 •

edited

Loading

alzimmermsft commented Oct 20, 2022 •

edited by kushagraThapar

Loading