From 0237fba77a436b7f53445dfbe35cb3073e19e703 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Tue, 27 Oct 2026 18:17:28 -0700 Subject: [PATCH] feat(images): duplicate cleanup keeps images' own categories (#332) The kept copy's own category wins; a kept copy without one receives the removed copy's, keyed by the deepest output source holding each file. The removed copy's row is never deleted, so a give-back or a failed delete loses nothing. The job log reports how many were kept. --- database/duplicate_copies.py | 15 ++- py/api/duplicate_files.py | 4 + py/api/duplicate_spellings.py | 25 ++++- tests/test_duplicate_image_categories.py | 136 +++++++++++++++++++++++ 4 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 tests/test_duplicate_image_categories.py diff --git a/database/duplicate_copies.py b/database/duplicate_copies.py index 2baea72b..22129786 100644 --- a/database/duplicate_copies.py +++ b/database/duplicate_copies.py @@ -11,6 +11,8 @@ IMMEDIATE`` transaction, so no row ever names a deleted file: - its favorite; - its image tags (``media_image_tags`` of its byte record), joined onto the kept copy's byte record as their union, in order; +- its own image category (#332), copied to the kept copy when that copy has + none; the removed copy's row is kept, so a give-back loses nothing; - its pending Needs review items, settled: only an item still ``pending`` at revision 0 with no decision row is removed (``review._SETTLE_ITEM``), so a decided item is never touched. @@ -22,6 +24,8 @@ import os from .annotation_schema import BYTES_TABLE, FILES_TABLE from .annotations import _BUMP_INDEX +from .image_categories import COPY_SQL as COPY_CATEGORY_SQL +from .image_categories import copy_image_category from .models import normalize_image_path, utc_now_iso from .review import _BUMP_REVIEW_INDEX, _PENDING_OF_LOCATOR, _SETTLE_ITEM from .review_changes import notify_review_change @@ -76,6 +80,7 @@ FILE_LINK_SQL = ( COPY_BYTE_SQL, COPY_TAG_COUNT_SQL, PENDING_COUNT_SQL, + COPY_CATEGORY_SQL, ) @@ -137,8 +142,10 @@ class DuplicateCopyOperations: removed key, kept root key, kept key)`` per listed root holding the file), ``removed_locator``, ``kept_locator``, ``sha256`` and the kept copy's ``kept_size`` and - ``kept_mtime_ns``. Returns ``{moved, merged, favorites, tags, - review}``. + ``kept_mtime_ns``; ``categories``: ``(removed file key, kept file + key)`` pairs of the image categories (#332). Returns ``{moved, + merged, favorites, tags, review, categories}`` (``categories``: the + image categories copied to the kept copy). """ counts = dict.fromkeys(("moved", "merged", "favorites", "tags", "review"), 0) kept_keys = [normalize_image_path(path) for path in move["kept"]] @@ -174,6 +181,10 @@ class DuplicateCopyOperations: for favorite in move.get("favorites", ()): counts["favorites"] += _move_favorite(conn, *favorite) counts["tags"] = self._move_copy_tags(conn, move) + counts["categories"] = sum( + copy_image_category(conn, removed, kept) + for removed, kept in move.get("categories", ()) + ) for (item_id,) in conn.execute( _PENDING_OF_LOCATOR, (move["removed_locator"],) ).fetchall(): diff --git a/py/api/duplicate_files.py b/py/api/duplicate_files.py index 6edf265d..dc7f511c 100644 --- a/py/api/duplicate_files.py +++ b/py/api/duplicate_files.py @@ -589,6 +589,7 @@ class DuplicateFilesMixin: spellings = await self._run_in_executor(self._source_spellings) removed, failure, keepers = 0, None, {} totals = dict.fromkeys(("moved", "merged", "favorites", "tags", "review"), 0) + totals["categories"] = 0 # image categories kept on the kept copies (#332) for done, target in enumerate(targets, start=1): if failure is not None: self._record_target(job, "kept", target, NOT_ATTEMPTED) @@ -635,6 +636,9 @@ class DuplicateFilesMixin: job.log_line( f"Added {_plural(totals['tags'], 'image tag')} to the kept copies" ) + if totals.get("categories"): + kept = _plural(totals["categories"], "image category", "image categories") + job.log_line(f"Kept {kept} on the kept copies") if totals["review"]: job.log_line( f"Settled {_plural(totals['review'], 'Needs review item')} " diff --git a/py/api/duplicate_spellings.py b/py/api/duplicate_spellings.py index 31e0f0b9..0e197aa4 100644 --- a/py/api/duplicate_spellings.py +++ b/py/api/duplicate_spellings.py @@ -18,7 +18,7 @@ Blocking: each lookup stats the file's folders once (cached). import os from pathlib import Path -from .images import _favorite_relative_key, _favorite_root_key +from .images import _favorite_relative_key, _favorite_root_key, image_category_key def _identity(path): @@ -78,6 +78,15 @@ class Spellings: ) return spelled + @property + def listed_roots(self): + """The listed roots (the output sources), in order.""" + return [root for root, _identity in self._listed] + + def category_key(self, path): + """*path*'s image category file key (#332), or None outside every source.""" + return image_category_key(self.listed_roots, path) + def favorites(self, path): """``{listed root: path below it (POSIX)}`` for each listed root holding it.""" path = os.path.abspath(str(path)) @@ -125,9 +134,23 @@ def copy_move(target, kept_stat, spellings): "targets": targets, "kept_path": target["keeper_abs"], "favorites": favorites, + "categories": _category_pairs(target, spellings), "removed_locator": target["locator"], "kept_locator": target["keeper_locator"], "sha256": target["hash"], "kept_size": kept_stat.st_size, "kept_mtime_ns": kept_stat.st_mtime_ns, } + + +def _category_pairs(target, spellings): + """``[(removed file key, kept file key)]`` for the image category (#332). + + From the two absolute paths, not the favorites tuples: those follow the + listed root, which differs from the file key when sources nest. + """ + removed = spellings.category_key(target["abs"]) + kept = spellings.category_key(target["keeper_abs"]) + if removed is None or kept is None: + return [] + return [(removed, kept)] diff --git a/tests/test_duplicate_image_categories.py b/tests/test_duplicate_image_categories.py new file mode 100644 index 00000000..c129fcbe --- /dev/null +++ b/tests/test_duplicate_image_categories.py @@ -0,0 +1,136 @@ +"""Duplicate cleanup keeps images' own categories (#332 MC11). + +The kept copy's own category wins; a kept copy without one receives the +removed copy's. The removed copy's row is never deleted, so a give-back +(the kept copy changed) and a failed delete lose nothing. +""" + +import asyncio +import os +from unittest.mock import patch + +try: + from tests.test_api_duplicate_files import WAIT, DuplicateFilesTestCase +except ImportError: # discovered with tests/ as the top-level directory + from test_api_duplicate_files import WAIT, DuplicateFilesTestCase + +from py.api import duplicate_files # noqa: E402 +from py.api.images import image_category_key # noqa: E402 +from py.config import GalleryConfig # noqa: E402 + + +class TestDuplicateCleanupKeepsCategories(DuplicateFilesTestCase): + def key(self, path): + return image_category_key(self.api._get_all_output_dirs(), path) + + def category(self, path): + return self.api.db.image_category(*self.key(path)) + + def set_category(self, path, value): + self.api.db.set_image_categories([self.key(path)], value) + + async def run_cleanup(self, keep="a.png"): + job, group = await self.found_group() + plan = await self.review(job.id, [self.keep_of(group, keep)]) + body = await self.apply(plan["token"]) + cleanup = self.api._jobs.get(body["job"]["id"]) + await asyncio.wait_for(asyncio.shield(cleanup.task), WAIT) + return cleanup + + async def test_kept_copy_without_a_category_receives_the_removed_one(self): + self.trio() + b = self.output_dir / "b.png" + self.set_category(b, "Portraits") + cleanup = await self.run_cleanup() + self.assertFalse(b.exists()) + self.assertEqual(self.category(self.output_dir / "a.png"), "Portraits") + # The removed copy's row stays (hidden while its file is gone). + self.assertEqual(self.category(b), "Portraits") + self.assertIn( + "Kept 1 image category on the kept copies", "\n".join(cleanup.log) + ) + + async def test_kept_copy_keeps_its_own_category(self): + self.trio() + self.set_category(self.output_dir / "a.png", "Landscapes") + self.set_category(self.output_dir / "b.png", "Portraits") + self.set_category(self.output_dir / "sub" / "c.png", "Scans") + cleanup = await self.run_cleanup() + self.assertEqual(self.category(self.output_dir / "a.png"), "Landscapes") + self.assertNotIn("image categor", "\n".join(cleanup.log)) + + async def test_two_removed_copies_give_the_first_category(self): + self.trio() + self.set_category(self.output_dir / "b.png", "Portraits") + self.set_category(self.output_dir / "sub" / "c.png", "Scans") + cleanup = await self.run_cleanup() + self.assertIn(self.category(self.output_dir / "a.png"), {"Portraits", "Scans"}) + self.assertIn( + "Kept 1 image category on the kept copies", "\n".join(cleanup.log) + ) + + async def test_give_back_loses_no_category(self): + self.trio() + self.set_category(self.output_dir / "b.png", "Portraits") + with patch.object(duplicate_files, "still_kept", return_value=False): + cleanup = await self.run_cleanup() + self.assertEqual(cleanup.counters["removed"], 0) + rows = {row["path"]: row["detail"] for row in cleanup.outcomes["kept"]} + self.assertEqual( + rows["b.png"], duplicate_files.ROW_DETAILS[duplicate_files.KEEPER_GONE] + ) + self.assertTrue((self.output_dir / "b.png").is_file()) + self.assertEqual(self.category(self.output_dir / "b.png"), "Portraits") + + async def test_a_failed_delete_keeps_both_categories(self): + self.trio() + self.set_category(self.output_dir / "b.png", "Portraits") + real = duplicate_files._unlink + + def unlink(path): + if os.path.basename(path) == "b.png": + raise PermissionError(13, "denied") + real(path) + + with patch.object(duplicate_files, "_unlink", side_effect=unlink): + await self.run_cleanup() + self.assertTrue((self.output_dir / "b.png").is_file()) + self.assertEqual(self.category(self.output_dir / "b.png"), "Portraits") + self.assertEqual(self.category(self.output_dir / "a.png"), "Portraits") + + async def test_nested_sources_use_the_deepest_source_keys(self): + self.write("sub/a.png", b"same", mtime=1) + self.write("sub/b.png", b"same", mtime=2) + # The subfolder is listed first, so the cleanup's listed root is the + # file key's source; the outer source still holds the files. + GalleryConfig.MONITORING_DIRECTORIES = [ + self._root(self.output_dir / "sub"), + self._root(self.output_dir), + ] + sub_key = duplicate_files.favorite_root_key(self.output_dir / "sub") + self.api.db.set_image_categories([(sub_key, "b.png")], "Portraits") + await self.run_cleanup() + self.assertFalse((self.output_dir / "sub" / "b.png").exists()) + self.assertEqual(self.api.db.image_category(sub_key, "a.png"), "Portraits") + outer_key = duplicate_files.favorite_root_key(self.output_dir) + self.assertIsNone(self.api.db.image_category(outer_key, "sub/a.png")) + + async def test_outer_source_listed_first_still_keys_by_the_deepest(self): + self.write("sub/a.png", b"same", mtime=1) + self.write("sub/b.png", b"same", mtime=2) + GalleryConfig.MONITORING_DIRECTORIES = [ + self._root(self.output_dir), + self._root(self.output_dir / "sub"), + ] + sub_key = duplicate_files.favorite_root_key(self.output_dir / "sub") + self.api.db.set_image_categories([(sub_key, "b.png")], "Portraits") + job, group = await self.found_group() + [kept] = [f for f in group["files"] if f["path"].endswith("a.png")][:1] + plan = await self.review( + job.id, [self.keep_of(group, kept["path"], kept["root_index"])] + ) + body = await self.apply(plan["token"]) + cleanup = self.api._jobs.get(body["job"]["id"]) + await asyncio.wait_for(asyncio.shield(cleanup.task), WAIT) + self.assertFalse((self.output_dir / "sub" / "b.png").exists()) + self.assertEqual(self.api.db.image_category(sub_key, "a.png"), "Portraits")