diff --git a/object_storage_api/repositories/image.py b/object_storage_api/repositories/image.py index be2081a..98d5d90 100644 --- a/object_storage_api/repositories/image.py +++ b/object_storage_api/repositories/image.py @@ -96,6 +96,29 @@ def list(self, entity_id: Optional[str], primary: Optional[bool], session: Clien images = self._images_collection.find(query, session=session) return [ImageOut(**image) for image in images] + def update(self, image_id: str, image: ImageIn, session: ClientSession = None) -> ImageOut: + """ + Updates an image from a MongoDB database. + + :param image_id: The ID of the image to update. + :param image: The image containing the update data. + :param session: PyMongo ClientSession to use for database operations. + :return: The updated image. + :raises InvalidObjectIdError: If the supplied `image_id` is invalid. + """ + + logger.info("Updating image metadata with ID: %s", image_id) + try: + image_id = CustomObjectId(image_id) + self._images_collection.update_one( + {"_id": image_id}, {"$set": image.model_dump(by_alias=True)}, session=session + ) + except InvalidObjectIdError as exc: + exc.status_code = 404 + exc.response_detail = "Image not found" + raise exc + return self.get(image_id=str(image_id), session=session) + def delete(self, image_id: str, session: ClientSession = None) -> None: """ Delete an image by its ID from a MongoDB database. diff --git a/object_storage_api/routers/image.py b/object_storage_api/routers/image.py index 4ed6e6e..5a5f5ff 100644 --- a/object_storage_api/routers/image.py +++ b/object_storage_api/routers/image.py @@ -8,7 +8,12 @@ from fastapi import APIRouter, Depends, File, Form, Path, Query, UploadFile, status -from object_storage_api.schemas.image import ImageMetadataSchema, ImagePostMetadataSchema, ImageSchema +from object_storage_api.schemas.image import ( + ImageMetadataSchema, + ImagePatchMetadataSchema, + ImagePostMetadataSchema, + ImageSchema, +) from object_storage_api.services.image import ImageService logger = logging.getLogger() @@ -80,6 +85,23 @@ def get_image( return image_service.get(image_id) +@router.patch( + path="/{image_id}", + summary="Update an image partially by ID", + response_description="Image updated successfully", +) +def partial_update_image( + image: ImagePatchMetadataSchema, + image_id: Annotated[str, Path(description="ID of the image to update")], + image_service: ImageServiceDep, +) -> ImageMetadataSchema: + # pylint: disable=missing-function-docstring + logger.info("Partially updating image with ID: %s", image_id) + logger.debug("Image data: %s", image) + + return image_service.update(image_id, image) + + @router.delete( path="/{image_id}", summary="Delete an image by ID", diff --git a/object_storage_api/schemas/image.py b/object_storage_api/schemas/image.py index 2eea290..e27fefb 100644 --- a/object_storage_api/schemas/image.py +++ b/object_storage_api/schemas/image.py @@ -9,12 +9,20 @@ from object_storage_api.schemas.mixins import CreatedModifiedSchemaMixin +class ImagePatchMetadataSchema(BaseModel): + """Base schema model for an image.""" + + title: Optional[str] = Field(default=None, description="Title of the image") + description: Optional[str] = Field(default=None, description="Description of the image") + file_name: Optional[str] = Field(default=None, description="File name of the image") + + class ImagePostMetadataSchema(BaseModel): """Base schema model for an image.""" - entity_id: str = Field(description="ID of the entity the image relates to") title: Optional[str] = Field(default=None, description="Title of the image") description: Optional[str] = Field(default=None, description="Description of the image") + entity_id: str = Field(description="ID of the entity the image relates to") class ImageMetadataSchema(CreatedModifiedSchemaMixin, ImagePostMetadataSchema): diff --git a/object_storage_api/services/image.py b/object_storage_api/services/image.py index f9906d9..1104152 100644 --- a/object_storage_api/services/image.py +++ b/object_storage_api/services/image.py @@ -13,7 +13,12 @@ from object_storage_api.core.image import generate_thumbnail_base64_str from object_storage_api.models.image import ImageIn from object_storage_api.repositories.image import ImageRepo -from object_storage_api.schemas.image import ImageMetadataSchema, ImagePostMetadataSchema, ImageSchema +from object_storage_api.schemas.image import ( + ImageMetadataSchema, + ImagePatchMetadataSchema, + ImagePostMetadataSchema, + ImageSchema, +) from object_storage_api.stores.image import ImageStore logger = logging.getLogger() @@ -97,6 +102,22 @@ def list(self, entity_id: Optional[str] = None, primary: Optional[bool] = None) images = self._image_repository.list(entity_id, primary) return [ImageMetadataSchema(**image.model_dump()) for image in images] + def update(self, image_id: str, image: ImagePatchMetadataSchema) -> ImageMetadataSchema: + """ + Update an image based on its ID. + + :param image_id: The ID of the image to update. + :param image: The image containing the fields to be updated. + :return: The updated image. + """ + stored_image = self._image_repository.get(image_id=image_id) + update_data = image.model_dump(exclude_unset=True) + updated_image = self._image_repository.update( + image_id=image_id, image=ImageIn(**{**stored_image.model_dump(), **update_data}) + ) + + return ImageMetadataSchema(**updated_image.model_dump()) + def delete(self, image_id: str) -> None: """ Delete an image by its ID. diff --git a/test/e2e/test_image.py b/test/e2e/test_image.py index 274ca95..f52aa93 100644 --- a/test/e2e/test_image.py +++ b/test/e2e/test_image.py @@ -4,8 +4,10 @@ from test.mock_data import ( IMAGE_GET_DATA_ALL_VALUES, - IMAGE_GET_METADATA_ALL_VALUES, + IMAGE_GET_METADATA_ALL_VALUES_A, + IMAGE_GET_METADATA_ALL_VALUES_B, IMAGE_GET_METADATA_REQUIRED_VALUES_ONLY, + IMAGE_PATCH_METADATA_DATA_ALL_VALUES_B, IMAGE_POST_METADATA_DATA_ALL_VALUES, IMAGE_POST_METADATA_DATA_REQUIRED_VALUES_ONLY, ) @@ -84,7 +86,7 @@ def test_create_with_all_values_provided(self): """Test creating an image with all values provided.""" self.post_image(IMAGE_POST_METADATA_DATA_ALL_VALUES, "image.jpg") - self.check_post_image_success(IMAGE_GET_METADATA_ALL_VALUES) + self.check_post_image_success(IMAGE_GET_METADATA_ALL_VALUES_A) def test_create_with_invalid_entity_id(self): """Test creating an image with an invalid `entity_id`.""" @@ -193,17 +195,17 @@ def post_test_images(self) -> list[dict]: return [ { - **IMAGE_GET_METADATA_ALL_VALUES, + **IMAGE_GET_METADATA_ALL_VALUES_A, "entity_id": entity_id_a, "id": image_a_id, }, { - **IMAGE_GET_METADATA_ALL_VALUES, + **IMAGE_GET_METADATA_ALL_VALUES_A, "entity_id": entity_id_a, "id": image_b_id, }, { - **IMAGE_GET_METADATA_ALL_VALUES, + **IMAGE_GET_METADATA_ALL_VALUES_A, "entity_id": entity_id_b, "id": image_c_id, }, @@ -307,6 +309,62 @@ def test_list_with_invalid_primary_filter(self): ) +class UpdateDSL(ListDSL): + """Base class for update tests.""" + + _patch_response_image: Response + + def patch_image(self, image_id: str, image_patch_data: dict) -> None: + """ + Patches an image with the given ID. + + :param image_id: ID of the image to be updated. + :param image_patch_data: Dictionary containing the image patch data as would be required for an + `ImagePatchSchema`. + """ + self._patch_response_image = self.test_client.patch(f"/images/{image_id}", json=image_patch_data) + + def check_patch_image_success(self, expected_image_get_data: dict) -> None: + """ + Checks that a prior call to `patch_image` gave a successful response with the expected data returned. + + :param expected_image_get_data: Dictionaries containing the expected image data as would be + required for an `ImageMetadataSchema`. + """ + assert self._patch_response_image.status_code == 200 + assert self._patch_response_image.json() == expected_image_get_data + + def check_patch_image_failed_with_detail(self, status_code: int, detail: str) -> None: + """ + Checks that prior call to `patch_image` gave a failed response with the expected code and detail. + + :param status_code: Expected status code to be returned. + :param detail: Expected detail to be returned. + """ + assert self._patch_response_image.status_code == status_code + assert self._patch_response_image.json()["detail"] == detail + + +class TestUpdate(UpdateDSL): + """Tests for updating an image.""" + + def test_partial_update_all_fields(self): + """Test updating every field of an image.""" + image_id = self.post_image(IMAGE_POST_METADATA_DATA_ALL_VALUES, "image.jpg") + self.patch_image(image_id, IMAGE_PATCH_METADATA_DATA_ALL_VALUES_B) + self.check_patch_image_success(IMAGE_GET_METADATA_ALL_VALUES_B) + + def test_partial_update_with_non_existent_id(self): + """Test updating a non-existent image.""" + self.patch_image(str(ObjectId()), {}) + self.check_patch_image_failed_with_detail(404, "Image not found") + + def test_partial_update_invalid_id(self): + """Test updating an image with an invalid ID.""" + self.patch_image("invalid-id", {}) + self.check_patch_image_failed_with_detail(404, "Image not found") + + class DeleteDSL(ListDSL): """Base class for delete tests.""" diff --git a/test/mock_data.py b/test/mock_data.py index e1ba282..7b2ce39 100644 --- a/test/mock_data.py +++ b/test/mock_data.py @@ -131,13 +131,23 @@ "url": ANY, } +IMAGE_PATCH_METADATA_DATA_ALL_VALUES_A = { + "title": "Report Title", + "description": "A damage report.", +} + +IMAGE_PATCH_METADATA_DATA_ALL_VALUES_B = { + "title": "Shattered Laser", + "description": "An image of a shattered laser.", + "file_name": "picture.jpg", +} IMAGE_POST_METADATA_DATA_ALL_VALUES = { **IMAGE_POST_METADATA_DATA_REQUIRED_VALUES_ONLY, - "title": "Report Title", - "description": "A damage report.", + **IMAGE_PATCH_METADATA_DATA_ALL_VALUES_A, } + IMAGE_IN_DATA_ALL_VALUES = { **IMAGE_POST_METADATA_DATA_ALL_VALUES, "id": str(ObjectId()), @@ -146,7 +156,7 @@ "thumbnail_base64": "UklGRjQAAABXRUJQVlA4ICgAAADQAQCdASoCAAEAAUAmJYwCdAEO/gOOAAD+qlQWHDxhNJOjVlqIb8AA", } -IMAGE_GET_METADATA_ALL_VALUES = { +IMAGE_GET_METADATA_ALL_VALUES_A = { **IMAGE_POST_METADATA_DATA_ALL_VALUES, **CREATED_MODIFIED_GET_DATA_EXPECTED, "id": ANY, @@ -155,7 +165,9 @@ "thumbnail_base64": "UklGRjQAAABXRUJQVlA4ICgAAADQAQCdASoCAAEAAUAmJYwCdAEO/gOOAAD+qlQWHDxhNJOjVlqIb8AA", } +IMAGE_GET_METADATA_ALL_VALUES_B = {**IMAGE_GET_METADATA_ALL_VALUES_A, **IMAGE_PATCH_METADATA_DATA_ALL_VALUES_B} + IMAGE_GET_DATA_ALL_VALUES = { - **IMAGE_GET_METADATA_ALL_VALUES, + **IMAGE_GET_METADATA_ALL_VALUES_A, "url": ANY, } diff --git a/test/unit/repositories/test_image.py b/test/unit/repositories/test_image.py index dfe0d41..3bcae62 100644 --- a/test/unit/repositories/test_image.py +++ b/test/unit/repositories/test_image.py @@ -277,6 +277,116 @@ def test_list_with_primary_and_entity_id(self): self.check_list_success() +# pylint: enable=duplicate-code + + +class UpdateDSL(ImageRepoDSL): + """Base class for `update` tests.""" + + _image_in: ImageIn + _expected_image_out: ImageOut + _updated_image_id: str + _updated_image: ImageOut + _update_exception: pytest.ExceptionInfo + + def set_update_data(self, new_image_in_data: dict): + """ + Assigns the update data to use during a call to `call_update`. + + :param new_image_in_data: New image data as would be required for an `ImageIn` database model to supply to the + `ImageRepo` `update` method. + """ + self._image_in = ImageIn(**new_image_in_data) + + def mock_update( + self, + new_image_in_data: dict, + ) -> None: + """ + Mocks database methods appropriately to test the `update` repo method. + + :param new_image_in_data: Dictionary containing the new image data as would be required for an `ImageIn` + database model (i.e. no created and modified times required). + """ + self.set_update_data(new_image_in_data) + + self._expected_image_out = ImageOut(**self._image_in.model_dump()) + RepositoryTestHelpers.mock_find_one(self.images_collection, self._expected_image_out.model_dump(by_alias=True)) + + def call_update(self, image_id: str) -> None: + """ + Calls the `ImageRepo` `update` method with the appropriate data from a prior call to `mock_update` + (or `set_update_data`). + + :param image_id: ID of the image to be updated. + """ + + self._updated_image_id = image_id + self._updated_image = self.image_repository.update(image_id, self._image_in, session=self.mock_session) + + def call_update_expecting_error(self, image_id: str, error_type: type[BaseException]) -> None: + """ + Calls the `ImageRepo` `update` method with the appropriate data from a prior call to `mock_update` + (or `set_update_data`) while expecting an error to be raised. + + :param image_id: ID of the image to be updated. + :param error_type: Expected exception to be raised. + """ + + with pytest.raises(error_type) as exc: + self.image_repository.update(image_id, self._image_in) + self._update_exception = exc + + def check_update_success(self) -> None: + """Checks that a prior call to `call_update` worked as expected.""" + + self.images_collection.update_one.assert_called_once_with( + { + "_id": ObjectId(self._updated_image_id), + }, + { + "$set": self._image_in.model_dump(by_alias=True), + }, + session=self.mock_session, + ) + + assert self._updated_image == self._expected_image_out + + def check_update_failed_with_exception(self, message: str) -> None: + """ + Checks that a prior call to `call_update_expecting_error` failed as expected, raising an exception + with the correct message. + + :param message: Expected message of the raised exception. + """ + + self.images_collection.update_one.assert_not_called() + + assert str(self._update_exception.value) == message + + +class TestUpdate(UpdateDSL): + """Tests for updating an image.""" + + def test_update(self): + """Test updating an image.""" + + image_id = str(ObjectId()) + + self.mock_update(IMAGE_IN_DATA_ALL_VALUES) + self.call_update(image_id) + self.check_update_success() + + def test_update_with_invalid_id(self): + """Test updating an image with an invalid ID.""" + + image_id = "invalid-id" + + self.set_update_data(IMAGE_IN_DATA_ALL_VALUES) + self.call_update_expecting_error(image_id, InvalidObjectIdError) + self.check_update_failed_with_exception("Invalid ObjectId value 'invalid-id'") + + class DeleteDSL(ImageRepoDSL): """Base class for `delete` tests.""" @@ -367,6 +477,3 @@ def test_delete_invalid_id(self): self.call_delete_expecting_error(image_id, InvalidObjectIdError) self.check_delete_failed_with_exception(f"Invalid ObjectId value '{image_id}'") - - -# pylint: enable=duplicate-code diff --git a/test/unit/services/test_image.py b/test/unit/services/test_image.py index d39649a..955afe5 100644 --- a/test/unit/services/test_image.py +++ b/test/unit/services/test_image.py @@ -2,7 +2,11 @@ Unit tests for the `ImageService` service. """ -from test.mock_data import IMAGE_IN_DATA_ALL_VALUES, IMAGE_POST_METADATA_DATA_ALL_VALUES +from test.mock_data import ( + IMAGE_IN_DATA_ALL_VALUES, + IMAGE_PATCH_METADATA_DATA_ALL_VALUES_A, + IMAGE_POST_METADATA_DATA_ALL_VALUES, +) from typing import List, Optional from unittest.mock import MagicMock, Mock, patch @@ -12,7 +16,12 @@ from object_storage_api.core.exceptions import InvalidObjectIdError from object_storage_api.models.image import ImageIn, ImageOut -from object_storage_api.schemas.image import ImageMetadataSchema, ImagePostMetadataSchema, ImageSchema +from object_storage_api.schemas.image import ( + ImageMetadataSchema, + ImagePatchMetadataSchema, + ImagePostMetadataSchema, + ImageSchema, +) from object_storage_api.services.image import ImageService @@ -249,6 +258,92 @@ def test_list(self): self.check_list_success() +class UpdateDSL(ImageServiceDSL): + """Base class for `update` tests.""" + + _stored_image: Optional[ImageOut] + _image_patch: ImagePatchMetadataSchema + _expected_image_in: ImageIn + _expected_image_out: ImageOut + _updated_image_id: str + _updated_image: MagicMock + + def mock_update(self, image_patch_data: dict, stored_image_post_data: Optional[dict]) -> None: + """ + Mocks the repository methods appropriately to test the `update` service method. + + :param image_id: ID of the image to be updated. + :param image_patch_data: Dictionary containing the patch data as would be required for an + `ImagePatchMetadataSchema` (i.e. no created and modified times required). + :param stored_image_post_data: Dictionary containing the image data for the existing stored + image as would be required for an `ImagePostMetadataSchema` (i.e. no created and modified + times required). + """ + # Stored image + self._stored_image = ( + ImageOut( + **ImageIn( + **stored_image_post_data, + ).model_dump(), + ) + if stored_image_post_data + else None + ) + self.mock_image_repository.get.return_value = self._stored_image + + # Patch schema + self._image_patch = ImagePatchMetadataSchema(**image_patch_data) + + # Construct the expected input for the repository + merged_image_data = {**(stored_image_post_data or {}), **image_patch_data} + self._expected_image_in = ImageIn(**merged_image_data) + + # Updated image + image_out = ImageOut( + **self._expected_image_in.model_dump(), + ) + + self.mock_image_repository.update.return_value = image_out + + self._expected_image_out = ImageMetadataSchema(**image_out.model_dump()) + + def call_update(self, image_id: str) -> None: + """ + Class the `ImageService` `update` method with the appropriate data from a prior call to `mock_update`. + + :param image_id: ID of the image to be updated. + """ + self._updated_image_id = image_id + self._updated_image = self.image_service.update(image_id, self._image_patch) + + def check_update_success(self) -> None: + """Checks that a prior call to `call_update` worked as updated.""" + # Ensure obtained old image + self.mock_image_repository.get.assert_called_once_with(image_id=self._updated_image_id) + + # Ensure updated with expected data + self.mock_image_repository.update.assert_called_once_with( + image_id=self._updated_image_id, image=self._expected_image_in + ) + + assert self._updated_image == self._expected_image_out + + +class TestUpdate(UpdateDSL): + """Tests for updating an image.""" + + def test_update(self): + """Test updating all fields of an image.""" + image_id = str(ObjectId()) + + self.mock_update( + image_patch_data=IMAGE_PATCH_METADATA_DATA_ALL_VALUES_A, + stored_image_post_data=IMAGE_IN_DATA_ALL_VALUES, + ) + self.call_update(image_id) + self.check_update_success() + + class DeleteDSL(ImageServiceDSL): """Base class for `delete` tests."""