add get check to delete e2e test and minor changes #38

object_storage_api/repositories/image.py

@@ -109,4 +109,4 @@ def delete(self, image_id: str, session: ClientSession = None) -> None:
             raise exc
         response = self._images_collection.delete_one(filter={"_id": image_id}, session=session)
         if response.deleted_count == 0:
-            raise MissingRecordError(f"Requested Image was not found: {image_id}", "image")
+            raise MissingRecordError(f"No image found with ID: {image_id}", "image")

object_storage_api/routers/image.py

@@ -69,6 +69,17 @@ def get_images(
     return image_service.list(entity_id, primary)
 
 
+@router.get(path="/{image_id}", summary="Get an image by ID", response_description="Single image")
+def get_image(
+    image_id: Annotated[str, Path(description="ID of the image to get")],
+    image_service: ImageServiceDep,
+) -> ImageSchema:
+    # pylint: disable=missing-function-docstring
+    logger.info("Getting image with ID: %s", image_id)
+
+    return image_service.get(image_id)
+
+
 @router.delete(
     path="/{image_id}",
     summary="Delete an image by ID",
@@ -82,14 +93,3 @@ def delete_image(
     # pylint: disable=missing-function-docstring
     logger.info("Deleting image with ID: %s", image_id)
     image_service.delete(image_id)
-
-
-@router.get(path="/{image_id}", summary="Get an image by ID", response_description="Single image")
-def get_image(
-    image_id: Annotated[str, Path(description="ID of the image to get")],
-    image_service: ImageServiceDep,
-) -> ImageSchema:
-    # pylint: disable=missing-function-docstring
-    logger.info("Getting image with ID: %s", image_id)
-
-    return image_service.get(image_id)

object_storage_api/stores/image.py

@@ -39,19 +39,6 @@ def upload(self, image_id: str, image_metadata: ImagePostMetadataSchema, upload_
 
         return object_key
 
-    def delete(self, object_key: str) -> None:
-        """
-        Deletes a given image from object storage.
-
-        :param object_key: Key of the image to delete.
-        """
-
-        logger.info("Deleting image file from the object storage")
-        s3_client.delete_object(
-            Bucket=object_storage_config.bucket_name.get_secret_value(),
-            Key=object_key,
-        )
-
     def create_presigned_get(self, image: ImageOut) -> str:
         """
         Generate a presigned URL to share an S3 object.
@@ -71,3 +58,16 @@ def create_presigned_get(self, image: ImageOut) -> str:
         )
 
         return response
+
+    def delete(self, object_key: str) -> None:
+        """
+        Deletes a given image from object storage.
+
+        :param object_key: Key of the image to delete.
+        """
+
+        logger.info("Deleting image file from the object storage")
+        s3_client.delete_object(
+            Bucket=object_storage_config.bucket_name.get_secret_value(),
+            Key=object_key,
+        )

test/e2e/test_image.py

@@ -165,15 +165,15 @@ def post_test_images(self) -> list[dict]:
         """
         Posts three images. The first two images have the same entity ID, the last image has a different one.
 
-        :return: List of dictionaries containing the expected item data returned from a get endpoint in
+        :return: List of dictionaries containing the expected image data returned from a get endpoint in
                  the form of an `ImageMetadataSchema`.
         """
         entity_id_a, entity_id_b = (str(ObjectId()) for _ in range(2))
 
-        # First item
+        # First image
         image_a_id = self.post_image({**IMAGE_POST_METADATA_DATA_ALL_VALUES, "entity_id": entity_id_a}, "image.jpg")
 
-        # Second item
+        # Second image
         image_b_id = self.post_image(
             {
                 **IMAGE_POST_METADATA_DATA_ALL_VALUES,
@@ -182,7 +182,7 @@ def post_test_images(self) -> list[dict]:
             "image.jpg",
         )
 
-        # Third item
+        # Third image
         image_c_id = self.post_image(
             {
                 **IMAGE_POST_METADATA_DATA_ALL_VALUES,
@@ -327,9 +327,9 @@ def check_delete_image_success(self) -> None:
         """Checks that a prior call to `delete_image` gave a successful response with the expected data
         returned."""
 
-        assert self._delete_response_image.status_code == 200
+        assert self._delete_response_image.status_code == 204
 
-    def check_delete_item_failed_with_detail(self) -> None:
+    def check_delete_image_failed_with_detail(self) -> None:
         """
         Checks that a prior call to `delete_image` gave a failed response with the expected code and
         error message.
@@ -353,14 +353,17 @@ def test_delete(self):
         self.delete_image(image_id)
         self.check_delete_image_success()
 
+        self.get_image(image_id)
+        self.check_get_image_failed()
+
     def test_delete_with_non_existent_id(self):
-        """Test deleting a non-existent item."""
+        """Test deleting a non-existent image."""
 
         self.delete_image(str(ObjectId()))
-        self.check_delete_item_failed_with_detail()
+        self.check_delete_image_failed_with_detail()
 
     def test_delete_with_invalid_id(self):
-        """Test deleting an item with an invalid ID."""
+        """Test deleting an image with an invalid ID."""
 
         self.delete_image("invalid_id")
-        self.check_delete_item_failed_with_detail()
+        self.check_delete_image_failed_with_detail()

test/unit/repositories/test_image.py

@@ -283,7 +283,7 @@ def mock_delete(self, deleted_count: int) -> None:
         """
         Mocks database methods appropriately to test the `delete` repo method.
 
-        :param deleted_count: Expected count of the number of delete imges.
+        :param deleted_count: Number of documents deleted successfully.
         """
         RepositoryTestHelpers.mock_delete_one(self.images_collection, deleted_count)
 
@@ -354,7 +354,7 @@ def test_delete_non_existent_id(self):
 
         self.mock_delete(0)
         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)
+        self.check_delete_failed_with_exception(f"No image found with ID: {image_id}", assert_delete=True)
 
     def test_delete_invalid_id(self):
         """Test deleting an image with an invalid ID."""