From 441a3ef9df38cd24b80f8f212b735f68c64d7601 Mon Sep 17 00:00:00 2001 From: vrubelroman Date: Wed, 16 Sep 2026 20:01:41 +0000 Subject: [PATCH] Verify deletion actually removed the file before reporting success MeTube's /delete only unlinks the file when it's configured with DELETE_FILE_ON_TRASHCAN=true; otherwise it just drops the entry from its own "done" list and returns {"status": "ok"} regardless -- we were trusting that response and marking the job "deleted" (offering a re-download) while the file was still sitting on mediaVM's disk the whole time. Now HEAD-check the media_url right after the delete call. If the file is still reachable, leave the job's status untouched (still "completed", still playable) and surface a clear 409 explaining MeTube's own config is why, rather than lying about local state. Co-Authored-By: Claude Sonnet 5 --- backend/app/api/videos.py | 9 +++++++ backend/app/services/download_jobs.py | 22 +++++++++++++++- frontend/src/components/DownloadButton.tsx | 5 ++++ tests/test_videos.py | 30 ++++++++++++++++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/backend/app/api/videos.py b/backend/app/api/videos.py index 3613856..375fac8 100644 --- a/backend/app/api/videos.py +++ b/backend/app/api/videos.py @@ -9,6 +9,7 @@ from app.models.channel import Channel from app.models.download_job import DownloadJob from app.models.video import Video from app.services.download_jobs import ( + DeleteDidNotRemoveFile, DeleteNotAllowed, MeTubeRejected, delete_local_copy, @@ -78,6 +79,14 @@ def delete_download(youtube_video_id: str, db: Session = Depends(get_db)) -> dic job = delete_local_copy(db, video) except DeleteNotAllowed as exc: raise HTTPException(status_code=400, detail=str(exc)) + except DeleteDidNotRemoveFile: + raise HTTPException( + status_code=409, + detail=( + "MeTube убрал запись из своего списка, но файл остался на диске " + "(на mediaVM выключена настройка DELETE_FILE_ON_TRASHCAN)" + ), + ) except Exception: logger.exception("Failed to delete local copy for %s", youtube_video_id) raise HTTPException(status_code=502, detail="MeTube is unavailable") diff --git a/backend/app/services/download_jobs.py b/backend/app/services/download_jobs.py index eb92edf..d8ac4b8 100644 --- a/backend/app/services/download_jobs.py +++ b/backend/app/services/download_jobs.py @@ -19,6 +19,15 @@ class DeleteNotAllowed(Exception): pass +class DeleteDidNotRemoveFile(Exception): + """MeTube accepted the /delete request but the file is still reachable + afterwards -- its DELETE_FILE_ON_TRASHCAN setting is very likely off on + that instance, which is outside this app's control (see AGENTS.md + constraint 11: never touch MeTube's own config/source).""" + + pass + + def get_latest_job(db: Session, video_id: int) -> DownloadJob | None: return ( db.query(DownloadJob) @@ -69,7 +78,18 @@ def delete_local_copy(db: Session, video: Video) -> DownloadJob: if not job.metube_job_id: raise DeleteNotAllowed("Missing MeTube job id, cannot request deletion") - MeTubeClient().delete_download(job.metube_job_id) + client = MeTubeClient() + client.delete_download(job.metube_job_id) + + # MeTube's /delete only unlinks the file if it's configured with + # DELETE_FILE_ON_TRASHCAN=true -- otherwise it just drops the entry from + # its own "done" list and the file stays put. We don't control that + # instance's config, so verify rather than trust the "ok" response. + if job.media_url and client.check_media(job.media_url): + raise DeleteDidNotRemoveFile( + "MeTube removed the entry but the file is still on disk " + "(its DELETE_FILE_ON_TRASHCAN setting is likely off)" + ) job.status = "deleted" job.media_url = None diff --git a/frontend/src/components/DownloadButton.tsx b/frontend/src/components/DownloadButton.tsx index 85ff346..b046fcf 100644 --- a/frontend/src/components/DownloadButton.tsx +++ b/frontend/src/components/DownloadButton.tsx @@ -75,6 +75,11 @@ function DownloadButton({ video }: Props) { > Удалить + {deleteMutation.isError && ( + + {(deleteMutation.error as Error).message} + + )} ) } diff --git a/tests/test_videos.py b/tests/test_videos.py index 4874711..3f62f52 100644 --- a/tests/test_videos.py +++ b/tests/test_videos.py @@ -184,6 +184,7 @@ def test_delete_download_success(client, db_session, monkeypatch): "app.services.metube_client.MeTubeClient.delete_download", lambda self, metube_job_id: calls.setdefault("id", metube_job_id) or {"status": "ok"}, ) + monkeypatch.setattr("app.services.metube_client.MeTubeClient.check_media", lambda self, url: False) resp = client.delete(f"/api/videos/{video.youtube_video_id}/download") @@ -195,3 +196,32 @@ def test_delete_download_success(client, db_session, monkeypatch): db_session.refresh(job) assert job.status == "deleted" assert job.media_url is None + + +def test_delete_download_file_still_present_returns_409(client, db_session, monkeypatch): + """MeTube can accept /delete and still leave the file on disk if its own + DELETE_FILE_ON_TRASHCAN config is off -- we must not lie about that.""" + from app.models.download_job import DownloadJob + + video = _seed_single_video(db_session) + job = DownloadJob( + video_id=video.id, + status="completed", + metube_job_id="vid1.vid1", + media_url="http://metube.local/download/f.mp4", + ) + db_session.add(job) + db_session.commit() + + monkeypatch.setattr( + "app.services.metube_client.MeTubeClient.delete_download", lambda self, metube_job_id: {"status": "ok"} + ) + monkeypatch.setattr("app.services.metube_client.MeTubeClient.check_media", lambda self, url: True) + + resp = client.delete(f"/api/videos/{video.youtube_video_id}/download") + + assert resp.status_code == 409 + + db_session.refresh(job) + assert job.status == "completed" + assert job.media_url == "http://metube.local/download/f.mp4"