diff --git a/src/backend/api/routers/vision.py b/src/backend/api/routers/vision.py index 0e8bb92..7901b67 100644 --- a/src/backend/api/routers/vision.py +++ b/src/backend/api/routers/vision.py @@ -177,8 +177,10 @@ async def delete_reference_image( 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) + # the file must still be there for it. And only if no other row (a + # copy-on-write sibling, #38.6) still references the same path. + if not await vision_service.file_still_referenced(db, reference.path): + vision_service.remove_reference_file(reference) @router.post("/preview") diff --git a/src/backend/services/recipe_service.py b/src/backend/services/recipe_service.py index 125658f..7faaeb1 100644 --- a/src/backend/services/recipe_service.py +++ b/src/backend/services/recipe_service.py @@ -12,6 +12,7 @@ from src.backend.models.orm.recipe import Recipe, RecipeVersion from src.backend.models.orm.setting import RecipeVersionAudit from src.backend.models.orm.task import RecipeSubtask, RecipeTask from src.backend.models.orm.user import User +from src.backend.models.orm.vision import VisionReferenceImage from src.backend.models.api.recipe import RecipeCreate, RecipeUpdate @@ -118,6 +119,28 @@ async def _copy_tasks_to_version( db.add(new_task) await db.flush() # get new_task.id + # Reference images (#38.6): the copied task points at the same files + # on disk as the source - same `path`, same acquisition metadata. A + # graph composed against a reference must keep working after the + # recipe is next edited, exactly like task_type/vision_json above. + # The file itself is only ever removed once no row references its + # path any more (see vision_service.file_still_referenced). + ref_images = (await db.execute( + select(VisionReferenceImage).where(VisionReferenceImage.task_id == task.id) + )).scalars().all() + for ref in ref_images: + db.add(VisionReferenceImage( + task_id=new_task.id, + path=ref.path, + station_id=ref.station_id, + device_code=ref.device_code, + calibration_snapshot=ref.calibration_snapshot, + engine_version=ref.engine_version, + note=ref.note, + expected_json=ref.expected_json, + acquired_at=ref.acquired_at, + )) + for sub in task.subtasks: new_sub = RecipeSubtask( task_id=new_task.id, diff --git a/src/backend/services/vision_service.py b/src/backend/services/vision_service.py index 3166633..58b1146 100644 --- a/src/backend/services/vision_service.py +++ b/src/backend/services/vision_service.py @@ -275,6 +275,22 @@ def remove_reference_file(reference: VisionReferenceImage) -> None: _reference_file_path(reference).unlink(missing_ok=True) +async def file_still_referenced(db: AsyncSession, path: str) -> bool: + """True if some VisionReferenceImage row still points at *path*. + + Copy-on-write (#38.6) can leave several rows sharing the same file on + disk (the copy carries over the source's `path` rather than duplicating + the file). The router must call this, after the row being deleted has + been committed, before it calls `remove_reference_file` - otherwise + deleting one task's reference would take the file out from under every + other task that still references it. + """ + result = await db.execute( + select(VisionReferenceImage.id).where(VisionReferenceImage.path == path).limit(1) + ) + return result.scalar_one_or_none() is not None + + async def save_reference_image( db: AsyncSession, task_id: int, diff --git a/src/backend/tests/test_vision_reference.py b/src/backend/tests/test_vision_reference.py index 0342fd2..3b5eeae 100644 --- a/src/backend/tests/test_vision_reference.py +++ b/src/backend/tests/test_vision_reference.py @@ -192,6 +192,78 @@ async def test_deleting_an_unknown_reference_image_is_404( assert response.status_code == 404 +@pytest.mark.asyncio +async def test_copy_on_write_carries_over_reference_images( + client, db_session, maker_user, measurement_tec_user, +): + """#38.6: a copy-on-write version must list the source task's reference + images (same path, same metadata), and the file is removed only once no + row references it any more.""" + 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), "note": "orig"}, + ) + assert created.status_code == 200 + old_reference_id = created.json()["id"] + path = created.json()["path"] + file_path = settings.upload_path / path + assert file_path.exists() + + # A measurement on v1 forces the next PUT onto the copy-on-write path. + measured = await client.post( + "/api/measurements/", + headers=auth_headers(measurement_tec_user), + json={"subtask_id": subtask.id, "version_id": version.id, "value": 10.0}, + ) + assert measured.status_code == 200 + + resp = await client.put( + f"/api/recipes/{recipe.id}", + headers=auth_headers(maker_user), + json={"change_notes": "Edit after measurement"}, + ) + assert resp.status_code == 200, resp.text + + db_session.expunge_all() + new_version, new_task, _ = await recipe_parts(db_session, recipe.id) + assert new_version.version_number == 2 + + listing = await client.get( + "/api/vision/reference-images", + headers=auth_headers(maker_user), + params={"task_id": new_task.id}, + ) + assert listing.status_code == 200 + body = listing.json() + assert len(body) == 1 + assert body[0]["path"] == path + assert body[0]["note"] == "orig" + new_reference_id = body[0]["id"] + assert new_reference_id != old_reference_id + + # Two rows share the same file: deleting one must leave it in place. + del1 = await client.delete( + f"/api/vision/reference-images/{old_reference_id}", + headers=auth_headers(maker_user), + ) + assert del1.status_code == 204 + assert file_path.exists() + + # Deleting the last row referencing the path removes the file too. + del2 = await client.delete( + f"/api/vision/reference-images/{new_reference_id}", + headers=auth_headers(maker_user), + ) + assert del2.status_code == 204 + assert not file_path.exists() + + @pytest.mark.asyncio async def test_preview_runs_the_graph_and_saves_no_measurement( client, db_session, maker_user, worker_says,