From 3a57e28cc666175c7c0d93dd94b74ad13e7f7609 Mon Sep 17 00:00:00 2001 From: Roland Date: Thu, 6 Aug 2026 21:52:30 -0600 Subject: [PATCH] fix: the deleted duplicate still held the path we renamed onto `delete_duplicate_videos` deletes the smaller copy sitting at the destination path, then renames the copy it keeps onto that path. `FileGroup.delete` unlinks the files right away but only queues the row deletion on the session, and SQLAlchemy emits UPDATEs before DELETEs within a flush -- so the rename's UPDATE of `primary_path` collided with the row that was about to be deleted: sqlite3.IntegrityError: UNIQUE constraint failed: file_group.primary_path The whole download transaction rolled back, so a re-downloaded video lost its DB record while its files stayed on disk (and the previous copy's files were already unlinked). Flush the delete before claiming the path. The earlier missing-file deletions in the same function never hit this because the `get_by_path` query between them autoflushes. Co-Authored-By: Claude Opus 5 (1M context) --- modules/videos/models.py | 5 ++++ modules/videos/test/test_video.py | 43 +++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/modules/videos/models.py b/modules/videos/models.py index 3b013937..e280c65d 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -533,6 +533,11 @@ class Video(ModelHelper, Base): existing_video = Video.get_by_path(session, video_path) if existing_video and largest_video != existing_video: delete_video(existing_video) + # `FileGroup.delete` unlinks the files immediately, but its DELETE is only queued on the session. The + # rename below claims `video_path` as its `primary_path`, and SQLAlchemy emits UPDATEs before DELETEs in a + # flush -- so without this flush the UPDATE hits the not-yet-deleted row and violates the unique index on + # `file_group.primary_path`, rolling back the entire download. + session.flush() for idx, video in enumerate(duplicate_videos): if video == existing_video: duplicate_videos.pop(idx) diff --git a/modules/videos/test/test_video.py b/modules/videos/test/test_video.py index 426389dc..99241352 100644 --- a/modules/videos/test/test_video.py +++ b/modules/videos/test/test_video.py @@ -237,6 +237,49 @@ async def test_delete_duplicate_video_tagged_duplicate(async_client, test_sessio assert video.file_group.tag_names == [tag.name, ] +@pytest.mark.asyncio +async def test_delete_duplicate_video_smaller_copy_at_destination(async_client, test_session, channel_factory, + video_factory): + """The kept Video can be renamed onto the path of the smaller duplicate that was just deleted.""" + channel = channel_factory(name='Channel Name') + video_path = channel.directory / f'{channel.name}_20000101_ABC123456_The video title.mp4' + + channel.info_json = {'entries': [{'id': 'ABC123456', 'title': 'The video title'}]} + entry = channel.info_json['entries'][0] + + # The smaller copy already occupies the destination path. + small = video_factory( + channel_id=channel.id, + title=f'{channel.name}_20000101_ABC123456_The video title', + upload_date=datetime(2000, 1, 1, tzinfo=timezone.utc), + source_id='ABC123456', + with_video_file=True, + ) + large = video_factory( + channel_id=channel.id, + title=f'{channel.name}_20000101_ABC123456_The video title ', # Trailing space, so a different path. + upload_date=datetime(2000, 1, 1, tzinfo=timezone.utc), + source_id='ABC123456', + with_video_file=True, + ) + small.file_group.url = large.file_group.url = 'https://example.com/video' + large.file_group.size = small.file_group.size + 1 + large_id = large.id + + test_session.commit() + assert small.video_path == video_path + + assert await Video.delete_duplicate_videos(test_session, 'https://example.com/video', entry['id'], video_path) + # The commit must not raise IntegrityError: the deleted duplicate still held `video_path` when the kept Video's + # `primary_path` was updated to it. + test_session.commit() + + video = test_session.query(Video).one() + assert video.id == large_id, 'The larger Video was not the one kept' + assert video.video_path == video_path + assert video.video_path.is_file() + + @pytest.mark.asyncio async def test_delete_renamed_video(async_client, test_session, channel_factory, video_factory, tag_factory): """If duplicate Video's exist, delete all the videos that do not have the new title."""