DataCite Xml updates- 4.7, language, Translator, Keyword URI, etc. - #12346
DataCite Xml updates- 4.7, language, Translator, Keyword URI, etc.#12346qqmyers wants to merge 15 commits into
Conversation
Could you write more about how Dataverse has a 'Translator' option? Or how do we expect someone depositing a dataset in a Dataverse repository to indicate that someone contributed as a translator? I ask because I don't see "Translator" in the deposit form's Contributor Type dropdown on Demo Dataverse or in the list of contributor types in the citation.tsv and citation.properties file that ships with Dataverse. I do see it in the citation metadata block that QDR is using, at https://data.qdr.syr.edu/api/metadatablocks/citation. Do you know yet if you're planning to merge this PR for the 6.11 release? I ask because in #12281 we're tracking other changes we're planning to make to the information that Dataverse includes in the DataCite XML export. That GitHub issue has a milestone label for 6.11, although we're not sure yet if we'll be able to finish it for the 6.11 release. |
|
Since Translator wasn't yet in the citation block, I've added it in the PR. When/if this gets merged is not my call. (As QDR is already deploying these changes, the priority should be based on whether Harvard or other GDCC members want it.) |
|
Nevermind! After sprint planning today, it's probable that we won't make the changes described in #12281 as part of the 6.11 version of Dataverse. |
|
OK, even so, it would be best to merge this before asking someone to write the same code again. Also - if it's fixed twice, git will flag a conflict before it can be merged, so no need to address it beforehand. |
|
Ah, thanks, I like that approach. Or maybe I should close that #12300 issue about the Keyword Term URIs, and update it and its parent issue at #12281 to point at that merging this PR (#12346) will also help align the DataCite/Datacite and OpenAIRE/oai_datacite exports. That way regardless of which changes are included in the next release of Dataverse, folks aren't re-writing code. |
|
I just marked this PR as Closing #12300 which should make a note there and auto-close it when this is merged. |
|
To this PR I added a "Needs sizing" status and made the "FY Priority" "Must have", so it's on the radar of folks prioritizing things. |
rtreacy
left a comment
There was a problem hiding this comment.
Claude
PR #12346 Review: DataCite XML updates — 4.7, language, Translator, Keyword URI, etc.
What it does
A grab-bag of DataCite XML export improvements: bumps schema version to 4.7, adds Translator as a contributor type, adds valueURI to from keywordTermURI, adds a element when exactly one citation-block language is set, accepts YYYY/YYYY-MM/YYYY-MM-DD in date-range fields, and aims to stop emitting the literal string "null" in date ranges when only one side of a range is filled in.
Critical: likely NullPointerException regression in writeDates()
This is the one I'd block on. In both the dateOfCollection and timePeriodCovered loops (XmlMetadataTemplate.java:756/759 and :782/785):
startDate = subField.getValue().trim();
endDate = subField.getValue().trim();
DatasetField.getValue() returns null whenever the field has no stored value (DatasetField.java:243-251, empty datasetFieldValues and no controlled-vocab value). That's exactly the situation this PR's own release note describes fixing: "avoids sending the word 'null' as part of a date range when the start or end date is unspecified." Previously that scenario produced a literal null reference that got silently stringified to "null" at the later concatenation ((startDate + "/" + endDate)), which is the bug being targeted — but the fix now calls .trim() directly on that same null return value before it ever reaches the new isValidYearMonthOrDay() guard, so instead of sending a wrong string, it throws an NPE.
generateXML() only catches XMLStreamException | IOException (line 77 in the fetched file) — an NPE here is unchecked and will propagate straight out, likely failing DOI registration/export entirely for any dataset where a "Date of Collection" or "Time Period Covered" entry has only a start or only an end date filled in (a very common real-world entry pattern). This isn't a hypothetical edge case; it's the exact scenario the PR is meant to handle.
The included test (testDataCiteXMLCreationAllFields, fixture dataset-all-defaults.txt) doesn't exercise this: every date-range entry in the fixture has both start and end populated, so the NPE path is never hit. I'd fix with something like:
String rawValue = subField.getValue();
startDate = rawValue != null ? rawValue.trim() : null;
(and add a test case with a one-sided range to lock in the actual fix).
Minor: citation.tsv displayOrder collision
contributorType Translator 14
contributorType Supervisor 15
contributorType Work Package Leader 16
contributorType Other 16 <- unchanged, now duplicates WPL
Inserting Translator at 14 correctly shifted Supervisor→15 and Work Package Leader→16, but Other (previously the unique 16) wasn't bumped to 17. This is only the displayOrder column (no DB uniqueness constraint, confirmed via ControlledVocabularyValue.java's @table — just a plain index), so it won't break metadata-block loading or corrupt existing data (values are referenced by DB id, not this column) — it just leaves Work Package Leader and Other tied for sort position in the UI dropdown. Easy one-line fix (Other → 17), worth doing for cleanliness even though low severity.
What checks out
- Schema version/location bump to 4.7 and the Translator addition to both citation.tsv, citation.properties, and the contributorTypes allow-list in XmlMetadataTemplate.java are all consistent with each other.
- valueURI attribute wiring for keywordTermURI mirrors the existing schemeURI pattern exactly — no issues.
- writeLanguage()'s use of dataset.getLatestVersionForCopy() matches the exact same established pattern already used a few methods above in writeSubjects() (same file) — not a new inconsistency, despite my first instinct to flag version-scoping here.
- isValidYearMonthOrDay() correctly delegates to Year/YearMonth/LocalDate.parse(), so semantically invalid dates (e.g., month 13, Feb 30) are properly rejected via DateTimeParseException, not just syntactically pattern-matched.
- The @see #writeContributorElement(...) → @see #writeEntityElements(...) javadoc fix is a legitimate correction of a stale reference (verified writeEntityElements is the real, current method name).
- New tests reasonably cover the additive happy paths: keyword valueURI, Translator contributor round-trip, and single-language emission.
Test coverage gaps
- No test for a one-sided date range (only start or only end) in either dateOfCollection or timePeriodCovered — this is precisely what's missing to catch the NPE above.
- No test for a syntactically invalid date string in these fields (e.g., "2020/13", "not-a-date") to confirm graceful rejection rather than an exception.
Verdict
Solid, well-motivated set of DataCite export improvements, but the date-handling change has a real regression risk that inverts the bug it's meant to fix (wrong string → crash) for the specific case called out in the PR description. I'd request a fix + a one-sided-range test before merging. The citation.tsv displayOrder duplicate is a trivial fix worth including in the same pass.
What this PR does / why we need it: This PR updates the citation block contributor types to add 'Translator' and makes a few small updates to the DataCite XML export/xml metadata sent to DataCite:
Which issue(s) this PR closes:
valueURIproperty of thesubjectproperty #12300Special notes for your reviewer: Note the timePeriodCovered and dateOfCollection fields show a placeholder with YYYY-MM-DD but in practice many people are using shorter year or year and month entries. These were not being sent to DataCite (as of ~2024 when validation was added). Not sure if it is worth changing the placeholder or not.
Suggestions on how to test this: The individual changes listed above should all be easy to test by filling in the relevant metadata. The XML produced should be visible on Fabrica or via API to get the DataCite XML export, or one could publish and see the export in the UI. There is a new test for invalid/null start/end dates which is passing.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: