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

Rewrite code of CitaviImporter to avoid JAXBContext #9539

Open
koppor opened this issue Jan 6, 2023 · 7 comments
Open

Rewrite code of CitaviImporter to avoid JAXBContext #9539

koppor opened this issue Jan 6, 2023 · 7 comments
Labels
good first issue An issue intended for project-newcomers. Varies in difficulty. import type: code-quality Issues related to code or architecture decisions
Milestone

Comments

@koppor
Copy link
Member

koppor commented Jan 6, 2023

The code of org.jabref.logic.importer.fileformat.CitaviXmlImporter is written OK, but relies on JAXB. It should be rewritten using StAX-Parser and thus getting rid of a JAXB dependency.

The code reads like we do not hit polymorphism in XMLElements (which is not supported by JAXB. See FasterXML/jackson-modules-base#127).

See /src/main/java/org/jabref/logic/importer/fileformat/MedlineImporter.java#L52 for some code hints.

Similar to #9673, the gradle build target generateEndnoteSource can be removed and the xsd file be removed from the source repository.

Negleted Implementation Option

Thus, it seems to be "just" to include the dependency to jackson module jakarta-xmlbind and to use Jackson's XMLMapper. Code examples are at https://stackify.com/java-xml-jackson/.

@koppor koppor added type: code-quality Issues related to code or architecture decisions import labels Jan 6, 2023
@koppor koppor changed the title Rewrite code of CitaviImporter to use Jackson Rewrite code of CitaviImporter to avoid JAXBContext Mar 18, 2023
@Siedlerchr Siedlerchr added the good first issue An issue intended for project-newcomers. Varies in difficulty. label May 18, 2023
@koppor koppor added this to the 5.11 milestone Sep 2, 2023
@koppor
Copy link
Member Author

koppor commented Sep 2, 2023

We should really work on that to get rid off buildSrc

@koppor
Copy link
Member Author

koppor commented Sep 2, 2023

Similar code refactoring: #9880

@ThiloteE ThiloteE added good first issue An issue intended for project-newcomers. Varies in difficulty. and removed good first issue An issue intended for project-newcomers. Varies in difficulty. labels Oct 9, 2023
@JoleneSun111
Copy link

Hello, I'm a new contributor, and I will carefully read the contribution guidelines. Could you please assign this issue to me? @koppor

@koppor koppor modified the milestones: 5.11, v6.0 Oct 12, 2023
@koppor
Copy link
Member Author

koppor commented Oct 13, 2023

@JoleneSun111 If you want, you can also try https://github.com/FasterXML/StaxMate. However, that piece of library is not well documented. I would assume that basic Java Stax transformation is enough to solve the issue. - It is not straight-forward, but IMHO manageable nevertheless. The code is backed by tests, so there should be check if everything goes smoothely.

I remember we had the discussions that there have to build local data structures for the lookup of IDs.

@ShailikaS
Copy link

Hi @koppor, I would like to work on this if this issue is still open?

@ThiloteE
Copy link
Member

Since there has not been any activity here since the abandoned pull-request from AvunArasi (#9928), I will assign you @ShailikaS.

@ThiloteE ThiloteE assigned ShailikaS and unassigned JoleneSun111 Dec 28, 2023
@koppor
Copy link
Member Author

koppor commented Jan 22, 2024

@ShailikaS you have been assigned. Are you still working on this? -- It could be a bit hard to implement...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue An issue intended for project-newcomers. Varies in difficulty. import type: code-quality Issues related to code or architecture decisions
Projects
Status: Free to take
Status: Free to take
Development

Successfully merging a pull request may close this issue.

5 participants