From 274d6fafa678957268c9a486a119de3af01a9228 Mon Sep 17 00:00:00 2001 From: Mohammed Taha Khan Date: Mon, 31 Aug 2026 17:47:04 +0200 Subject: [PATCH] fix(harvester): merge new abstracts and author identifiers on update --- .../inspire_harvester/update/config.py | 3 + .../update/fields/creatibutors.py | 56 +++++++--- site/tests/conftest.py | 18 +++ .../test_creatibutors_update.py | 104 ++++++++++++++++++ 4 files changed, 168 insertions(+), 13 deletions(-) create mode 100644 site/tests/inspire_harvester/test_creatibutors_update.py diff --git a/site/cds_rdm/inspire_harvester/update/config.py b/site/cds_rdm/inspire_harvester/update/config.py index e701b0cb..2abf4da1 100644 --- a/site/cds_rdm/inspire_harvester/update/config.py +++ b/site/cds_rdm/inspire_harvester/update/config.py @@ -33,6 +33,9 @@ "metadata.subjects": ListOfDictAppendUniqueUpdate(key_field="subject"), "metadata.languages": ListOfDictAppendUniqueUpdate(key_field="id"), "metadata.description": OverwriteFieldUpdate(), + "metadata.additional_descriptions": ListOfDictAppendUniqueUpdate( + key_field="description" + ), "metadata.title": OverwriteFieldUpdate(), "custom_fields.thesis:thesis": ThesisFieldUpdate(), "custom_fields.cern:accelerators": ListOfDictAppendUniqueUpdate(key_field="id"), diff --git a/site/cds_rdm/inspire_harvester/update/fields/creatibutors.py b/site/cds_rdm/inspire_harvester/update/fields/creatibutors.py index 8cdc286d..e32637cb 100644 --- a/site/cds_rdm/inspire_harvester/update/fields/creatibutors.py +++ b/site/cds_rdm/inspire_harvester/update/fields/creatibutors.py @@ -78,19 +78,36 @@ def _union_affiliations(self, cur_list, inc_list): return out - def _key(self, creator: dict): - """Return a hashable key for matching a creator/contributor.""" + def _keys(self, creator: dict): + """Return every key a creator/contributor may be matched on. + + Identifiers come first, but the name key is always included so that an + author who gains an identifier upstream still matches the stored entry. + """ p = creator.get("person_or_org") or {} - ids = p.get("identifiers") or [] - for i in ids: - if i.get("scheme") and i.get("identifier"): - return ("id", i["scheme"], i["identifier"]) - return ( - "name", - (p.get("family_name") or "").lower(), - (p.get("given_name") or "").lower(), - (p.get("name") or "").lower(), + keys = [ + ("id", i["scheme"], i["identifier"]) + for i in p.get("identifiers") or [] + if i.get("scheme") and i.get("identifier") + ] + keys.append( + ( + "name", + (p.get("family_name") or "").lower(), + (p.get("given_name") or "").lower(), + (p.get("name") or "").lower(), + ) ) + return keys + + def _key(self, creator: dict): + """Return the primary key of a creator/contributor, used for logging.""" + return self._keys(creator)[0] + + def _schemes(self, creator: dict): + """Return the identifier schemes carried by a creator/contributor.""" + p = creator.get("person_or_org") or {} + return {i["scheme"] for i in p.get("identifiers") or [] if i.get("scheme")} def _merge_creator(self, cur, inc): """Merge a single current creator entry with its incoming counterpart.""" @@ -140,11 +157,24 @@ def update(self, current, incoming, path, ctx): # index current index = {} for i, c in enumerate(cur_list): - index.setdefault(self._key(c), []).append(i) + for key in self._keys(c): + index.setdefault(key, []).append(i) for inc in inc_list: k = self._key(inc) - matches = index.get(k, []) + *id_keys, name_key = self._keys(inc) + matches = [] + for key in id_keys: + matches = index.get(key, []) + if matches: + break + else: + inc_schemes = self._schemes(inc) + matches = [ + i + for i in index.get(name_key, []) + if not (self._schemes(cur_list[i]) & inc_schemes) + ] if not matches: if self.strict: diff --git a/site/tests/conftest.py b/site/tests/conftest.py index 191c89c7..f8ac8506 100644 --- a/site/tests/conftest.py +++ b/site/tests/conftest.py @@ -1520,6 +1520,24 @@ def description_type_v(app, description_type): "type": "descriptiontypes", }, ) + vocab = vocabulary_service.create( + system_identity, + { + "id": "series-information", + "title": {"en": "Series information"}, + "props": {"datacite": "SeriesInformation"}, + "type": "descriptiontypes", + }, + ) + vocab = vocabulary_service.create( + system_identity, + { + "id": "technical-info", + "title": {"en": "Technical info"}, + "props": {"datacite": "TechnicalInfo"}, + "type": "descriptiontypes", + }, + ) Vocabulary.index.refresh() diff --git a/site/tests/inspire_harvester/test_creatibutors_update.py b/site/tests/inspire_harvester/test_creatibutors_update.py new file mode 100644 index 00000000..c858a71d --- /dev/null +++ b/site/tests/inspire_harvester/test_creatibutors_update.py @@ -0,0 +1,104 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2026 CERN. +# +# CDS-RDM is free software; you can redistribute it and/or modify it under +# the terms of the MIT License; see LICENSE file for more details. + +"""CreatibutorsFieldUpdate matching tests.""" + +from cds_rdm.inspire_harvester.update.fields.creatibutors import CreatibutorsFieldUpdate + +PATH = "metadata.creators" + + +def _person(family="", given="", name="", orcid=None): + """Build a creator entry.""" + person = { + "type": "personal", + "family_name": family, + "given_name": given, + "name": name or ", ".join(part for part in (family, given) if part), + } + if orcid: + person["identifiers"] = [{"scheme": "orcid", "identifier": orcid}] + return {"person_or_org": person} + + +def _merge(current_creators, incoming_creators): + strategy = CreatibutorsFieldUpdate(strict=True) + return strategy.update( + {"metadata": {"creators": current_creators}}, + {"metadata": {"creators": incoming_creators}}, + PATH, + ctx=None, + ) + + +def test_name_key_uses_family_given_and_full_name(): + """Matching is on the full name tuple, not given name alone.""" + keys = CreatibutorsFieldUpdate()._keys( + _person(family="Rossi", given="Anna", name="Rossi, Anna") + ) + + assert ("name", "rossi", "anna", "rossi, anna") in keys + + +def test_same_given_name_different_family_is_not_merged(): + """Two people who share a given name are not treated as one person.""" + result = _merge( + [_person(family="Rossi", given="Anna", name="Rossi, Anna")], + [_person(family="Bianchi", given="Anna", name="Bianchi, Anna")], + ) + + stored = result.updated["metadata"]["creators"][0]["person_or_org"] + assert stored["family_name"] == "Rossi" + assert [w.kind for w in result.warnings] == ["new_creator"] + + +def test_stored_name_only_gains_orcid_from_inspire(): + """Stored author keyed by name still matches incoming ORCID.""" + result = _merge( + [_person(family="Doe", given="John", name="Doe, John")], + [ + _person( + family="Doe", + given="John", + name="Doe, John", + orcid="0000-0002-1825-0097", + ) + ], + ) + + assert result.warnings == [] + assert result.updated["metadata"]["creators"][0]["person_or_org"]["identifiers"] == [ + {"scheme": "orcid", "identifier": "0000-0002-1825-0097"} + ] + + +def test_same_name_different_orcids_are_not_merged(): + """Two people with the same name and different ORCIDs stay separate.""" + result = _merge( + [ + _person( + family="Smith", + given="John", + name="Smith, John", + orcid="0000-0001-0000-0001", + ) + ], + [ + _person( + family="Smith", + given="John", + name="Smith, John", + orcid="0000-0002-0000-0002", + ) + ], + ) + + assert [w.kind for w in result.warnings] == ["new_creator"] + stored = result.updated["metadata"]["creators"][0]["person_or_org"] + assert stored["identifiers"] == [ + {"scheme": "orcid", "identifier": "0000-0001-0000-0001"} + ]