From a20ef4893ab8e992c328ea69cbe20b7d900f8bd3 Mon Sep 17 00:00:00 2001 From: Roland Date: Fri, 23 Oct 2026 12:50:11 -0600 Subject: [PATCH] Remove a file's Tags Directory links when WROLPi deletes it When WROLPi deleted a tagged file (a forced delete, duplicate Video cleanup, or a poster conversion), the source was unlinked but its Tags Directory hardlink was not. The link became the only copy, so the tags sync refused to delete it and warned on every sync. The links of a file are now removed before WROLPi removes it, and only if the link is the same file as its source. Co-Authored-By: Claude Opus 5.5 --- modules/videos/lib.py | 4 ++++ modules/videos/models.py | 2 ++ modules/videos/test/test_lib.py | 17 ++++++++++++++++ modules/videos/test/test_video.py | 27 ++++++++++++++++++++++++ wrolpi/files/models.py | 21 ++++++++++++++++++- wrolpi/test/test_tags.py | 34 +++++++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 1 deletion(-) diff --git a/modules/videos/lib.py b/modules/videos/lib.py index 6d34bd91..ebaeba34 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -214,6 +214,10 @@ def convert_or_generate_poster(video: Video) -> Tuple[Optional[pathlib.Path], Op old: pathlib.Path = video.poster_path new = old.with_suffix('.jpg') + if old != new: + # The old poster is about to be removed; its Tags Directory link would be left as the only copy. + video.file_group.unlink_tag_directory_links(old) + if old != new and new.exists(): # Destination JPEG already exists (it may have the wrong format). old.unlink() diff --git a/modules/videos/models.py b/modules/videos/models.py index 3637cec8..8e75bbe2 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -515,6 +515,8 @@ class Video(ModelHelper, Base): def delete_video(video_: Video): """Delete a Video with any of its tags.""" + # Untagging forgets which links are this Video's, so remove them first. + video_.file_group.unlink_tag_directory_links() for tag_name_ in video_.file_group.tag_names: video_.file_group.untag(session, tag_name_) video_.delete() diff --git a/modules/videos/test/test_lib.py b/modules/videos/test/test_lib.py index 2fd3bd6c..be1f043a 100644 --- a/modules/videos/test/test_lib.py +++ b/modules/videos/test/test_lib.py @@ -593,3 +593,20 @@ def test_download_settings_codec_validation(): with pytest.raises(ValidationError): DownloadSettings(audio_codecs=['wma']) + + +@pytest.mark.asyncio +async def test_convert_poster_removes_tags_directory_link(async_client, test_session, test_directory, video_factory, + tag_factory, await_switches): + """Converting a tagged Video's poster removes the old poster's Tags Directory link.""" + tag = await tag_factory() + video = video_factory(with_video_file=True, with_poster_ext='webp', tag_names=[tag.name]) + test_session.commit() + await await_switches() + webp_link = test_directory / 'tags' / tag.name / video.poster_path.name + assert webp_link.is_file() + + new_poster, _ = lib.convert_or_generate_poster(video) + + assert new_poster.suffix == '.jpg' + assert not webp_link.exists(), 'The converted poster left its link behind' diff --git a/modules/videos/test/test_video.py b/modules/videos/test/test_video.py index 93277ac1..5ccbee9e 100644 --- a/modules/videos/test/test_video.py +++ b/modules/videos/test/test_video.py @@ -240,6 +240,33 @@ async def test_delete_duplicate_video_tagged_duplicate(async_client, test_sessio assert video.file_group.tag_names == [tag.name, ] +@pytest.mark.asyncio +async def test_delete_duplicate_video_leaves_no_tags_directory_orphans(async_client, test_session, test_directory, + channel_factory, video_factory, tag_factory, + await_switches): + """Deleting a tagged duplicate Video removes its Tags Directory links; only links to existing files remain.""" + channel = channel_factory(name='Channel Name') + video_path = channel.directory / f'{channel.name}_20000101_ABC123456_The video title.mp4' + tag = await tag_factory() + for title in (f'{channel.name}_20000101_ABC123456_The video title', + f'{channel.name}_20000101_ABC123456_The video title '): + video = video_factory(channel_id=channel.id, title=title, upload_date=datetime(2000, 1, 1, tzinfo=timezone.utc), + source_id='ABC123456', with_video_file=True, with_info_json=True, tag_names=[tag.name]) + video.file_group.url = 'https://example.com/video' + test_session.commit() + await await_switches() + tags_directory = test_directory / 'tags' + assert len([i for i in tags_directory.rglob('*.mp4')]) == 2 + + assert await Video.delete_duplicate_videos(test_session, 'https://example.com/video', 'ABC123456', video_path) + test_session.commit() + await await_switches() + + links = [i for i in tags_directory.rglob('*') if i.is_file() and i.name != 'README.txt'] + assert links, 'The kept Video is still linked' + assert all(i.stat().st_nlink > 1 for i in links), 'A link was left behind with no source file' + + @pytest.mark.asyncio async def test_delete_duplicate_video_smaller_copy_at_destination(async_client, test_session, channel_factory, video_factory): diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index b73ff63b..008843d3 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -19,7 +19,7 @@ from wrolpi.errors import FileGroupIsTagged, UnknownFile from wrolpi.files import indexers from wrolpi.media_path import MediaPathType from wrolpi.files.progress import save_recently_viewed_config -from wrolpi.tags import Tag, TagFile, save_tags_config, sync_tags_directory +from wrolpi.tags import Tag, TagFile, save_tags_config, sync_tags_directory, get_tags_directory from wrolpi.vars import PYTEST logger = logger.getChild(__name__) @@ -494,6 +494,7 @@ class FileGroup(ModelHelper, Base): if self.tag_files and not force: raise FileGroupIsTagged(f'Cannot delete {self} because it has tags') + self.unlink_tag_directory_links() for path in self.my_paths(): path.unlink(missing_ok=True) @@ -835,6 +836,24 @@ class FileGroup(ModelHelper, Base): self.do_index() self.indexed = True + def unlink_tag_directory_links(self, *paths: pathlib.Path): + """Delete this FileGroup's links in the Tags Directory (only those of `paths`, if provided). + + Call this before the source files are removed. A link is only deleted if it is the same file as its source, + so a file that is not this FileGroup's link is never deleted.""" + if not self.tag_files: + return + tags_directory = get_tags_directory() + for file, link in self.get_tag_directory_paths_map().items(): + if paths and file not in paths: + continue + link = tags_directory / link + try: + if link.samefile(file): + link.unlink() + except FileNotFoundError: + pass + def get_tag_directory_paths_map(self) -> dict[pathlib.Path, str]: """Return all links that should exist for this FileGroup in the Tags Directory.""" tag_names = self.tag_names diff --git a/wrolpi/test/test_tags.py b/wrolpi/test/test_tags.py index 04f9e77a..59453008 100644 --- a/wrolpi/test/test_tags.py +++ b/wrolpi/test/test_tags.py @@ -680,6 +680,40 @@ async def test_tags_directory_all_files_deleted(test_session, test_directory, ta assert poster_link.is_file(), 'Poster link should be preserved when all sources deleted' +@pytest.mark.asyncio +async def test_tags_directory_tagged_file_group_deleted(async_client, test_session, test_directory, tag_factory, + video_bytes, image_bytes_factory, await_switches): + """Deleting a tagged FileGroup removes its Tags Directory links, but never a file that is not one of them.""" + video_file = test_directory / 'video.mp4' + poster_file = test_directory / 'video.jpg' + video_file.write_bytes(video_bytes) + poster_file.write_bytes(image_bytes_factory()) + fg = FileGroup.from_paths(test_session, video_file, poster_file) + test_session.add(fg) + test_session.commit() + + tag = await tag_factory('favorite') + fg.add_tag(test_session, tag.id) + test_session.commit() + await await_switches() + + video_link = test_directory / 'tags' / tag.name / 'video.mp4' + poster_link = test_directory / 'tags' / tag.name / 'video.jpg' + assert video_link.is_file() and poster_link.is_file() + # The poster link was replaced with a different file; it is not this FileGroup's link. + poster_link.unlink() + poster_link.write_bytes(b'not a link') + + body = dict(file_group_ids=[fg.id], force=True) + request, response = await async_client.post('/api/files/delete_groups', content=json.dumps(body)) + assert response.status_code == HTTPStatus.NO_CONTENT + await await_switches() + + assert not video_file.exists() and not poster_file.exists() + assert not video_link.exists(), 'The link to the deleted file should be removed' + assert poster_link.read_bytes() == b'not a link', 'A file that is not a link to the FileGroup is kept' + + @pytest.mark.asyncio async def test_tags_directory_disabled(test_session, test_directory, tag_factory, video_factory, await_switches): """Test that Tag Directory is NOT synchronized when tags_directory setting is disabled."""