From 70e2072b79ff3cb2cdf6a4992ff80dff3cf660dd Mon Sep 17 00:00:00 2001 From: AdrianoDev Date: Tue, 6 Oct 2026 11:27:23 +0200 Subject: [PATCH] File di riferimento rimossi con il task, immagini animate rifiutate, filtro production_run_id (#43) Co-Authored-By: Claude Sonnet 5.5 --- src/backend/api/routers/measurements.py | 6 +- src/backend/api/routers/tasks.py | 21 ++- src/backend/api/routers/vision.py | 21 +++ .../test_task_cleanup_animated_run_filter.py | 120 ++++++++++++++++++ src/frontend/flask_app/blueprints/measure.py | 12 +- src/frontend/flask_app/tests/test_measure.py | 32 ++++- 6 files changed, 197 insertions(+), 15 deletions(-) create mode 100644 src/backend/tests/test_task_cleanup_animated_run_filter.py diff --git a/src/backend/api/routers/measurements.py b/src/backend/api/routers/measurements.py index e291153..38e1342 100644 --- a/src/backend/api/routers/measurements.py +++ b/src/backend/api/routers/measurements.py @@ -174,6 +174,7 @@ def _build_measurement_filters( date_from: datetime | None, date_to: datetime | None, pass_fail: str | None, + production_run_id: int | None = None, ) -> list: """Build SQLAlchemy filter conditions shared by list and CSV export.""" filters = [] @@ -199,6 +200,8 @@ def _build_measurement_filters( filters.append(Measurement.measured_at <= date_to) if pass_fail is not None: filters.append(Measurement.pass_fail == pass_fail) + if production_run_id is not None: + filters.append(Measurement.production_run_id == production_run_id) return filters @@ -213,6 +216,7 @@ async def get_measurements( date_from: datetime | None = Query(None), date_to: datetime | None = Query(None), pass_fail: str | None = Query(None, pattern="^(pass|warning|fail)$"), + production_run_id: int | None = Query(None, gt=0), page: int = Query(1, ge=1), per_page: int = Query(50, ge=1, le=500), user: User = Depends(get_current_user), @@ -228,7 +232,7 @@ async def get_measurements( """ filters = _build_measurement_filters( recipe_id, version_id, subtask_id, measured_by, lot_number, - serial_number, date_from, date_to, pass_fail, + serial_number, date_from, date_to, pass_fail, production_run_id, ) # Build query diff --git a/src/backend/api/routers/tasks.py b/src/backend/api/routers/tasks.py index bf85d78..037a436 100644 --- a/src/backend/api/routers/tasks.py +++ b/src/backend/api/routers/tasks.py @@ -19,7 +19,8 @@ from src.backend.models.api.task import ( TaskUpdate, validate_tolerance_order, ) -from src.backend.services import recipe_service +from src.backend.models.orm.vision import VisionReferenceImage +from src.backend.services import recipe_service, vision_service router = APIRouter(tags=["tasks"]) @@ -366,8 +367,24 @@ async def delete_task( task = await _task_for_write(db, task, user) + # The rows go with the task (migration 014 cascades them), the files do not: + # collect them now, remove the files only once the commit is durable - same + # order as vision.py::delete_reference_image. Rows are deleted explicitly so + # file_still_referenced sees the post-delete state even where the database + # does not enforce the cascade. + references = (await db.execute( + select(VisionReferenceImage).where(VisionReferenceImage.task_id == task.id) + )).scalars().all() + for reference in references: + await db.delete(reference) + await db.delete(task) - await db.flush() + await db.commit() + + # A copy-on-write sibling may still point at the same file (#38.6). + for reference in references: + if not await vision_service.file_still_referenced(db, reference.path): + vision_service.remove_reference_file(reference) # --------------------------------------------------------------------------- diff --git a/src/backend/api/routers/vision.py b/src/backend/api/routers/vision.py index 7901b67..e93c30b 100644 --- a/src/backend/api/routers/vision.py +++ b/src/backend/api/routers/vision.py @@ -4,7 +4,10 @@ It does not know, and must not know, whether the server or the station computed the numbers: that is what lets a station's configuration change without touching the frontend or the recipes. """ +import io + from fastapi import APIRouter, Depends, File, Form, HTTPException, UploadFile, status +from PIL import Image from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -90,9 +93,27 @@ async def _read_image(image: UploadFile) -> bytes: status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, detail="the file's content does not match a supported image format", ) + _reject_animated(content) return content +def _reject_animated(content: bytes) -> None: + """Animated GIF/WebP/APNG have no single frame to measure (decision of + 06/10/2026, #43): refuse them instead of silently reading frame 0.""" + try: + with Image.open(io.BytesIO(content)) as img: + animated = getattr(img, "is_animated", False) or getattr(img, "n_frames", 1) > 1 + except Exception: + # Unreadable here is not this check's business: the magic bytes passed + # and the worker decides what it can decode. + return + if animated: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail="animated images are not supported", + ) + + @router.post("/execute", response_model=VisionExecuteResponse) async def execute( task_id: int = Form(...), diff --git a/src/backend/tests/test_task_cleanup_animated_run_filter.py b/src/backend/tests/test_task_cleanup_animated_run_filter.py new file mode 100644 index 0000000..f4e4644 --- /dev/null +++ b/src/backend/tests/test_task_cleanup_animated_run_filter.py @@ -0,0 +1,120 @@ +"""#43: reference files removed with their task, animated images refused, +GET /api/measurements filtered by production run.""" +import io + +import pytest +from PIL import Image +from sqlalchemy import select + +from src.backend.config import settings +from src.backend.models.orm.measurement import Measurement +from src.backend.models.orm.vision import VisionReferenceImage +from src.backend.tests.conftest import auth_headers, create_test_recipe +from src.backend.tests.test_vision_upload_limits import a_png, recipe_parts + + +@pytest.fixture(autouse=True) +def redirect_uploads(monkeypatch, tmp_path): + monkeypatch.setattr(settings, "upload_dir", str(tmp_path)) + + +async def _upload_reference(client, user, task_id): + resp = await client.post( + "/api/vision/reference-images", headers=auth_headers(user), + files={"image": ("p.png", a_png(), "image/png")}, + data={"task_id": str(task_id)}, + ) + assert resp.status_code in (200, 201), resp.text + + +@pytest.mark.asyncio +async def test_delete_task_removes_the_reference_file(client, db_session, maker_user): + recipe = await create_test_recipe(db_session, maker_user.id) + _, task, _ = await recipe_parts(db_session, recipe.id) + await db_session.commit() + await _upload_reference(client, maker_user, task.id) + ref = (await db_session.execute(select(VisionReferenceImage))).scalars().one() + path = settings.upload_path / ref.path + assert path.exists() + + resp = await client.delete(f"/api/tasks/{task.id}", headers=auth_headers(maker_user)) + + assert resp.status_code == 204 + assert not path.exists() + + +@pytest.mark.asyncio +async def test_delete_task_keeps_a_file_another_task_still_references( + client, db_session, maker_user, +): + r1 = await create_test_recipe(db_session, maker_user.id) + r2 = await create_test_recipe(db_session, maker_user.id, code="REC-002") + _, t1, _ = await recipe_parts(db_session, r1.id) + _, t2, _ = await recipe_parts(db_session, r2.id) + await db_session.commit() + await _upload_reference(client, maker_user, t1.id) + ref = (await db_session.execute(select(VisionReferenceImage))).scalars().one() + db_session.add(VisionReferenceImage(task_id=t2.id, path=ref.path)) + await db_session.commit() + path = settings.upload_path / ref.path + + resp = await client.delete(f"/api/tasks/{t1.id}", headers=auth_headers(maker_user)) + + assert resp.status_code == 204 + assert path.exists() + + +def _animated(fmt: str, frames: int) -> bytes: + imgs = [Image.new("L", (16, 16), color=i * 40) for i in range(frames)] + buf = io.BytesIO() + imgs[0].save(buf, format=fmt, save_all=True, append_images=imgs[1:], duration=50) + return buf.getvalue() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("fmt,mime,frames,expected", [ + ("GIF", "image/gif", 2, 422), + ("WEBP", "image/webp", 2, 422), + ("GIF", "image/gif", 1, 201), +]) +async def test_animated_images_are_refused( + client, db_session, maker_user, fmt, mime, frames, expected, +): + recipe = await create_test_recipe(db_session, maker_user.id) + _, task, _ = await recipe_parts(db_session, recipe.id) + await db_session.commit() + + resp = await client.post( + "/api/vision/reference-images", headers=auth_headers(maker_user), + files={"image": ("a", _animated(fmt, frames), mime)}, + data={"task_id": str(task.id)}, + ) + + if expected == 422: + assert resp.status_code == 422 + assert "animated" in resp.json()["detail"] + else: + assert resp.status_code in (200, 201), resp.text + + +@pytest.mark.asyncio +async def test_measurements_filter_by_production_run(client, db_session, maker_user): + recipe = await create_test_recipe(db_session, maker_user.id) + version, _, subtask = await recipe_parts(db_session, recipe.id) + for run in (7, 7, 8, None): + db_session.add(Measurement( + subtask_id=subtask.id, version_id=version.id, measured_by=maker_user.id, + value=1.0, pass_fail="pass", deviation=0.0, input_method="manual", + production_run_id=run, + )) + await db_session.commit() + + resp = await client.get( + "/api/measurements/", headers=auth_headers(maker_user), + params={"production_run_id": 7}, + ) + + assert resp.status_code == 200 + body = resp.json() + assert body["total"] == 2 + assert {m["production_run_id"] for m in body["items"]} == {7} diff --git a/src/frontend/flask_app/blueprints/measure.py b/src/frontend/flask_app/blueprints/measure.py index 9f83b76..256f14b 100644 --- a/src/frontend/flask_app/blueprints/measure.py +++ b/src/frontend/flask_app/blueprints/measure.py @@ -386,14 +386,12 @@ def task_complete(recipe_id: int): # Load measurements if version_id provided measurements = [] if version_id: - # Scoped to this measurement session, not the whole version (#30): a - # second production run on the same recipe version used to pull in - # every other station's/day's readings too. GET /api/measurements has - # no production_run_id filter yet (that's lotto A3), so lot/serial + - # the current operator is the closest available identity - MANCA: - # once the backend accepts production_run_id, prefer it when the - # active run is known (see _current_run_id()). + # Scoped to this measurement session, not the whole version (#30): the + # active production run when known, else lot/serial + current operator. params = {"version_id": version_id, "per_page": 500} + run_id = _current_run_id() + if run_id: + params["production_run_id"] = run_id if lot_number: params["lot_number"] = lot_number if serial_number: diff --git a/src/frontend/flask_app/tests/test_measure.py b/src/frontend/flask_app/tests/test_measure.py index 28b72eb..680387a 100644 --- a/src/frontend/flask_app/tests/test_measure.py +++ b/src/frontend/flask_app/tests/test_measure.py @@ -338,6 +338,7 @@ class TestTaskComplete: # The summary serialises the version number, so the recipe carries one. {"id": 1, "code": "REC-001", "name": "Test Recipe", "version": 1}, [{"id": 1, "title": "Task 1", "order_index": 0, "subtasks": []}], + None, # current run {"items": []}, # measurements ] @@ -357,13 +358,14 @@ class TestTaskComplete: mock_api_client.get.side_effect = [ {"id": 1, "code": "REC-001", "name": "Test Recipe", "version": 1}, [{"id": 1, "title": "Task 1", "order_index": 0, "subtasks": []}], + None, # current run {"items": [], "pages": 1}, ] resp = logged_in_client.get("/measure/complete/1?version_id=5") assert resp.status_code == 200 - endpoint, kwargs = mock_api_client.get.call_args_list[2] + endpoint, kwargs = mock_api_client.get.call_args_list[3] assert endpoint[0] == "/api/measurements" params = kwargs["params"] assert params["version_id"] == "5" @@ -371,6 +373,24 @@ class TestTaskComplete: assert params["serial_number"] == "S1" assert params["measured_by"] == 1 # session["user_id"] from the fixture + def test_measurements_filtered_by_the_active_production_run( + self, logged_in_client, mock_api_client, + ): + """#43: when the station has a run open, the summary asks for that run.""" + mock_api_client.get.side_effect = [ + {"id": 1, "code": "REC-001", "name": "Test Recipe", "version": 1}, + [{"id": 1, "title": "Task 1", "order_index": 0, "subtasks": []}], + {"id": 77}, # current run + {"items": [], "pages": 1}, + ] + + resp = logged_in_client.get("/measure/complete/1?version_id=5") + assert resp.status_code == 200 + + endpoint, kwargs = mock_api_client.get.call_args_list[3] + assert endpoint[0] == "/api/measurements" + assert kwargs["params"]["production_run_id"] == 77 + def test_measurements_without_lot_or_serial_still_scope_to_the_operator( self, logged_in_client, mock_api_client, ): @@ -379,13 +399,14 @@ class TestTaskComplete: mock_api_client.get.side_effect = [ {"id": 1, "code": "REC-001", "name": "Test Recipe", "version": 1}, [{"id": 1, "title": "Task 1", "order_index": 0, "subtasks": []}], + None, # current run {"items": [], "pages": 1}, ] resp = logged_in_client.get("/measure/complete/1?version_id=5") assert resp.status_code == 200 - endpoint, kwargs = mock_api_client.get.call_args_list[2] + endpoint, kwargs = mock_api_client.get.call_args_list[3] params = kwargs["params"] assert "lot_number" not in params assert "serial_number" not in params @@ -404,15 +425,16 @@ class TestTaskComplete: mock_api_client.get.side_effect = [ {"id": 1, "code": "REC-001", "name": "Test Recipe", "version": 1}, [{"id": 1, "title": "Task 1", "order_index": 0, "subtasks": [subtask]}], + None, # current run page1, page2, ] resp = logged_in_client.get("/measure/complete/1?version_id=5") assert resp.status_code == 200 - assert mock_api_client.get.call_count == 4 - first_page_call = mock_api_client.get.call_args_list[2] - second_page_call = mock_api_client.get.call_args_list[3] + assert mock_api_client.get.call_count == 5 + first_page_call = mock_api_client.get.call_args_list[3] + second_page_call = mock_api_client.get.call_args_list[4] assert first_page_call.kwargs["params"]["page"] == 1 assert second_page_call.kwargs["params"]["page"] == 2