From ad0ec88f27ca423947e10755217eb86d65274db7 Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:34:51 -0600 Subject: [PATCH] Remove a file's Tags Directory links when WROLPi deletes it (#861) * 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 * Remove Tags Directory links on path deletes; keep a valid poster's link The file browser deletes by path (`lib.delete`), which unlinks the sources before any FileGroup is deleted, so its tagged files still left orphaned links. It now removes the links of the files it is about to delete, or of every FileGroup in a deleted directory. A poster is no longer unlinked from the Tags Directory unless it is actually removed: a valid JPEG that is not named .jpg is kept with its link, and a converted poster's links are removed only once the conversion succeeds. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- modules/videos/lib.py | 7 ++- modules/videos/models.py | 2 + modules/videos/test/test_lib.py | 34 +++++++++++++++ modules/videos/test/test_video.py | 27 ++++++++++++ wrolpi/files/lib.py | 10 +++++ wrolpi/files/models.py | 29 ++++++++++++- wrolpi/test/test_tags.py | 71 +++++++++++++++++++++++++++++++ 7 files changed, 178 insertions(+), 2 deletions(-) diff --git a/modules/videos/lib.py b/modules/videos/lib.py index 6d34bd91..c5dd210f 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -216,15 +216,20 @@ def convert_or_generate_poster(video: Video) -> Tuple[Optional[pathlib.Path], Op if old != new and new.exists(): # Destination JPEG already exists (it may have the wrong format). + video.file_group.unlink_tag_directory_links(old) old.unlink() old = new if not is_valid_poster(old): - # Poster is not valid, convert it and place it in the new location. + # Poster is not valid, convert it and place it in the new location. Conversion removes `old`, so its + # Tags Directory links are found now and removed only once the conversion succeeds. + old_links = video.file_group.get_tag_directory_links(old) if old != new else [] try: convert_image(old, new) if not new.is_file() or not new.stat().st_size: raise FileNotFoundError(f'Failed to convert poster: {new}') + for link in old_links: + link.unlink(missing_ok=True) logger.info(f'Converted invalid poster {repr(str(old))} to {repr(str(new))}') return new, None except Exception as e: 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..b37d7a51 100644 --- a/modules/videos/test/test_lib.py +++ b/modules/videos/test/test_lib.py @@ -593,3 +593,37 @@ 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' + + +@pytest.mark.asyncio +async def test_valid_poster_keeps_tags_directory_link(async_client, test_session, test_directory, video_factory, + tag_factory, await_switches): + """A valid JPEG poster that is not named .jpg is not converted, so its Tags Directory link is kept.""" + tag = await tag_factory() + video = video_factory(with_video_file=True, with_poster_ext='jpeg', tag_names=[tag.name]) + test_session.commit() + await await_switches() + poster_link = test_directory / 'tags' / tag.name / video.poster_path.name + assert poster_link.is_file() + + lib.convert_or_generate_poster(video) + + assert video.poster_path.is_file() + assert poster_link.is_file() and poster_link.samefile(video.poster_path) 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/lib.py b/wrolpi/files/lib.py index da05b252..dce501cb 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -427,6 +427,16 @@ async def delete(*paths: Union[str, pathlib.Path], force: bool = False, post_pro return [i.__json__() for i in tagged_file_groups] had_tagged = bool(tagged_file_groups) + # Remove the Tags Directory links of the files about to be deleted, while they can still be matched to + # their sources. A deleted directory takes every file of a FileGroup inside it. + directories = [i for i in paths if i.is_dir()] + files = [i for i in paths if i.is_file()] + for file_group in tagged_file_groups: + if any(i == file_group.directory or i in file_group.directory.parents for i in directories): + file_group.unlink_tag_directory_links() + elif files: + file_group.unlink_tag_directory_links(*files) + for path in paths: ignored_directories = get_wrolpi_config().ignored_directories if ignored_directories and str(path) in ignored_directories: diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index b73ff63b..2d9955b5 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,32 @@ class FileGroup(ModelHelper, Base): self.do_index() self.indexed = True + def get_tag_directory_links(self, *paths: pathlib.Path) -> List[pathlib.Path]: + """Return this FileGroup's links in the Tags Directory (only those of `paths`, if provided). + + A link is only returned if it is the same file as its source, so the sources must still exist.""" + if not self.tag_files: + return [] + tags_directory = get_tags_directory() + links = [] + 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): + links.append(link) + except FileNotFoundError: + pass + return links + + 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 file that is not this FileGroup's link is never deleted.""" + for link in self.get_tag_directory_links(*paths): + link.unlink(missing_ok=True) + 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..32affa09 100644 --- a/wrolpi/test/test_tags.py +++ b/wrolpi/test/test_tags.py @@ -680,6 +680,77 @@ 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 +@pytest.mark.parametrize('delete_directory', [False, True]) +async def test_tags_directory_tagged_path_deleted(async_client, test_session, test_directory, tag_factory, + video_bytes, image_bytes_factory, await_switches, delete_directory): + """Force-deleting a tagged file (or its directory) by path removes the deleted files' Tags Directory links.""" + directory = test_directory / 'videos' + directory.mkdir() + video_file = directory / 'video.mp4' + poster_file = 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() + + # Delete the whole directory, or only the poster. + body = dict(paths=['videos' if delete_directory else 'videos/video.jpg'], force=True) + request, response = await async_client.post('/api/files/delete', content=json.dumps(body)) + assert response.status_code == HTTPStatus.NO_CONTENT + await await_switches() + + assert not poster_link.exists(), 'The link to the deleted file should be removed' + if delete_directory: + assert not video_link.exists(), 'The link to the deleted file should be removed' + else: + assert video_link.is_file() and video_link.samefile(video_file), 'The link of a kept file 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."""