feat(vision): immagini di riferimento non orfane, GET/DELETE (#38.5)
save_reference_image scrive il file dopo il flush riuscito; il router
lo cancella se il commit successivo fallisce. Aggiunti GET
/api/vision/reference-images?task_id= (lista, Maker) e DELETE
/api/vision/reference-images/{id} (cancella riga e file, Maker).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -12,7 +12,9 @@ from src.backend.api.middleware.api_key import require_maker, require_measuremen
|
||||
from src.backend.api.routers.files import ALLOWED_IMAGE_TYPES, validate_file_size
|
||||
from src.backend.config import settings
|
||||
from src.backend.database import get_db
|
||||
from src.backend.models.api.vision import VisionExecuteResponse, VisionMeasurementResult
|
||||
from src.backend.models.api.vision import (
|
||||
VisionExecuteResponse, VisionMeasurementResult, VisionReferenceImageResponse,
|
||||
)
|
||||
from src.backend.models.orm.task import RecipeTask
|
||||
from src.backend.models.orm.user import User
|
||||
from src.backend.services import vision_service
|
||||
@@ -128,7 +130,7 @@ async def execute(
|
||||
|
||||
|
||||
# Composing a recipe and keeping a reference image are Maker operations, not
|
||||
# measuring: both endpoints below gate on `require_maker`, not the
|
||||
# measuring: every endpoint below gates on `require_maker`, not the
|
||||
# `require_measurement_tec` that `execute` above uses.
|
||||
|
||||
|
||||
@@ -146,10 +148,39 @@ async def add_reference_image(
|
||||
db, task.id, content, note=note,
|
||||
extension=_IMAGE_EXTENSIONS[image.content_type],
|
||||
)
|
||||
await db.commit()
|
||||
try:
|
||||
await db.commit()
|
||||
except Exception:
|
||||
# The row never became durable - the file `save_reference_image`
|
||||
# wrote after its flush would otherwise outlive it as an orphan.
|
||||
vision_service.remove_reference_file(reference)
|
||||
raise
|
||||
return {"id": reference.id, "path": reference.path, "note": reference.note}
|
||||
|
||||
|
||||
@router.get("/reference-images", response_model=list[VisionReferenceImageResponse])
|
||||
async def get_reference_images(
|
||||
task_id: int,
|
||||
db: AsyncSession = Depends(get_db),
|
||||
current_user: User = Depends(require_maker),
|
||||
) -> list[VisionReferenceImageResponse]:
|
||||
await _task_or_404(db, task_id)
|
||||
return await vision_service.list_reference_images(db, task_id)
|
||||
|
||||
|
||||
@router.delete("/reference-images/{reference_id}", status_code=204)
|
||||
async def delete_reference_image(
|
||||
reference_id: int,
|
||||
db: AsyncSession = Depends(get_db),
|
||||
current_user: User = Depends(require_maker),
|
||||
) -> None:
|
||||
reference = await vision_service.delete_reference_image(db, reference_id)
|
||||
await db.commit()
|
||||
# Only now: if the commit above had failed, the row would still exist and
|
||||
# the file must still be there for it.
|
||||
vision_service.remove_reference_file(reference)
|
||||
|
||||
|
||||
@router.post("/preview")
|
||||
async def preview(
|
||||
task_id: int = Form(...),
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
"""Pydantic schemas for the vision execution response."""
|
||||
from pydantic import BaseModel
|
||||
from datetime import datetime
|
||||
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
|
||||
class VisionMeasurementResult(BaseModel):
|
||||
@@ -14,3 +16,16 @@ class VisionExecuteResponse(BaseModel):
|
||||
vision_result_id: int
|
||||
engine_version: str
|
||||
measurements: list[VisionMeasurementResult]
|
||||
|
||||
|
||||
class VisionReferenceImageResponse(BaseModel):
|
||||
"""One saved reference image, as `GET /api/vision/reference-images` lists it."""
|
||||
model_config = ConfigDict(from_attributes=True)
|
||||
|
||||
id: int
|
||||
task_id: int
|
||||
path: str
|
||||
note: str | None = None
|
||||
station_id: int | None = None
|
||||
device_code: str | None = None
|
||||
acquired_at: datetime
|
||||
|
||||
@@ -260,6 +260,21 @@ async def execute_task(
|
||||
return result, saved
|
||||
|
||||
|
||||
def _reference_file_path(reference: VisionReferenceImage) -> Path:
|
||||
return settings.upload_path / reference.path
|
||||
|
||||
|
||||
def remove_reference_file(reference: VisionReferenceImage) -> None:
|
||||
"""Delete a reference image's file, if it is still there.
|
||||
|
||||
Called by the router only once the row's fate is durable: after a
|
||||
successful commit when the row was just deleted, or to undo a
|
||||
`save_reference_image` whose row failed to commit (see both call sites).
|
||||
`missing_ok` because "already gone" must never be an error here.
|
||||
"""
|
||||
_reference_file_path(reference).unlink(missing_ok=True)
|
||||
|
||||
|
||||
async def save_reference_image(
|
||||
db: AsyncSession,
|
||||
task_id: int,
|
||||
@@ -276,17 +291,22 @@ async def save_reference_image(
|
||||
`extension` must match the upload's real content type (the router derives
|
||||
it from `image.content_type`) - writing a jpeg to disk as `<uuid>.png`
|
||||
would be a lie the filename tells forever.
|
||||
|
||||
The file is written only after the row has flushed successfully - a bad
|
||||
FK or constraint fails at `flush()`, before anything exists on disk, so
|
||||
there is nothing to clean up. The caller still has to remove the file if
|
||||
the *commit* that follows fails (`remove_reference_file`, from the
|
||||
router): a flush is not durable, and an orphaned file from a rolled-back
|
||||
row is exactly what #38 found.
|
||||
"""
|
||||
# `upload_path` is the resolved Path, `upload_dir` the bare setting: the
|
||||
# rest of the codebase uses the former (see production_export_service).
|
||||
folder = settings.upload_path / "vision" / "reference" / str(task_id)
|
||||
folder.mkdir(parents=True, exist_ok=True)
|
||||
name = f"{uuid4().hex}.{extension}"
|
||||
(folder / name).write_bytes(image_bytes)
|
||||
relative_path = str(Path("vision") / "reference" / str(task_id) / name)
|
||||
|
||||
reference = VisionReferenceImage(
|
||||
task_id=task_id,
|
||||
path=str(Path("vision") / "reference" / str(task_id) / name),
|
||||
path=relative_path,
|
||||
note=note,
|
||||
station_id=station_id,
|
||||
device_code=device_code,
|
||||
@@ -294,10 +314,46 @@ async def save_reference_image(
|
||||
)
|
||||
db.add(reference)
|
||||
await db.flush()
|
||||
|
||||
folder = _reference_file_path(reference).parent
|
||||
folder.mkdir(parents=True, exist_ok=True)
|
||||
_reference_file_path(reference).write_bytes(image_bytes)
|
||||
|
||||
await db.refresh(reference)
|
||||
return reference
|
||||
|
||||
|
||||
async def list_reference_images(
|
||||
db: AsyncSession, task_id: int,
|
||||
) -> list[VisionReferenceImage]:
|
||||
result = await db.execute(
|
||||
select(VisionReferenceImage)
|
||||
.where(VisionReferenceImage.task_id == task_id)
|
||||
.order_by(VisionReferenceImage.acquired_at)
|
||||
)
|
||||
return list(result.scalars().all())
|
||||
|
||||
|
||||
async def delete_reference_image(
|
||||
db: AsyncSession, reference_id: int,
|
||||
) -> VisionReferenceImage:
|
||||
"""Flush the row's deletion. The caller (router) removes the file only
|
||||
once the commit that makes the deletion durable has actually succeeded -
|
||||
deleting it any earlier would leave the row pointing at a file that is
|
||||
already gone if the commit then failed.
|
||||
"""
|
||||
reference = (await db.execute(
|
||||
select(VisionReferenceImage).where(VisionReferenceImage.id == reference_id)
|
||||
)).scalar_one_or_none()
|
||||
if reference is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND, detail="Reference image not found",
|
||||
)
|
||||
await db.delete(reference)
|
||||
await db.flush()
|
||||
return reference
|
||||
|
||||
|
||||
async def preview(db: AsyncSession, task: RecipeTask, image_bytes: bytes) -> dict:
|
||||
"""Run the graph and save nothing. Composing is not measuring."""
|
||||
if not task.vision_json:
|
||||
|
||||
@@ -85,6 +85,113 @@ async def test_a_reference_image_keeps_its_provenance(
|
||||
assert saved.path
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_file_survives_a_failed_commit_as_nothing(
|
||||
client, db_session, maker_user, monkeypatch,
|
||||
):
|
||||
"""#38.5: the file is written only after a successful flush, and removed
|
||||
if the commit that follows still fails - never an orphan on disk."""
|
||||
recipe = await create_test_recipe(db_session, maker_user.id)
|
||||
version, task, subtask = await recipe_parts(db_session, recipe.id)
|
||||
await db_session.commit()
|
||||
|
||||
original_commit = db_session.commit
|
||||
|
||||
async def boom(*args, **kwargs):
|
||||
raise RuntimeError("simulated commit failure")
|
||||
monkeypatch.setattr(db_session, "commit", boom)
|
||||
|
||||
try:
|
||||
# The ASGI transport re-raises an unhandled app exception rather
|
||||
# than turning it into a response - the same way it would surface
|
||||
# in production logs. What matters here is what is left on disk.
|
||||
with pytest.raises(RuntimeError, match="simulated commit failure"):
|
||||
await client.post(
|
||||
"/api/vision/reference-images",
|
||||
headers=auth_headers(maker_user),
|
||||
files={"image": ("ref.png", an_image(), "image/png")},
|
||||
data={"task_id": str(task.id)},
|
||||
)
|
||||
finally:
|
||||
# The db_session fixture commits again on its own teardown - restore
|
||||
# the real commit so that does not also blow up.
|
||||
monkeypatch.setattr(db_session, "commit", original_commit)
|
||||
|
||||
folder = settings.upload_path / "vision" / "reference" / str(task.id)
|
||||
assert not folder.exists() or list(folder.iterdir()) == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reference_images_are_listed_for_their_task(
|
||||
client, db_session, maker_user,
|
||||
):
|
||||
recipe = await create_test_recipe(db_session, maker_user.id)
|
||||
version, task, subtask = await recipe_parts(db_session, recipe.id)
|
||||
await db_session.commit()
|
||||
|
||||
for _ in range(2):
|
||||
response = await client.post(
|
||||
"/api/vision/reference-images",
|
||||
headers=auth_headers(maker_user),
|
||||
files={"image": ("ref.png", an_image(), "image/png")},
|
||||
data={"task_id": str(task.id)},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
listing = await client.get(
|
||||
"/api/vision/reference-images",
|
||||
headers=auth_headers(maker_user),
|
||||
params={"task_id": task.id},
|
||||
)
|
||||
|
||||
assert listing.status_code == 200
|
||||
body = listing.json()
|
||||
assert len(body) == 2
|
||||
assert {row["task_id"] for row in body} == {task.id}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_deleting_a_reference_image_removes_the_row_and_the_file(
|
||||
client, db_session, maker_user,
|
||||
):
|
||||
recipe = await create_test_recipe(db_session, maker_user.id)
|
||||
version, task, subtask = await recipe_parts(db_session, recipe.id)
|
||||
await db_session.commit()
|
||||
|
||||
created = await client.post(
|
||||
"/api/vision/reference-images",
|
||||
headers=auth_headers(maker_user),
|
||||
files={"image": ("ref.png", an_image(), "image/png")},
|
||||
data={"task_id": str(task.id)},
|
||||
)
|
||||
reference_id = created.json()["id"]
|
||||
file_path = settings.upload_path / created.json()["path"]
|
||||
assert file_path.exists()
|
||||
|
||||
response = await client.delete(
|
||||
f"/api/vision/reference-images/{reference_id}",
|
||||
headers=auth_headers(maker_user),
|
||||
)
|
||||
|
||||
assert response.status_code == 204
|
||||
assert (await db_session.execute(
|
||||
select(VisionReferenceImage).where(VisionReferenceImage.id == reference_id)
|
||||
)).scalar_one_or_none() is None
|
||||
assert not file_path.exists()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_deleting_an_unknown_reference_image_is_404(
|
||||
client, db_session, maker_user,
|
||||
):
|
||||
response = await client.delete(
|
||||
"/api/vision/reference-images/999999",
|
||||
headers=auth_headers(maker_user),
|
||||
)
|
||||
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_preview_runs_the_graph_and_saves_no_measurement(
|
||||
client, db_session, maker_user, worker_says,
|
||||
|
||||
Reference in New Issue
Block a user