Add Get Endpoint for Image metadata and presigned url

object_storage_api/core/exceptions.py

@@ -6,6 +6,9 @@
 # TODO: Some of this file is identical to the one in inventory-management-system-api - Use common repo?
 
 
+from typing import Optional
+
+
 class BaseAPIException(Exception):
     """
     Base exception for API errors.
@@ -19,17 +22,21 @@ class BaseAPIException(Exception):
 
     detail: str
 
-    def __init__(self, detail: str):
+    def __init__(self, detail: str, response_detail: Optional[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):
     """
@@ -53,3 +60,25 @@ class InvalidImageFileError(BaseAPIException):
 
     status_code = 422
     response_detail = "File given is not a valid image"
+
+
+class MissingRecordError(DatabaseError):
+    """
+    A specific database record was requested but could not be found.
+    """
+
+    status_code = 404
+    response_detail = "Requested record was not found"
+
+    def __init__(self, detail: str, entity_name: Optional[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 detail.
+        """
+        super().__init__(detail)
+
+        if entity_name is not None:
+            self.response_detail = f"{entity_name.capitalize()} not found"

object_storage_api/repositories/image.py

@@ -10,6 +10,7 @@
 
 from object_storage_api.core.custom_object_id import CustomObjectId
 from object_storage_api.core.database import DatabaseDep
+from object_storage_api.core.exceptions import InvalidObjectIdError, MissingRecordError
 from object_storage_api.models.image import ImageIn, ImageOut
 
 logger = logging.getLogger()
@@ -42,20 +43,27 @@ def create(self, image: ImageIn, session: ClientSession = None) -> ImageOut:
         result = self._images_collection.insert_one(image.model_dump(by_alias=True), session=session)
         return self.get(str(result.inserted_id), session=session)
 
-    def get(self, image_id: str, session: ClientSession = None) -> Optional[ImageOut]:
+    def get(self, image_id: str, session: ClientSession = None) -> ImageOut:
         """
         Retrieve an image by its ID from a MongoDB database.
 
         :param image_id: ID of the image to retrieve.
         :param session: PyMongo ClientSession to use for database operations.
-        :return: Retrieved image or `None` if not found.
+        :return: Retrieved image if found.
+        :raises MissingRecordError: If the supplied `image_id` is non-existent.
+        :raises InvalidObjectIdError: If the supplied `image_id` is invalid.
         """
-        image_id = CustomObjectId(image_id)
         logger.info("Retrieving image with ID: %s from the database", image_id)
-        image = self._images_collection.find_one({"_id": image_id}, session=session)
+        try:
+            image_id = CustomObjectId(image_id)
+            image = self._images_collection.find_one({"_id": image_id}, session=session)
+        except InvalidObjectIdError as exc:
+            exc.status_code = 404
+            exc.response_detail = "Image not found"
+            raise exc
         if image:
             return ImageOut(**image)
-        return None
+        raise MissingRecordError(detail=f"Image with image_id {image_id} was not found.", entity_name="image")
 
     def list(self, entity_id: Optional[str], primary: Optional[bool], session: ClientSession = None) -> list[ImageOut]:
         """

object_storage_api/routers/image.py

@@ -6,9 +6,9 @@
 import logging
 from typing import Annotated, Optional
 
-from fastapi import APIRouter, Depends, File, Form, Query, UploadFile, status
+from fastapi import APIRouter, Depends, File, Form, Path, Query, UploadFile, status
 
-from object_storage_api.schemas.image import ImagePostMetadataSchema, ImageSchema
+from object_storage_api.schemas.image import ImageMetadataSchema, ImagePostMetadataSchema, ImageSchema
 from object_storage_api.services.image import ImageService
 
 logger = logging.getLogger()
@@ -36,7 +36,7 @@ def create_image(
     upload_file: Annotated[UploadFile, File(description="Image file")],
     title: Annotated[Optional[str], Form(description="Title of the image")] = None,
     description: Annotated[Optional[str], Form(description="Description of the image")] = None,
-) -> ImageSchema:
+) -> ImageMetadataSchema:
     # pylint: disable=missing-function-docstring
     logger.info("Creating a new image")
 
@@ -57,7 +57,7 @@ def get_images(
     image_service: ImageServiceDep,
     entity_id: Annotated[Optional[str], Query(description="Filter images by entity ID")] = None,
     primary: Annotated[Optional[bool], Query(description="Filter images by primary")] = None,
-) -> list[ImageSchema]:
+) -> list[ImageMetadataSchema]:
     # pylint: disable=missing-function-docstring
     logger.info("Getting images")
 
@@ -67,3 +67,13 @@ def get_images(
         logger.debug("Primary filter: '%s'", primary)
 
     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)

object_storage_api/schemas/image.py

@@ -4,7 +4,7 @@
 
 from typing import Optional
 
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, HttpUrl
 
 from object_storage_api.schemas.mixins import CreatedModifiedSchemaMixin
 
