diff --git a/TESTING.md b/TESTING.md index 16a22edb..3512e465 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2458,6 +2458,29 @@ files to the system trash. **No code path unlinks or overwrites a media file.** back records of files that are in the trash (Missing-file links cleans them up). The legacy gallery lists every file on disk, removed or not. Hard links have one mark per path, and on FAT/exFAT/SMB a same-size replacement can stay hidden. + Since #501 the thumbnail jobs (System › Thumbnails' counts and its missing, rebuild and + convert runs, and the previews after a scan) and the legacy `GET /scan_output_dir` + listing skip hidden files too; an on-demand preview the legacy gallery asks for is still + made, and a one-image auto-tag (`/autotag/single` + `/autotag/apply`) is an explicit + write that shows the file again. System › Maintenance **Missing-file links** also + forgets the marks that hide nothing any more (the file is gone from a folder that still + exists, or its bytes changed), counted in its scope ("K removed images whose files are + gone or changed") and job line, judged only under a source that is readable and lists + something (an unplugged drive's empty mount point keeps its marks), never for a source + no longer configured, and re-checked at apply (`plan-changed`). The sidebar's Images, + Favorites and Needs review counts and the collection counts are read again when an + `image_delete` job ends in the tab that followed it (`JOB_END_COUNTS`, `state.mjs`). + Tests: `tests/test_image_removal_gaps.py`, the sidebar case in + `tests/browser/workspace-images-delete.spec.mjs`. +- **Trash folders inside an output root** (#501, `utils/media_folders.py`): no walker + descends a folder below a root named `.Trash`, `.Trash-`, `.Trashes` or + `$RECYCLE.BIN` (any case), as none descends `thumbnails`: the workspace and legacy + listings (`_walk_media`, `_settled_entry`), the metadata scan and duplicate find + (`_collect_media_files`), the legacy auto-tag listing (`_collect_scan_files`) and the + delete review (`trash-folder`). The image monitor tests every folder of the new file's + absolute path, so it also ignores outputs written into a source that itself lies + inside such a folder. Tests: `tests/test_media_folders.py`, + `tests/test_image_removal_gaps.py`. - **Trash backends** (`utils/trash.py`, no dependency): Linux and BSD implement the freedesktop Trash specification (home trash on the same mount, else the mount's `.Trash/$uid` or `.Trash-$uid`), reserve the name with `O_EXCL` and move with diff --git a/database/image_removal.py b/database/image_removal.py index 3360433c..58099e03 100644 --- a/database/image_removal.py +++ b/database/image_removal.py @@ -144,6 +144,23 @@ def removed_from_library(conn, path) -> bool: return False +def mark_hides(marks: dict, locator: str, path) -> bool: + """Whether a removal mark in *marks* hides the file at *path* (blocking). + + *marks* is ``{locator: (size, mtime_ns)}``. The mark of *locator* hides + the file while ``os.lstat(path)`` still has the size and ``mtime_ns`` it + was removed with; a file that cannot be looked at is not hidden. + """ + mark = marks.get(locator) + if mark is None: + return False + try: + info = os.lstat(path) + except OSError: + return False + return (info.st_size, info.st_mtime_ns) == tuple(mark) + + def write_mark(conn, locator, size, mtime_ns): _write_mark(conn, locator, size, mtime_ns, utc_now_iso()) diff --git a/database/maintenance.py b/database/maintenance.py index a76d2d9c..6fddc494 100644 --- a/database/maintenance.py +++ b/database/maintenance.py @@ -15,6 +15,8 @@ change: ``cleanup_orphaned_images``), counted only under a source that can be read now, as the health check's ``missing_files`` judges them (``health.link_states``). It deletes database rows only, never a file. + It also forgets the removal marks (#480) that hide nothing any more: the + file is gone or changed, under a readable source (#501, ``_stale_marks``). ``preview_maintenance`` reads one snapshot on its own ``mode=ro`` connection (``health.readonly_uri``) and closes it in ``finally``; files are looked at @@ -41,6 +43,8 @@ from pathlib import Path from .execution_contexts import LIVE_HINT_CTES from .health import BATCH_ROWS, link_states, readable_roots, readonly_uri +from .image_removal_schema import TABLE as MARKS_TABLE +from .image_removal_schema import clear_marks, has_marks_table from .pairs import PairDependencyError from .workflow_documents import documents_of_rows, release_documents @@ -187,6 +191,8 @@ class MaintenanceDraft: protected: dict media: dict details: list = field(default_factory=list) + # missing_links only: locators of removal marks whose file is gone (#501). + marks: list = field(default_factory=list) def public(self): """The plan fields a response may carry (no fingerprint, no targets).""" @@ -289,6 +295,90 @@ def _delete_ids(conn, table, ids): return removed +def _mark_rows(conn, only=None): + """``[(locator, size, mtime_ns)]`` of the removal marks (all, or those + among *only*), sorted by locator.""" + if not has_marks_table(conn): + return [] + sql = f"SELECT locator, size, mtime_ns FROM {MARKS_TABLE}" # nosec B608 + if only is None: + return sorted(tuple(row) for row in conn.execute(sql)) + found = [] + for batch in _batches(sorted(only)): + found.extend( + tuple(row) + for row in conn.execute( + f"{sql} WHERE locator IN ({_marks(batch)})", batch # nosec B608 + ) + ) + return sorted(found) + + +def _lists_something(folder): + """Whether *folder* holds at least one entry (an unplugged drive's mount + point, or an unbound volume, is an empty folder that reads fine).""" + try: + with os.scandir(folder) as entries: + return next(entries, None) is not None + except OSError: + return False + + +def _name_listed(path): + """Whether *path*'s folder lists its name, compared under ``normcase``. + + A locator is ``normcase``d; on a case-sensitive NTFS folder ``lstat`` of + the lower-cased spelling fails for a file that is there. + """ + folder, name = os.path.split(path) + wanted = os.path.normcase(name) + try: + with os.scandir(folder) as entries: + return any(os.path.normcase(entry.name) == wanted for entry in entries) + except OSError: + return True # cannot tell: keep the mark + + +def _stale_mark(locator, size, mtime_ns): + """Whether the file of a mark is gone or has other bytes (blocking).""" + try: + info = os.lstat(locator) + except (FileNotFoundError, NotADirectoryError): + return os.path.isdir(os.path.dirname(locator)) and not _name_listed(locator) + except OSError: + return False + return (info.st_size, info.st_mtime_ns) != (size, mtime_ns) + + +def _stale_marks(marks, roots): + """``[(locator, owner)]`` of the marks that hide nothing any more (#501). + + *marks* are ``(locator, size, mtime_ns)`` rows. A mark is stale when its + file was changed, or is gone from a folder that still exists. Only marks + under a source that can be read now and lists something are judged (an + unplugged drive's marks, or its empty mount point's, stay); any doubt + keeps the mark. *owner* is the index of the longest source holding it. + Locators are ``normcase(realpath)`` paths, so the sources are too. + """ + readable = readable_roots(roots) + prefixes = [ + os.path.normcase(os.path.realpath(root)).rstrip("\\/") + os.sep + for root in roots + ] + judged = {} + stale = [] + for locator, size, mtime_ns in marks: + owners = [i for i, prefix in enumerate(prefixes) if locator.startswith(prefix)] + if not owners: + continue # under no source now (a source removed from the settings) + owner = max(owners, key=lambda i: len(prefixes[i])) + if owner not in judged: + judged[owner] = readable[owner] and _lists_something(roots[owner]) + if judged[owner] and _stale_mark(locator, size, mtime_ns): + stale.append((locator, owner)) + return stale + + def _read_snapshot(db_path, read): """``read(conn)`` on one ``mode=ro`` snapshot; the connection always closes.""" conn = sqlite3.connect(readonly_uri(db_path), uri=True) @@ -431,8 +521,11 @@ class MaintenanceOperations: if operation == "workflow_storage": return self._plan_workflow_storage() if operation == "missing_links": - rows = _read_snapshot(self.model.db_path, self._link_rows) - return self._plan_missing_links(rows, roots) + rows, marks = _read_snapshot( + self.model.db_path, + lambda conn: (self._link_rows(conn), _mark_rows(conn)), + ) + return self._plan_missing_links(rows, roots, _stale_marks(marks, roots)) reader = ( self._plan_duplicate_prompts if operation == "duplicate_prompts" @@ -563,13 +656,15 @@ class MaintenanceOperations: ) @staticmethod - def _links_fingerprint(candidates): + def _links_fingerprint(candidates, stale=()): fingerprint = _Fingerprint("missing_links") for row, owner in candidates: fingerprint.add([row[0], row[1], row[3], owner]) + for locator, owner in stale: + fingerprint.add(["mark", locator, owner]) return fingerprint.hexdigest() - def _plan_missing_links(self, rows, roots): + def _plan_missing_links(self, rows, roots, stale=()): readable = readable_roots(roots) candidates, unavailable, outside = [], 0, 0 for row, owner, state in link_states(rows, roots, readable): @@ -597,29 +692,47 @@ class MaintenanceOperations: ) ) count = len(candidates) + parts = [] + if count: + parts.append( + _plural(count, "link to a missing file", "links to missing files") + ) + if stale: + parts.append( + _plural( + len(stale), + "removed image whose file is gone or changed", + "removed images whose files are gone or changed", + ) + ) + details = [ + { + "label": _relative(row[1], roots[owner]), + "detail": f"{_source_name(roots[owner], owner)} · the file is missing", + } + for row, owner in candidates + ] + [ + { + "label": _relative(locator, os.path.realpath(roots[owner])), + "detail": ( + f"{_source_name(roots[owner], owner)} · removed image, " + "the file is gone or changed" + ), + } + for locator, owner in stale + ] return MaintenanceDraft( operation="missing_links", - fingerprint=self._links_fingerprint(candidates), + fingerprint=self._links_fingerprint(candidates, stale), targets=[row[0] for row, _owner in candidates], scope={ - "count": count, - "label": ( - _plural(count, "link to a missing file", "links to missing files") - if count - else "No links to missing files" - ), + "count": count + len(stale), + "label": "; ".join(parts) or "No links to missing files", }, protected={"count": unavailable + outside, "reasons": reasons}, media={"deletes_files": False, "label": LINKS_ONLY}, - details=[ - { - "label": _relative(row[1], roots[owner]), - "detail": ( - f"{_source_name(roots[owner], owner)} · the file is missing" - ), - } - for row, owner in candidates[:MAX_DETAILS] - ], + details=details[:MAX_DETAILS], + marks=[locator for locator, _owner in stale], ) def _plan_optimize(self): @@ -669,14 +782,18 @@ class MaintenanceOperations: # ── apply ───────────────────────────────────────────────────────── - def apply_maintenance(self, operation, fingerprint, targets, roots=()) -> dict: + def apply_maintenance( + self, operation, fingerprint, targets, roots=(), marks=() + ) -> dict: """Apply a reviewed plan exactly, or raise PlanChanged (blocking). Inside ``BEGIN IMMEDIATE`` the fingerprint is computed again on the write connection; when it differs nothing is written. ``targets`` and ``roots`` are the plan's. Returns ``{changed, rows}``: how many records changed and up to ``MAX_RESULT_ROWS`` descriptions of them - (``{prompt_id[, kept_id]}`` or ``{image_path}``). + (``{prompt_id[, kept_id]}`` or ``{image_path}``). ``marks`` are a + missing-links plan's stale removal marks; that apply also returns + ``marks``, how many it forgot. """ if operation not in MAINTENANCE_OPERATIONS or operation == "workflow_storage": raise ValueError(f"Unknown maintenance operation {operation!r}") @@ -689,9 +806,12 @@ class MaintenanceOperations: appliers = { "duplicate_prompts": self._apply_duplicate_prompts, "unused_components": self._apply_unused_components, - "missing_links": self._apply_missing_links, } with self._pair_transaction() as conn: + if operation == "missing_links": + return self._apply_missing_links( + conn, fingerprint, targets, roots, list(marks) + ) return appliers[operation](conn, fingerprint, targets, roots) def _apply_duplicate_prompts(self, conn, fingerprint, targets, _roots): @@ -720,7 +840,7 @@ class MaintenanceOperations: ] return {"changed": changed, "rows": rows} - def _apply_missing_links(self, conn, fingerprint, targets, roots): + def _apply_missing_links(self, conn, fingerprint, targets, roots, marks): rows = [] for batch in _batches(sorted(targets)): rows.extend( @@ -737,10 +857,14 @@ class MaintenanceOperations: if state != "missing": # back, or its source went away raise PlanChanged("missing_links") candidates.append((row, owner)) - if self._links_fingerprint(candidates) != fingerprint: + stale = _stale_marks(_mark_rows(conn, marks) if marks else [], roots) + if [locator for locator, _owner in stale] != marks: # a mark went or is valid + raise PlanChanged("missing_links") + if self._links_fingerprint(candidates, stale) != fingerprint: raise PlanChanged("missing_links") changed = _delete_ids(conn, "generated_images", [row[0] for row in rows]) return { "changed": changed, + "marks": clear_marks(conn, marks), "rows": [{"image_path": row[1]} for row in rows[:MAX_RESULT_ROWS]], } diff --git a/py/api/admin.py b/py/api/admin.py index a6b123d2..a9f820e2 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -36,10 +36,13 @@ except ImportError: try: from ...utils import exit_signal + from ...utils.media_folders import is_skipped_folder except ImportError: from utils import exit_signal + from utils.media_folders import is_skipped_folder try: + from ...database.image_removal import mark_hides from ...database.pair_ingestion import SCAN_MARKS, ingest_resolved_pair from ...database.run_attribution import ( RecordedOutput, @@ -60,6 +63,7 @@ try: decide_file_prompt, ) except ImportError: + from database.image_removal import mark_hides from database.pair_ingestion import SCAN_MARKS, ingest_resolved_pair from database.run_attribution import ( RecordedOutput, @@ -118,10 +122,11 @@ DUPLICATE_FIND_LEGACY_TITLE = "Find duplicate image files (gallery page)" def _collect_media_files(output_dirs, extensions): """Media files under ``output_dirs`` (blocking), bounded and symlink-free. - Walks each root without following symlinks, skips ``thumbnails`` - directories and symlinked files, never descends more than MAX_SCAN_DEPTH - levels and stops after MAX_SCAN_FILES files in total. Extensions match - case-insensitively and each path is listed once. + Walks each root without following symlinks, skips ``thumbnails`` and + trash folders (``utils.media_folders``) and symlinked files, never + descends more than MAX_SCAN_DEPTH levels and stops after MAX_SCAN_FILES + files in total. Extensions match case-insensitively and each path is + listed once. """ wanted = {ext.lower() for ext in extensions} found = [] @@ -136,7 +141,7 @@ def _collect_media_files(output_dirs, extensions): dirnames[:] = sorted( d for d in dirnames - if d != "thumbnails" + if not is_skipped_folder(d) and depth < MAX_SCAN_DEPTH and not os.path.islink(os.path.join(current, d)) ) @@ -893,11 +898,8 @@ class AdminRoutesMixin: stat = os.stat(path) except OSError: continue - mark = marks.get(os.path.normcase(os.path.realpath(str(path)))) - if mark is not None and tuple(mark) == ( - stat.st_size, - stat.st_mtime_ns, - ): + locator = os.path.normcase(os.path.realpath(str(path))) + if locator in marks and mark_hides(marks, locator, path): continue key = _physical_file_key(path, stat) if key not in seen: diff --git a/py/api/autotag_routes.py b/py/api/autotag_routes.py index edc8ca82..d3ff1629 100644 --- a/py/api/autotag_routes.py +++ b/py/api/autotag_routes.py @@ -23,6 +23,11 @@ from .prompts import thumbnail_url_for from . import thumbnail_settings from .. import model_gate +try: + from ...utils.media_folders import is_skipped_folder +except ImportError: + from utils.media_folders import is_skipped_folder + from aiohttp import web _INTERNAL_ERROR = "An internal error occurred. Check server logs for details." @@ -1319,8 +1324,9 @@ class AutotagRoutesMixin: def _collect_scan_files(output_path: Path): """Media files under ``output_path`` from a bounded, symlink-free walk. - Descends at most ``_SCAN_MAX_DEPTH`` levels, skips ``thumbnails`` - trees and symlinked files, and stops after ``_SCAN_MAX_FILES`` files. + Descends at most ``_SCAN_MAX_DEPTH`` levels, skips ``thumbnails`` and + trash folders (``utils.media_folders``) and symlinked files, and stops + after ``_SCAN_MAX_FILES`` files. Returns ``(paths, truncated)``. """ found = [] @@ -1330,7 +1336,7 @@ class AutotagRoutesMixin: dirnames[:] = sorted( name for name in dirnames - if name != "thumbnails" and depth + 1 <= _SCAN_MAX_DEPTH + if not is_skipped_folder(name) and depth + 1 <= _SCAN_MAX_DEPTH ) for name in sorted(filenames): if Path(name).suffix.lower() not in _MEDIA_EXTENSIONS: @@ -1357,6 +1363,10 @@ class AutotagRoutesMixin: files, truncated = await self._run_in_executor( self._collect_scan_files, output_path ) + # #501: a file removed from the library is not offered for tagging. + files = await self._run_in_executor( + self._without_removed, output_path, files + ) has_thumbnails = (output_path / "thumbnails").is_dir() images = [] diff --git a/py/api/image_delete.py b/py/api/image_delete.py index fe011149..44d38737 100644 --- a/py/api/image_delete.py +++ b/py/api/image_delete.py @@ -47,12 +47,16 @@ from .maintenance import MAINTENANCE_PREVIEW_LEASE, _fail_job, _failure, backup_ try: from ...database import image_removal_schema + from ...database.image_removal import mark_hides from ...database.annotation_schema import MAX_LOCATOR_LENGTH from ...utils import trash + from ...utils.media_folders import is_trash_folder except ImportError: from database import image_removal_schema + from database.image_removal import mark_hides from database.annotation_schema import MAX_LOCATOR_LENGTH from utils import trash + from utils.media_folders import is_trash_folder IMAGE_DELETE = "delete_images" MODES = ("library", "trash") @@ -61,7 +65,6 @@ TITLES = { "trash": "Delete images (move to the trash)", } MAX_DETAIL_ROWS = 50 -TRASH_FOLDERS = frozenset({".trash", ".trashes", "$recycle.bin"}) # Reason codes (design §4.4) and the sentence each shows. REASONS = { @@ -93,19 +96,12 @@ SOURCES_CHANGED = "The sources changed since the review; nothing was changed." RECORDS_FAILED = "The image records could not be removed; the job stopped." -def _is_trash_folder(name: str) -> bool: - folded = name.casefold() - return folded in TRASH_FOLDERS or ( - folded.startswith(".trash-") and folded[7:].isdigit() - ) - - def lexical_problem(relative: str) -> str | None: """The refusal code a root-relative POSIX path earns before any file access.""" parts = relative.split("/") if "thumbnails" in parts[:-1]: return "thumbnail" - if any(_is_trash_folder(part) for part in parts[:-1]): + if any(is_trash_folder(part) for part in parts[:-1]): return "trash-folder" if os.path.splitext(parts[-1])[1].lower() not in DELETABLE_MEDIA_EXTENSIONS: return "not-media" @@ -443,37 +439,49 @@ class ImageDeleteMixin: if not marks: return set() located = self._workspace_locators(listings) - hidden = set() - for locator, (size, mtime_ns) in marks.items(): - entry = located.get(locator) - if entry is None: - continue - try: - info = os.lstat(entry.path) - except OSError: - continue - if (info.st_size, info.st_mtime_ns) == (size, mtime_ns): - hidden.add(entry.normalized) - return hidden + return { + entry.normalized + for locator, entry in ((locator, located.get(locator)) for locator in marks) + if entry is not None and mark_hides(marks, locator, entry.path) + } def _removed_paths(self, paths) -> set: """The absolute paths among *paths* a removal mark hides (blocking).""" marks = self._removal_marks() if not marks: return set() - hidden = set() - for path in paths: - mark = marks.get(os.path.normcase(os.path.realpath(str(path)))) - if mark is None: - continue - try: - info = os.lstat(path) - except OSError: - continue - if (info.st_size, info.st_mtime_ns) == tuple(mark): - hidden.add(str(path)) + return { + str(path) + for path in paths + if mark_hides(marks, os.path.normcase(os.path.realpath(str(path))), path) + } + + def _removal_check_under(self, root): + """``hidden(path) -> bool`` for files a walk of *root* found (blocking). + + A walk never follows a link below *root*, so a file's locator is + ``normcase(join(realpath(root), relative))``: no ``realpath`` per + file, and an ``lstat`` only for a marked one. Without marks it + answers False at once. + """ + marks = self._removal_marks() + if not marks: + return lambda _path: False + root = os.fspath(root) + real = os.path.realpath(root) + + def hidden(path): + relative = os.path.relpath(os.fspath(path), root) + locator = os.path.normcase(os.path.join(real, relative)) + return mark_hides(marks, locator, path) + return hidden + def _without_removed(self, root, paths) -> list: + """*paths* found by a walk of *root*, less the hidden ones (blocking).""" + removed = self._removal_check_under(root) + return [path for path in paths if not removed(path)] + def parse_mode(value): """The apply's ``mode``, or None when it is not one of MODES.""" diff --git a/py/api/images.py b/py/api/images.py index 38286e2e..eb5636d9 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -66,6 +66,7 @@ try: from ...database.models import normalize_image_path from ...database.run_attribution import recorded_output_lookup from ...utils.card_seed import card_seed, header_params, parsed_positive + from ...utils.media_folders import in_skipped_folder, is_skipped_folder from ...utils.parallel import map_parallel from ...utils.json_strict import dumps_strict from ...utils.inspection_response import ( @@ -91,6 +92,7 @@ except ImportError: from database.models import normalize_image_path from database.run_attribution import recorded_output_lookup from utils.card_seed import card_seed, header_params, parsed_positive + from utils.media_folders import in_skipped_folder, is_skipped_folder from utils.parallel import map_parallel from utils.json_strict import dumps_strict from utils.inspection_response import ( @@ -915,7 +917,8 @@ def _name_suffix(name): def _walk_media(output_path, max_files): """Yield the ``os.DirEntry`` of each media file under *output_path*. - Symlinks are never followed, ``thumbnails/`` is skipped, descent stops + Symlinks are never followed, ``thumbnails/`` and trash folders are + skipped (``utils.media_folders``, #501), descent stops at MAX_SCAN_DEPTH and at most *max_files* entries are produced. The order is ``os.walk``'s (top-down, subfolders in ``os.scandir`` order, files by name), which the gallery's ties in mtime keep. Each entry's type comes @@ -927,8 +930,6 @@ def _walk_media(output_path, max_files): produced = 0 while stack: top, rel_parts = stack.pop() - if "thumbnails" in rel_parts: - continue dirs, files = [], [] try: with os.scandir(top) as entries: @@ -952,7 +953,7 @@ def _walk_media(output_path, max_files): if len(rel_parts) >= MAX_SCAN_DEPTH: continue for entry in reversed(dirs): - if not entry.is_symlink(): + if not entry.is_symlink() and not is_skipped_folder(entry.name): stack.append((entry.path, rel_parts + (entry.name,))) @@ -976,7 +977,7 @@ def _settled_entry(root, relative): *folders, name = relative.split(os.sep) if _name_suffix(name).lower() not in MEDIA_EXTENSIONS: return None - if "thumbnails" in folders or len(folders) > MAX_SCAN_DEPTH: + if in_skipped_folder(folders) or len(folders) > MAX_SCAN_DEPTH: return None folder = os.fspath(root) for part in ("", *folders): @@ -3075,7 +3076,9 @@ class ImageRoutesMixin: no symlinks), each preview folder is listed once, and at most MAX_THUMBNAIL_FILES targets are queued per batch (a job plans again after each batch); ``remaining`` counts the targets beyond that cap. - Sources in *exclude* (worked on by an earlier batch) are left out. + Sources in *exclude* (worked on by an earlier batch) are left out, and + so are files removed from the library (#501: a removal mark hides + them, and their previews went with their records). ``stop()`` true ends the walk before the next file. """ settings = writer_settings(settings) @@ -3084,11 +3087,14 @@ class ImageRoutesMixin: plan = {key: 0 for key in _PLAN_COUNTS} plan.update(targets=[], fresh_files=[]) folders = {} + removed = self._removal_check_under(output_path) for src in _iter_media_files(output_path, MAX_GALLERY_FILES): if stop is not None and stop(): break if exclude and src in exclude: continue + if removed(src): + continue rel_no_ext = src.relative_to(output_path).with_suffix("") stem = f"{rel_no_ext.name}{thumbnail_settings.THUMB_SUFFIX}" dst = thumbnails_dir / rel_no_ext.parent / (stem + settings.extension) diff --git a/py/api/maintenance.py b/py/api/maintenance.py index 4a518efd..e7d34020 100644 --- a/py/api/maintenance.py +++ b/py/api/maintenance.py @@ -281,7 +281,7 @@ def short_space_message(short): ) -def _changed_line(operation, count): +def _changed_line(operation, count, marks=0): if operation == "optimize": return "Optimized the database" if operation == "duplicate_prompts": @@ -291,7 +291,11 @@ def _changed_line(operation, count): noun = "unused prompt component" if count == 1 else "unused prompt components" return f"Removed {count:,} {noun}" noun = "link to a missing file" if count == 1 else "links to missing files" - return f"Removed {count:,} {noun}" + line = f"Removed {count:,} {noun}" + if marks: + gone = "removed image" if marks == 1 else "removed images" + line += f"; forgot {marks:,} {gone} whose files are gone or changed" + return line def take_safety_backup(model, label="backup"): @@ -419,6 +423,7 @@ class MaintenanceRoutesMixin: draft.public(), { "targets": draft.targets, + "marks": draft.marks, "roots": roots, "protected": draft.protected["count"], "rows": draft.scope["count"], @@ -653,6 +658,7 @@ class MaintenanceRoutesMixin: plan.fingerprint, payload["targets"], payload["roots"], + payload.get("marks", ()), ) except PlanChanged: return _fail_job(job, "plan-changed", PLAN_CHANGED) @@ -664,7 +670,7 @@ class MaintenanceRoutesMixin: ) job.record_counted("changed", result["changed"], rows) job.record_counted("protected", payload["protected"]) - line = _changed_line(plan.operation, result["changed"]) + line = _changed_line(plan.operation, result["changed"], result.get("marks", 0)) job.log_line(line) job.update(progress=100, status_line=f"{line} · {backup_note(backup)}") diff --git a/tests/browser/workspace-images-delete.spec.mjs b/tests/browser/workspace-images-delete.spec.mjs index bf727731..7051166b 100644 --- a/tests/browser/workspace-images-delete.spec.mjs +++ b/tests/browser/workspace-images-delete.spec.mjs @@ -73,6 +73,19 @@ for (const mode of ['library', 'trash']) { }); } +// #501: the sidebar's Images count (read once at mount) follows a finished delete without a reload. +test('a finished delete re-reads the sidebar Images count', async ({ page }) => { + await openImages(page); + const badge = page.locator('#primary-nav a[data-route="images"] .nav-count'); + await expect(badge).toHaveText(/^\d[\d,]*$/); + const before = Number((await badge.textContent()).replace(/,/g, '')); + await check(page, 0); + const { primary } = await deleteChecked(page, 'library'); + await primary.click(); + await expect(toast(page)).toContainText('Removed 1 image from the library.'); + await expect(badge).toHaveText((before - 1).toLocaleString('en-US')); +}); + test('Cancel changes nothing', async ({ page }) => { const before = await openImages(page); await check(page, 0); diff --git a/tests/test_image_removal_gaps.py b/tests/test_image_removal_gaps.py new file mode 100644 index 00000000..cb961720 --- /dev/null +++ b/tests/test_image_removal_gaps.py @@ -0,0 +1,251 @@ +"""Removal-mark gaps after #480, and trash folders inside output roots (#501). + +- Thumbnail jobs and the legacy auto-tag listing skip a library-removed file. +- Missing-file links also forgets the marks of files that are gone (only under + a source that can be read now). +- Every walker skips a trash folder that sits inside an output root. +""" + +import asyncio +import os +import shutil +import sqlite3 +from pathlib import Path +from unittest.mock import patch + +try: + from tests.disk_waits import DISK_BOUND_SECONDS + from tests.test_image_delete_routes import ImageDeleteTestCase +except ImportError: # discovered with tests/ as the top-level directory + from disk_waits import DISK_BOUND_SECONDS + from test_image_delete_routes import ImageDeleteTestCase + +from py.api import images as images_module +from py.api.admin import _collect_media_files +from py.api.autotag_routes import AutotagRoutesMixin +from utils.image_monitor import ImageGenerationHandler + +MAINTENANCE_PREVIEW = "/prompt_manager/maintenance/preview" +MAINTENANCE_APPLY = "/prompt_manager/maintenance/apply" + + +class _GapCase(ImageDeleteTestCase): + def marks(self): + conn = sqlite3.connect(self.api.db.model.db_path) + try: + return [ + row[0] for row in conn.execute("SELECT locator FROM image_removals") + ] + finally: + conn.close() + + def revision(self): + conn = sqlite3.connect(self.api.db.model.db_path) + try: + return conn.execute( + "SELECT revision FROM image_removals_meta WHERE singleton_id=1" + ).fetchone()[0] + finally: + conn.close() + + +class TestThumbnailJobsSkipHiddenFiles(_GapCase): + async def test_missing_and_rebuild_plans_leave_a_removed_file_out(self): + self.write("hidden.png", b"hidden bytes", mtime=1_000_000) + self.write("shown.png", b"shown bytes", mtime=2_000_000) + await self.delete(["hidden.png"], "library") + thumbs = self.output_dir / "thumbnails" + for mode in ("missing", "rebuild"): + with self.subTest(mode=mode): + plan = self.api._plan_thumbnails(self.output_dir, thumbs, mode=mode) + sources = [src.name for src, _dst, _video in plan["targets"]] + self.assertEqual(sources, ["shown.png"]) + self.assertEqual(plan["missing"], 1) + + async def test_a_changed_file_gets_its_preview_again(self): + self.write("hidden.png", b"hidden bytes", mtime=1_000_000) + await self.delete(["hidden.png"], "library") + self.write("hidden.png", b"new and longer bytes", mtime=3_000_000) + plan = self.api._plan_thumbnails( + self.output_dir, self.output_dir / "thumbnails" + ) + self.assertEqual([t[0].name for t in plan["targets"]], ["hidden.png"]) + + +class TestLegacyAutotagListing(_GapCase): + async def test_scan_output_dir_leaves_a_removed_file_out(self): + self.write("hidden.png", b"hidden bytes", mtime=1_000_000) + self.write("shown.png", b"shown bytes", mtime=2_000_000) + await self.delete(["hidden.png"], "library") + with patch.object( + self.api, "_find_comfyui_output_dir", return_value=str(self.output_dir) + ): + response = await self.client.get("/prompt_manager/scan_output_dir") + body = await response.json() + self.assertEqual([image["filename"] for image in body["images"]], ["shown.png"]) + + +class TestMissingFileLinksPrunesMarks(_GapCase): + async def links_review(self, status=200): + return await self.post( + MAINTENANCE_PREVIEW, {"operation": "missing_links", "options": {}}, status + ) + + async def apply(self, token, status=202): + body = await self.post(MAINTENANCE_APPLY, {"token": token}, status) + if status != 202: + return body + job = self.api._jobs.get(body["job"]["id"]) + await asyncio.wait_for(asyncio.shield(job.task), DISK_BOUND_SECONDS) + return job + + async def removed(self, name): + self.write("other.png", b"another output") # the source lists something + self.write(name, f"bytes of {name}".encode(), mtime=1_000_000) + await self.delete([name], "library") + self.assertEqual(len(self.marks()), 1) + + async def test_the_mark_of_a_gone_file_is_counted_and_forgotten(self): + await self.removed("gone.png") + # Not an unlink by PromptManager: the user deleted it elsewhere. + os.remove(self.output_dir / "gone.png") + plan = await self.links_review() + self.assertEqual(plan["scope"]["count"], 1) + self.assertEqual( + plan["scope"]["label"], "1 removed image whose file is gone or changed" + ) + before = self.revision() + job = await self.apply(plan["token"]) + self.assertEqual(job.state, "finished", job.summary()) + self.assertEqual(self.marks(), []) + self.assertGreater(self.revision(), before) + + async def test_the_mark_of_a_present_file_stays(self): + await self.removed("kept.png") + plan = await self.links_review() + self.assertEqual(plan["scope"]["count"], 0) + self.assertEqual(plan["scope"]["label"], "No links to missing files") + self.assertEqual(len(self.marks()), 1) + + async def test_a_mark_under_an_unreadable_source_stays(self): + await self.removed("away.png") + os.remove(self.output_dir / "away.png") + with patch( + "database.maintenance.readable_roots", + side_effect=lambda r: [False] * len(r), + ): + plan = await self.links_review() + self.assertEqual(plan["scope"]["count"], 0) + self.assertEqual(len(self.marks()), 1) + + async def test_the_mark_of_a_changed_file_is_forgotten(self): + await self.removed("changed.png") + self.write("changed.png", b"other and longer bytes", mtime=4_000_000) + plan = await self.links_review() + self.assertEqual(plan["scope"]["count"], 1) + job = await self.apply(plan["token"]) + self.assertEqual(job.state, "finished", job.summary()) + self.assertEqual(self.marks(), []) + + async def test_marks_under_an_empty_source_stay(self): + # An unplugged drive leaves its mount point: an empty, readable folder. + await self.removed("mounted.png") + for child in list(self.output_dir.iterdir()): + if child.is_dir(): + shutil.rmtree(child) + else: + os.remove(child) + plan = await self.links_review() + self.assertEqual(plan["scope"]["count"], 0) + self.assertEqual(len(self.marks()), 1) + + async def test_a_name_the_folder_still_lists_is_not_gone(self): + # A case-sensitive NTFS folder: lstat of the normcased locator fails. + await self.removed("Listed.png") + real_lstat = os.lstat + locator = self.marks()[0] + + def lstat(path, *args, **kwargs): + if os.fspath(path) == locator: + raise FileNotFoundError(path) + return real_lstat(path, *args, **kwargs) + + with ( + patch("database.maintenance.os.lstat", side_effect=lstat), + patch("database.maintenance.os.path.normcase", side_effect=str.lower), + ): + from database.maintenance import _stale_marks + + stale = _stale_marks([(locator, 1, 1)], [str(self.output_dir)]) + self.assertEqual(stale, []) + + async def test_a_file_back_before_the_apply_changes_the_plan(self): + await self.removed("back.png") + path = self.output_dir / "back.png" + saved = Path(self.tmpdir) / "saved.png" + shutil.copy2(path, saved) + os.remove(path) + plan = await self.links_review() + shutil.copy2(saved, path) + job = await self.apply(plan["token"]) + self.assertEqual(job.state, "failed", job.summary()) + self.assertEqual(len(self.marks()), 1) + + +class TestWalkersSkipTrashFolders(_GapCase): + def seed(self): + self.write("kept.png", b"kept") + self.write( + "x.png", b"trashed x", folder=self.output_dir / ".Trash-1000" / "files" + ) + self.write( + "y.png", b"trashed y", folder=self.output_dir / "sub" / "$RECYCLE.BIN" + ) + self.write("z.png", b"trashed z", folder=self.output_dir / ".Trashes") + + def test_walk_media_and_the_settled_entry(self): + self.seed() + walked = sorted( + Path(entry.path).name + for entry in images_module._walk_media(self.output_dir, 100) + ) + self.assertEqual(walked, ["kept.png"]) + for relative in ( + os.path.join(".Trash-1000", "files", "x.png"), + os.path.join("sub", "$RECYCLE.BIN", "y.png"), + ): + with self.subTest(relative=relative): + self.assertIsNone( + images_module._settled_entry(self.output_dir, relative) + ) + + def test_scan_duplicate_and_autotag_walkers(self): + self.seed() + collected = [ + Path(p).name for p in _collect_media_files([self.output_dir], [".png"]) + ] + self.assertEqual(collected, ["kept.png"]) + found, _truncated = AutotagRoutesMixin._collect_scan_files(self.output_dir) + self.assertEqual([p.name for p in found], ["kept.png"]) + + def test_the_image_monitor(self): + monitor = ImageGenerationHandler.__new__(ImageGenerationHandler) + monitor.supported_extensions = (".png",) + base = str(self.output_dir) + self.assertTrue(monitor.is_image_file(os.path.join(base, "a.png"))) + for folder in (".Trash-1000", "$Recycle.Bin", ".Trashes", ".trash"): + with self.subTest(folder=folder): + self.assertFalse( + monitor.is_image_file(os.path.join(base, folder, "files", "a.png")) + ) + + async def test_the_workspace_listing(self): + self.seed() + self.assertEqual(await self.listed(), ["kept.png"]) + + def test_a_root_named_like_a_trash_folder_still_lists(self): + root = Path(self.tmpdir) / ".Trash-1000" + (root / "inner").mkdir(parents=True) + (root / "inner" / "a.png").write_bytes(b"a") + walked = [Path(e.path).name for e in images_module._walk_media(root, 10)] + self.assertEqual(walked, ["a.png"]) diff --git a/tests/test_media_folders.py b/tests/test_media_folders.py new file mode 100644 index 00000000..d866aa93 --- /dev/null +++ b/tests/test_media_folders.py @@ -0,0 +1,41 @@ +"""The shared folder skip every media walker uses (#501).""" + +import unittest + +from utils.media_folders import in_skipped_folder, is_skipped_folder, is_trash_folder + + +class TrashFolderNameTests(unittest.TestCase): + def test_trash_folder_names_in_any_case(self): + for name in ( + ".Trash", + ".trash", + ".Trash-1000", + ".trash-0", + ".Trashes", + "$RECYCLE.BIN", + "$Recycle.Bin", + ): + with self.subTest(name=name): + self.assertTrue(is_trash_folder(name)) + self.assertTrue(is_skipped_folder(name)) + + def test_other_names_are_not_trash(self): + for name in (".Trash-abc", ".Trash-", "trash", "Recycle", "RECYCLE.BIN", "x"): + with self.subTest(name=name): + self.assertFalse(is_trash_folder(name)) + + def test_thumbnails_is_skipped_exactly(self): + self.assertTrue(is_skipped_folder("thumbnails")) + self.assertFalse(is_skipped_folder("Thumbnails")) + self.assertFalse(is_trash_folder("thumbnails")) + + def test_folder_parts(self): + self.assertTrue(in_skipped_folder(("a", ".Trash-1000", "files"))) + self.assertTrue(in_skipped_folder(("thumbnails",))) + self.assertFalse(in_skipped_folder(("a", "b"))) + self.assertFalse(in_skipped_folder(())) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/image_monitor.py b/utils/image_monitor.py index d2fc1c19..18c62c1f 100644 --- a/utils/image_monitor.py +++ b/utils/image_monitor.py @@ -45,6 +45,7 @@ from .foreign_runs import ( store_foreign, ) from .hashing import generate_prompt_hash +from .media_folders import in_skipped_folder from .metadata_extractor import ComfyUIMetadataExtractor from .parallel import note_refusal, refused_at_shutdown from .logging_config import get_logger @@ -428,11 +429,12 @@ class ImageGenerationHandler(FileSystemEventHandler): Returns: True if the file has a supported image extension and is not - inside a thumbnails directory, False otherwise + inside a thumbnails or trash folder, False otherwise """ - # Skip files in thumbnails directory - those are derivatives, not - # generated images - if "/thumbnails/" in filepath or "\\thumbnails\\" in filepath: + # Previews are derivatives, and a trash folder inside an output + # folder holds thrown-away files (#501): neither is a new output. + folders = filepath.replace("\\", "/").split("/")[:-1] + if in_skipped_folder(folders): return False return filepath.lower().endswith(self.supported_extensions) diff --git a/utils/media_folders.py b/utils/media_folders.py new file mode 100644 index 00000000..6052a001 --- /dev/null +++ b/utils/media_folders.py @@ -0,0 +1,31 @@ +"""The folders no media walker lists below an output root (#501). + +``thumbnails`` holds PromptManager's own previews. A trash folder that sits +inside an output root (``D:\\$RECYCLE.BIN`` when ``D:\\`` is a root, +``.Trash-1000`` at a mount point, ``.Trashes`` on a macOS volume) holds files +the user threw away: they are not outputs, so they are never listed, scanned, +linked, tagged or previewed. Every walker asks these predicates instead of +testing names itself; only folders *below* a root are tested. +""" + +TRASH_FOLDER_NAMES = frozenset({".trash", ".trashes", "$recycle.bin"}) +PREVIEW_FOLDER = "thumbnails" + + +def is_trash_folder(name: str) -> bool: + """A trash folder name, in any case: ``.Trash``, ``.Trash-``, + ``.Trashes`` or ``$RECYCLE.BIN``.""" + folded = name.casefold() + if folded in TRASH_FOLDER_NAMES: + return True + return folded.startswith(".trash-") and folded[7:].isdigit() + + +def is_skipped_folder(name: str) -> bool: + """A folder no walker descends: ``thumbnails`` (exact) or a trash folder.""" + return name == PREVIEW_FOLDER or is_trash_folder(name) + + +def in_skipped_folder(parts) -> bool: + """Whether any of the folder names *parts* is skipped.""" + return any(is_skipped_folder(part) for part in parts) diff --git a/web/js/workspace/index.mjs b/web/js/workspace/index.mjs index ad50c0db..951abd1c 100644 --- a/web/js/workspace/index.mjs +++ b/web/js/workspace/index.mjs @@ -1,7 +1,8 @@ import '../api-client.js'; import { createButton, createScope } from '../shared/toolkit/core.js'; import { routes, mountRouter, summarizeStats, routeTitle, loadNavCounts, navigateTo, COLLECTION_ROUTE, - LIBRARY_WRITE_COUNTS, NAV_COUNT_ROUTES, FAVORITE_WRITE_COUNTS, FAVORITE_COUNT_DELAY_MS, REVIEW_WRITE_COUNTS } from './state.mjs'; + LIBRARY_WRITE_COUNTS, NAV_COUNT_ROUTES, FAVORITE_WRITE_COUNTS, FAVORITE_COUNT_DELAY_MS, REVIEW_WRITE_COUNTS, + JOB_END_COUNTS } from './state.mjs'; import { mountWorkspaceTheme } from './theme.mjs'; import { forgetImagesVisit, mountImages, onFavoriteChange } from './images.mjs'; import { forgetLibraryVisit, mountLibrary, mountNewPromptAction, onLibraryWrite } from './library.mjs'; @@ -170,16 +171,20 @@ function mountWorkspace() { favoriteCountTimer = setTimeout(() => refreshCounts(FAVORITE_WRITE_COUNTS), FAVORITE_COUNT_DELAY_MS); })); scope.cleanup(() => clearTimeout(favoriteCountTimer)); - // A metadata scan that ends may have added items to Needs review: re-read its count once per ended scan. - const runningScans = new Set(); + // A job that ends re-reads the counts it can change, once per ended job: a metadata scan may have added items to + // Needs review; deleting images (#480) hides or trashes them, with their favorites, review items and collection + // memberships (#501). + const runningJobs = new Map(); scope.cleanup(activity.subscribe(() => { - let ended = false; + const ended = new Set(); for (const job of activity.jobs()) { - if (job.kind !== 'scan') continue; - if (job.state === 'running') runningScans.add(job.id); - else if (runningScans.delete(job.id)) ended = true; + if (!JOB_END_COUNTS[job.kind]) continue; + if (job.state === 'running') runningJobs.set(job.id, job.kind); + else if (runningJobs.delete(job.id)) ended.add(job.kind); } - if (ended) refreshCounts(REVIEW_WRITE_COUNTS); + if (!ended.size) return; + refreshCounts([...new Set([...ended].flatMap(kind => JOB_END_COUNTS[kind]))]); + if (ended.has('image_delete')) collections.load(); })); refreshCounts(NAV_COUNT_ROUTES); // S9: a finished restore replaced the library. The cached visits are dropped, the counts and saved views read diff --git a/web/js/workspace/state.mjs b/web/js/workspace/state.mjs index 9af7739c..a2b056c9 100644 --- a/web/js/workspace/state.mjs +++ b/web/js/workspace/state.mjs @@ -129,6 +129,11 @@ export const LIBRARY_WRITE_COUNTS = Object.freeze(['library', 'tags']); export const FAVORITE_WRITE_COUNTS = Object.freeze(['favorites']); /** The count a review decision or a finished metadata scan can change. */ export const REVIEW_WRITE_COUNTS = Object.freeze(['review']); +/** Job kind -> the counts re-read once when such a job ends (a scan feeds Needs review; #501: a delete hides images). */ +export const JOB_END_COUNTS = Object.freeze({ + scan: REVIEW_WRITE_COUNTS, + image_delete: Object.freeze(['images', 'favorites', 'review']), +}); // Trailing delay that coalesces quick star toggles into one Favorites count read (#249 M1). export const FAVORITE_COUNT_DELAY_MS = 300;