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