diff --git a/backend/app/routes/albums.py b/backend/app/routes/albums.py index fdbed2378..9d66f8102 100644 --- a/backend/app/routes/albums.py +++ b/backend/app/routes/albums.py @@ -1,5 +1,11 @@ from fastapi import APIRouter, HTTPException, status, Body, Path import uuid + +from typing import Callable, TypeVar +from typing_extensions import ParamSpec +from functools import wraps +import sqlite3 +from app.logging.setup_logging import get_logger from app.schemas.album import ( GetAlbumsResponse, CreateAlbumRequest, @@ -38,6 +44,38 @@ album_util_create_from_memory, ) +logger = get_logger(__name__) + +P = ParamSpec("P") +R = TypeVar("R") + + +def handle_route_exceptions( + error_title: str, error_message: str +) -> Callable[[Callable[P, R]], Callable[P, R]]: + def decorator(func: Callable[P, R]) -> Callable[P, R]: + @wraps(func) + def wrapper(*args: P.args, **kwargs: P.kwargs) -> R: + try: + return func(*args, **kwargs) + except HTTPException: + raise + except Exception as e: + logger.error(f"Error in {func.__name__} route: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=ErrorResponse( + success=False, + error=error_title, + message=error_message, + ).model_dump(), + ) + + return wrapper + + return decorator + + router = APIRouter() @@ -63,6 +101,9 @@ def _internal_error(message: str) -> HTTPException: # GET /albums/ - Get all albums (including locked ones) @router.get("/", response_model=GetAlbumsResponse) +@handle_route_exceptions( + "Internal Server Error", "An unexpected error occurred while fetching albums." +) def get_albums(): """Get all albums. Always returns both locked and unlocked albums.""" albums = db_get_all_albums() @@ -94,6 +135,9 @@ def get_albums(): # POST /albums/ - Create a new album @router.post("/", response_model=CreateAlbumResponse) +@handle_route_exceptions( + "Internal Server Error", "An unexpected error occurred while creating the album." +) def create_album(body: CreateAlbumRequest): existing_album = db_get_album_by_name(body.name) if existing_album: @@ -105,8 +149,8 @@ def create_album(body: CreateAlbumRequest): album_id, body.name, body.description, body.is_locked, body.password ) return CreateAlbumResponse(success=True, album_id=album_id) - except Exception as e: - raise _internal_error(f"Failed to create album: {e}") from e + except sqlite3.IntegrityError: + raise _album_exists(body.name) # POST /albums/from-memory - Create an album from a curated memory @@ -147,8 +191,6 @@ def create_album_from_memory( ) from e except AlbumNameTakenError as e: raise _album_exists(body.name) from e - except Exception as e: - raise _internal_error(f"Failed to create album from memory: {e}") from e return CreateAlbumFromMemoryResponse( success=True, @@ -159,6 +201,9 @@ def create_album_from_memory( # GET /albums/{album_id} - Get specific album details @router.get("/{album_id}", response_model=GetAlbumResponse) +@handle_route_exceptions( + "Internal Server Error", "An unexpected error occurred while fetching the album." +) def get_album(album_id: str = Path(...)): album = db_get_album(album_id) if not album: @@ -169,38 +214,31 @@ def get_album(album_id: str = Path(...)): ).model_dump(), ) - try: - # Get image count for the album - image_ids = db_get_album_images(album_id) - image_count = len(image_ids) - - is_locked = album["is_locked"] - album_obj = Album( - album_id=album["album_id"], - album_name=album["album_name"], - description=album["description"] or "", - is_locked=is_locked, - # Same reasoning as the listing: the cover gives away the contents. - cover_image_path=(None if is_locked else db_get_album_cover_path(album_id)), - image_count=image_count, - created_at=album["created_at"], - updated_at=album["updated_at"], - ) - return GetAlbumResponse(success=True, data=album_obj) - except Exception as e: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=ErrorResponse( - success=False, - error="Internal Server Error", - message=f"Failed to fetch album: {str(e)}", - ).model_dump(), - ) + # Get image count for the album + image_ids = db_get_album_images(album_id) + image_count = len(image_ids) + + is_locked = album["is_locked"] + album_obj = Album( + album_id=album["album_id"], + album_name=album["album_name"], + description=album["description"] or "", + is_locked=is_locked, + cover_image_path=(None if is_locked else db_get_album_cover_path(album_id)), + image_count=image_count, + created_at=album["created_at"], + updated_at=album["updated_at"], + ) + return GetAlbumResponse(success=True, data=album_obj) # PUT /albums/{album_id} - Update Album @router.put("/{album_id}", response_model=SuccessResponse) +@handle_route_exceptions( + "Failed to Update Album", "An unexpected error occurred while updating the album." +) def update_album(album_id: str = Path(...), body: UpdateAlbumRequest = Body(...)): + album = db_get_album(album_id) if not album: raise HTTPException( @@ -238,18 +276,17 @@ def update_album(album_id: str = Path(...), body: UpdateAlbumRequest = Body(...) album_id, body.name, body.description, body.is_locked, body.password ) return SuccessResponse(success=True, msg="Album updated successfully") - except Exception as e: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ErrorResponse( - success=False, error="Failed to Update Album", message=str(e) - ).model_dump(), - ) + except sqlite3.IntegrityError: + raise _album_exists(body.name) # DELETE /albums/{album_id} - Delete an album @router.delete("/{album_id}", response_model=SuccessResponse) +@handle_route_exceptions( + "Failed to Delete Album", "An unexpected error occurred while deleting the album." +) def delete_album(album_id: str = Path(...)): + album = db_get_album(album_id) if not album: raise HTTPException( @@ -261,26 +298,22 @@ def delete_album(album_id: str = Path(...)): ).model_dump(), ) - try: - db_delete_album(album_id) - return SuccessResponse(success=True, msg="Album deleted successfully") - except Exception as e: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=ErrorResponse( - success=False, error="Failed to Delete Album", message=str(e) - ).model_dump(), - ) + db_delete_album(album_id) + return SuccessResponse(success=True, msg="Album deleted successfully") # GET /albums/{album_id}/images - Get all images in an album @router.post("/{album_id}/images/get", response_model=GetAlbumImagesResponse) +@handle_route_exceptions( + "Failed to Retrieve Images", "An unexpected error occurred while retrieving images." +) # GET requests do not accept a body by default. # Since we need to send a password securely, switching this to POST -- necessary. # Open to suggestions if better approach possible. def get_album_images( album_id: str = Path(...), body: GetAlbumImagesRequest = Body(...) ): + album = db_get_album(album_id) if not album: raise HTTPException( @@ -312,21 +345,17 @@ def get_album_images( ).model_dump(), ) - try: - image_ids = db_get_album_images(album_id) - return GetAlbumImagesResponse(success=True, image_ids=image_ids) - except Exception as e: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=ErrorResponse( - success=False, error="Failed to Retrieve Images", message=str(e) - ).model_dump(), - ) + image_ids = db_get_album_images(album_id) + return GetAlbumImagesResponse(success=True, image_ids=image_ids) # POST /albums/{album_id}/images - Add images to an album @router.post("/{album_id}/images", response_model=SuccessResponse) +@handle_route_exceptions( + "Failed to Add Images", "An unexpected error occurred while adding images." +) def add_images_to_album(album_id: str = Path(...), body: ImageIdsRequest = Body(...)): + album = db_get_album(album_id) if not album: raise HTTPException( @@ -353,9 +382,9 @@ def add_images_to_album(album_id: str = Path(...), body: ImageIdsRequest = Body( return SuccessResponse( success=True, msg=f"Added {len(body.image_ids)} images to album" ) - except Exception as e: + except (ValueError, TypeError) as e: raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + status_code=status.HTTP_400_BAD_REQUEST, detail=ErrorResponse( success=False, error="Failed to Add Images", message=str(e) ).model_dump(), @@ -364,7 +393,11 @@ def add_images_to_album(album_id: str = Path(...), body: ImageIdsRequest = Body( # DELETE /albums/{album_id}/images/{image_id} - Remove image from album @router.delete("/{album_id}/images/{image_id}", response_model=SuccessResponse) +@handle_route_exceptions( + "Failed to Remove Image", "An unexpected error occurred while removing the image." +) def remove_image_from_album(album_id: str = Path(...), image_id: str = Path(...)): + album = db_get_album(album_id) if not album: raise HTTPException( @@ -381,9 +414,9 @@ def remove_image_from_album(album_id: str = Path(...), image_id: str = Path(...) return SuccessResponse( success=True, msg="Image removed from album successfully" ) - except Exception as e: + except ValueError as e: raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + status_code=status.HTTP_404_NOT_FOUND, detail=ErrorResponse( success=False, error="Failed to Remove Image", message=str(e) ).model_dump(), @@ -392,9 +425,13 @@ def remove_image_from_album(album_id: str = Path(...), image_id: str = Path(...) # DELETE /albums/{album_id}/images - Remove multiple images from album @router.delete("/{album_id}/images", response_model=SuccessResponse) +@handle_route_exceptions( + "Failed to Remove Images", "An unexpected error occurred while removing the images." +) def remove_images_from_album( album_id: str = Path(...), body: ImageIdsRequest = Body(...) ): + album = db_get_album(album_id) if not album: raise HTTPException( @@ -416,15 +453,7 @@ def remove_images_from_album( ).model_dump(), ) - try: - db_remove_images_from_album(album_id, body.image_ids) - return SuccessResponse( - success=True, msg=f"Removed {len(body.image_ids)} images from album" - ) - except Exception as e: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=ErrorResponse( - success=False, error="Failed to Remove Images", message=str(e) - ).model_dump(), - ) + db_remove_images_from_album(album_id, body.image_ids) + return SuccessResponse( + success=True, msg=f"Removed {len(body.image_ids)} images from album" + ) diff --git a/backend/tests/test_albums.py b/backend/tests/test_albums.py index 3f1a524ff..1d0d35c60 100644 --- a/backend/tests/test_albums.py +++ b/backend/tests/test_albums.py @@ -694,3 +694,38 @@ def test_create_album_from_memory_duplicate_name( assert response.status_code == 409 assert response.json()["detail"]["error"] == "Album Already Exists" mock_create.assert_not_called() + + +class TestAlbumRouteErrors: + """Test suite for verifying that album routes correctly handle exceptions.""" + + def test_unexpected_exceptions_mapped_to_500(self): + """Verify that an unexpected exception returns a 500 error.""" + from unittest.mock import patch + + with patch("app.routes.albums.db_get_all_albums") as mock_get_all: + mock_get_all.side_effect = Exception("Database explosion") + response = client.get("/albums/") + assert response.status_code == 500 + json_resp = response.json() + assert json_resp["detail"]["success"] is False + assert "unexpected error" in json_resp["detail"]["message"].lower() + + def test_duplicate_album_integrity_error(self): + """Verify that a database IntegrityError returns a 409 conflict.""" + import sqlite3 + from unittest.mock import patch + + with patch("app.routes.albums.db_get_album_by_name", return_value=None): + with patch( + "app.routes.albums.db_insert_album", + side_effect=sqlite3.IntegrityError("Unique constraint failed"), + ): + response = client.post( + "/albums/", + json={"name": "Duplicate", "description": "This should fail"}, + ) + assert response.status_code == 409 + json_resp = response.json() + assert json_resp["detail"]["success"] is False + assert "album already exists" in json_resp["detail"]["error"].lower() diff --git a/backend/tests/test_memory_signals_db.py b/backend/tests/test_memory_signals_db.py index 9a712d976..d6bd453e0 100644 --- a/backend/tests/test_memory_signals_db.py +++ b/backend/tests/test_memory_signals_db.py @@ -76,6 +76,7 @@ def test_db(monkeypatch: pytest.MonkeyPatch) -> Iterator[str]: "app.database.folders", "app.database.yolo_mapping", "app.database.albums", + "app.database.connection", "app.database.faces", "app.database.face_clusters", "app.database.videos",