@@ -19,12 +19,20 @@ class ImagePostMetadataSchema(BaseModel):
     description: Optional[str] = Field(default=None, description="Description of the image")
 
 
-class ImageSchema(CreatedModifiedSchemaMixin, ImagePostMetadataSchema):
+class ImageMetadataSchema(CreatedModifiedSchemaMixin, ImagePostMetadataSchema):
     """
-    Schema model for an image get request response.
+    Schema model for an image's metadata.
     """
 
     id: str = Field(description="ID of the image")
     file_name: str = Field(description="File name of the image")
     primary: bool = Field(description="Whether the image is the primary for its related entity")
     thumbnail_base64: str = Field(description="Thumbnail of the image as a base64 encoded byte string")
+
+
+class ImageSchema(ImageMetadataSchema):
+    """
+    Schema model for an image get request response.
+    """
+
+    url: HttpUrl = Field(description="Presigned get URL to get the image file")

object_storage_api/services/image.py

@@ -13,7 +13,7 @@
 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 ImagePostMetadataSchema, ImageSchema
+from object_storage_api.schemas.image import ImageMetadataSchema, ImagePostMetadataSchema, ImageSchema
 from object_storage_api.stores.image import ImageStore
 
 logger = logging.getLogger()
@@ -38,7 +38,7 @@ def __init__(
         self._image_repository = image_repository
         self._image_store = image_store
 
-    def create(self, image_metadata: ImagePostMetadataSchema, upload_file: UploadFile) -> ImageSchema:
+    def create(self, image_metadata: ImagePostMetadataSchema, upload_file: UploadFile) -> ImageMetadataSchema:
         """
         Create a new image.
 
@@ -73,9 +73,20 @@ def create(self, image_metadata: ImagePostMetadataSchema, upload_file: UploadFil
 
         image_out = self._image_repository.create(image_in)
 
-        return ImageSchema(**image_out.model_dump())
+        return ImageMetadataSchema(**image_out.model_dump())
 
-    def list(self, entity_id: Optional[str] = None, primary: Optional[bool] = None) -> list[ImageSchema]:
+    def get(self, image_id: str) -> ImageSchema:
+        """
+        Retrieve an image's metadata with its presigned get url by its ID.
+
+        :param image_id: ID of the image to retrieve.
+        :return: An image's metadata with a presigned get url if it is obtained.
+        """
+        image = self._image_repository.get(image_id=image_id)
+        presigned_url = self._image_store.create_presigned_get(image)
+        return ImageSchema(**image.model_dump(), url=presigned_url)
+
+    def list(self, entity_id: Optional[str] = None, primary: Optional[bool] = None) -> list[ImageMetadataSchema]:
         """
         Retrieve a list of images based on the provided filters.
 
@@ -84,4 +95,4 @@ def list(self, entity_id: Optional[str] = None, primary: Optional[bool] = None)
         :return: List of images or an empty list if no images are retrieved.
         """
         images = self._image_repository.list(entity_id, primary)
-        return [ImageSchema(**image.model_dump()) for image in images]
+        return [ImageMetadataSchema(**image.model_dump()) for image in images]

object_storage_api/stores/image.py

@@ -7,6 +7,7 @@
 from fastapi import UploadFile
 
 from object_storage_api.core.object_store import object_storage_config, s3_client
+from object_storage_api.models.image import ImageOut
 from object_storage_api.schemas.image import ImagePostMetadataSchema
 
 logger = logging.getLogger()
@@ -37,3 +38,22 @@ def upload(self, image_id: str, image_metadata: ImagePostMetadataSchema, upload_
         )
 
         return object_key
+
+    def create_presigned_get(self, image: ImageOut) -> str:
+        """Generate a presigned URL to share an S3 object.
+
+        :param image: `ImageOut` model of the image.
+        :return: Presigned url to get the image.
+        """
+        logger.info("Generating presigned url to get image from object storage")
+        response = s3_client.generate_presigned_url(
+            "get_object",
+            Params={
+                "Bucket": object_storage_config.bucket_name.get_secret_value(),
+                "Key": image.object_key,
+                "ResponseContentDisposition": f'inline; filename="{image.file_name}"',
+            },
+            ExpiresIn=object_storage_config.presigned_url_expiry_seconds,
+        )
+
+        return response

test/e2e/test_image.py

@@ -4,7 +4,8 @@
 
 from test.mock_data import (
     IMAGE_GET_DATA_ALL_VALUES,
-    IMAGE_GET_DATA_REQUIRED_VALUES_ONLY,
+    IMAGE_GET_METADATA_ALL_VALUES,
+    IMAGE_GET_METADATA_REQUIRED_VALUES_ONLY,
     IMAGE_POST_METADATA_DATA_ALL_VALUES,
     IMAGE_POST_METADATA_DATA_REQUIRED_VALUES_ONLY,
 )
@@ -52,7 +53,7 @@ def check_post_image_success(self, expected_image_get_data: dict) -> None:
         Checks that a prior call to `post_image` gave a successful response with the expected data returned.
 
         :param expected_image_get_data: Dictionary containing the expected image data returned as would be
-                                        required for an `ImageSchema`.
+                                        required for an `ImageMetadataSchema`.
         """
 
         assert self._post_response_image.status_code == 201
@@ -77,13 +78,13 @@ def test_create_with_only_required_values_provided(self):
         """Test creating an image with only required values provided."""
 
         self.post_image(IMAGE_POST_METADATA_DATA_REQUIRED_VALUES_ONLY, "image.jpg")
-        self.check_post_image_success(IMAGE_GET_DATA_REQUIRED_VALUES_ONLY)
+        self.check_post_image_success(IMAGE_GET_METADATA_REQUIRED_VALUES_ONLY)
 
     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_DATA_ALL_VALUES)
