From ffa38cc3eb406f1427dcb7b876ff816d99d67a84 Mon Sep 17 00:00:00 2001 From: Anikesh Suresh Date: Thu, 28 Nov 2024 13:35:45 +0000 Subject: [PATCH] Implement unit repo tests #38 --- object_storage_api/core/exceptions.py | 23 ++++- object_storage_api/repositories/image.py | 8 +- object_storage_api/stores/image.py | 11 ++- test/unit/repositories/conftest.py | 14 +++ test/unit/repositories/test_image.py | 107 +++++++++++++++++++++++ 5 files changed, 155 insertions(+), 8 deletions(-) diff --git a/object_storage_api/core/exceptions.py b/object_storage_api/core/exceptions.py index 1acfd98..8cc3c49 100644 --- a/object_storage_api/core/exceptions.py +++ b/object_storage_api/core/exceptions.py @@ -19,17 +19,21 @@ class BaseAPIException(Exception): detail: str - def __init__(self, detail: str): + def __init__(self, detail: str, response_detail: str = None): """ Initialise the exception. :param detail: Specific detail of the exception (just like Exception would take - this will only be logged and not returned in a response). + :param response_detail: Generic detail of the exception that will be returned in a response. """ super().__init__(detail) self.detail = detail + if response_detail is not None: + self.response_detail = response_detail + class DatabaseError(BaseAPIException): """ @@ -60,5 +64,18 @@ class MissingRecordError(DatabaseError): A specific database record was requested but could not be found. """ - status_code = 422 - response_detail = "Requested Record was not found" + status_code = 404 + response_detail = "Requested record was not found" + + def __init__(self, detail: str, entity_name: str = None): + """ + Initialise the exception. + + :param detail: Specific detail of the exception (just like Exception would take - this will only be logged + and not returned in a response). + :param entity_name: Name of the entity to include in the response. + """ + super().__init__(detail) + + if entity_name is not None: + self.response_detail = f"{entity_name.capitalize()} not found" diff --git a/object_storage_api/repositories/image.py b/object_storage_api/repositories/image.py index f209baf..3aee4ac 100644 --- a/object_storage_api/repositories/image.py +++ b/object_storage_api/repositories/image.py @@ -87,13 +87,13 @@ def list(self, entity_id: Optional[str], primary: Optional[bool], session: Clien def delete(self, image_id: str, session: ClientSession = None) -> str: try: new_image_id = CustomObjectId(image_id) - except InvalidObjectIdError as exc: - exc.response_detail = f"Invalid image_id given: {image_id}" - raise exc + except InvalidObjectIdError as e: + exc = MissingRecordError(f"Invalid image_id given: {image_id}", "image") + raise exc from e response = self._images_collection.find_one_and_delete( filter={"_id": new_image_id}, projection={"object_key": True}, session=session ) if response is None: - exc = MissingRecordError(f"Requested Image was not found: Image ID {image_id}") + exc = MissingRecordError(f"Requested Image was not found: {image_id}", "image") raise exc return response["object_key"] diff --git a/object_storage_api/stores/image.py b/object_storage_api/stores/image.py index 4df3e5f..50f2d87 100644 --- a/object_storage_api/stores/image.py +++ b/object_storage_api/stores/image.py @@ -39,7 +39,16 @@ def upload(self, image_id: str, image_metadata: ImagePostMetadataSchema, upload_ return object_key def delete(self, object_key: str) -> None: - s3_client.delete_object( + res = s3_client.delete_object( Bucket=object_storage_config.bucket_name.get_secret_value(), Key=object_key, ) + + print(res) + + response = s3_client.delete_object( + Bucket=object_storage_config.bucket_name.get_secret_value(), + Key="images/14295f7n1029435f7h3201945h/asdfu7u9081v23g4578gh1", + ) + + print(response) diff --git a/test/unit/repositories/conftest.py b/test/unit/repositories/conftest.py index d8a2874..939cd97 100644 --- a/test/unit/repositories/conftest.py +++ b/test/unit/repositories/conftest.py @@ -77,3 +77,17 @@ def mock_find(collection_mock: Mock, documents: List[dict]) -> None: cursor_mock = MagicMock(Cursor) cursor_mock.__iter__.return_value = iter(documents) collection_mock.find.return_value = cursor_mock + + def mock_find_one_and_delete(collection_mock: Mock, document: dict | None) -> None: + """ + Mocks the `find_one` method of the MongoDB database collection mock to return a specific document. + + :param collection_mock: Mocked MongoDB database collection instance. + :param document: The document to be returned by the `find_one` method. + """ + if collection_mock.find_one_and_delete.side_effect is None: + collection_mock.find_one_and_delete.side_effect = [document] + else: + documents = list(collection_mock.find_one_and_delete.side_effect) + documents.append(document) + collection_mock.find_one_and_delete.side_effect = documents diff --git a/test/unit/repositories/test_image.py b/test/unit/repositories/test_image.py index c50fdcc..3630a8c 100644 --- a/test/unit/repositories/test_image.py +++ b/test/unit/repositories/test_image.py @@ -10,6 +10,7 @@ import pytest from bson import ObjectId +from object_storage_api.core.exceptions import MissingRecordError from object_storage_api.models.image import ImageIn, ImageOut from object_storage_api.repositories.image import ImageRepo @@ -167,3 +168,109 @@ def test_list_with_primary_and_entity_id(self): self.mock_list([IMAGE_IN_DATA_ALL_VALUES]) self.call_list(primary=True, entity_id=IMAGE_IN_DATA_ALL_VALUES["entity_id"]) self.check_list_success() + + +class DeleteDSL(ImageRepoDSL): + """Base class for `delete` tests.""" + + _delete_image_id: str + _delete_exception: pytest.ExceptionInfo + _expected_image_out: ImageOut + _obtained_image_out: ImageOut + _expected_object_key: str + _obtained_object_key: str + + def mock_delete(self, image_id: str, image_in_data: dict = None) -> None: + """ + Mocks database methods appropriately to test the `delete` repo method. + + :param deleted_count: Number of documents deleted successfully. + """ + if image_in_data: + image_in_data["id"] = image_id + self._expected_image_out = ImageOut(**ImageIn(**image_in_data).model_dump()) if image_in_data else None + RepositoryTestHelpers.mock_find_one_and_delete( + self.images_collection, self._expected_image_out.model_dump() if image_in_data else None + ) + if self._expected_image_out: + self._expected_object_key = self._expected_image_out.object_key + + def call_delete(self, image_id: str) -> None: + """ + Calls the `ImageRepo` `delete` method. + + :param image_id: ID of the image to be deleted. + """ + + self._delete_image_id = image_id + self._obtained_object_key = self.image_repository.delete(image_id, session=self.mock_session) + + def call_delete_expecting_error(self, image_id: str, error_type: type[BaseException]) -> None: + """ + Calls the `ImageRepo` `delete` method while expecting an error to be raised. + + :param image_id: ID of the image to be deleted. + :param error_type: Expected exception to be raised. + """ + + self._delete_image_id = image_id + with pytest.raises(error_type) as exc: + self.image_repository.delete(image_id, session=self.mock_session) + self._delete_exception = exc + + def check_delete_success(self) -> None: + """Checks that a prior call to `call_delete` worked as expected.""" + + self.images_collection.find_one_and_delete.assert_called_once_with( + filter={"_id": ObjectId(self._delete_image_id)}, projection={"object_key": True}, session=self.mock_session + ) + self._obtained_object_key = self._expected_object_key + + def check_delete_failed_with_exception(self, message: str, assert_delete: bool = False) -> None: + """ + Checks that a prior call to `call_delete_expecting_error` worked as expected, raising an exception + with the correct message. + + :param message: Expected message of the raised exception. + :param assert_delete: Whether the `delete_one` method is expected to be called or not. + """ + + if not assert_delete: + self.images_collection.find_one_and_delete.assert_not_called() + else: + self.images_collection.find_one_and_delete.assert_called_once_with( + filter={"_id": ObjectId(self._delete_image_id)}, + projection={"object_key": True}, + session=self.mock_session, + ) + + assert str(self._delete_exception.value) == message + + +class TestDelete(DeleteDSL): + """Tests for deleting an image.""" + + def test_delete(self): + """Test deleting an image.""" + image_id = str(ObjectId()) + + self.mock_delete(image_id, IMAGE_IN_DATA_ALL_VALUES) + self.call_delete(image_id) + self.check_delete_success() + + def test_delete_non_existent_id(self): + """Test deleting an image with a non-existent ID.""" + + image_id = str(ObjectId()) + + self.mock_delete(image_id, None) + self.call_delete_expecting_error(image_id, MissingRecordError) + self.check_delete_failed_with_exception(f"Requested Image was not found: {image_id}", assert_delete=True) + + def test_delete_invalid_id(self): + """Test deleting an image with an invalid ID.""" + + image_id = "invalid-id" + + self.call_delete_expecting_error(image_id, MissingRecordError) + self.check_delete_failed_with_exception(f"Invalid image_id given: {image_id}")