diff --git a/modules/videos/lib.py b/modules/videos/lib.py index ebaeba34..c5dd210f 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -214,21 +214,22 @@ 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). + 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/test/test_lib.py b/modules/videos/test/test_lib.py index be1f043a..b37d7a51 100644 --- a/modules/videos/test/test_lib.py +++ b/modules/videos/test/test_lib.py @@ -610,3 +610,20 @@ async def test_convert_poster_removes_tags_directory_link(async_client, test_ses 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/wrolpi/files/lib.py b/wrolpi/files/lib.py index 2bebcc8c..bb7997fc 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 008843d3..2d9955b5 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -836,23 +836,31 @@ 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). + 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). - 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.""" + 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 + 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): - link.unlink() + 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.""" diff --git a/wrolpi/test/test_tags.py b/wrolpi/test/test_tags.py index 59453008..32affa09 100644 --- a/wrolpi/test/test_tags.py +++ b/wrolpi/test/test_tags.py @@ -714,6 +714,43 @@ async def test_tags_directory_tagged_file_group_deleted(async_client, test_sessi 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."""