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

[Task] Replace JSoup with Jackson #300

Closed
andrewazores opened this issue Nov 20, 2023 · 2 comments · Fixed by #389 or cryostatio/cryostat#449
Closed

[Task] Replace JSoup with Jackson #300

andrewazores opened this issue Nov 20, 2023 · 2 comments · Fixed by #389 or cryostatio/cryostat#449
Assignees
Labels
chore Refactor, rename, cleanup, etc.

Comments

@andrewazores
Copy link
Member

JSoup is used for some light processing of .jfc XML documents. Unfortunately, we also expose the JSoup Document class as part of our own public API.

  1. The Document type should be abstracted away so that any library used for processing XML is an internal implementation detail of this library
  2. JSoup should be replaced by Jackson, since Jackson handles both XML and JSON and is already used in other parts of the Cryostat project as well.
@andrewazores andrewazores added the chore Refactor, rename, cleanup, etc. label Nov 20, 2023
@andrewazores andrewazores moved this to Todo in 3.0.0 release Feb 13, 2024
@Josh-Matsuoka Josh-Matsuoka self-assigned this Apr 22, 2024
@Josh-Matsuoka
Copy link
Contributor

Digging into this a bit further and experimenting with it, it looks like there are 2 potential issues we're going to run into with this

The first is that attributes are lost when deserializing then reserializing, the attributes are turned into normal tags which would break the jfc schema. [1]

The second issue is that it doesn't support cases where there is an attribute and later a tag with the same name. This occurs in the JFC templates where the setting tag has an attribute 'control' and there is a separate 'control' tag later on. [2]

[1] FasterXML/jackson-dataformat-xml#613
[2] FasterXML/jackson-dataformat-xml#555

We should still abstract away the Document type, but I think it's better to keep using jsoup for the moment.

@andrewazores
Copy link
Member Author

andrewazores commented Apr 30, 2024

Good findings, thanks. It's unfortunate but I think your conclusion that we need to keep the dependency is correct, or else we can't do any validations or transformations on .jfcs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc.
Projects
No open projects
Status: Done
2 participants