From c4cce386e44f32ae368edee07525e93d8392d12e Mon Sep 17 00:00:00 2001 From: Mohammed Taha Khan Date: Mon, 10 Aug 2026 09:11:47 +0200 Subject: [PATCH] fix(harvester): keep first or CDS DOI and warn on extras --- .../transform/mappers/identifiers.py | 23 ++++--- .../inspire_harvester/test_harvester_job.py | 9 +-- .../inspire_harvester/test_transformer.py | 60 ++++++++++++++++--- 3 files changed, 70 insertions(+), 22 deletions(-) diff --git a/site/cds_rdm/inspire_harvester/transform/mappers/identifiers.py b/site/cds_rdm/inspire_harvester/transform/mappers/identifiers.py index dcc58425..7611e9a0 100644 --- a/site/cds_rdm/inspire_harvester/transform/mappers/identifiers.py +++ b/site/cds_rdm/inspire_harvester/transform/mappers/identifiers.py @@ -75,7 +75,7 @@ def map_value(self, src_record, ctx, logger): Prefer a CDS/DataCite-prefix DOI as the main PID. Any other DOIs become related identifiers (via ctx.extra_related_dois). Multiple non-CDS DOIs - with no CDS DOI remain an error. + use the first as main and the rest as related (with a warning). """ src_metadata = src_record.get("metadata", {}) DATACITE_PREFIX = current_app.config["DATACITE_PREFIX"] @@ -117,12 +117,16 @@ def map_value(self, src_record, ctx, logger): if cds_dois: main = cds_dois[0] extras = other_dois - elif len(other_dois) > 1: - ctx.errors.append("More than 1 DOI was found.") - return None - elif len(other_dois) == 1: + elif other_dois: main = other_dois[0] - extras = [] + extras = other_dois[1:] + if extras: + logger.warning( + "Multiple DOIs found; using one as main and others as " + "related identifiers. " + f"| details: main={main.get('value')}, " + f"related={[e.get('value') for e in extras]}" + ) else: return None @@ -286,12 +290,17 @@ def map_value(self, src_record, ctx, logger): } ) + has_cds_doi = any( + d.get("value", "").startswith(current_app.config["DATACITE_PREFIX"]) + for d in src_metadata.get("dois", []) + ) + extra_rel = "isversionof" if has_cds_doi else "isvariantformof" for doi in ctx.extra_related_dois: identifiers.append( { "identifier": doi, "scheme": "doi", - "relation_type": {"id": "isversionof"}, + "relation_type": {"id": extra_rel}, "resource_type": {"id": "publication-other"}, } ) diff --git a/site/tests/inspire_harvester/test_harvester_job.py b/site/tests/inspire_harvester/test_harvester_job.py index ad17050e..b794ee3c 100644 --- a/site/tests/inspire_harvester/test_harvester_job.py +++ b/site/tests/inspire_harvester/test_harvester_job.py @@ -10,9 +10,7 @@ from pathlib import Path from urllib.parse import parse_qs, urlparse -import pytest from invenio_access.permissions import system_identity -from invenio_jobs.errors import TaskExecutionPartialError from invenio_rdm_records.proxies import current_rdm_records_service from invenio_rdm_records.records.api import RDMRecord @@ -412,14 +410,13 @@ def mock_requests_get_pagination( content = json.load(f) return mock_requests_get(url, mock_content=content) - with pytest.raises(TaskExecutionPartialError) as e: - run_harvester_mock(ds_config, mock_requests_get_pagination) + run_harvester_mock(ds_config, mock_requests_get_pagination) RDMRecord.index.refresh() created_records = current_rdm_records_service.search(system_identity) - # 14/15 - one record with multiple DOIS will raise an error - assert created_records.total == 14 + # All 15 records create successfully (multi-DOI records keep one as main PID) + assert created_records.total == 15 created_record1 = current_rdm_records_service.search( system_identity, diff --git a/site/tests/inspire_harvester/test_transformer.py b/site/tests/inspire_harvester/test_transformer.py index 59f74d5f..46dc43d8 100644 --- a/site/tests/inspire_harvester/test_transformer.py +++ b/site/tests/inspire_harvester/test_transformer.py @@ -345,8 +345,10 @@ def test_transform_dois_invalid(mock_is_doi, running_app): assert len(ctx.errors) == 1 -def test_transform_dois_multiple(running_app): - """Test DOIMapper errors when multiple non-CDS DOIs and no CDS DOI.""" +@patch("cds_rdm.inspire_harvester.transform.mappers.identifiers.is_doi") +def test_transform_dois_multiple(mock_is_doi, running_app): + """First non-CDS DOI is main; remaining DOIs become related with a warning.""" + mock_is_doi.return_value = True src_metadata = { "dois": [ {"value": "10.1000/test1"}, @@ -361,15 +363,20 @@ def test_transform_dois_multiple(running_app): mapper = DOIMapper() src_record = {"metadata": src_metadata, "created": "2023-01-01"} - result = mapper.map_value(src_record, ctx, logger) - assert result is None - assert len(ctx.errors) == 1 - assert "More than 1 DOI was found." in ctx.errors[0] + with patch.object(logger, "warning") as mock_warning: + result = mapper.map_value(src_record, ctx, logger) + + assert result["doi"]["identifier"] == "10.1000/test1" + assert result["doi"]["provider"] == "external" + assert ctx.extra_related_dois == ["10.1000/test2"] + assert not ctx.errors + mock_warning.assert_called_once() + assert "Multiple DOIs found" in mock_warning.call_args[0][0] @patch("cds_rdm.inspire_harvester.transform.mappers.identifiers.is_doi") def test_transform_dois_cds_with_external(mock_is_doi, running_app): - """CDS DOI becomes main PID; other DOIs go to related identifiers.""" + """CDS DOI becomes main PID; other DOIs go to related identifiers without warning.""" mock_is_doi.return_value = True src_metadata = { "dois": [ @@ -384,12 +391,14 @@ def test_transform_dois_cds_with_external(mock_is_doi, running_app): mapper = DOIMapper() src_record = {"metadata": src_metadata, "created": "2023-01-01"} - result = mapper.map_value(src_record, ctx, logger) + with patch.object(logger, "warning") as mock_warning: + result = mapper.map_value(src_record, ctx, logger) assert result["doi"]["identifier"] == "10.17181/cds-doi" assert result["doi"]["provider"] == "datacite" assert ctx.extra_related_dois == ["10.1000/external"] assert not ctx.errors + mock_warning.assert_not_called() @patch("cds_rdm.inspire_harvester.transform.mappers.identifiers.is_doi") @@ -402,7 +411,40 @@ def test_transform_related_identifiers_includes_extra_dois(mock_is_doi, running_ ctx.extra_related_dois.append("10.1000/external") logger = Logger(inspire_id="12345") mapper = RelatedIdentifiersMapper() - src_record = {"metadata": {}, "created": "2023-01-01"} + src_record = { + "metadata": {"dois": [{"value": "10.1000/test1"}, {"value": "10.1000/external"}]}, + "created": "2023-01-01", + } + + result = mapper.map_value(src_record, ctx, logger) + + assert { + "identifier": "10.1000/external", + "scheme": "doi", + "relation_type": {"id": "isvariantformof"}, + "resource_type": {"id": "publication-other"}, + } in result + + +@patch("cds_rdm.inspire_harvester.transform.mappers.identifiers.is_doi") +def test_transform_related_identifiers_extra_dois_cds_main(mock_is_doi, running_app): + """Extra DOIs use isversionof when a CDS DOI is present on the source.""" + mock_is_doi.return_value = True + ctx = MetadataSerializationContext( + resource_type=ResourceType.OTHER, inspire_id="12345" + ) + ctx.extra_related_dois.append("10.1000/external") + logger = Logger(inspire_id="12345") + mapper = RelatedIdentifiersMapper() + src_record = { + "metadata": { + "dois": [ + {"value": "10.1000/external"}, + {"value": "10.17181/cds-doi"}, + ] + }, + "created": "2023-01-01", + } result = mapper.map_value(src_record, ctx, logger)