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.
This commit is contained in:
@@ -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():
|
||||
|
||||
@@ -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')} "
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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")
|
||||
Reference in New Issue
Block a user