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) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-08-06 21:52:30 -06:00 • committed by lrnselfreliance
parent 4b9593580d
commit 3a57e28cc6
2 changed files with 48 additions and 0 deletions

View file

@ -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)

View file

@ -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."""