Merge pull request #12 from swissgeo/GPS-898-feat-add-delete-endpoint

GPS-898: add delete drawing feature

Author: hbollon

app/api/wps.py

@@ -9,7 +9,7 @@
 from typing import Annotated
 
 from fastapi import APIRouter, File, Form, Request, UploadFile
-from fastapi.responses import StreamingResponse
+from fastapi.responses import Response, StreamingResponse
 
 from app.core.drawings import DrawingsService, DrawingsServiceDep
 from app.schemas.drawings import DrawingsCreateResponse, DrawingsUpdateResponse
@@ -38,6 +38,14 @@
     File(description="The KMZ file to upload. Only KMZ files are accepted."),
 ]
 
+AdminIdForm = Annotated[
+    uuid.UUID,
+    Form(
+        description="Admin identifier required to authorize the operation",
+        examples=["00000000-0000-0000-0000-000000000000"],
+    ),
+]
+
 
 @router.post(
     "/drawings",
@@ -104,13 +112,7 @@ async def get_drawing(
 async def update_drawing(  # noqa: PLR0913, PLR0917
     request: Request,
     drawing_id: uuid.UUID,
-    admin_id: Annotated[
-        uuid.UUID,
-        Form(
-            description="Admin identifier required to update the drawing",
-            examples=["00000000-0000-0000-0000-000000000000"],
-        ),
-    ],
+    admin_id: AdminIdForm,
     file: KmzFile,
     sha256: Sha256Form,
     drawings: DrawingsServiceDep,
@@ -124,3 +126,26 @@ async def update_drawing(  # noqa: PLR0913, PLR0917
     timestamps.
     """
     return await drawings.update_drawing(drawing_id, admin_id, file, request, sha256)
+
+
+@router.delete(
+    "/drawings/{drawing_id}",
+    status_code=204,
+    responses={
+        403: {"model": ErrorResponse},
+        404: {"model": ErrorResponse},
+        500: {"model": ErrorResponse},
+    },
+)
+async def delete_drawing(
+    drawing_id: uuid.UUID,
+    admin_id: AdminIdForm,
+    drawings: DrawingsServiceDep,
+) -> Response:
+    """Delete a KMZ drawing.
+
+    The admin_id must match the stored drawing metadata, otherwise the request
+    is rejected with 403. The deletion is permanent.
+    """
+    await drawings.delete_drawing(drawing_id, admin_id)
+    return Response(status_code=204)

app/core/drawings.py

@@ -272,6 +272,37 @@ async def get_drawing(self, drawing_id: uuid.UUID) -> tuple[AsyncIterator[bytes]
 
         return self._s3.get_drawing(s3_key), s3_key
 
+    async def delete_drawing(self, drawing_id: uuid.UUID, admin_id: uuid.UUID) -> None:
+        """Delete a KMZ drawing from S3.
+
+        Verifies the drawing exists and the admin_id matches the stored metadata
+        before deleting the object. The deletion is permanent, the only way to
+        recover it is through S3 versioning.
+
+        Args:
+            drawing_id: The UUID of the drawing to delete.
+            admin_id: Admin identifier that must match the stored drawing.
+
+        Raises:
+            DrawingNotFoundError: If no drawing exists with the given identifier.
+            AdminIdMismatchError: If the admin_id does not match the stored one.
+            S3Error: If the S3 delete operation fails.
+
+        """
+        s3_key = self.build_s3_key(drawing_id)
+
+        # Head the object first so a missing drawing surfaces as a 404 before
+        # the delete is attempted
+        existing = await self._s3.head_drawing(s3_key)
+
+        if existing.get("admin-id") != str(admin_id):
+            logger.warning("admin_id mismatch for drawing %s", drawing_id)
+            raise AdminIdMismatchError
+
+        await self._s3.delete_drawing(s3_key)
+
+        logger.info("Drawing deleted: id=%s", drawing_id)
+
 
 async def get_drawings_service(
     s3: S3ServiceDep,

app/core/s3.py

@@ -117,6 +117,22 @@ async def head_drawing(self, key: str) -> dict[str, str]:
         else:
             return metadata
 
+    async def delete_drawing(self, key: str) -> None:
+        """Delete a KMZ file from S3.
+
+        Args:
+            key: S3 object key
+
+        Raises:
+            S3Error: If the S3 delete fails
+
+        """
+        try:
+            await self._client.delete_object(Bucket=self._bucket, Key=key)
+        except botocore.exceptions.BotoCoreError as e:
+            logger.exception("S3 delete failed for key %s", key)
+            raise S3Error(f"S3 delete failed for key {key}: {e}") from e
+
     async def check_bucket(self) -> bool:
         """Check whether the configured S3 bucket is accessible.
 

app/tests/test_drawings_endpoints.py

@@ -311,3 +311,72 @@ def test_create_drawing_body_size_within_limit(client: TestClient, valid_kmz_byt
         data={"sha256": _sha256(valid_kmz_bytes)},
     )
     assert response.status_code == 201
+
+
+def test_delete_drawing_success(client: TestClient, valid_kmz_bytes: bytes):
+    """DELETE an existing drawing with the correct admin_id returns 204 and removes it."""
+    create_resp = client.post(
+        "/api/wps/v1/drawings",
+        files={"file": ("test.kmz", valid_kmz_bytes, "application/vnd.google-earth.kmz")},
+        data={"sha256": _sha256(valid_kmz_bytes)},
+    )
+    assert create_resp.status_code == 201
+    drawing_id = create_resp.json()["id"]
+    admin_id = create_resp.json()["admin_id"]
+
+    response = client.request(
+        "DELETE",
+        f"/api/wps/v1/drawings/{drawing_id}",
+        data={"admin_id": admin_id},
+    )
+    assert response.status_code == 204
+
+    # The drawing must no longer be retrievable
+    get_resp = client.get(f"/api/wps/v1/drawings/{drawing_id}")
+    assert get_resp.status_code == 404
+
+
+def test_delete_drawing_wrong_admin_id(client: TestClient, valid_kmz_bytes: bytes):
+    """DELETE with a mismatched admin_id returns 403 Forbidden."""
+    create_resp = client.post(
+        "/api/wps/v1/drawings",
+        files={"file": ("test.kmz", valid_kmz_bytes, "application/vnd.google-earth.kmz")},
+        data={"sha256": _sha256(valid_kmz_bytes)},
+    )
+    assert create_resp.status_code == 201
+    drawing_id = create_resp.json()["id"]
+
+    response = client.request(
+        "DELETE",
+        f"/api/wps/v1/drawings/{drawing_id}",
+        data={"admin_id": str(uuid.uuid4())},
+    )
+    assert response.status_code == 403
+    assert "detail" in response.json()
+
+
+def test_delete_drawing_not_found(client: TestClient):
+    """DELETE a non-existent drawing returns 404 Not Found."""
+    response = client.request(
+        "DELETE",
+        "/api/wps/v1/drawings/00000000-0000-0000-0000-000000000000",
+        data={"admin_id": str(uuid.uuid4())},
+    )
+    assert response.status_code == 404
+    assert "detail" in response.json()
+
+
+def test_delete_drawing_missing_admin_id(client: TestClient):
+    """DELETE without the admin_id form field returns 422 Unprocessable Entity."""
+    response = client.request("DELETE", "/api/wps/v1/drawings/00000000-0000-0000-0000-000000000000")
+    assert response.status_code == 422
+
+
+def test_delete_drawing_invalid_uuid(client: TestClient):
+    """DELETE with a malformed UUID returns 422 Unprocessable Entity."""
+    response = client.request(
+        "DELETE",
+        "/api/wps/v1/drawings/not-a-valid-uuid",
+        data={"admin_id": str(uuid.uuid4())},
+    )
+    assert response.status_code == 422

app/tests/test_s3_service.py

@@ -213,3 +213,39 @@ async def test_check_bucket_failure(settings) -> None:
     svc = S3Service(client=client, bucket=settings.aws_s3_bucket_name)
 
     assert await svc.check_bucket() is False
+
+
+@pytest.mark.asyncio
+async def test_delete_drawing_success(settings) -> None:
+    """Upload then delete, verify the object is gone."""
+    session = aioboto3.Session()
+    async with session.client("s3", endpoint_url=settings.aws_endpoint_url) as client:  # type: ignore  # noqa: PGH003
+        svc = S3Service(client=client, bucket=settings.aws_s3_bucket_name)
+        key = "drawings/delete-test.kmz"
+        data = b"content to delete"
+        metadata = {
+            "sha256": "jkl012",
+            "admin-id": "11111111-1111-1111-1111-111111111111",
+            "created-at": "2026-01-01T00:00:00+00:00",
+            "modified-at": "2026-01-01T00:00:00+00:00",
+        }
+
+        await svc.upload_drawing(
+            key, io.BytesIO(data), "application/vnd.google-earth.kmz", metadata
+        )
+
+        await svc.delete_drawing(key)
+
+        with pytest.raises(DrawingNotFoundError):
+            await svc.head_drawing(key)
+
+
+@pytest.mark.asyncio
+async def test_delete_drawing_botocore_error(settings) -> None:
+    """A BotoCoreError during delete should raise S3Error."""
+    client = MagicMock()
+    client.delete_object = AsyncMock(side_effect=botocore.exceptions.BotoCoreError())
+    svc = S3Service(client=client, bucket=settings.aws_s3_bucket_name)
+
+    with pytest.raises(S3Error):
+        await svc.delete_drawing("drawings/x.kmz")

pyproject.toml

@@ -14,9 +14,9 @@ dependencies = [
     # https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation#readme
     "httptools~=0.8.0",
     "opentelemetry-exporter-otlp~=1.41",
-    "opentelemetry-instrumentation-asgi==0.62b1",     # asgi is used by fastapi
-    "opentelemetry-instrumentation-botocore==0.62b1",
-    "opentelemetry-instrumentation-fastapi==0.62b1",
+    "opentelemetry-instrumentation-asgi==0.65b0",     # asgi is used by fastapi
+    "opentelemetry-instrumentation-botocore==0.65b0",
+    "opentelemetry-instrumentation-fastapi==0.65b0",
     "opentelemetry-sdk~=1.41",
     "orjson~=3.11",
     "pydantic-settings~=2.13",

scripts/smoke_test_drawings_api.sh

@@ -325,6 +325,32 @@ else
     fail "  Downloaded content does NOT match updated file"
 fi
 
+# --- DELETE /api/wps/v1/drawings/{id} ---
+echo -e "\n${BOLD}=== DELETE /api/wps/v1/drawings/{id} ===${NC}"
+
+HTTP=$(curl -s -o /dev/null -w '%{http_code}' -X DELETE \
+    -F "admin_id=$WRONG_ADMIN_ID" \
+    "$BASE_URL/api/wps/v1/drawings/$DRAWING_ID")
+assert_status "Reject wrong admin_id" 403 "$HTTP"
+
+HTTP=$(curl -s -o /dev/null -w '%{http_code}' -X DELETE \
+    -F "admin_id=$ADMIN_ID" \
+    "$BASE_URL/api/wps/v1/drawings/00000000-0000-0000-0000-000000000000")
+assert_status "Delete non-existent drawing → 404" 404 "$HTTP"
+
+HTTP=$(curl -s -o /dev/null -w '%{http_code}' -X DELETE \
+    "$BASE_URL/api/wps/v1/drawings/$DRAWING_ID")
+assert_status "Delete without admin_id form field → 422" 422 "$HTTP"
+
+HTTP=$(curl -s -o /dev/null -w '%{http_code}' -X DELETE \
+    -F "admin_id=$ADMIN_ID" \
+    "$BASE_URL/api/wps/v1/drawings/$DRAWING_ID")
+assert_status "Delete existing drawing → 204" 204 "$HTTP"
+
+HTTP=$(curl -s -o /dev/null -w '%{http_code}' \
+    "$BASE_URL/api/wps/v1/drawings/$DRAWING_ID")
+assert_status "Deleted drawing no longer retrievable → 404" 404 "$HTTP"
+
 # --- OpenAPI spec ---
 echo -e "\n${BOLD}=== OpenAPI Spec ===${NC}"
 

scripts/test_s3_local.py

@@ -29,7 +29,7 @@
 TEST_FILE = "France.kmz"
 
 
-async def main() -> None:
+async def main() -> None:  # noqa: PLR0915
     import aioboto3  # noqa: PLC0415
 
     session = aioboto3.Session()
@@ -94,6 +94,19 @@ async def main() -> None:
         except DrawingNotFoundError:
             print("OK (DrawingNotFoundError)")
 
+        # Delete
+        print("6. delete_drawing ...", end=" ", flush=True)
+        await svc.delete_drawing(key)
+        print("OK")
+
+        # Head after delete (should be gone)
+        print("7. head_drawing (after delete) ...", end=" ", flush=True)
+        try:
+            await svc.head_drawing(key)
+            print("FAIL (expected DrawingNotFoundError)")
+        except DrawingNotFoundError:
+            print("OK (DrawingNotFoundError)")
+
         print()
         print("All checks passed ✓")