Skip to content

Insert jabref-entrytype into meta.xml#16210

Merged
subhramit merged 8 commits into
JabRef:mainfrom
pluto-han:Add-metadata
Jul 9, 2026
Merged

Insert jabref-entrytype into meta.xml#16210
subhramit merged 8 commits into
JabRef:mainfrom
pluto-han:Add-metadata

Conversation

@pluto-han

Copy link
Copy Markdown
Collaborator

Related issues and pull requests

Closes #16209

PR Description

This PR inserts jabref-entrytype information into LibreOffice document's meta.xml.

Example:

{
  "schemaVersion": 1,
  "citationTypeMap": {
    "Keen2020": {
      "jabrefEntryType": "article"
    },
    "Zotero-123": {
      "jabrefEntryType": "inproceedings"
    }
  }
}

Two scenarios will trigger this logic:

  1. Inserting citations from JabRef
  2. First time reading a reference from Zotero

Steps to test

  1. Create a test.odt in LibreOffice, cites some references from Zotero and save
image
  1. Rename .odt to .zip, unzip and open meta.xml

  2. Look for keyword meta:user-defined, notice that there are only Zotero side records

image
  1. Open JabRef, connect the same document, repeat 2 again

  2. Notice that there are JabRef side records

image

AI usage

I use codex 5.5 to help me create test and parsing/inserting metadata logic. All AI-generated code was reviewed by myself.


Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • I manually tested my changes in running JabRef (always required)
  • I added JUnit tests for changes (if applicable)
  • I added screenshots in the PR description (if change is visible to the user)
  • [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

AI usage

I use codex 5.5 to help me create test and parsing/inserting metadata logic. All AI-generated code was reviewed by myself.


Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • I manually tested my changes in running JabRef (always required)
  • I added JUnit tests for changes (if applicable)
  • I added screenshots in the PR description (if change is visible to the user)
  • [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Store JabRef entry types in LibreOffice meta.xml (jabref-entrytype)

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Persist citation key → JabRef entry type mapping in LibreOffice document custom properties.
• Populate metadata when connecting to a document and when inserting/updating citations.
• Add unit tests for metadata merging and Zotero entry-type parsing behavior.
Diagram

graph TD
  OOBibBase["OOBibBase connect"] --> Doc{{"UNO XTextDocument"}} --> Metadata["CitationEntryTypeMetadata"] --> UserProp["UnoUserDefinedProperty"] --> MetaXml[("meta.xml custom props")]
  CSLC["CSLCitationOOAdapter"] --> Doc
  ZoteroMarks["Zotero ref marks"] --> Metadata
  subgraph Legend
    direction LR
    _mod["Module/Class"] ~~~ _api{{"UNO API"}} ~~~ _store[("Document storage")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. One custom property per citation key
  • ➕ Avoids maintaining a JSON schema/versioning layer
  • ➕ Can update a single key without rewriting a large blob
  • ➖ Can create many document properties and clutter LibreOffice custom properties UI
  • ➖ Harder to enumerate/manage and potentially slower over UNO calls
2. Store entry type inside each reference mark payload
  • ➕ Keeps metadata co-located with the citation mark
  • ➕ No document-level shared state to merge
  • ➖ Requires rewriting existing marks to backfill types
  • ➖ More invasive changes to mark format and parsing; higher compatibility risk
3. Rely solely on CSL type mapping (no persistence)
  • ➕ No extra metadata written to documents
  • ➕ Simpler runtime behavior
  • ➖ Doesn’t solve the cross-tool consistency problem (JabRef vs Zotero)
  • ➖ Cannot preserve user-corrected/overridden types across sessions

Recommendation: The chosen approach (a single versioned JSON blob in a document-level user-defined property) is a good tradeoff: it’s non-invasive to existing marks, easy to merge/update, and stays compatible with LibreOffice’s standard metadata mechanism. If this metadata could grow significantly, consider adding a lightweight pruning strategy (e.g., only cited keys) or size safeguards in a follow-up.

Files changed (4) +284 / -1

Enhancement (3) +168 / -1
OOBibBase.javaCapture Zotero entry types on document connect +14/-1

Capture Zotero entry types on document connect

• On successful document selection/connection, the code now triggers storage of Zotero-derived entry types into document metadata. Exceptions from UNO property access are caught and logged to avoid breaking the connect flow.

jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBase.java

CitationEntryTypeMetadata.javaAdd JSON metadata helper for jabref-entrytype custom property +151/-0

Add JSON metadata helper for jabref-entrytype custom property

• Introduces a utility that reads/writes a versioned JSON structure under the document user-defined property "jabref-entrytype". It merges JabRef entry types by citation key and can parse Zotero reference marks, optionally overriding parsed types using already-stored metadata.

jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java

CSLCitationOOAdapter.javaPersist entry-type metadata after citation insert and style update +3/-0

Persist entry-type metadata after citation insert and style update

• After inserting references and after recomputing citations for a new style, the adapter now stores entry types for the affected (cited) entries into the document-level metadata property.

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java

Tests (1) +116 / -0
CitationEntryTypeMetadataTest.javaUnit tests for entry-type metadata merge and Zotero parsing behavior +116/-0

Unit tests for entry-type metadata merge and Zotero parsing behavior

• Adds tests verifying JSON merge semantics (add, keep, update) and that Zotero CSL parsing maps to JabRef types while allowing stored metadata to override the parsed type.

jablib/src/test/java/org/jabref/logic/openoffice/CitationEntryTypeMetadataTest.java

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Optional.of(entry.getValue()) NPE risk ✓ Resolved 📘 Rule violation ≡ Correctness
Description
CitationEntryTypeMetadata.readEntryTypes wraps entry.getValue() with Optional.of(...), which
will throw a NullPointerException if the deserialized citationTypeMap contains a null value (e.g.,
valid JSON like "citationTypeMap": {"Keen2020": null}). This breaks the intended fail-open
behavior for malformed/partial metadata, can crash metadata reads and prevent metadata-based
entry-type restoration, and violates idiomatic absence handling.
Code

jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[R92-97]

+        for (Map.Entry<String, CitationEntryType> entry : getCitationTypeMap(entryTypeMetadata).entrySet()) {
+            Optional.of(entry.getValue())
+                    .map(citationEntryType -> citationEntryType.jabrefEntryType)
+                    .filter(value -> !StringUtil.isBlank(value))
+                    .ifPresent(value -> result.put(entry.getKey(), value));
+        }
Evidence
The checklist calls for idiomatic absence handling and avoiding null/Optional anti-patterns, but
readEntryTypes currently assumes entry.getValue() is non-null and uses Optional.of(...) on it.
Because the metadata is deserialized from JSON and the model explicitly allows nullable values
(@Nullable), null map entries can exist at runtime, and Optional.of(null) will throw, turning
otherwise parseable (but partial/malformed) metadata into a hard failure instead of being treated as
“absent/empty.”

jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[92-97]
jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[84-97]
jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[139-146]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CitationEntryTypeMetadata.readEntryTypes` uses `Optional.of(entry.getValue())`, which throws a `NullPointerException` when a `citationTypeMap` entry value is `null` (possible when parsing/deserializing JSON metadata, e.g., `"citationTypeMap": {"Keen2020": null}`). This causes metadata reading to crash instead of failing open (skipping missing values), which can prevent metadata-based entry-type restoration.
## Issue Context
This class parses user-provided/embedded document metadata (e.g., `meta.xml` user-defined properties) and is expected to be robust to missing/null fields. The parsed model allows nulls (`@Nullable`), so null entries can exist at runtime even when the metadata is malformed-but-parseable.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[84-99]
- jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[92-97]
- jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[139-146]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Gson null causes NPE ✓ Resolved 🐞 Bug ☼ Reliability
Description
CitationEntryTypeMetadata.parseMetadata wraps GSON.fromJson(...) with Optional.of(...), which throws
NullPointerException if Gson returns null (e.g., metadata content is JSON literal "null"). This
unchecked exception can escape storeZoteroEntryTypes/storeEntryTypes and break document connection
or citation insertion/update flows.
Code

jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[R119-126]

+    private static Optional<EntryTypeMetadata> parseMetadata(String metadata) {
+        if (StringUtil.isBlank(metadata)) {
+            return Optional.empty();
+        }
+
+        try {
+            return Optional.of(GSON.fromJson(metadata, EntryTypeMetadata.class));
+        } catch (JsonParseException e) {
Evidence
parseMetadata currently uses Optional.of(...) on the return value of GSON.fromJson, which can
be null; the repo itself notes that Gson can return null unless guarded. Callers (e.g.,
document-connection path) only catch UNO checked exceptions, so an NPE would propagate and break the
flow.

jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[119-129]
jablib/src/main/java/org/jabref/logic/openoffice/ZoteroCitationMarkParser.java[70-78]
jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBase.java[106-111]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`parseMetadata` uses `Optional.of(GSON.fromJson(...))`, which can throw `NullPointerException` when Gson returns `null` (e.g., when the stored metadata string is the JSON literal `null`). This NPE is unchecked and is not handled by callers like `OOBibBase.storeZoteroEntryTypes`, so it can disrupt core flows.
### Issue Context
The codebase already acknowledges that Gson can return null unless guarded (see comment in `ZoteroCitationMarkParser.parseCslJsonItems`).
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[119-129]
- jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBase.java[106-111]
### Suggested fix
1. Change `Optional.of(GSON.fromJson(...))` to:
- `EntryTypeMetadata parsed = GSON.fromJson(metadata, EntryTypeMetadata.class);`
- `return Optional.ofNullable(parsed);`
2. Consider broadening the `try` block to also handle unexpected runtime failures defensively (e.g., `RuntimeException`) and return `Optional.empty()`.
3. Add/extend a unit test ensuring `readEntryTypes("null")` returns an empty map and does not throw.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Trivial comment in parser ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The added comment in parseZoteroEntries restates what the following code does rather than
explaining intent or rationale. This adds noise without documenting a “why”.
Code

jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[R104-112]

+            if (ReferenceMark.isZoteroReferenceMarkName(referenceMarkName)) {
+                // If citation key is in metadata, then use the entry type in metadata
+                for (BibEntry entry : ZoteroCitationMarkParser.parse(referenceMarkName)) {
+                    entry.getCitationKey()
+                         .map(entryTypesInMetaData::get)
+                         .filter(value -> !StringUtil.isBlank(value))
+                         .map(EntryTypeFactory::parse)
+                         .ifPresent(entry::setType);
+                    entries.add(entry);
Evidence
The compliance rule disallows comments that merely restate the code. The added line `// If citation
key is in metadata, then use the entry type in metadata` directly describes the immediately
following chain and does not provide additional rationale.

AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why': AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why': AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why': AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why': AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why': AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why': AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why': AGENTS.md: Avoid Trivial Comments; Comments Should Explain 'Why'
jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[104-112]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added comment narrates the code instead of explaining intent/rationale.
## Issue Context
Comments should capture non-obvious reasoning, constraints, or “why”, not restate the implementation.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java[104-112]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java Outdated
Comment thread jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java Outdated
Comment thread jablib/src/main/java/org/jabref/logic/openoffice/CitationEntryTypeMetadata.java Outdated
}

try {
return Optional.ofNullable(GSON.fromJson(metadata, EntryTypeMetadata.class));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think GSSon.fromJson is non null

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Tried to address Qodo's comment

static List<BibEntry> parseZoteroEntries(List<String> referenceMarkNames, Map<String, String> entryTypesInMetaData) {
List<BibEntry> entries = new ArrayList<>();
for (String referenceMarkName : referenceMarkNames) {
if (ReferenceMark.isZoteroReferenceMarkName(referenceMarkName)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

private static void applyStoredEntryType(BibEntry entry, Map<String, String> entryTypesByCitationKey) {
    entry.getCitationKey()
         .map(entryTypesByCitationKey::get)
         .filter(entryType -> !StringUtil.isBlank(entryType))
         .map(EntryTypeFactory::parse)
         .ifPresent(entry::setType);
}

}

EntryTypeMetadata entryTypeMetadata = parsedMetadata.get();
Map<String, String> result = new LinkedHashMap<>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can init the hashmap with the size of the metadata map, you know the size)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated

assertEquals(1, entries.size());
BibEntry entry = entries.getFirst();
assertEquals(StandardEntryType.Book, entry.getType());
assertEquals(Optional.of("Zotero-587"), entry.getCitationKey());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use assertEquals over the whole entry
assertEquals(expectedBibEntry, actualEntry)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated

private CitationEntryTypeMetadata() {
}

public static void storeZoteroEntryTypes(XTextDocument document)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would keep the schema as a POJO and add another class. for the mehtods that work with it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

separation of concerns

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Extracted schema into CitationEntryTypeMetadata and made a manager

@subhramit subhramit added this pull request to the merge queue Jul 9, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Jul 9, 2026
Merged via the queue into JabRef:main with commit 725d1c8 Jul 9, 2026
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: no-bot-comments status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inserting jabref-entrytype info

3 participants