diff --git a/wrolpi/files/downloader.py b/wrolpi/files/downloader.py index 43f90d01..1090fa18 100644 --- a/wrolpi/files/downloader.py +++ b/wrolpi/files/downloader.py @@ -95,8 +95,8 @@ class FileDownloader(Downloader, ABC): output_path = None try: output_path = await self.download_file(download, prepared.url, prepared.destination, ctx=ctx) - fg = await upsert_file(output_path, tag_names=download.tag_names) - return ExecutedFile(output_path=output_path, location=fg.location) + upserted = await upsert_file(output_path, tag_names=download.tag_names) + return ExecutedFile(output_path=output_path, location=upserted.location) except IgnoredDirectoryError: # `upsert_file` says this is an ignored file, that's fine. return ExecutedFile( diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index e6d1818d..7a9aca64 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -2066,7 +2066,14 @@ def remove_ignored_directory(directory: Union[pathlib.Path, str]): get_wrolpi_config().ignored_directories = kept -async def upsert_file(file: pathlib.Path | str, tag_names: List[str] = None) -> FileGroup: +@dataclasses.dataclass(frozen=True) +class UpsertedFile: + """The FileGroup `upsert_file` created or updated. Plain values: its Session is gone once it returns.""" + file_group_id: int + location: str + + +async def upsert_file(file: pathlib.Path | str, tag_names: List[str] = None) -> UpsertedFile: """Insert/update all files in the provided file's FileGroup.""" if not file or not file.is_file(): raise InvalidFile(f'Cannot upsert file that does not exist: {file}') @@ -2130,11 +2137,12 @@ async def upsert_file(file: pathlib.Path | str, tag_names: List[str] = None) -> # Commit all changes in one transaction session.commit() + upserted = UpsertedFile(file_group_id=file_group.id, location=file_group.location) # Upsert directories after FileGroup is committed upsert_directories([], file.parents) - return file_group + return upserted def get_file_location_href(file: pathlib.Path) -> str: diff --git a/wrolpi/files/test/test_lib.py b/wrolpi/files/test/test_lib.py index 193ff7b8..7072803e 100644 --- a/wrolpi/files/test/test_lib.py +++ b/wrolpi/files/test/test_lib.py @@ -1338,7 +1338,8 @@ async def test_upsert_file_video_with_channel(async_client, test_session, test_d info_json_file.write_text(json.dumps({'duration': 5})) # Upsert the file, it should be modeled. - fg: FileGroup = await lib.upsert_file(video_file) + upserted = await lib.upsert_file(video_file) + fg = test_session.query(FileGroup).filter_by(id=upserted.file_group_id).one() assert fg.model == 'video', 'Upserted file should have been modeled.' assert fg.my_video_files(), 'Video file was upserted.' assert fg.my_json_files(), 'Info json file should have been found near video file.' @@ -1350,6 +1351,21 @@ async def test_upsert_file_video_with_channel(async_client, test_session, test_d assert video.info_json_path, 'Info json file not found near video file.' +@pytest.mark.asyncio +async def test_upsert_file_result_outlives_its_session(test_session, test_directory, video_bytes): + """`upsert_file`'s Session is gone once it returns (production frees it), so its result must not need it.""" + video_file = test_directory / 'video.mp4' + video_file.write_bytes(video_bytes) + + upserted = await lib.upsert_file(video_file) + sqlalchemy.orm.close_all_sessions() + + location = upserted.location + file_group = test_session.query(FileGroup).one() + assert location == file_group.location + assert upserted.file_group_id == file_group.id + + @only_macos def test_get_real_path_name(test_directory): (test_directory / 'foo.txt').touch()