diff --git a/backend/aris/routes/file_annotations.py b/backend/aris/routes/file_annotations.py index 78ecf2a3..2a5ce9cd 100644 --- a/backend/aris/routes/file_annotations.py +++ b/backend/aris/routes/file_annotations.py @@ -5,7 +5,7 @@ from fastapi import APIRouter, Depends, HTTPException, status from pydantic import BaseModel, ConfigDict -from sqlalchemy import and_, or_, select +from sqlalchemy import and_, func, or_, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -274,15 +274,20 @@ async def create_annotation_message( raise HTTPException(status_code=403, detail="Comment permission required") # Private annotations: single note only. Shared annotations: unlimited (thread). + # Lock the annotation row to prevent race conditions where two concurrent + # requests both pass the count check before either inserts. if annotation.visibility != AnnotationVisibility.SHARED: - count_query = select(AnnotationMessage).where( + await db.execute( + select(Annotation).where(Annotation.id == annotation_id).with_for_update() + ) + count_query = select(func.count()).select_from(AnnotationMessage).where( and_( AnnotationMessage.annotation_id == annotation_id, AnnotationMessage.deleted_at.is_(None), ) ) count_result = await db.execute(count_query) - if len(count_result.scalars().all()) >= 1: + if count_result.scalar_one() >= 1: raise HTTPException( status_code=400, detail="Annotation already has a note" ) diff --git a/backend/tests/test_routes/test_routes_file_annotations.py b/backend/tests/test_routes/test_routes_file_annotations.py index 826e017c..41166803 100644 --- a/backend/tests/test_routes/test_routes_file_annotations.py +++ b/backend/tests/test_routes/test_routes_file_annotations.py @@ -444,6 +444,35 @@ async def test_private_annotation_single_note_limit( assert "already has a note" in resp2.json()["detail"] +async def test_private_annotation_note_limit_after_delete_and_recreate( + client: AsyncClient, authenticated_user +): + """After deleting a note, a new one can be created (soft-delete aware).""" + headers = {"Authorization": f"Bearer {authenticated_user['token']}"} + file_id = await _create_file(client, headers, authenticated_user["user_id"]) + ann = await _create_annotation(client, headers, file_id) + + resp1 = await client.post( + f"/annotations/{ann['id']}/messages", + headers=headers, + json={"content": "Original note"}, + ) + assert resp1.status_code == 201 + msg_id = resp1.json()["id"] + + delete_resp = await client.delete( + f"/annotations/messages/{msg_id}", headers=headers + ) + assert delete_resp.status_code == 204 + + resp2 = await client.post( + f"/annotations/{ann['id']}/messages", + headers=headers, + json={"content": "Replacement note"}, + ) + assert resp2.status_code == 201 + + async def test_shared_annotation_multiple_notes( client: AsyncClient, authenticated_user,