From 5427beb5fbf41c4ef39850ded9eb5597f868df2f Mon Sep 17 00:00:00 2001 From: Roland Date: Fri, 19 Jun 2026 11:39:37 -0600 Subject: [PATCH] Fix swapped arguments in download_info_json Video.get_by_url call `download_info_json` called `Video.get_by_url(url, session)`, but the method signature is `get_by_url(session, url)`. This passed the URL string where the session was expected, raising `AttributeError: 'str' object has no attribute 'query'` whenever a Video metadata refresh ran. Swap the arguments at both call sites and add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- modules/videos/downloader.py | 4 ++-- modules/videos/test/test_downloader.py | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py index 7422a287..ede0e364 100755 --- a/modules/videos/downloader.py +++ b/modules/videos/downloader.py @@ -1187,7 +1187,7 @@ class VideoDownloader(Downloader, ABC): with get_db_session() as session: # Find the existing video, replace its info json. - video = Video.get_by_url(url, session) + video = Video.get_by_url(session, url) location = video.location @@ -1202,7 +1202,7 @@ class VideoDownloader(Downloader, ABC): with get_db_session(commit=True) as session: # Find the existing video, replace its info json. - video = Video.get_by_url(url, session) + video = Video.get_by_url(session, url) video.replace_info_json(info_json) if parent_download_url := settings.get('parent_download_url'): diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index c65fb69b..e15fca7c 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -538,6 +538,29 @@ async def test_video_download(test_session, test_directory, mock_video_extract_i assert 'formats' not in video.get_info_json(), 'formats should be deleted from info json' +@pytest.mark.asyncio +async def test_download_info_json_finds_existing_video(test_session, test_directory, video_factory): + """`download_info_json` looks up the existing Video by its URL. + + Regression: `Video.get_by_url` was called with its arguments swapped — `(url, session)` instead of + `(session, url)` — which raised `AttributeError: 'str' object has no attribute 'query'`.""" + source_id = 'video-source-id' + url = 'https://example.com/the-video' + video = video_factory(with_info_json={'id': source_id}, source_id=source_id) + video.file_group.url = url + test_session.commit() + + download = Download(url=url, info_json={'id': source_id}, settings={}) + + # The info_json download itself fails here, but the buggy existing-video lookup runs first. + with mock.patch('modules.videos.downloader.download_video_info_json') as mock_download: + mock_download.side_effect = Exception('no network') + result = await VideoDownloader.download_info_json(download, video.video_path) + + assert result.success is False, 'Download should fail gracefully once the info json download fails' + mock_download.assert_called_once_with(url) + + @pytest.mark.asyncio async def test_download_result(test_session, test_directory, video_download_manager, mock_video_process_runner, mock_video_extract_info, image_file, await_switches):