Bug/safo6 NRL-2089 Log error when delete fails (#1174)

sandyforresternhs · web-flow · commit ca3603ac90ab · 2026-04-01T13:58:24.000+01:00
diff --git a/layer/nrlf/core/dynamodb/repository.py b/layer/nrlf/core/dynamodb/repository.py
@@ -344,14 +344,18 @@ def delete_by_id(self, id_: str, can_ignore_delete_fail: bool = False):
         key = f"D#{id_}"
         try:
             self.table.delete_item(Key={"pk": key, "sk": key})
+            logger.log(LogReference.REPOSITORY027, id=id_)
         except Exception as exc:
-            if can_ignore_delete_fail:
-                logger.log(
-                    LogReference.REPOSITORY026a,
-                    exc_info=sys.exc_info(),
-                    stacklevel=5,
-                    error=str(exc),
-                )
+            logger.log(
+                (
+                    LogReference.REPOSITORY026a
+                    if can_ignore_delete_fail
+                    else LogReference.REPOSITORY026b
+                ),
+                exc_info=sys.exc_info(),
+                stacklevel=5,
+                error=str(exc),
+            )
 
     def _query(self, **kwargs) -> Iterator[DocumentPointer]:
         """
diff --git a/layer/nrlf/core/dynamodb/tests/test_repository.py b/layer/nrlf/core/dynamodb/tests/test_repository.py
@@ -1,7 +1,32 @@
+from unittest.mock import patch
+
 import pytest
+from moto import mock_aws
 
 from nrlf.core.constants import PointerTypes
-from nrlf.core.dynamodb.repository import _get_sk_ids_for_type
+from nrlf.core.dynamodb.model import DocumentPointer
+from nrlf.core.dynamodb.repository import (
+    DocumentPointerRepository,
+    _get_sk_ids_for_type,
+)
+from nrlf.core.log_references import LogReference
+from nrlf.tests.data import load_document_reference
+from nrlf.tests.dynamodb import mock_repository
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def make_document_pointer(id: str = "Y05868-99999-99999-999999") -> DocumentPointer:
+    doc_ref = load_document_reference("Y05868-736253002-Valid")
+    doc_ref.id = id
+    return DocumentPointer.from_document_reference(doc_ref)
+
+
+# ---------------------------------------------------------------------------
+# _get_sk_ids_for_type
+# ---------------------------------------------------------------------------
 
 
 def test_get_sk_ids_for_type_exception_thrown_for_invalid_type():
@@ -30,4 +55,89 @@ def test_get_sk_ids_for_type_exception_thrown_if_new_type_has_no_category():
     )
 
 
-# TODO: Add unit tests for Repository Class
+# ---------------------------------------------------------------------------
+# DocumentPointerRepository.supersede
+# ---------------------------------------------------------------------------
+
+
+@mock_aws
+@mock_repository
+def test_supersede_creates_new_and_deletes_old(repository: DocumentPointerRepository):
+    old_doc = make_document_pointer(id="Y05868-OLD0001")
+    repository.create(old_doc)
+    assert repository.get_by_id("Y05868-OLD0001") is not None
+
+    new_doc = make_document_pointer(id="Y05868-NEW0001")
+    result = repository.supersede(new_doc, ids_to_delete=["Y05868-OLD0001"])
+
+    assert result.id == "Y05868-NEW0001"
+    assert repository.get_by_id("Y05868-NEW0001") is not None
+    assert repository.get_by_id("Y05868-OLD0001") is None
+
+
+@mock_aws
+@mock_repository
+@patch("nrlf.core.dynamodb.repository.logger")
+def test_supersede_with_can_ignore_delete_fail(
+    mock_logger,
+    repository: DocumentPointerRepository,
+):
+    new_doc = make_document_pointer(id="Y05868-NEW0003")
+
+    with patch.object(
+        repository.table,
+        "delete_item",
+        side_effect=Exception("simulated delete failure"),
+    ):
+        result = repository.supersede(
+            new_doc,
+            ids_to_delete=["Y05868-NONEXISTENT"],
+            can_ignore_delete_fail=True,
+        )
+
+    assert result.id == "Y05868-NEW0003"
+    log_codes = [c.args[0] for c in mock_logger.log.call_args_list]
+    assert LogReference.REPOSITORY026a in log_codes
+
+
+# ---------------------------------------------------------------------------
+# DocumentPointerRepository.delete_by_id
+# ---------------------------------------------------------------------------
+
+
+@mock_aws
+@mock_repository
+def test_delete_by_id_removes_document_pointer(repository: DocumentPointerRepository):
+    doc = make_document_pointer()
+    repository.create(doc)
+    assert repository.get_by_id(doc.id) is not None
+
+    repository.delete_by_id(doc.id)
+
+    assert repository.get_by_id(doc.id) is None
+
+
+@pytest.mark.parametrize(
+    "ignore_delete_fail, log_reference",
+    [(True, LogReference.REPOSITORY026a), (False, LogReference.REPOSITORY026b)],
+)
+@mock_aws
+@mock_repository
+@patch("nrlf.core.dynamodb.repository.logger")
+def test_delete_by_id_logs_correct_reference_based_on_can_ignore_delete_fail(
+    mock_logger,
+    repository: DocumentPointerRepository,
+    ignore_delete_fail,
+    log_reference,
+):
+    with patch.object(
+        repository.table,
+        "delete_item",
+        side_effect=Exception("simulated delete failure"),
+    ):
+        repository.delete_by_id(
+            "Y05868-NONEXISTENT", can_ignore_delete_fail=ignore_delete_fail
+        )
+
+    log_codes = [c.args[0] for c in mock_logger.log.call_args_list]
+    assert log_reference in log_codes
diff --git a/layer/nrlf/core/log_references.py b/layer/nrlf/core/log_references.py
@@ -151,6 +151,9 @@ class LogReference(Enum):
     REPOSITORY026a = _Reference(
         "EXCEPTION", "Ignoring failure to delete resource from DynamoDB"
     )
+    REPOSITORY026b = _Reference(
+        "EXCEPTION", "Failed to delete superseded resource from DynamoDB"
+    )
     REPOSITORY027 = _Reference("INFO", "Successfully deleted item from DynamoDB")
     REPOSITORY028 = _Reference("INFO", "Received page of search results")
     REPOSITORY028a = _Reference("DEBUG", "Received page of search results with result")
diff --git a/layer/nrlf/tests/dynamodb.py b/layer/nrlf/tests/dynamodb.py
@@ -1,3 +1,6 @@
+import functools
+import inspect
+
 from nrlf.core.boto import get_dynamodb_resource
 from nrlf.core.config import Config
 from nrlf.core.dynamodb.repository import DocumentPointerRepository
@@ -40,6 +43,7 @@ def create_document_pointer_table(config: Config, dynamodb: DynamoDBServiceResou
 
 
 def mock_repository(func):
+    @functools.wraps(func)
     def wrapped_function(*args, **kwargs):
         config = Config()
         dynamodb = get_dynamodb_resource()
@@ -49,4 +53,12 @@ def wrapped_function(*args, **kwargs):
 
         return func(*args, **kwargs, repository=repository)
 
+    # Remove 'repository' from the visible signature so pytest doesn't try
+    # to inject it as a test parameter (it's already injected by this decorator).
+    sig = inspect.signature(func)
+    params_minus_repository = [
+        p for name, p in sig.parameters.items() if name != "repository"
+    ]
+    wrapped_function.__signature__ = sig.replace(parameters=params_minus_repository)
+
     return wrapped_function
