From 3a4fa4e8ef3cf434442129cd099af6bbd14a1071 Mon Sep 17 00:00:00 2001
From: Anikesh Suresh <sureshanikesh@yahoo.co.uk>
Date: Thu, 5 Dec 2024 11:34:59 +0000
Subject: [PATCH] added error handling TODO: docstrings, tests, merge get
 endpoint #35

---
 object_storage_api/core/exceptions.py    | 48 +++++++++++++++++++++++-
 object_storage_api/repositories/image.py | 15 ++++++--
 object_storage_api/services/image.py     |  2 +-
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/object_storage_api/core/exceptions.py b/object_storage_api/core/exceptions.py
index 095c051..6aca36c 100644
--- a/object_storage_api/core/exceptions.py
+++ b/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):
     """
@@ -42,9 +49,23 @@ class InvalidObjectIdError(DatabaseError):
     The provided value is not a valid ObjectId.
     """
 
-    status_code = 422
+    status_code = 404
     response_detail = "Invalid ID given"
 
+    def __init__(self, detail: str, response_detail: Optional[str] = None, 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 response_detail: Generic detail of the exception to be returned in the response.
+        :param entity_name: Name of the entity to include in the response detail.
+        """
+        super().__init__(detail, response_detail)
+
+        if entity_name is not None:
+            self.response_detail = f"{entity_name.capitalize()} not found"
+
 
 class InvalidImageFileError(BaseAPIException):
     """
@@ -53,3 +74,26 @@ 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, response_detail: Optional[str] = None, 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 response_detail: Generic detail of the exception to be returned in the response.
+        :param entity_name: Name of the entity to include in the response detail.
+        """
+        super().__init__(detail, response_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 a245307..3bced93 100644
--- a/object_storage_api/repositories/image.py
+++ b/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
 from object_storage_api.models.image import ImageIn, ImageOut
 
 logger = logging.getLogger()
@@ -94,10 +95,16 @@ def update(self, image_id: str, image: ImageIn, session: ClientSession = None) -
         """
 
         logger.info("Updating image metadata with ID: %s", image_id)
-        image_id = CustomObjectId(image_id)
+        entity_name = "image"
+        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:
+            raise InvalidObjectIdError(detail=f"Invalid ObjectId value '{image_id}'", entity_name=entity_name) from exc
 
-        self._images_collection.update_one(
-            {"_id": image_id}, {"$set": image.model_dump(by_alias=True)}, session=session
-        )
         image = self.get(image_id=str(image_id), session=session)
+        if image is None:
+            raise MissingRecordError(detail=f"No image found with ID: {image_id}", entity_name=entity_name)
         return image
diff --git a/object_storage_api/services/image.py b/object_storage_api/services/image.py
index 94f9b05..c0f06cd 100644
--- a/object_storage_api/services/image.py
+++ b/object_storage_api/services/image.py
@@ -90,7 +90,7 @@ def update(self, image_id: str, image: ImagePatchMetadataSchema) -> ImageSchema:
         """
         Update an image based on its ID.
 
-        :param image_id: The ID of the image to updtae.
+        :param image_id: The ID of the image to update.
         :param image: The new update data for the image.
         :return: List of images or an empty list if no images are retrieved.
         """