diff --git a/layer/nrlf/core/dynamodb/repository.py b/layer/nrlf/core/dynamodb/repository.py index 26a36deef..f9a2414e0 100644 --- 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 index 75a8b605d..15c201d07 100644 --- 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 index 34152a78e..cf2dea3ea 100644 --- 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 index 233185d73..df027b131 100644 --- 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