diff --git a/src/event/crud.py b/src/event/crud.py index ae5e7dc..d45fc4f 100644 --- a/src/event/crud.py +++ b/src/event/crud.py @@ -12,6 +12,12 @@ from event.tables import EventDB from image_asset.tables import ImageAssetDB +MEDIA_BASE_URL = settings.media_base_url.rstrip("/") + + +def make_image_url(storage_key: str | None) -> str | None: + return f"{MEDIA_BASE_URL}/{storage_key}" if storage_key is not None else None + async def get_events(db_session: AsyncSession, q: GetEventQueryParams) -> list[Event]: query = select(EventDB, ImageAssetDB.storage_key).outerjoin(ImageAssetDB, EventDB.image_id == ImageAssetDB.image_id) @@ -29,12 +35,9 @@ async def get_events(db_session: AsyncSession, q: GetEventQueryParams) -> list[E ) rows = (await db_session.execute(query)).all() - media_base_url = settings.media_base_url.rstrip("/") return [ - Event.model_validate(event).model_copy( - update={"image_url": f"{media_base_url}/{storage_key}" if storage_key is not None else None} - ) + Event.model_validate(event).model_copy(update={"image_url": make_image_url(storage_key)}) for event, storage_key in rows ] @@ -43,6 +46,19 @@ async def get_event_by_eid(db_session: AsyncSession, eid: int) -> EventDB | None return await db_session.get(EventDB, eid) +async def get_event_with_image_url(db_session: AsyncSession, eid: int) -> tuple[EventDB, str | None] | None: + query = ( + select(EventDB, ImageAssetDB.storage_key) + .outerjoin(ImageAssetDB, EventDB.image_id == ImageAssetDB.image_id) + .where(EventDB.eid == eid) + ) + row = (await db_session.execute(query)).one_or_none() + if row is None: + return None + db_event, storage_key = row + return db_event, make_image_url(storage_key) + + async def get_events_by_group_id(db_session: AsyncSession, group_id: UUID) -> Sequence[EventDB]: query = select(EventDB).where(EventDB.group_id == group_id).order_by(EventDB.start_datetime, EventDB.end_datetime) diff --git a/src/event/urls.py b/src/event/urls.py index 03a9e2d..ee995b9 100644 --- a/src/event/urls.py +++ b/src/event/urls.py @@ -4,9 +4,11 @@ from fastapi import APIRouter, Body, Depends, HTTPException, Query, status from fastapi.encoders import jsonable_encoder from pydantic import BaseModel, Field, ValidationError +from sqlalchemy.exc import IntegrityError import database import event.crud +import image_asset.crud from dependencies import MonthPath, YearPath, perm_admin from event.models import ( Event, @@ -18,6 +20,7 @@ GroupEventDeleteResponse, ) from event.tables import EventDB +from image_asset.tables import ImageAssetDB from utils.shared_models import DetailModel router = APIRouter( @@ -43,12 +46,19 @@ async def get_all_events(db_session: database.DBSession, q: Annotated[GetEventQu response_model=Event, status_code=status.HTTP_201_CREATED, responses={ - 500: {"description": "failed to fetch new event", "model": DetailModel}, + 400: {"description": "Image asset doesn't exist.", "model": DetailModel}, + 500: {"description": "Failed to fetch new event", "model": DetailModel}, }, operation_id="create_event", dependencies=[Depends(perm_admin)], ) async def create_event(db_session: database.DBSession, body: EventCreate): + image_url = None + if body.image_id: + image = await image_asset.crud.get_image_asset_by_id(db_session, body.image_id) + if image is None: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Image asset doesn't exist.") + image_url = event.crud.make_image_url(image.storage_key) new_event = EventDB(**body.model_dump()) event.crud.create_event( db_session, @@ -58,7 +68,10 @@ async def create_event(db_session: database.DBSession, body: EventCreate): await db_session.commit() await db_session.refresh(new_event) - return new_event + response = Event.model_validate(new_event) + response.image_url = image_url + + return response @router.post( @@ -115,29 +128,61 @@ async def add_event_to_group(db_session: database.DBSession, group_id: uuid.UUID "/{eid}", description="Update an Event detail", response_model=Event, - responses={404: {"description": "Event doesn't exist."}}, + responses={ + 400: {"description": "Image asset doesn't exist."}, + 404: {"description": "Event doesn't exist."}, + 409: {"description": "Concurrent change caused an issue."}, + }, operation_id="update_event", dependencies=[Depends(perm_admin)], ) async def update_event(db_session: database.DBSession, eid: int, body: EventUpdate): - db_event = await event.crud.get_event_by_eid(db_session, eid) - if db_event is None: + result = await event.crud.get_event_with_image_url(db_session, eid) + if result is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Event doesn't exist.") - db_data = Event.model_validate(db_event) - patch_data = body.model_dump(exclude_unset=True) + db_event, image_url = result + + event_data = Event.model_validate(db_event).model_copy(update={"image_url": image_url}) + patch_data = body.model_dump(exclude_unset=True) # does not include image_url + updated_data = event_data.model_dump() | patch_data + + if "image_id" in patch_data and patch_data["image_id"] != db_event.image_id: + new_image_id = patch_data["image_id"] + + # The patched data explicitly set image_id to None, so clear the image. + if new_image_id is None: + updated_data["image_url"] = None + else: + image = await db_session.get(ImageAssetDB, new_image_id, with_for_update=True) + if image is None: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Image asset doesn't exist.", + ) + updated_data["image_url"] = event.crud.make_image_url(image.storage_key) try: - updated = Event.model_validate(db_data.model_dump() | patch_data) + updated = Event.model_validate(updated_data) except ValidationError as e: raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=jsonable_encoder(e.errors()) + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail=jsonable_encoder( + e.errors(), + ), ) from e for key, value in patch_data.items(): setattr(db_event, key, value) - await db_session.commit() + try: + await db_session.commit() + except IntegrityError as e: + await db_session.rollback() + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Error when committing.", + ) from e return updated diff --git a/src/image_asset/constants.py b/src/image_asset/constants.py new file mode 100644 index 0000000..6290557 --- /dev/null +++ b/src/image_asset/constants.py @@ -0,0 +1,21 @@ +from enum import StrEnum +from pathlib import Path + + +class ImageAssetCategory(StrEnum): + EVENTS = "events" + EXECS = "execs" + PHOTOS = "photos" + + +IMAGE_ASSET_MAPPING: dict[ImageAssetCategory, Path] = { + ImageAssetCategory.EVENTS: Path("events"), + ImageAssetCategory.EXECS: Path("execs"), + ImageAssetCategory.PHOTOS: Path("photos"), +} + +ALLOWED_IMAGE_TYPES = {"JPEG": "jpg", "PNG": "png", "WEBP": "webp"} + +MAX_PIXELS = 8_000_000 + +MAX_ATTEMPTS = 3 diff --git a/src/image_asset/crud.py b/src/image_asset/crud.py index a25d312..07f056c 100644 --- a/src/image_asset/crud.py +++ b/src/image_asset/crud.py @@ -5,6 +5,10 @@ from image_asset.tables import ImageAssetDB +async def get_image_asset_by_id(db_session: database.DBSession, image_id: int) -> ImageAssetDB | None: + return await db_session.get(ImageAssetDB, image_id) + + async def get_all_image_assets(db_session: database.DBSession) -> list[ImageAssetDB]: query = select(ImageAssetDB).order_by(ImageAssetDB.image_id.desc()) return list((await db_session.scalars(query)).all()) diff --git a/src/image_asset/urls.py b/src/image_asset/urls.py index 30ed755..ec7fefc 100644 --- a/src/image_asset/urls.py +++ b/src/image_asset/urls.py @@ -3,25 +3,26 @@ import warnings from datetime import UTC, datetime from pathlib import Path -from uuid import uuid4 +from typing import Annotated +from uuid import UUID, uuid4 -from fastapi import APIRouter, Depends, HTTPException, UploadFile, status +from fastapi import APIRouter, Depends, HTTPException, Response, UploadFile, status from PIL import Image, UnidentifiedImageError +from pydantic import WithJsonSchema +from sqlalchemy.exc import IntegrityError +from starlette.status import HTTP_500_INTERNAL_SERVER_ERROR import database import image_asset.crud from config import settings from dependencies import perm_admin +from image_asset.constants import ALLOWED_IMAGE_TYPES, IMAGE_ASSET_MAPPING, MAX_ATTEMPTS, MAX_PIXELS, ImageAssetCategory from image_asset.models import ImageAsset from image_asset.tables import ImageAssetDB from utils.shared_models import DetailModel _logger = logging.getLogger(__name__) -ALLOWED_IMAGE_TYPES = {"JPEG": "jpg", "PNG": "png", "WEBP": "webp"} - -MAX_PIXELS = 8_000_000 - async def validate_upload(file: UploadFile) -> str: """ @@ -71,6 +72,31 @@ async def validate_upload(file: UploadFile) -> str: return image_format +def _make_storage_key(category: ImageAssetCategory | None, uuid: UUID, image_format: str) -> str: + if category is not None: + cat_path = IMAGE_ASSET_MAPPING.get(category) + if cat_path is None: + raise ValueError(f"Image asset mapping missing for category: {category}") + return f"images/{cat_path}/{uuid}.{image_format}" + + return f"images/{uuid}.{image_format}" + + +def _create_file(dest: Path, file: UploadFile) -> None: + dest.parent.mkdir(parents=True, exist_ok=True) + output = dest.open("xb") + + try: + with output: + shutil.copyfileobj(file.file, output) + except Exception: + try: + dest.unlink(missing_ok=True) + except OSError: + _logger.exception("Failed to clean up image after failed file write: %s.", dest) + raise + + router = APIRouter( prefix="/image", tags=["media"], @@ -95,52 +121,118 @@ async def get_all_image_assets(db_session: database.DBSession): response_model=ImageAsset, status_code=status.HTTP_201_CREATED, responses={ - 400: {"description": "Image is invalid", "model": DetailModel}, - 403: {"description": "Must be a website admin", "model": DetailModel}, - 413: {"description": f"Maximum resolution of {MAX_PIXELS / 1_000_000} megapixels", "model": DetailModel}, + 400: {"description": "Image or category is invalid.", "model": DetailModel}, + 403: {"description": "Must be a website admin.", "model": DetailModel}, + 413: {"description": f"Maximum resolution of {MAX_PIXELS / 1_000_000} megapixels.", "model": DetailModel}, 415: {"description": "Image format not supported.", "model": DetailModel}, - 500: {"description": "Saving image failed", "model": DetailModel}, + 500: {"description": "Server had an issue saving the image.", "model": DetailModel}, }, operation_id="create_image_asset", dependencies=[Depends(perm_admin)], ) -async def create_image_asset_from_upload(file: UploadFile, db_session: database.DBSession): +async def create_image_asset_from_upload( + db_session: database.DBSession, + file: Annotated[UploadFile, WithJsonSchema({"type": "string", "format": "binary"})], + category: ImageAssetCategory | None = None, +): image_format = await validate_upload(file) - storage_key = f"images/{uuid4()}.{image_format}" - destination = settings.media_root / storage_key - - try: - destination.parent.mkdir(parents=True, exist_ok=True) + retries = 0 + while retries < MAX_ATTEMPTS: + committed = False + file_created = False + uid = uuid4() + try: + storage_key = _make_storage_key(category, uid, image_format) + except ValueError as e: + _logger.error(e) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Image asset mapping missing" + ) from e + destination = settings.media_root / storage_key + await file.seek(0) - with destination.open("wb") as output: - shutil.copyfileobj(file.file, output) - except OSError as error: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Failed to save image to storage.", - ) from error + try: + _create_file(destination, file) + file_created = True + except FileExistsError: + retries += 1 + continue - new_img_asset = ImageAssetDB( - storage_key=storage_key, - original_filename=file.filename, + try: + new_img_asset = ImageAssetDB( + storage_key=storage_key, + original_filename=file.filename, + ) + image_asset.crud.create_image_asset(db_session, new_img_asset) + await db_session.commit() + committed = True + await db_session.refresh(new_img_asset) + return ImageAsset.model_validate(new_img_asset) + except IntegrityError: + await db_session.rollback() + try: + destination.unlink(missing_ok=True) + except OSError: + _logger.exception("Failed to remove image: %s", destination) + continue + except Exception as e: + _logger.exception(e) + if not committed: + await db_session.rollback() + if file_created: + try: + destination.unlink(missing_ok=True) + except OSError: + _logger.exception("Failed to remove image: %s", destination) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to make image asset, unhandled exception.", + ) from e + finally: + retries += 1 + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed to make image asset, exhausted retries." ) + +@router.delete( + "/{image_id}", + description="Delete an image asset and its associated file if it's not referenced by anything else.", + status_code=status.HTTP_204_NO_CONTENT, + responses={ + 403: {"description": "Must be a website admin", "model": DetailModel}, + 404: {"description": "Image asset doesn't exist", "model": DetailModel}, + 409: {"description": "Image asset is still referenced", "model": DetailModel}, + 500: {"description": "Deleting image file failed.", "model": DetailModel}, + }, + operation_id="delete_image_asset", + dependencies=[Depends(perm_admin)], +) +async def delete_image_asset(db_session: database.DBSession, image_id: int): + db_entry = await image_asset.crud.get_image_asset_by_id(db_session, image_id) + if db_entry is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Image can't be found") + + image_path = settings.media_root / db_entry.storage_key + try: - image_asset.crud.create_image_asset(db_session, new_img_asset) + await image_asset.crud.delete_image_asset(db_session, db_entry) await db_session.commit() - await db_session.refresh(new_img_asset) - except Exception as e: + except IntegrityError as e: await db_session.rollback() + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Image is still referenced by other objects and cannot be deleted.", + ) from e - try: - destination.unlink(missing_ok=True) - except OSError: - # This logs to ensure we know there's now an orphaned file being stored. - _logger.info("Failed to clean up image after failed DB insertion: %s.", destination) + try: + image_path.unlink(missing_ok=True) + except OSError as e: + _logger.error("Failed to delete image file: %s", image_path) raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Failed to clean up image after failed write.", + detail="Database entry was deleted, but deleting the image file failed.", ) from e - return new_img_asset + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/tests/integration/test_events.py b/tests/integration/test_events.py index 19309a2..ea03639 100644 --- a/tests/integration/test_events.py +++ b/tests/integration/test_events.py @@ -7,11 +7,17 @@ from config import settings from database import DBSession from event.constants import EventStatusEnum +from event.models import EventCreate from event.tables import EventDB from image_asset.tables import ImageAssetDB pytestmark = pytest.mark.asyncio(loop_scope="session") +UPDATE_EVENT_START = datetime(2030, 1, 1, 18, tzinfo=UTC) +UPDATE_EVENT_END = UPDATE_EVENT_START + timedelta(hours=2) +ORIGINAL_IMAGE_KEY = "images/update-event-original.png" +REPLACEMENT_IMAGE_KEY = "images/update-event-replacement.png" + async def seed_events(db_session: DBSession) -> None: now = datetime.now(UTC) @@ -58,6 +64,34 @@ async def seed_events(db_session: DBSession) -> None: await db_session.commit() +async def seed_event_for_update(db_session: DBSession) -> tuple[int, int, int]: + original_image = ImageAssetDB( + storage_key=ORIGINAL_IMAGE_KEY, + original_filename="update-event-original.png", + ) + replacement_image = ImageAssetDB( + storage_key=REPLACEMENT_IMAGE_KEY, + original_filename="update-event-replacement.png", + ) + db_session.add_all([original_image, replacement_image]) + await db_session.flush() + + event = EventDB( + name="Event to update", + description="Original description.", + start_datetime=UPDATE_EVENT_START, + end_datetime=UPDATE_EVENT_END, + status=EventStatusEnum.SCHEDULED, + image_id=original_image.image_id, + ) + db_session.add(event) + await db_session.flush() + + ids = event.eid, original_image.image_id, replacement_image.image_id + await db_session.commit() + return ids + + @pytest.mark.parametrize( ("params", "expected_names"), [ @@ -121,3 +155,160 @@ async def test__get_events_rejects_invalid_boolean_query_params(client: AsyncCli response = await client.get("/api/event", params={"current": "not-a-boolean"}) assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT + + +async def test__update_event_preserves_image_when_image_id_is_omitted( + db_session: DBSession, + admin_client: AsyncClient, +): + event_id, original_image_id, _ = await seed_event_for_update(db_session) + + response = await admin_client.patch( + f"/api/event/{event_id}", + json={"name": "Updated event"}, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["name"] == "Updated event" + assert response.json()["image_id"] == original_image_id + assert response.json()["image_url"] == f"{settings.media_base_url.rstrip('/')}/{ORIGINAL_IMAGE_KEY}" + + db_event = await db_session.get(EventDB, event_id, populate_existing=True) + assert db_event is not None + assert db_event.name == "Updated event" + assert db_event.image_id == original_image_id + + +async def test__update_event_preserves_image_when_image_id_is_unchanged( + db_session: DBSession, + admin_client: AsyncClient, +): + event_id, original_image_id, _ = await seed_event_for_update(db_session) + + response = await admin_client.patch( + f"/api/event/{event_id}", + json={"image_id": original_image_id}, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["image_id"] == original_image_id + assert response.json()["image_url"] == f"{settings.media_base_url.rstrip('/')}/{ORIGINAL_IMAGE_KEY}" + + db_event = await db_session.get(EventDB, event_id, populate_existing=True) + assert db_event is not None + assert db_event.image_id == original_image_id + + +async def test__update_event_replaces_image( + db_session: DBSession, + admin_client: AsyncClient, +): + event_id, _, replacement_image_id = await seed_event_for_update(db_session) + + response = await admin_client.patch( + f"/api/event/{event_id}", + json={"image_id": replacement_image_id}, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["image_id"] == replacement_image_id + assert response.json()["image_url"] == f"{settings.media_base_url.rstrip('/')}/{REPLACEMENT_IMAGE_KEY}" + + db_event = await db_session.get(EventDB, event_id, populate_existing=True) + assert db_event is not None + assert db_event.image_id == replacement_image_id + + +async def test__update_event_clears_image_when_image_id_is_null( + db_session: DBSession, + admin_client: AsyncClient, +): + event_id, _, _ = await seed_event_for_update(db_session) + + response = await admin_client.patch( + f"/api/event/{event_id}", + json={"image_id": None}, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["image_id"] is None + assert response.json()["image_url"] is None + + db_event = await db_session.get(EventDB, event_id, populate_existing=True) + assert db_event is not None + assert db_event.image_id is None + + +async def test__update_event_rejects_missing_image_without_modifying_event( + db_session: DBSession, + admin_client: AsyncClient, +): + event_id, original_image_id, _ = await seed_event_for_update(db_session) + + response = await admin_client.patch( + f"/api/event/{event_id}", + json={"image_id": 0}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"detail": "Image asset doesn't exist."} + + db_event = await db_session.get(EventDB, event_id, populate_existing=True) + assert db_event is not None + assert db_event.image_id == original_image_id + + +async def test__update_event_validates_the_merged_time_range( + db_session: DBSession, + admin_client: AsyncClient, +): + event_id, _, _ = await seed_event_for_update(db_session) + + response = await admin_client.patch( + f"/api/event/{event_id}", + json={"start_datetime": (UPDATE_EVENT_END + timedelta(hours=1)).isoformat()}, + ) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT + + db_event = await db_session.get(EventDB, event_id, populate_existing=True) + assert db_event is not None + assert db_event.start_datetime == UPDATE_EVENT_START + + +async def test__update_event_returns_not_found_for_missing_event(admin_client: AsyncClient): + response = await admin_client.patch("/api/event/0", json={"name": "Missing event"}) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json() == {"detail": "Event doesn't exist."} + + +async def test__update_event_requires_authentication(client: AsyncClient): + response = await client.patch("/api/event/0", json={"name": "Unauthorized update"}) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +async def test__create_event_returns_url(db_session: DBSession, admin_client: AsyncClient): + image = ImageAssetDB( + storage_key="images/new-event.png", + original_filename="new-event.png", + ) + db_session.add(image) + await db_session.flush() + now = datetime.now(UTC).isoformat() + + response = await admin_client.post( + "/api/event", + json={ + "name": "New Event", + "description": "Description", + "start_datetime": now, + "end_datetime": now, + "status": EventStatusEnum.SCHEDULED, + "image_id": image.image_id, + }, + ) + + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["image_url"] == f"{settings.media_base_url}/images/new-event.png" diff --git a/tests/integration/test_image_asset.py b/tests/integration/test_image_asset.py index ffff950..f697d07 100644 --- a/tests/integration/test_image_asset.py +++ b/tests/integration/test_image_asset.py @@ -15,8 +15,8 @@ import image_asset.urls as image_urls from config import settings from database import DBSession +from image_asset.constants import MAX_ATTEMPTS, MAX_PIXELS, ImageAssetCategory from image_asset.models import ImageAsset -from image_asset.urls import MAX_PIXELS pytestmark = pytest.mark.asyncio(loop_scope="session") @@ -118,6 +118,11 @@ async def test__upload_image_asset(client: AsyncClient): assert response.status_code == status.HTTP_401_UNAUTHORIZED +async def test__delete_image_asset_requires_authentication(client: AsyncClient): + response = await client.delete("/api/image/1") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + # TODO: Unauthorized client @@ -191,6 +196,119 @@ async def test__admin_upload_good_image( assert saved_file.is_file() +@pytest.mark.parametrize( + ("category", "directory"), + [ + (ImageAssetCategory.EVENTS, "events"), + (ImageAssetCategory.EXECS, "execs"), + (ImageAssetCategory.PHOTOS, "photos"), + ], +) +async def test__admin_upload_image_to_category( + db_session: DBSession, + admin_client: AsyncClient, + tmp_path: Path, + category: ImageAssetCategory, + directory: str, +): + response = await admin_client.post( + "/api/image", + params={"category": category.value}, + files={"file": ("test.png", make_image(), "image/png")}, + ) + + assert response.status_code == status.HTTP_201_CREATED + + asset = ImageAsset.model_validate(response.json()) + + assert asset.storage_key.startswith(f"images/{directory}/") + assert asset.storage_key.endswith(".png") + assert await db_session.get(image_asset.crud.ImageAssetDB, asset.image_id) is not None + assert (tmp_path / asset.storage_key).is_file() + + +async def test__admin_upload_invalid_category( + db_session: DBSession, + admin_client: AsyncClient, + tmp_path: Path, +): + response = await admin_client.post( + "/api/image", + params={"category": "invalid"}, + files={"file": ("test.png", make_image(), "image/png")}, + ) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT + assert await image_asset.crud.get_all_image_assets(db_session) == [] + assert not any(path.is_file() for path in tmp_path.rglob("*")) + + +async def test__admin_upload_retries_without_overwriting_existing_file( + db_session: DBSession, + admin_client: AsyncClient, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + collision_uuid = UUID("00000000-0000-0000-0000-000000000001") + successful_uuid = UUID("00000000-0000-0000-0000-000000000002") + generated_uuids = iter((collision_uuid, successful_uuid)) + monkeypatch.setattr(image_urls, "uuid4", lambda: next(generated_uuids)) + + existing_file = tmp_path / f"images/{collision_uuid}.png" + existing_file.parent.mkdir(parents=True) + existing_file.write_bytes(b"existing file") + + response = await admin_client.post( + "/api/image", + files={"file": ("test.png", make_image(), "image/png")}, + ) + + assert response.status_code == status.HTTP_201_CREATED + + asset = ImageAsset.model_validate(response.json()) + + assert asset.storage_key == f"images/{successful_uuid}.png" + assert existing_file.read_bytes() == b"existing file" + assert (tmp_path / asset.storage_key).is_file() + assert await db_session.get(image_asset.crud.ImageAssetDB, asset.image_id) is not None + + +async def test__admin_upload_retries_after_storage_key_conflict( + db_session: DBSession, + admin_client: AsyncClient, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + collision_uuid = UUID("00000000-0000-0000-0000-000000000001") + successful_uuid = UUID("00000000-0000-0000-0000-000000000002") + collision_storage_key = f"images/{collision_uuid}.png" + + existing_asset = image_asset.crud.ImageAssetDB( + storage_key=collision_storage_key, + original_filename="existing.png", + created_at=datetime.now(UTC), + ) + image_asset.crud.create_image_asset(db_session, existing_asset) + await db_session.commit() + + generated_uuids = iter((collision_uuid, successful_uuid)) + monkeypatch.setattr(image_urls, "uuid4", lambda: next(generated_uuids)) + + response = await admin_client.post( + "/api/image", + files={"file": ("test.png", make_image(), "image/png")}, + ) + + assert response.status_code == status.HTTP_201_CREATED + + asset = ImageAsset.model_validate(response.json()) + + assert asset.storage_key == f"images/{successful_uuid}.png" + assert not (tmp_path / collision_storage_key).exists() + assert (tmp_path / asset.storage_key).is_file() + assert len(await image_asset.crud.get_all_image_assets(db_session)) == 2 + + @pytest.mark.parametrize( ("filename", "content", "content_type", "http_status"), [ @@ -232,12 +350,18 @@ async def test__admin_upload_invalid_image( assert not any(path.is_file() for path in tmp_path.rglob("*")) -async def test__admin_failed_db_insert_is_cleaned_up( +async def test__admin_exhausted_storage_key_conflicts_are_cleaned_up( db_session: DBSession, admin_client: AsyncClient, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ): fixed_uuid = UUID("00000000-0000-0000-0000-000000000001") - monkeypatch.setattr(image_urls, "uuid4", lambda: fixed_uuid) + generated_uuids = [] + + def generate_fixed_uuid(): + generated_uuids.append(fixed_uuid) + return fixed_uuid + + monkeypatch.setattr(image_urls, "uuid4", generate_fixed_uuid) storage_key = f"images/{fixed_uuid}.png" @@ -258,6 +382,71 @@ async def test__admin_failed_db_insert_is_cleaned_up( ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert response.json() == {"detail": "Failed to make image asset, exhausted retries."} + assert len(generated_uuids) == MAX_ATTEMPTS saved_file = tmp_path / storage_key assert not saved_file.exists() + assets = await image_asset.crud.get_all_image_assets(db_session) + assert [asset.storage_key for asset in assets] == [storage_key] + + +async def test__admin_delete_image_asset(db_session: DBSession, admin_client: AsyncClient, tmp_path: Path): + upload_response = await admin_client.post( + "/api/image", + files={"file": ("test.png", make_image(), "image/png")}, + ) + assert upload_response.status_code == status.HTTP_201_CREATED + + asset = ImageAsset.model_validate(upload_response.json()) + saved_file = tmp_path / asset.storage_key + assert saved_file.is_file() + + delete_response = await admin_client.delete(f"/api/image/{asset.image_id}") + + assert delete_response.status_code == status.HTTP_204_NO_CONTENT + assert delete_response.content == b"" + assert await db_session.get(image_asset.crud.ImageAssetDB, asset.image_id) is None + assert not saved_file.exists() + + +async def test__admin_delete_missing_image_asset(admin_client: AsyncClient): + response = await admin_client.delete("/api/image/0") + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json() == {"detail": "Image can't be found"} + + +async def test__admin_cannot_delete_referenced_image_asset( + db_session: DBSession, + admin_client: AsyncClient, + tmp_path: Path, +): + upload_response = await admin_client.post( + "/api/image", + files={"file": ("referenced.png", make_image(), "image/png")}, + ) + assert upload_response.status_code == status.HTTP_201_CREATED + + asset = ImageAsset.model_validate(upload_response.json()) + saved_file = tmp_path / asset.storage_key + + event_response = await admin_client.post( + "/api/event", + json={ + "name": "Event with an image", + "description": "The image asset must remain available.", + "start_datetime": "2026-09-05T12:00:00-07:00", + "end_datetime": "2026-09-05T13:00:00-07:00", + "status": "scheduled", + "image_id": asset.image_id, + }, + ) + assert event_response.status_code == status.HTTP_201_CREATED + + delete_response = await admin_client.delete(f"/api/image/{asset.image_id}") + + assert delete_response.status_code == status.HTTP_409_CONFLICT + assert delete_response.json() == {"detail": "Image is still referenced by other objects and cannot be deleted."} + assert await db_session.get(image_asset.crud.ImageAssetDB, asset.image_id) is not None + assert saved_file.is_file() diff --git a/tests/unit/test_image_asset.py b/tests/unit/test_image_asset.py index 27de1a5..a5b5dfa 100644 --- a/tests/unit/test_image_asset.py +++ b/tests/unit/test_image_asset.py @@ -4,7 +4,8 @@ from fastapi import HTTPException, UploadFile, status from PIL import Image -from image_asset.urls import ALLOWED_IMAGE_TYPES, MAX_PIXELS, validate_upload +from image_asset.constants import ALLOWED_IMAGE_TYPES, MAX_PIXELS +from image_asset.urls import validate_upload pytestmark = pytest.mark.unit