+        self.check_post_image_success(IMAGE_GET_METADATA_ALL_VALUES)
 
     def test_create_with_invalid_entity_id(self):
         """Test creating an image with an invalid `entity_id`."""
@@ -98,13 +99,62 @@ def test_create_with_invalid_image_file(self):
         self.check_post_image_failed_with_detail(422, "File given is not a valid image")
 
 
-# pylint:disable=fixme
-# TODO: Inherit from GetDSL when added
-class ListDSL(CreateDSL):
-    """Base class for list tests."""
+class GetDSL(CreateDSL):
+    """Base class for get tests."""
 
     _get_response_image: Response
 
+    def get_image(self, image_id: str) -> None:
+        """
+        Gets an image with the given ID.
+
+        :param image_id: The ID of the image to be obtained.
+        """
+        self._get_response_image = self.test_client.get(f"/images/{image_id}")
+
+    def check_get_image_success(self, expected_image_data: dict) -> None:
+        """
+        Checks that a prior call to `get_image` gave a successful response with the expected data returned.
+
+        :param expected_image_data: Dictionary containing the expected image data as would be required
+            for an `ImageMetadataSchema`.
+        """
+        assert self._get_response_image.status_code == 200
+        assert self._get_response_image.json() == expected_image_data
+
+    def check_get_image_failed(self) -> None:
+        """
+        Checks that prior call to `get_image` gave a failed response.
+
+        """
+        assert self._get_response_image.status_code == 404
+        assert self._get_response_image.json()["detail"] == "Image not found"
+
+
+class TestGet(GetDSL):
+    "Tests for getting an image."
+
+    def test_get_with_valid_image_id(self):
+        """Test getting an image with a valid image ID."""
+        image_id = self.post_image(IMAGE_POST_METADATA_DATA_ALL_VALUES, "image.jpg")
+        self.get_image(image_id)
+        self.check_get_image_success(IMAGE_GET_DATA_ALL_VALUES)
+
+    def test_get_with_invalid_image_id(self):
+        """Test getting an image with an invalid image ID."""
+        self.get_image("sdfgfsdg")
+        self.check_get_image_failed()
+
+    def test_get_with_non_existent_image_id(self):
+        """Test getting an image with a non-existent image ID."""
+        image_id = str(ObjectId())
+        self.get_image(image_id)
+        self.check_get_image_failed()
+
+
+class ListDSL(GetDSL):
+    """Base class for list tests."""
+
     def get_images(self, filters: Optional[dict] = None) -> None:
         """
         Gets a list of images with the given filters.
@@ -118,7 +168,7 @@ 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
-                 the form of an `ImageSchema`.
+                 the form of an `ImageMetadataSchema`.
         """
         entity_id_a, entity_id_b = (str(ObjectId()) for _ in range(2))
 
@@ -145,17 +195,17 @@ def post_test_images(self) -> list[dict]:
 
         return [
             {
-                **IMAGE_GET_DATA_ALL_VALUES,
+                **IMAGE_GET_METADATA_ALL_VALUES,
                 "entity_id": entity_id_a,
                 "id": image_a_id,
             },
             {
-                **IMAGE_GET_DATA_ALL_VALUES,
+                **IMAGE_GET_METADATA_ALL_VALUES,
                 "entity_id": entity_id_a,
                 "id": image_b_id,
             },
             {
-                **IMAGE_GET_DATA_ALL_VALUES,
+                **IMAGE_GET_METADATA_ALL_VALUES,
                 "entity_id": entity_id_b,
                 "id": image_c_id,
             },
@@ -166,7 +216,7 @@ def check_get_images_success(self, expected_images_get_data: list[dict]) -> None
         Checks that a prior call to `get_images` gave a successful response with the expected data returned.
 
         :param expected_images_get_data: List of dictionaries containing the expected image data as would
-            be required for an `ImageSchema`.
+            be required for an `ImageMetadataSchema`.
         """
         assert self._get_response_image.status_code == 200
         assert self._get_response_image.json() == expected_images_get_data