mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-25 18:40:06 -04:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
04779b8557
commit
2b2361ed64
4 changed files with 145 additions and 0 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue