Skip to content
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

Upgrade json-smart from 2.4.2 to 2.4.7 #1299

Merged
merged 9 commits into from
Jun 28, 2021
Merged

Upgrade json-smart from 2.4.2 to 2.4.7 #1299

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f2f7174
Upgrade json-smart from 2.4.2 to 2.4.5
cliu123 Jun 24, 2021
510c7a5
Keep accessors-smart-2.4.2
cliu123 Jun 24, 2021
56e7210
Revert "Keep accessors-smart-2.4.2"
cliu123 Jun 26, 2021
4c16daf
Revert "Upgrade json-smart from 2.4.2 to 2.4.5"
cliu123 Jun 26, 2021
9ba5d57
Exclude json-smart dependency
cliu123 Jun 26, 2021
667725e
Replace json-smart with Jackson
cliu123 Jun 28, 2021
db9bd1a
Revert "Replace json-smart with Jackson"
cliu123 Jun 28, 2021
52ce88c
Revert "Exclude json-smart dependency"
cliu123 Jun 28, 2021
d83faa6
Upgrade json-smart to 2.4.7
cliu123 Jun 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@
<dependency>
<groupId>net.minidev</groupId>
<artifactId>json-smart</artifactId>
<version>2.4.5</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not to upgrade json-smart to 2.4.7 that is more widely used compared to 2.4.5?

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, it will be better to remove the dependency on json-smart completely.

Copy link
Member Author

@cliu123 cliu123 Jun 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Additionally, it will be better to remove the dependency on json-smart completely.
Tried replacing json-smart with Jackson, but Jackson automatically wraps the values in Json with quotes, which fails the tests currently relying on json-smar. The following is the failure:

org.junit.ComparisonFailure: 
Expected :{"http_certificates_list":"[{issuer_dn=CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com, subject_dn=CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE, san=[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]], not_before=2020-02-17T16:19:25Z, not_after=2022-02-16T16:19:25Z}]","transport_certificates_list":"[{issuer_dn=CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com, subject_dn=CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE, san=[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]], not_before=2020-02-17T16:19:25Z, not_after=2022-02-16T16:19:25Z}]"}
Actual   :{"http_certificates_list":[{"issuer_dn":"CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com","subject_dn":"CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE","san":"[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]]","not_before":"2020-02-17T16:19:25Z","not_after":"2022-02-16T16:19:25Z"}],"transport_certificates_list":[{"issuer_dn":"CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com","subject_dn":"CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE","san":"[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]]","not_before":"2020-02-17T16:19:25Z","not_after":"2022-02-16T16:19:25Z"}]}

Probably Jackson is not the answer. Will need explore more options.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cliu123 I don't see why jackson is not an option. The expected is not properly constructed.

</dependency>
<dependency>
<groupId>net.minidev</groupId>
<artifactId>accessors-smart</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this is necessary?

Copy link
Member Author

@cliu123 cliu123 Jun 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

json-smart-2.4.5 introduces accessors-smart-2.4.3 that is not available in Maven Central. Had to have accessors-smart remain 2.4.2.

Failed to execute goal on project opensearch-security: Could not resolve dependencies for project org.opensearch:opensearch-security:jar:1.0.0.0-rc1: Could not find artifact net.minidev:accessors-smart:jar:2.4.3 in central (https://repo.maven.apache.org/maven2) -> [Help 1]

Failing CI: https://github.com/opensearch-project/security/runs/2906736355?check_suite_focus=true

Copy link
Contributor

Choose a reason for hiding this comment

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

https://mvnrepository.com/artifact/net.minidev/accessors-smart/2.4.3

Copy link
Member Author

Choose a reason for hiding this comment

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

accessors-smart-2.4.3.jar is missing at here. But accessors-smart-2.4.2.jar is available.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created an issue: netplex/json-smart-v2#76

<version>2.4.2</version>
</dependency>
</dependencies>
Expand Down