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.

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

Use a single JSON library #100

Open
aalmiray opened this issue Feb 8, 2023 · 6 comments
Open

Use a single JSON library #100

aalmiray opened this issue Feb 8, 2023 · 6 comments

Comments

@aalmiray
Copy link

aalmiray commented Feb 8, 2023

At the moment both Jackson and GSON are configured as dependencies. It would be better if there was only one dependency for processing JSON. Hopeful Jackson will be the chosen one.

in-toto-java/pom.xml

Lines 57 to 74 in 73afcb4

<!-- Jackson Dependencies -->
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.14.1</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.datatype</groupId>
<artifactId>jackson-datatype-jsr310</artifactId>
<version>2.14.1</version>
</dependency>
<!-- gson: object to json converter -->
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.10.1</version>
<scope>compile</scope>
</dependency>

@adityasaky
Copy link
Member

Hi @aalmiray, do you think you could submit a patch converging on one library? We essentially have two implementations under the hood due to a major refactor a while ago and looks like this was a side effect. Thanks!

@aalmiray
Copy link
Author

aalmiray commented Feb 8, 2023

Sure. My personal preference would be Jackson as stated before. Does that match the goals of the project?

@adityasaky
Copy link
Member

cc @SantiagoTorres @Alos

@Alos
Copy link
Collaborator

Alos commented Feb 13, 2023

When I implemented the new version of the client it was my preference to use Jackson too. However, we did not remove gson for "compatibility" reasons. We could spend some time making sure the gson generated JSON matches the one from Jackson, but I think it requires the addition of more detail unit tests (Especially for the legacy stuff)

@adityasaky
Copy link
Member

I think switching everything over to jackson is fine. @aalmiray do you want to take a stab at it and working on parts that may have some issues with the switch?

@aalmiray
Copy link
Author

@adityasaky Yes, I can work on a spike in the coming weeks. I may come back with questions 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants