From 2b2361ed64db6675738b4ab2d227351c9e6c122c Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Mon, 5 Oct 2026 15:03:34 -0600 Subject: [PATCH] Save playlists config when a playlist's file is renamed or moved (#781) playlists.yaml references file items by path. Renaming, moving, or reorganizing a FileGroup updated the database but not the config, so the next import (startup or global refresh) could not find the file and silently dropped the item from the playlist. Co-authored-by: Claude Opus 5.5 --- wrolpi/collections/config.py | 19 ++++++ .../collections/test/test_playlists_config.py | 64 +++++++++++++++++++ wrolpi/collections/test/test_reorganize.py | 53 +++++++++++++++ wrolpi/files/lib.py | 9 +++ 4 files changed, 145 insertions(+) diff --git a/wrolpi/collections/config.py b/wrolpi/collections/config.py index e758243b..93744147 100644 --- a/wrolpi/collections/config.py +++ b/wrolpi/collections/config.py @@ -274,6 +274,25 @@ def get_playlists_config() -> PlaylistsConfig: return playlists_config +def file_groups_moved(session, file_group_ids): + """Call after FileGroups change path (rename/move/reorganize). + + playlists.yaml references file items by path, so it must be re-saved (and the playlist + directories re-synced) or the next import will not find the file and will drop the item. + + `file_group_ids` may be an iterable of ids or a select of ids.""" + from .models import Collection, CollectionItem + + in_playlist = session.query(CollectionItem.id) \ + .join(Collection, Collection.id == CollectionItem.collection_id) \ + .filter(Collection.kind == 'playlist', CollectionItem.file_group_id.in_(file_group_ids)) \ + .first() + if in_playlist: + save_playlists_config.activate_switch() + from .sync import sync_playlists_directory + sync_playlists_directory.activate_switch() + + @register_switch_handler('import_playlists_config') def import_playlists_config(): """Re-import playlists.yaml into the database. diff --git a/wrolpi/collections/test/test_playlists_config.py b/wrolpi/collections/test/test_playlists_config.py index fcea78e5..6f1f851c 100644 --- a/wrolpi/collections/test/test_playlists_config.py +++ b/wrolpi/collections/test/test_playlists_config.py @@ -1,6 +1,7 @@ """Tests for PlaylistsConfig disaster recovery: a missing or empty playlists.yaml must never delete playlists from the database (mirroring ChannelsConfig's guards).""" from copy import deepcopy +from http import HTTPStatus import pytest @@ -262,3 +263,66 @@ async def test_custom_directory_round_trips(test_session, test_directory, playli test_session.expire_all() collection = test_session.query(Collection).filter_by(name='Custom Dir').one() assert str(collection.directory) == str(custom) + + +async def _assert_playlist_survives_reimport(test_session, playlists_config, new_path): + """playlists.yaml must hold the FileGroup's new path, so a re-import keeps the item.""" + test_session.expire_all() + file_refs = [i['file'] for p in playlists_config.playlists for i in p['items'] if 'file' in i] + assert file_refs == [new_path] + + # Re-import (startup / end of a global refresh) rebuilds items from the config file. + playlists_config.import_config() + test_session.expire_all() + collection = test_session.query(Collection).filter_by(name='Moved', kind='playlist').one() + assert [i.item_kind for i in collection.items] == ['file', 'url'] + assert str(collection.items[0].file_group.primary_path).endswith(new_path) + + +def _make_file_playlist(test_session, playlists_config, pdf): + from wrolpi.files.models import FileGroup + + file_group = FileGroup.from_paths(test_session, pdf) + collection = Collection(name='Moved', kind='playlist') + test_session.add(collection) + test_session.flush([collection]) + collection.add_file_group(file_group, session=test_session) + collection.add_url(test_session, '/map?x=1', title='Spot') + test_session.commit() + playlists_config.dump_config() + + +@pytest.mark.asyncio +async def test_rename_file_updates_playlists_config( + test_session, test_directory, playlists_config, async_client, await_switches): + """Renaming a playlist's file must write the new path to playlists.yaml, otherwise the next + re-import cannot find the file and drops the item.""" + pdf = test_directory / 'guide.pdf' + pdf.write_bytes(b'%PDF-1.4 x') + _make_file_playlist(test_session, playlists_config, pdf) + + request, response = await async_client.post( + '/api/files/rename', json=dict(path='guide.pdf', new_name='manual.pdf')) + assert response.status_code == HTTPStatus.NO_CONTENT + await await_switches() + + await _assert_playlist_survives_reimport(test_session, playlists_config, 'manual.pdf') + + +@pytest.mark.asyncio +async def test_move_file_updates_playlists_config( + test_session, test_directory, playlists_config, async_client, await_switches, + await_background_tasks): + """Moving a playlist's file must write the new path to playlists.yaml.""" + pdf = test_directory / 'guide.pdf' + pdf.write_bytes(b'%PDF-1.4 x') + (test_directory / 'docs').mkdir() + _make_file_playlist(test_session, playlists_config, pdf) + + request, response = await async_client.post( + '/api/files/move', json=dict(paths=['guide.pdf'], destination='docs')) + assert response.status_code == HTTPStatus.NO_CONTENT + await await_background_tasks() + await await_switches() + + await _assert_playlist_survives_reimport(test_session, playlists_config, 'docs/guide.pdf') diff --git a/wrolpi/collections/test/test_reorganize.py b/wrolpi/collections/test/test_reorganize.py index bc78954c..a916fafd 100644 --- a/wrolpi/collections/test/test_reorganize.py +++ b/wrolpi/collections/test/test_reorganize.py @@ -784,6 +784,59 @@ async def test_reorganize_preserves_tag_association(async_client, test_directory assert fg.primary_path == new_primary_path, "File should have moved" +@pytest.mark.asyncio +async def test_reorganize_updates_playlists_config(async_client, test_directory, video_factory, await_switches): + """Reorganizing a playlist's file must write the new path to playlists.yaml, otherwise the next + re-import cannot find the file and drops the item.""" + from copy import deepcopy + from wrolpi.collections.config import get_playlists_config + from wrolpi.files.worker import file_worker, FileTask, FileTaskType + + playlists_config = get_playlists_config() + playlists_config._config = deepcopy(playlists_config.default_config) + + channel_dir = test_directory / 'videos' / 'test_channel' + channel_dir.mkdir(parents=True, exist_ok=True) + with get_db_session(commit=True) as session: + collection = Collection(name='Test Channel', kind='channel', directory=channel_dir, + file_format='%(title)s.%(ext)s') + session.add(collection) + session.flush() + channel = Channel(name='Test Channel', collection_id=collection.id, directory=channel_dir) + session.add(channel) + session.commit() + channel_id = channel.id + + video = video_factory(channel_id=channel_id, title='test_video', with_video_file=True) + + with get_db_session(commit=True) as session: + file_group = session.query(Video).get(video.id).file_group + old_primary_path = file_group.primary_path + playlist = Collection(name='Watch', kind='playlist') + session.add(playlist) + session.flush([playlist]) + playlist.add_file_group(file_group, session=session) + playlists_config.dump_config() + + new_primary_path = channel_dir / 'renamed_video.mp4' + task = FileTask( + task_type=FileTaskType.reorganize, + paths=[], + move_mappings=[(old_primary_path, new_primary_path)], + job_id='test-reorganize-playlist', + ) + await file_worker.handle_reorganize(task) + await await_switches() + + file_refs = [i['file'] for p in playlists_config.playlists for i in p['items'] if 'file' in i] + assert file_refs == ['videos/test_channel/renamed_video.mp4'] + + playlists_config.import_config() + with get_db_session() as session: + playlist = session.query(Collection).filter_by(name='Watch', kind='playlist').one() + assert [i.file_group.primary_path for i in playlist.items] == [new_primary_path] + + @pytest.mark.asyncio async def test_reorganize_then_refresh_preserves_filegroup_id_and_tags( async_client, test_directory, video_factory, tag_factory, test_tags_config, refresh_files): diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index 36de4fcd..61e620d3 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -1697,6 +1697,10 @@ def _bulk_update_file_groups_db(session: Session, chunk_plan: Dict[pathlib.Path, ''' session.execute(sa_text(sql), dict(updates=updates)) + from wrolpi.collections.config import file_groups_moved + new_paths = [str(new_path) for new_path in chunk_plan.values()] + file_groups_moved(session, session.query(FileGroup.id).filter(FileGroup.primary_path.in_(new_paths))) + def _json_serial(obj): """JSON serializer for objects not serializable by default json code.""" @@ -1745,6 +1749,8 @@ def _bulk_update_file_groups_reorganize(updates: List[dict]): # during tests, but SQLAlchemy's identity map doesn't know about raw SQL updates. with get_db_session() as session: session.expire_all() + from wrolpi.collections.config import file_groups_moved + file_groups_moved(session, [update['id'] for update in updates]) def delete_directory(directory: pathlib.Path, recursive: bool = False, force: bool = False): @@ -1797,6 +1803,9 @@ async def rename_file(path: pathlib.Path, new_name: str) -> pathlib.Path: else: fg.move(new_path) + from wrolpi.collections.config import file_groups_moved + file_groups_moved(session, [fg.id]) + return new_path