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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

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:

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

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)

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

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:

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

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."""