From 8769d7c9e6929783b00f6881647fee6865bf7dee Mon Sep 17 00:00:00 2001 From: Mohammed Taha Khan Date: Tue, 20 Oct 2026 15:30:17 +0200 Subject: [PATCH] fix(harvester): clearer error when match hits a tombstoned record --- .../cds_rdm/inspire_harvester/load/matcher.py | 6 +++ site/cds_rdm/inspire_harvester/writer.py | 9 ++++- site/tests/inspire_harvester/test_matcher.py | 37 +++++++++++++++++-- site/tests/inspire_harvester/test_writer.py | 22 +++++++++++ 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/site/cds_rdm/inspire_harvester/load/matcher.py b/site/cds_rdm/inspire_harvester/load/matcher.py index 6d29e815..8dc81801 100644 --- a/site/cds_rdm/inspire_harvester/load/matcher.py +++ b/site/cds_rdm/inspire_harvester/load/matcher.py @@ -249,6 +249,12 @@ def match_migrated(self, entry, logger): record = current_rdm_records_service.read_latest( self.identity, id_=parent_pid.pid_value ) + if record._record.deletion_status.is_deleted: + raise WriterError( + "Matched CDS-RDM record is tombstoned - update the " + "CDSRDM id on INSPIRE to the live record. | details: " + f"cds_id={record.id}, legacy_recid={cds_id}" + ) logger.debug(f"Found record by legacy recid {cds_id}: {record.id}") if record.id not in matched_ids: matched_ids.append(record.id) diff --git a/site/cds_rdm/inspire_harvester/writer.py b/site/cds_rdm/inspire_harvester/writer.py index ce759077..ac227ae4 100644 --- a/site/cds_rdm/inspire_harvester/writer.py +++ b/site/cds_rdm/inspire_harvester/writer.py @@ -13,6 +13,7 @@ from invenio_access.permissions import system_identity from invenio_access.utils import get_identity from invenio_rdm_records.proxies import current_rdm_records_service +from invenio_rdm_records.services.errors import RecordDeletedException from invenio_users_resources.proxies import current_users_service from invenio_vocabularies.datastreams.errors import WriterError from invenio_vocabularies.datastreams.writers import BaseWriter @@ -141,7 +142,13 @@ def _update_record( """Return True if published, None if skipped or validation failed.""" entry = {k: v for k, v in stream_entry.entry.items() if k != "_inspire_ctx"} ctx = stream_entry.entry["_inspire_ctx"] - record = current_rdm_records_service.read(self.identity, record_pid) + try: + record = current_rdm_records_service.read(self.identity, record_pid) + except RecordDeletedException: + raise WriterError( + "Matched CDS-RDM record is tombstoned - update the CDSRDM id " + f"on INSPIRE to the live record. | details: cds_id={record_pid}" + ) record_dict = record.to_dict() errors = self.record_validator.validate( mode="update", diff --git a/site/tests/inspire_harvester/test_matcher.py b/site/tests/inspire_harvester/test_matcher.py index c6fd4df4..86346e1b 100644 --- a/site/tests/inspire_harvester/test_matcher.py +++ b/site/tests/inspire_harvester/test_matcher.py @@ -161,7 +161,9 @@ def test_matcher_finds_record_by_legacy_recid( logger = Mock() stream_entry = legacy_entry("2765541") mock_get_pid.return_value = Mock(pid_value="parent-1") - mock_read_latest.return_value = Mock(id="rec-1") + mock_read_latest.return_value = Mock( + id="rec-1", _record=Mock(deletion_status=Mock(is_deleted=False)) + ) result = matcher.match(stream_entry, inspire_id="111", logger=logger) @@ -169,6 +171,30 @@ def test_matcher_finds_record_by_legacy_recid( assert result.record_pid == "rec-1" +@patch("cds_rdm.inspire_harvester.load.matcher.current_rdm_records_service.read_latest") +@patch("cds_rdm.inspire_harvester.load.matcher.get_pid_by_legacy_recid") +def test_matcher_errors_when_legacy_recid_hits_tombstoned_record( + mock_get_pid, mock_read_latest, running_app +): + """Tombstoned lrecid match should tell curators to fix the CDSRDM id on INSPIRE.""" + matcher = RecordMatcher() + logger = Mock() + stream_entry = legacy_entry("2689813") + mock_get_pid.return_value = Mock(pid_value="parent-1") + mock_read_latest.return_value = Mock( + id="xcmbd-mvr19", + _record=Mock(deletion_status=Mock(is_deleted=True)), + ) + + with pytest.raises(WriterError, match="tombstoned") as exc_info: + matcher.match(stream_entry, inspire_id="1743143", logger=logger) + + message = str(exc_info.value) + assert "update the CDSRDM id on INSPIRE" in message + assert "cds_id=xcmbd-mvr19" in message + assert "legacy_recid=2689813" in message + + @patch("cds_rdm.inspire_harvester.load.matcher.current_rdm_records_service.read_latest") @patch("cds_rdm.inspire_harvester.load.matcher.get_pid_by_legacy_recid") def test_matcher_tries_each_legacy_cds_id_until_pidstore_hits( @@ -179,7 +205,9 @@ def test_matcher_tries_each_legacy_cds_id_until_pidstore_hits( logger = Mock() stream_entry = legacy_entry("2798711", "2765541") mock_get_pid.side_effect = [NoResultFound(), Mock(pid_value="parent-1")] - mock_read_latest.return_value = Mock(id="rec-1") + mock_read_latest.return_value = Mock( + id="rec-1", _record=Mock(deletion_status=Mock(is_deleted=False)) + ) result = matcher.match(stream_entry, inspire_id="111", logger=logger) @@ -199,7 +227,10 @@ def test_matcher_is_ambiguous_when_legacy_ids_hit_different_records( logger = Mock() stream_entry = legacy_entry("2765541", "2798711") mock_get_pid.side_effect = [Mock(pid_value="parent-1"), Mock(pid_value="parent-2")] - mock_read_latest.side_effect = [Mock(id="rec-1"), Mock(id="rec-2")] + mock_read_latest.side_effect = [ + Mock(id="rec-1", _record=Mock(deletion_status=Mock(is_deleted=False))), + Mock(id="rec-2", _record=Mock(deletion_status=Mock(is_deleted=False))), + ] result = matcher.match(stream_entry, inspire_id="111", logger=logger) diff --git a/site/tests/inspire_harvester/test_writer.py b/site/tests/inspire_harvester/test_writer.py index d1951a79..e790dab3 100644 --- a/site/tests/inspire_harvester/test_writer.py +++ b/site/tests/inspire_harvester/test_writer.py @@ -14,6 +14,7 @@ from invenio_access.permissions import system_identity from invenio_rdm_records.proxies import current_rdm_records, current_rdm_records_service from invenio_rdm_records.records.api import RDMRecord +from invenio_rdm_records.services.errors import RecordDeletedException from invenio_vocabularies.datastreams import StreamEntry from invenio_vocabularies.datastreams.errors import WriterError @@ -258,6 +259,27 @@ def test_writer_errors_when_legacy_recid_is_unknown(running_app, scientific_comm assert "was not found in the old CDS" in result.errors[0] +@patch("cds_rdm.inspire_harvester.writer.current_rdm_records_service.read") +def test_writer_errors_when_matched_record_is_tombstoned( + mock_read, running_app, scientific_community +): + """Reading a tombstoned match should yield a curator-facing harvest error.""" + writer = InspireWriter() + writer.matcher.match = Mock( + return_value=MatchResult(found=True, record_pid="xcmbd-mvr19") + ) + mock_read.side_effect = RecordDeletedException(Mock(id="xcmbd-mvr19")) + writer._create_record = Mock() + + result = writer.write(legacy_entry("2689813")) + + writer._create_record.assert_not_called() + assert result.errors + assert "tombstoned" in result.errors[0] + assert "update the CDSRDM id on INSPIRE" in result.errors[0] + assert "cds_id=xcmbd-mvr19" in result.errors[0] + + def test_writer_errors_when_pidstore_missed_a_migrated_record( running_app, scientific_community ):