diff --git a/cds_migrator_kit/rdm/README.md b/cds_migrator_kit/rdm/README.md index 5c23c6bc..d7bc090b 100644 --- a/cds_migrator_kit/rdm/README.md +++ b/cds_migrator_kit/rdm/README.md @@ -326,6 +326,8 @@ DELETE FROM files_files; DELETE FROM pidstore_pid WHERE pid_type = 'lrecid'; DELETE FROM pidstore_pid WHERE pid_type = 'recid'; DELETE FROM pidstore_pid WHERE pid_type = 'doi'; +DELETE FROM pidstore_pid WHERE pid_type = 'cdsrn'; +DELETE FROM pidstore_pid WHERE pid_type = 'apprn'; DELETE FROM cds_clc_record_sync; DELETE FROM cds_migration_legacy_records; ``` @@ -333,13 +335,13 @@ DELETE FROM cds_migration_legacy_records; - Cleanup indexed documents from opensearch ``` -POST /cds-rdm-dev-rdmrecords/_delete_by_query +POST /cds-rdm-rdmrecords/_delete_by_query { "query": { "match_all": {} } } -POST /cds-rdm-dev-communities/_delete_by_query +POST /cds-rdm-communities/_delete_by_query { "query": { "match_all": {} diff --git a/cds_migrator_kit/rdm/records/load/entities/record.py b/cds_migrator_kit/rdm/records/load/entities/record.py index e94fc617..b88222f2 100644 --- a/cds_migrator_kit/rdm/records/load/entities/record.py +++ b/cds_migrator_kit/rdm/records/load/entities/record.py @@ -156,6 +156,39 @@ def load_files( self.migration_logger.add_log(exc, record={"record": self.record_entry}) raise e + def set_default_preview(self, identity, draft, version_files, uow): + """Set default_preview to the first file not labelled erratum or addendum. + + Files whose 8564_y description contains 'erratum' or 'addendum' + (case-insensitive) are supplementary documents and must not become + the record's preview file. If every file carries such a label there + is nothing useful to promote, so we leave the field unset. + """ + if not version_files: + return + + _skip = {"erratum", "addendum"} + preview_key = next( + ( + file_data["key"] + for file_data in version_files.values() + if not any( + term in (file_data["metadata"].get("description") or "").lower() + for term in _skip + ) + ), + None, + ) + + if preview_key is None: + return + + draft_dict = draft.to_dict() + draft_dict.setdefault("files", {})["default_preview"] = preview_key + current_rdm_records_service.update_draft( + identity, draft["id"], data=draft_dict, uow=uow + ) + def load_access(self, draft, access_dict: VersionAccess, uow): """Set this version's access on the published record.""" record = draft._record @@ -250,6 +283,7 @@ def pre_publish(self, identity, versions, version, draft, uow): self.load_access(draft, access, uow) self.load_files(draft, files, uow) + self.set_default_preview(identity, draft, files, uow) return draft diff --git a/cds_migrator_kit/rdm/records/transform/config.py b/cds_migrator_kit/rdm/records/transform/config.py index 156f4a72..0926d581 100644 --- a/cds_migrator_kit/rdm/records/transform/config.py +++ b/cds_migrator_kit/rdm/records/transform/config.py @@ -159,6 +159,57 @@ FILE_SUBFORMATS_TO_DROP = ["pdfa", "unstamped"] +# Experiment IDs that live in the experiments MARC field but should be +# stored as studies (cern:studies) rather than experiments (cern:experiments). +EXPERIMENTS_AS_STUDIES = { + "IS01", + "IS01-1", + "IS01-2", + "IS01-3", + "IS01-4", + "IS01-5", + "IS01-6", + "IS01-7", + "IS01-8", + "IS01-9", + "IS01-10", + "IS01-11", + "IS01-12", + "IS01-13", + "IS01-15", + "IS01-16", + "IS01-18", + "IS01-20", + "IS01-21", + "IS01-22", + "IS01-23", + "IS01-24", + "IS01-26", + "IS01-28", + "IS01-29", + "IS01-31", + "IS01-32", + "IS01-35", + "IS01-37", + "IS01-38", + "IS01-39", + "IS01-40", + "IS01-42", + "IS01-43", + "IS01-44", + "IS01-45", + "IS01-49", + "IS01-51", + "IS01-B", + "IS01-C", + "IS01-D", + "IS01-E", + "IS250", + # "IS", + # "IS02", + "IS142", +} + # Legacy experiment names remapped to vocabulary ids before lookup EXPERIMENT_ALIASES = { "t2k": "RE13", @@ -170,6 +221,12 @@ "dampe": "RE29", "compass": "NA58", "compass na58": "NA58", + "cern isolde": "ISOLDE", + "mistral is346": "IS346", + "mistral is373": "IS373", + "witch is433": "IS433", + "isoltrap": "IS394", + "is485 (rex-isolde)": "IS485", } # 693__e values that are keywords rather than real experiments (curated as diff --git a/cds_migrator_kit/rdm/records/transform/mappers/custom_fields.py b/cds_migrator_kit/rdm/records/transform/mappers/custom_fields.py index 071e6dbd..84a1e56e 100644 --- a/cds_migrator_kit/rdm/records/transform/mappers/custom_fields.py +++ b/cds_migrator_kit/rdm/records/transform/mappers/custom_fields.py @@ -246,9 +246,6 @@ class JournalMapper(CustomFieldMapper): def apply(self, ctx): """Set ctx.custom_fields["journal:journal"].""" - # `_773_m_seen` is an internal bookkeeping field, just to check that we only have one 773__m - # per record as agreed with SIS. - ctx.dojson_entry.pop("_773_m_seen", None) journal = ctx.dojson_entry.get("custom_fields", {}).get("journal:journal", {}) if journal and not journal.get("title"): ctx.flag_curation( diff --git a/cds_migrator_kit/rdm/records/transform/models/_config.py b/cds_migrator_kit/rdm/records/transform/models/_config.py index 181bdf9f..467b4dbb 100644 --- a/cds_migrator_kit/rdm/records/transform/models/_config.py +++ b/cds_migrator_kit/rdm/records/transform/models/_config.py @@ -85,4 +85,5 @@ "999C5d", # old INSPIRE attr "999C69", # See decision log "999C6c", # See decision log + "999C5e", # See decision log } diff --git a/cds_migrator_kit/rdm/records/transform/models/isolde.py b/cds_migrator_kit/rdm/records/transform/models/isolde.py index b4b8a3cd..4620364b 100644 --- a/cds_migrator_kit/rdm/records/transform/models/isolde.py +++ b/cds_migrator_kit/rdm/records/transform/models/isolde.py @@ -7,6 +7,7 @@ """CDS-RDM ISOLDE model.""" +from cds_migrator_kit.rdm.records.transform.models._config import IGNORE_SYSTEM_KEYS from cds_migrator_kit.rdm.records.transform.models.base_publication_record import ( rdm_base_publication_model, ) @@ -17,13 +18,15 @@ class ISOLDEModel(CdsOverdo): """Translation model for ISOLDE.""" - __query__ = '693__.a:"CERN ISOLDE" AND (980__:ARTICLE OR 980__:PREPRINT OR 980__:conferencepaper OR 980__:NOTE OR 980__:REPORT) -980__:DELETED -980__:DUMMY' + __query__ = ( + '(693__.a:"CERN ISOLDE" OR 693__.a:"CERN Isolde") AND ' + "(980__:ARTICLE OR 980__:Article OR 980__:PREPRINT OR " + "980__:ConferencePaper OR 980__:conferencepaper OR " + "980__:NOTE OR 980__:REPORT) " + "-980__:DELETED -980__:DUMMY" + ) - # ResearchModel.__ignore_keys__ covers the shared publication baseline - # (035__ oai tags, 852__h/c holdings, 999C5*/6* citations, 8564_ file - # subfields, 773__ SIS fields, contributor emails, etc.). - # Only ISOLDE-specific additions are listed here. - __ignore_keys__ = ResearchModel.__ignore_keys__ + __ignore_keys__ = IGNORE_SYSTEM_KEYS | ResearchModel.__ignore_keys__ _default_fields = { "custom_fields": {}, diff --git a/cds_migrator_kit/rdm/records/transform/xml_processing/rules/base.py b/cds_migrator_kit/rdm/records/transform/xml_processing/rules/base.py index 4bd29e06..e14a08c4 100644 --- a/cds_migrator_kit/rdm/records/transform/xml_processing/rules/base.py +++ b/cds_migrator_kit/rdm/records/transform/xml_processing/rules/base.py @@ -30,6 +30,7 @@ CONTROLLED_SUBJECTS_SCHEMES, IDENTIFIERS_SCHEMES_TO_DROP, KEYWORD_SCHEMES_TO_DROP, + EXPERIMENTS_AS_STUDIES, PID_SCHEMES_TO_STORE_IN_IDENTIFIERS, PID_SCHEMES_TO_STORE_IN_RELATED_IDENTIFIERS, RECOGNISED_KEYWORD_SCHEMES, @@ -241,6 +242,15 @@ def custom_fields_693(self, key, value): for custom_field_name, subfield in fields_map.items(): custom_field = _custom_fields.get(custom_field_name, []) new_values = [v for v in force_list(value.get(subfield, "")) if v] + + if custom_field_name == "cern:experiments": + studies = [v for v in new_values if v.upper() in EXPERIMENTS_AS_STUDIES] + new_values = [ + v for v in new_values if v.upper() not in EXPERIMENTS_AS_STUDIES + ] + if studies: + _custom_fields.setdefault("cern:studies", []).extend(studies) + custom_field += new_values _custom_fields[custom_field_name] = custom_field @@ -369,6 +379,8 @@ def aleph_number(self, key, value): Attention: 035 might contain aleph number https://github.com/CERNDocumentServer/cds-migrator-kit/issues/21 + + If the record is migrated to the repository, it has 970__d with redirection URL. """ aleph = StringValue(value.get("a")).parse() identifiers = self.get("identifiers") @@ -376,6 +388,9 @@ def aleph_number(self, key, value): if aleph and new_id not in identifiers: return {"scheme": "aleph", "identifier": aleph} else: + d_value = value.get("d", "").strip().lower() + if d_value and not d_value.startswith("https://repository.cern"): + raise UnexpectedValue(field=key, subfield="d", value=value, stage="transform") raise IgnoreKey("identifiers") @@ -493,6 +508,18 @@ def _pids(self, key, value): if not is_doi_id and is_handle_id and (scheme == "doi" or scheme == "urn/hdl"): scheme = "handle" + if qualifier == "erratum": + related_identifiers = self.get("related_identifiers", []) + new_id = { + "identifier": identifier, + "scheme": scheme, + "relation_type": {"id": "issupplementedby"}, + } + if new_id not in related_identifiers: + related_identifiers.append(new_id) + self["related_identifiers"] = related_identifiers + raise IgnoreKey("_pids") + if qualifier == "publication" or qualifier == "thesis": # if we have a qualifier for one of these two, we know it references # an external resource (checked in DB and individually on records) @@ -693,6 +720,9 @@ def licenses(self, key, value): self["custom_fields"] = _custom_fields if not license_id: + # Bronze meaning no license on the document + if qualifier == "bronze": + raise IgnoreKey("rights") # 540__f/b without a license id — side effects (funding model, copyright) already applied raise UnexpectedValue( "License title missing", field=key, subfield="a", value=value @@ -727,7 +757,7 @@ def licenses(self, key, value): _license = { "title": {"en": license_id}, "link": license_url, - "description": description, + "description": {"en": description} if description else None, } return _license diff --git a/cds_migrator_kit/rdm/records/transform/xml_processing/rules/research.py b/cds_migrator_kit/rdm/records/transform/xml_processing/rules/research.py index 87a8f70a..03a0de12 100644 --- a/cds_migrator_kit/rdm/records/transform/xml_processing/rules/research.py +++ b/cds_migrator_kit/rdm/records/transform/xml_processing/rules/research.py @@ -273,27 +273,35 @@ def journal(self, key, value): related_ids.append(isbn_related_id) self["related_identifiers"] = related_ids + # 773__m qualifies the journal reference (e.g. the main publication or an + # erratum). Per the curators' decision, the erratum linking is handled + # entirely at the DOI level (0247_ $q -> related identifier with + # "issupplementedby"), so this note carries no extra information and is + # ignored. Any other value is surfaced for curation. if "m" in value: - # As discussed with SIS, we only ignore 773__m if there is only one in the record - # and its value is `publication` - if self.get("_773_m_seen"): - raise UnexpectedValue( - "Multiple 773__m seen. Record requires manual curation.", - subfield="m", - field=key, - value=value, - ) - - m_value = value.get("m") - if m_value != "publication": - raise UnexpectedValue( - f'Only value "publication" can be ignored for 773__m. Value "{m_value}" requires manual curation.', - subfield="m", - field=key, - value=value, - ) + allowed_m_values = {"publication", "erratum"} + m_values = value.get("m") + if not isinstance(m_values, (list, tuple)): + m_values = (m_values,) + for m_value in m_values: + if StringValue(m_value).parse().lower() not in allowed_m_values: + raise UnexpectedValue( + "Unexpected 773__m value. Record requires manual curation.", + subfield="m", + field=key, + value=value, + ) - self["_773_m_seen"] = True + # A repeated 773__c (dojson returns a tuple) is ambiguous page/session data + # that can't be reduced to a single value - flag for manual curation instead + # of crashing on the tuple when it is parsed downstream. + if isinstance(value.get("c"), (list, tuple)): + raise UnexpectedValue( + "Multiple 773__c values. Record requires manual curation.", + subfield="c", + field=key, + value=value, + ) # p/n/v are journal-specific; c alone with w is a conference proceedings artid is_journal = any(f in value for f in ["p", "n", "v"]) @@ -434,15 +442,27 @@ def internal_notes(self, key, value): @model.over("contributors", "^901__") @for_each_value def organisation(self, key, value): - """Translates organisation contributors.""" - contributor = value.get("u", "") - return { - "person_or_org": { - "type": "organizational", - "name": contributor, - }, - "role": {"id": "hostinginstitution"}, - } + """Translates organisation contributors. + + 901__ may carry multiple ``u`` affiliation values; each becomes its own + organizational contributor. + """ + contributors = self.get("contributors", []) + for name in force_list(value.get("u", "")): + name = StringValue(name).parse().strip() + if not name: + continue + contributors.append( + { + "person_or_org": { + "type": "organizational", + "name": name, + }, + "role": {"id": "hostinginstitution"}, + } + ) + self["contributors"] = contributors + raise IgnoreKey("contributors") @model.over("request_reviewers", "^906__", override=True) diff --git a/cds_migrator_kit/rdm/streams.yaml b/cds_migrator_kit/rdm/streams.yaml index 28d64b40..780a036d 100644 --- a/cds_migrator_kit/rdm/streams.yaml +++ b/cds_migrator_kit/rdm/streams.yaml @@ -339,7 +339,7 @@ records: files_dump_dir: cds_migrator_kit/rdm/data/isolde/files/ missing_users: cds_migrator_kit/rdm/data/users communities_ids: - - "77d6a84a-596e-414f-817b-1c43e9c66258" + - "0e0493e2-d312-48ac-93ad-a379cca428d5" load: legacy_pids_to_redirect: cds_migrator_kit/rdm/data/isolde/duplicated_pids.json hie_isolde: diff --git a/tests/cds-rdm/test_base_rules.py b/tests/cds-rdm/test_base_rules.py index 70c3034c..55f76a0b 100644 --- a/tests/cds-rdm/test_base_rules.py +++ b/tests/cds-rdm/test_base_rules.py @@ -12,6 +12,7 @@ from cds_migrator_kit.errors import UnexpectedValue from cds_migrator_kit.rdm.records.transform.xml_processing.rules.base import ( + _pids, custom_fields_693, imprint_info, normalize, @@ -463,3 +464,66 @@ def test_empty_datafield_is_ignored(self): ) result = rdm_base_record_model.do(marc_record) assert result["rights"] == [{"title": {"en": "CC-BY"}}] + + +class TestPidsDOI: + """Test the _pids rule (0247_) DOI handling. + + Per the curators' decision the erratum linking is handled at the DOI + level: the main (non-erratum) DOI becomes the record DOI, while each + erratum DOI goes to related_identifiers with an "issupplementedby" + relation. + """ + + def test_non_erratum_doi_becomes_main_record_doi(self): + """A DOI without an erratum qualifier is stored as the record DOI.""" + record = {} + with pytest.raises(IgnoreKey): + _pids(record, "0247_", {"2": "DOI", "a": "10.1103/PhysRevLett.1.1"}) + assert record["_pids"]["doi"] == {"identifier": "10.1103/PhysRevLett.1.1"} + assert "related_identifiers" not in record + + def test_erratum_doi_goes_to_related_identifiers_as_supplement(self): + """An erratum DOI becomes a related identifier, not the record DOI.""" + record = {} + with pytest.raises(IgnoreKey): + _pids( + record, + "0247_", + {"2": "DOI", "a": "10.1103/PhysRevLett.1.2", "q": "erratum"}, + ) + assert record["related_identifiers"] == [ + { + "identifier": "10.1103/PhysRevLett.1.2", + "scheme": "doi", + "relation_type": {"id": "issupplementedby"}, + } + ] + # the erratum DOI must not be promoted to the main record DOI + assert "_pids" not in record or "doi" not in record.get("_pids", {}) + + def test_multiple_erratum_dois_each_added_as_supplement(self): + """One or more erratum DOIs are all kept as related identifiers. + + dojson yields repeated 0247_ fields separately, so the rule is + invoked once per field; each erratum DOI accumulates. + """ + record = {} + for doi in ("10.1/err.a", "10.1/err.b"): + with pytest.raises(IgnoreKey): + _pids(record, "0247_", {"2": "DOI", "a": doi, "q": "erratum"}) + + identifiers = [r["identifier"] for r in record["related_identifiers"]] + assert identifiers == ["10.1/err.a", "10.1/err.b"] + assert all( + r["relation_type"] == {"id": "issupplementedby"} + for r in record["related_identifiers"] + ) + + def test_erratum_doi_not_duplicated(self): + """The same erratum DOI seen twice is only stored once.""" + record = {} + for _ in range(2): + with pytest.raises(IgnoreKey): + _pids(record, "0247_", {"2": "DOI", "a": "10.1/err.a", "q": "erratum"}) + assert len(record["related_identifiers"]) == 1 diff --git a/tests/cds-rdm/test_publications_rules.py b/tests/cds-rdm/test_publications_rules.py index 9ef382a7..9dd8f5f1 100644 --- a/tests/cds-rdm/test_publications_rules.py +++ b/tests/cds-rdm/test_publications_rules.py @@ -23,6 +23,7 @@ issn, journal, meeting, + organisation, resource_type, udc, ) @@ -705,6 +706,63 @@ def test_962_matching_real_journal_pages_preserves_journal(self): assert result[0]["resource_type"] == {"id": "publication-conferenceproceeding"} assert record["custom_fields"]["journal:journal"]["title"] == "Journal" + @pytest.mark.parametrize("m_value", ["publication", "erratum"]) + def test_773_m_known_value_is_ignored(self, m_value): + """773__m publication/erratum is ignored. + + The erratum linking is handled at the DOI level, so the note itself + carries no extra data: the record still migrates and the journal + fields are populated normally. + """ + record = {} + result = journal(record, "773__", {"p": "Nature", "v": "5", "m": m_value}) + + journal_info = result.get("journal:journal", {}) + assert journal_info["title"] == "Nature" + assert journal_info["volume"] == "5" + + def test_773_m_known_value_case_insensitive(self): + """773__m matching is case/whitespace insensitive.""" + record = {} + result = journal(record, "773__", {"p": "Nature", "m": " Erratum "}) + assert result["journal:journal"]["title"] == "Nature" + + def test_773_m_unexpected_value_raises(self): + """An unknown 773__m value is surfaced for manual curation.""" + with pytest.raises(UnexpectedValue, match="Unexpected 773__m value"): + journal({}, "773__", {"p": "Nature", "m": "something-else"}) + + def test_773_m_repeated_all_known_ignored(self): + """Repeated 773__m (a tuple) is accepted when every value is known.""" + record = {} + result = journal( + record, "773__", {"p": "Nature", "m": ("publication", "erratum")} + ) + assert result["journal:journal"]["title"] == "Nature" + + def test_773_m_repeated_with_one_unknown_raises(self): + """Repeated 773__m raises if any value is unknown.""" + with pytest.raises(UnexpectedValue, match="Unexpected 773__m value"): + journal({}, "773__", {"p": "Nature", "m": ("publication", "bogus")}) + + def test_773_c_repeated_raises_for_curation(self): + """A repeated 773__c (a tuple) is ambiguous and flagged for curation. + + dojson returns the subfield as a tuple; rather than crashing on it (or + silently picking one value) the record is surfaced for manual curation. + """ + with pytest.raises(UnexpectedValue, match="Multiple 773__c values"): + journal( + {}, + "773__", + {"p": "Phys. Lett. B", "m": "publication", "c": ("062701", "209904")}, + ) + + def test_773_c_repeated_in_conference_raises_for_curation(self): + """A repeated 773__c in a conference 773 is also flagged for curation.""" + with pytest.raises(UnexpectedValue, match="Multiple 773__c values"): + journal({}, "773__", {"w": "C05-03-12", "c": ("175-181", "WEC02")}) + class TestMeetingDateFrom518: """Test meeting date (518__r/__d) handling in related_identifiers. @@ -979,6 +1037,70 @@ def test_abbreviation_empty_a_ignored(self): abbreviation(record, "691__", {"a": ""}) +class TestOrganisation: + """Test organisation function from research.py (901__ rule).""" + + def _org(self, name): + return { + "person_or_org": {"type": "organizational", "name": name}, + "role": {"id": "hostinginstitution"}, + } + + def test_single_affiliation(self): + """A single 901__u becomes one organizational contributor.""" + record = {} + with pytest.raises(IgnoreKey): + organisation(record, "901__", {"u": "CERN"}) + assert record["contributors"] == [self._org("CERN")] + + def test_multiple_affiliations_as_tuple(self): + """Multiple 901__u values each become a separate contributor.""" + record = {} + with pytest.raises(IgnoreKey): + organisation( + record, + "901__", + { + "u": ( + "Darmstadt, Tech. Hochsch.", + "Munich, Tech. U.", + "Leuven U.", + ) + }, + ) + assert record["contributors"] == [ + self._org("Darmstadt, Tech. Hochsch."), + self._org("Munich, Tech. U."), + self._org("Leuven U."), + ] + + def test_appends_to_existing_contributors(self): + """901__ organisations are appended after existing contributors.""" + existing = { + "person_or_org": { + "type": "personal", + "family_name": "Doe", + "given_name": "Jane", + }, + "role": {"id": "other"}, + } + record = {"contributors": [existing]} + with pytest.raises(IgnoreKey): + organisation(record, "901__", {"u": ("CERN", "DESY")}) + assert record["contributors"] == [ + existing, + self._org("CERN"), + self._org("DESY"), + ] + + def test_empty_values_skipped(self): + """Empty / whitespace-only 901__u values are skipped.""" + record = {} + with pytest.raises(IgnoreKey): + organisation(record, "901__", {"u": ("", " ", "CERN")}) + assert record["contributors"] == [self._org("CERN")] + + class TestMeetingFrom111And711: """Tests for meeting (111__/711__ rule) - builds meeting:meeting custom field."""