fix(vision): reference image sopravvive al copy-on-write (#38.6)
_copy_tasks_to_version non copiava le righe VisionReferenceImage del task sorgente: il task copiato perdeva le immagini di riferimento del grafo di visione. Ora copia le righe sul task nuovo, stesso path e stessi metadati (stesso file su disco, non duplicato). Di conseguenza il file va cancellato solo se nessun'altra riga VisionReferenceImage referenzia lo stesso path: nuova vision_service.file_still_referenced(), usata dalla route DELETE prima di remove_reference_file. Test: copy-on-write elenca l'immagine sulla nuova versione; DELETE di una delle due righe lascia il file; DELETE dell'ultima lo rimuove. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -177,8 +177,10 @@ async def delete_reference_image(
|
|||||||
reference = await vision_service.delete_reference_image(db, reference_id)
|
reference = await vision_service.delete_reference_image(db, reference_id)
|
||||||
await db.commit()
|
await db.commit()
|
||||||
# Only now: if the commit above had failed, the row would still exist and
|
# Only now: if the commit above had failed, the row would still exist and
|
||||||
# the file must still be there for it.
|
# the file must still be there for it. And only if no other row (a
|
||||||
vision_service.remove_reference_file(reference)
|
# 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")
|
@router.post("/preview")
|
||||||
|
|||||||
@@ -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.setting import RecipeVersionAudit
|
||||||
from src.backend.models.orm.task import RecipeSubtask, RecipeTask
|
from src.backend.models.orm.task import RecipeSubtask, RecipeTask
|
||||||
from src.backend.models.orm.user import User
|
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
|
from src.backend.models.api.recipe import RecipeCreate, RecipeUpdate
|
||||||
|
|
||||||
|
|
||||||
@@ -118,6 +119,28 @@ async def _copy_tasks_to_version(
|
|||||||
db.add(new_task)
|
db.add(new_task)
|
||||||
await db.flush() # get new_task.id
|
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:
|
for sub in task.subtasks:
|
||||||
new_sub = RecipeSubtask(
|
new_sub = RecipeSubtask(
|
||||||
task_id=new_task.id,
|
task_id=new_task.id,
|
||||||
|
|||||||
@@ -275,6 +275,22 @@ def remove_reference_file(reference: VisionReferenceImage) -> None:
|
|||||||
_reference_file_path(reference).unlink(missing_ok=True)
|
_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(
|
async def save_reference_image(
|
||||||
db: AsyncSession,
|
db: AsyncSession,
|
||||||
task_id: int,
|
task_id: int,
|
||||||
|
|||||||
@@ -192,6 +192,78 @@ async def test_deleting_an_unknown_reference_image_is_404(
|
|||||||
assert response.status_code == 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
|
@pytest.mark.asyncio
|
||||||
async def test_preview_runs_the_graph_and_saves_no_measurement(
|
async def test_preview_runs_the_graph_and_saves_no_measurement(
|
||||||
client, db_session, maker_user, worker_says,
|
client, db_session, maker_user, worker_says,
|
||||||
|
|||||||
Reference in New Issue
Block a user