File downloads: return plain values from upsert_file

Every file download failed after the file was saved, with DetachedInstanceError reading
`fg.location`.  `upsert_file` returned a FileGroup from a session it had committed, and the
caller read it after the session was gone.  SQLAlchemy 1.3 kept that session alive in a
reference cycle until the next garbage collection, so the read worked by accident; 2.0 frees it
as the `with` block ends.  Failed downloads retry, re-downloading their files.

`upsert_file` now returns `UpsertedFile(file_group_id, location)`, read inside its session.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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(

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:

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