From ac093f21584d6f9a83d623e806484800862b6051 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Wed, 21 Oct 2026 19:32:14 -0700 Subject: [PATCH 1/2] fix: stamp metadata_imported_at on tag-reading library scans (#992) Hovering a browser card's tag button lazy-imports a comic whose has_metadata is false, i.e. metadata_imported_at and metadata_mtime are both NULL. metadata_mtime stays NULL for any archive with no embedded tags file. metadata_imported_at was only stamped by forced imports, so a library scan with "Import Tags on Library Scan" on (the default) read every comic but stamped none. On any install scanned since 0043, every comic without embedded tags read as un-imported. Each hover then forced an import that found nothing but still rewrote the row, purged and rebuilt the cover and broadcast LIBRARY_CHANGED, reloading the browser once per comic. - Stamp at finish whenever the pass read tags: forced, or the IMPORT_METADATA flag on. A flag-off scan still leaves rows NULL for the lazy importer. The flag read moves to FinishImporter so extract and finish share it. - Batch the stamp's path__in by IMPORTER_LINK_FK_BATCH_SIZE. First scans of large libraries now reach it, and a single IN list would exceed SQLite's default 32766-variable cap. - Migration 0060 backfills NULL rows with 0043's seed when the IMPORT_METADATA flag is on (a missing row counts as on). Co-Authored-By: Claude Opus 5.5 --- NEWS.md | 2 + codex/librarian/scribe/importer/finish.py | 36 +++-- .../librarian/scribe/importer/read/extract.py | 8 +- .../0060_backfill_metadata_imported_at.py | 44 ++++++ tests/test_lazy_import_marker.py | 127 +++++++++++++++++- 5 files changed, 192 insertions(+), 25 deletions(-) create mode 100644 codex/migrations/0060_backfill_metadata_imported_at.py diff --git a/NEWS.md b/NEWS.md index 7dbae0719..85e37db4b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -16,6 +16,8 @@ border-radius: 128px; - Online tag match picker shows series and issue details to help disambiguate similar covers and titles. - Fixes + - Browsing comics without embedded tags no longer re-imports them and + reloads the page. - Searches for a universe designation no longer miss imported comics. ## v2.5.5 diff --git a/codex/librarian/scribe/importer/finish.py b/codex/librarian/scribe/importer/finish.py index 24daced27..d5d2fe651 100644 --- a/codex/librarian/scribe/importer/finish.py +++ b/codex/librarian/scribe/importer/finish.py @@ -1,5 +1,6 @@ """The main importer class.""" +from itertools import batched from operator import itemgetter from time import perf_counter from types import MappingProxyType @@ -8,6 +9,7 @@ from django.db.models.functions import Now from humanize import intcomma, naturaldelta +from codex.choices.admin import AdminFlagChoices from codex.librarian.notifier.tasks import ( FAILED_IMPORTS_CHANGED_TASK, LIBRARY_CHANGED_TASK, @@ -18,7 +20,9 @@ from codex.librarian.scribe.importer.statii import IMPORTER_STATII from codex.librarian.scribe.search.status import SEARCH_INDEX_STATII from codex.librarian.scribe.status import SCRIBE_STATII +from codex.models.admin import AdminFlag from codex.models.comic import Comic +from codex.settings import IMPORTER_LINK_FK_BATCH_SIZE _REPORT_MAP = MappingProxyType( { @@ -91,24 +95,34 @@ def _log_finish(self) -> None: self.log.success(log_txt) self._log_phase_times() + def is_import_metadata(self) -> bool: + """Return whether this pass reads tags, forced or by admin flag.""" + if self.task.force_import_metadata: + return True + key = AdminFlagChoices.IMPORT_METADATA.value + return AdminFlag.objects.only("on").get(key=key).on + def _stamp_metadata_imported(self) -> None: """ - Mark every comic this forced import pass touched as import-complete. + Mark every comic this tag-reading pass touched as import-complete. Path-scoped so it covers SKIPPED no-metadata comics that never reach - the per-comic write path. Idempotent (``__isnull=True``) and gated on - ``force_import_metadata`` so ordinary watcher-driven imports never - rewrite the column. + the per-comic write path. Idempotent (``__isnull=True``). A scan with + the import-tags flag off reads no tags, so it leaves the column NULL + for the lazy importer to pick up. """ - if not self.task.force_import_metadata: - return if not (paths := self.metadata_import_paths): return - Comic.objects.filter( - library=self.library, - path__in=paths, - metadata_imported_at__isnull=True, - ).update(metadata_imported_at=Now()) + if not self.is_import_metadata(): + return + # A first scan reaches here with every path in the library, so + # batch path__in to stay under SQLite's variable limit. + for batch_paths in batched(paths, IMPORTER_LINK_FK_BATCH_SIZE): + Comic.objects.filter( + library=self.library, + path__in=batch_paths, + metadata_imported_at__isnull=True, + ).update(metadata_imported_at=Now()) def finish(self) -> None: """Perform final tasks when the apply is done.""" diff --git a/codex/librarian/scribe/importer/read/extract.py b/codex/librarian/scribe/importer/read/extract.py index e91f1711c..c4749eaf0 100644 --- a/codex/librarian/scribe/importer/read/extract.py +++ b/codex/librarian/scribe/importer/read/extract.py @@ -9,7 +9,6 @@ from comicbox.formats.comicbox.schema import PAGE_COUNT_KEY from comicbox.process import ReadResult, iter_process_files -from codex.choices.admin import AdminFlagChoices from codex.librarian.scribe.importer.const import ( EXTRACTED, EXTRACTED_STAT_ONLY, @@ -20,7 +19,6 @@ AggregateMetadataImporter, ) from codex.librarian.scribe.importer.statii.read import ImporterReadComicsStatus -from codex.models.admin import AdminFlag from codex.models.comic import Comic from codex.settings import COMICBOX_CONFIG, LOGLEVEL from codex.startup.loguru import CODEX_LOG_FORMAT @@ -102,11 +100,7 @@ def _filesystem_mtime_prefilter( def _get_import_metadata_flag(self) -> bool: """Set import_metadata flag.""" - if self.task.force_import_metadata: - import_metadata = True - else: - key = AdminFlagChoices.IMPORT_METADATA.value - import_metadata = AdminFlag.objects.only("on").get(key=key).on + import_metadata = self.is_import_metadata() if not import_metadata: self.log.warning("Admin flag set to NOT import tags.") return import_metadata diff --git a/codex/migrations/0060_backfill_metadata_imported_at.py b/codex/migrations/0060_backfill_metadata_imported_at.py new file mode 100644 index 000000000..756a71791 --- /dev/null +++ b/codex/migrations/0060_backfill_metadata_imported_at.py @@ -0,0 +1,44 @@ +""" +Mark comics scanned with tag import on as import-complete. + +Before this release, only a forced or lazy import stamped +``Comic.metadata_imported_at``. A library scan with the import-tags flag on +read every comic but stamped none, so a comic with no embedded metadata file +read as un-imported forever. Hovering its tag button re-imported it, purged +and rebuilt its cover and reloaded the browser. 0043 backfilled the rows that +existed then; this backfills the ones scanned since, with the same seed. + +Only when the import-tags flag is on: a scan with it off read no tags, and +its NULL rows are what the lazy importer is for. A missing flag row counts +as on, its default. +""" + +from django.db import migrations +from django.db.models.functions import Coalesce + +# AdminFlagChoices.IMPORT_METADATA, frozen at this migration's point in time. +_IMPORT_METADATA_KEY = "IM" + + +def _backfill_metadata_imported_at(apps, _schema_editor) -> None: + """Stamp un-stamped comics if library scans import tags.""" + admin_flag_model = apps.get_model("codex", "adminflag") + flag = admin_flag_model.objects.filter(key=_IMPORT_METADATA_KEY).first() + if flag is not None and not flag.on: + return + comic_model = apps.get_model("codex", "comic") + comic_model.objects.filter(metadata_imported_at__isnull=True).update( + metadata_imported_at=Coalesce("metadata_mtime", "updated_at") + ) + + +class Migration(migrations.Migration): + """Backfill Comic.metadata_imported_at for comics scanned since 0043.""" + + dependencies = [ + ("codex", "0059_series_filters_and_fts_summary"), + ] + + operations = [ + migrations.RunPython(_backfill_metadata_imported_at, migrations.RunPython.noop), + ] diff --git a/tests/test_lazy_import_marker.py b/tests/test_lazy_import_marker.py index 25cf7b68f..a34508818 100644 --- a/tests/test_lazy_import_marker.py +++ b/tests/test_lazy_import_marker.py @@ -4,35 +4,53 @@ ``Comic.metadata_mtime`` is comicbox's *embedded* metadata mtime, which stays NULL forever for an archive with no metadata file. The tag-icon hover gate and the lazy-import worker used to key off ``metadata_mtime``, so a no-metadata comic -re-fired the importer endlessly. ``metadata_imported_at`` records that a forced -import pass actually ran, independent of embedded metadata: +re-fired the importer endlessly. ``metadata_imported_at`` records that a +tag-reading import pass actually ran, independent of embedded metadata: -* ``FinishImporter.finish`` stamps every comic a *forced* import touched (by +* ``FinishImporter.finish`` stamps every comic a tag-reading import touched (by path, so it covers the SKIPPED no-metadata comics that never reach the - per-comic write path); a non-forced watcher import leaves the column alone. + per-comic write path). That is any forced import, and any library scan with + the import-tags flag on; a scan with the flag off reads no tags and leaves the + column NULL for the lazy importer. * ``LazyImporter.lazy_import`` gates on ``metadata_imported_at__isnull=True``, so a comic is imported once and then excluded from further hover-triggered passes. +* Migration 0060 stamps the rows scans left NULL before scans stamped them. """ +import importlib import shutil +import zipfile from pathlib import Path from threading import Event, Lock from typing import Final, override -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch +from django.apps import apps from django.core.cache import cache from django.test import TestCase from loguru import logger +from codex.choices.admin import AdminFlagChoices from codex.librarian.scribe.importer.importer import ComicImporter from codex.librarian.scribe.importer.tasks import ImportTask from codex.librarian.scribe.lazy_importer import LazyImporter from codex.librarian.scribe.tasks import LazyImportComicsTask from codex.models import Comic, Imprint, Library, Publisher, Series, Volume +from codex.models.admin import AdminFlag from tests.tmp_dirs import tmp_dir +_MIGRATION = importlib.import_module( + "codex.migrations.0060_backfill_metadata_imported_at" +) +_backfill = _MIGRATION._backfill_metadata_imported_at # noqa: SLF001 + _LIB_PATH: Final = tmp_dir(Path(__file__).stem) _COMIC_PATH: Final = str(_LIB_PATH / "bare.cbz") +_SCAN_LIB_PATH: Final = tmp_dir(f"{Path(__file__).stem}-scan") + + +def _set_import_metadata_flag(*, on: bool) -> None: + AdminFlag.objects.filter(key=AdminFlagChoices.IMPORT_METADATA.value).update(on=on) class LazyImportMarkerTestCase(TestCase): @@ -98,12 +116,72 @@ def test_forced_finish_stamps_marker(self) -> None: self.comic.refresh_from_db() assert self.comic.metadata_imported_at is not None - def test_non_forced_finish_leaves_marker_null(self) -> None: - """An ordinary watcher import must not touch the marker.""" + def test_forced_finish_stamps_marker_with_import_tags_off(self) -> None: + """A forced pass reads tags whatever the scan flag says.""" + _set_import_metadata_flag(on=False) + self._make_importer(force=True).finish() + self.comic.refresh_from_db() + assert self.comic.metadata_imported_at is not None + + def test_scan_finish_stamps_marker_with_import_tags_on(self) -> None: + """A scan that read tags stamps the comic, so hovers never re-import it.""" + self._make_importer(force=False).finish() + self.comic.refresh_from_db() + assert self.comic.metadata_imported_at is not None + + def test_stamp_covers_every_batch(self) -> None: + """A path set larger than one batch still stamps every comic.""" + other_path = str(_LIB_PATH / "other.cbz") + Path(other_path).write_bytes(b"") + other = Comic.objects.create( + library=self.library, + path=other_path, + issue_number=2, + name="other", + publisher=self.comic.publisher, + imprint=self.comic.imprint, + series=self.comic.series, + volume=self.comic.volume, + size=1, + page_count=1, + ) + importer = self._make_importer(force=False) + importer.metadata_import_paths = frozenset({_COMIC_PATH, other_path}) + with patch( + "codex.librarian.scribe.importer.finish.IMPORTER_LINK_FK_BATCH_SIZE", 1 + ): + importer.finish() + for comic in (self.comic, other): + comic.refresh_from_db() + assert comic.metadata_imported_at is not None + + def test_scan_finish_leaves_marker_null_with_import_tags_off(self) -> None: + """A scan that read no tags leaves the comic for the lazy importer.""" + _set_import_metadata_flag(on=False) self._make_importer(force=False).finish() self.comic.refresh_from_db() assert self.comic.metadata_imported_at is None + def test_backfill_stamps_with_import_tags_on(self) -> None: + """0060 stamps the rows a tag-reading scan left NULL.""" + _backfill(apps, None) + self.comic.refresh_from_db() + assert self.comic.metadata_imported_at == self.comic.updated_at + + def test_backfill_stamps_with_no_flag_row(self) -> None: + """A missing flag row counts as its default, on.""" + AdminFlag.objects.filter(key=AdminFlagChoices.IMPORT_METADATA.value).delete() + _backfill(apps, None) + self.comic.refresh_from_db() + assert self.comic.metadata_imported_at is not None + + def test_backfill_skips_with_import_tags_off(self) -> None: + """0060 leaves the rows of a scan that read no tags to the lazy importer.""" + _set_import_metadata_flag(on=False) + _backfill(apps, None) + self.comic.refresh_from_db() + assert self.comic.metadata_imported_at is None + def test_stamp_is_idempotent(self) -> None: """A second forced pass does not move an already-set marker.""" self._make_importer(force=True).finish() @@ -136,3 +214,38 @@ def test_lazy_import_skips_already_marked(self) -> None: LazyImportComicsTask(collection="comics", pks=frozenset({self.comic.pk})) ) assert not self._enqueued_import_tasks(queue) + + +class ScanStampsTaglessComicTestCase(TestCase): + """Issue 992: a scanned comic with no tags file must not read as un-imported.""" + + @override + def setUp(self) -> None: + """Seed admin flags, a library and a CBZ with pages but no tags.""" + from codex.startup import init_admin_flags + + cache.clear() + init_admin_flags() + _SCAN_LIB_PATH.mkdir(exist_ok=True, parents=True) + self.path = str(_SCAN_LIB_PATH / "tagless.cbz") # pyright: ignore[reportUninitializedInstanceVariable] + with zipfile.ZipFile(self.path, "w") as zf: + zf.writestr("page1.jpg", b"\xff\xd8\xff\xe0not-a-real-jpeg") + self.library = Library.objects.create(path=str(_SCAN_LIB_PATH)) # pyright: ignore[reportUninitializedInstanceVariable] + + @override + def tearDown(self) -> None: + shutil.rmtree(_SCAN_LIB_PATH, ignore_errors=True) + cache.clear() + + def test_scan_stamps_tagless_comic(self) -> None: + """An ordinary scan creates the comic already stamped.""" + task = ImportTask( + library_id=self.library.pk, files_created=frozenset({self.path}) + ) + importer = ComicImporter(task, logger, MagicMock(), Lock(), Event()) + assert importer._run_per_comic_phases_chunked() # noqa: SLF001 + importer.finish() + + comic = Comic.objects.get(path=self.path) + assert comic.metadata_mtime is None + assert comic.metadata_imported_at is not None From 12739a99af5bbdbd9e69512d37bce5e6c3019099 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Wed, 21 Oct 2026 19:49:12 -0700 Subject: [PATCH 2/2] fix: skip the row write for a re-import that changes nothing (#992) A forced or lazy import of an unchanged comic, typically one with no embedded tags, still rewrote the row. The query phase kept it in UPDATE_COMICS with an empty diff "to set stat", so update_comics wrote the six always-update fields, purged and rebuilt the cover and bumped updated_at. TimestampUpdater then restamped its collections and finish broadcast LIBRARY_CHANGED, reloading every open browser. update_comics now skips a comic when the update would write nothing new: - no scalar diff and no FK link change remain after the query phase's pruning, so Force Update still relinks a wrong publisher; - no m2m link to create or delete, so a stray genre still gets the row write that re-syncs its search entry and timestamp; - no TagWriter series_attrs ride along; - presave derives the same stat, size, date, decade and age-rating index from the file on disk, so a replaced file with the same tags still updates and gets a new cover; - the chunk's update_all_fks changed no rows. FK bulk updates don't write updated_at, and the FK search re-sync only runs beside per-comic FTS entries. A corrected tag or collection row reaches the browser and the index only through the comics the import rewrites, so such a chunk keeps the old full update. With nothing written, counts stay zero, so finish neither clears caches nor broadcasts. The metadata_imported_at stamp is path-scoped and still lands. An import that rewrote no comics left the empty FTS inputs behind, because an operate with no entries returns before consuming them; import_search_index now drains them on exit. Co-Authored-By: Claude Opus 5.5 --- NEWS.md | 4 +- .../scribe/importer/create/comics.py | 61 +++++- .../scribe/importer/create/foreign_keys.py | 1 + codex/librarian/scribe/importer/init.py | 5 + .../scribe/importer/query/update_comics.py | 3 +- .../scribe/importer/search/update.py | 6 +- tests/importer/test_noop_update.py | 198 ++++++++++++++++++ tests/importer/test_update_none.py | 8 +- 8 files changed, 275 insertions(+), 11 deletions(-) create mode 100644 tests/importer/test_noop_update.py diff --git a/NEWS.md b/NEWS.md index 85e37db4b..89d69632f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -16,8 +16,8 @@ border-radius: 128px; - Online tag match picker shows series and issue details to help disambiguate similar covers and titles. - Fixes - - Browsing comics without embedded tags no longer re-imports them and - reloads the page. + - Browsing comics without embedded tags, or re-importing unchanged comics, + no longer rebuilds their covers and reloads the page. - Searches for a universe designation no longer miss imported comics. ## v2.5.5 diff --git a/codex/librarian/scribe/importer/create/comics.py b/codex/librarian/scribe/importer/create/comics.py index 80f2f93c6..f6aa34f13 100644 --- a/codex/librarian/scribe/importer/create/comics.py +++ b/codex/librarian/scribe/importer/create/comics.py @@ -12,9 +12,11 @@ BULK_UPDATE_COMIC_FIELDS_SET, COLLECTION_FIELD_NAMES, CREATE_COMICS, + DELETE_M2MS, FTS_CREATE, FTS_UPDATE, LINK_FKS, + LINK_M2MS, NON_FTS_FIELDS, PATH_FIELD_NAME, UPDATE_COMICS, @@ -33,6 +35,10 @@ if TYPE_CHECKING: from codex.models.collections import BrowserCollectionModel +# What ``presave`` derives from the file and the row. An update that would +# change none of them, the timestamp aside, writes nothing new. +_NOOP_CHECK_FIELDS = tuple(sorted(ALWAYS_UPDATE_COMIC_FIELDS - {"updated_at"})) + class CreateComicsImporter(CreateForeignKeyLinksImporter): """Create comics methods.""" @@ -66,10 +72,55 @@ def _record_moved_source_collections( ) self.moved_source_collections.setdefault(model, set()).add(old_pk) + def _is_noop_update( + self, + comic: Comic, + md: dict, + link_md: dict, + m2m_deleted_pks: frozenset[int], + ) -> bool: + """ + Return whether updating this comic would write nothing new. + + A forced or lazy import of an unchanged comic, typically one with no + embedded tags, used to rewrite the row anyway: that purged and rebuilt + its cover and broadcast LIBRARY_CHANGED, reloading every browser. The + query phase has already pruned every link that is already correct, so + no scalar, FK or m2m change remains. If ``presave`` then derives the + same stat and the same values from the file on disk, the row is + current. A chunk that changed tag or collection rows keeps every comic + (see ``chunk_fks_updated``). + """ + if ( + self.chunk_fks_updated + or md + or link_md + or comic.path in self.task.series_attrs + ): + return False + if ( + comic.path in self.metadata.get(LINK_M2MS, {}) + or comic.pk in m2m_deleted_pks + ): + return False + before = tuple(getattr(comic, field_name) for field_name in _NOOP_CHECK_FIELDS) + comic.presave() + return before == tuple( + getattr(comic, field_name) for field_name in _NOOP_CHECK_FIELDS + ) + def _update_comic_values( - self, comic: Comic, update_comics: list, comic_pks: list, changed_fields: set + self, + comic: Comic, + update_comics: list, + comic_pks: list, + changed_fields: set, + m2m_deleted_pks: frozenset[int], ) -> None: md = self.metadata[UPDATE_COMICS].pop(comic.pk, {}) + link_md = self.get_comic_fk_links(comic.pk, comic.path, for_create=False) + if self._is_noop_update(comic, md, link_md, m2m_deleted_pks): + return for field_name, value in md.items(): setattr(comic, field_name, value) changed_fields.update(md) @@ -82,7 +133,6 @@ def _update_comic_values( field_name: getattr(comic, f"{field_name}_id") for field_name in COLLECTION_FIELD_NAMES } - link_md = self.get_comic_fk_links(comic.pk, comic.path, for_create=False) changed_fields.update(link_md) for field_name, value in link_md.items(): set_value = value @@ -125,12 +175,17 @@ def update_comics(self) -> int: update_comics = [] comic_pks = [] changed_fields: set[str] = set() + m2m_deleted_pks = frozenset( + comic_pk + for rows in self.metadata.get(DELETE_M2MS, {}).values() + for comic_pk, _ in rows + ) for comic in comics: if self.abort_event.is_set(): return count try: self._update_comic_values( - comic, update_comics, comic_pks, changed_fields + comic, update_comics, comic_pks, changed_fields, m2m_deleted_pks ) except Exception: self.log.exception(f"Error preparing {comic} for update.") diff --git a/codex/librarian/scribe/importer/create/foreign_keys.py b/codex/librarian/scribe/importer/create/foreign_keys.py index 784ded9c6..512fbd0bf 100644 --- a/codex/librarian/scribe/importer/create/foreign_keys.py +++ b/codex/librarian/scribe/importer/create/foreign_keys.py @@ -309,6 +309,7 @@ def update_all_fks(self) -> int: ) count += self.bulk_update_all_models(update_status) finally: + self.chunk_fks_updated = bool(count) self.metadata.pop(UPDATE_FKS, None) self.status_controller.finish(update_status) return count diff --git a/codex/librarian/scribe/importer/init.py b/codex/librarian/scribe/importer/init.py index 9f820c1cd..25d8051b8 100644 --- a/codex/librarian/scribe/importer/init.py +++ b/codex/librarian/scribe/importer/init.py @@ -156,6 +156,11 @@ def __init__( self.moved_source_collections: dict[ type[BrowserCollectionModel | Reprint], set[int] ] = {} + # Whether this chunk's ``update_all_fks`` changed any rows. Those + # rows reach the browser's refresh gate and the search index only + # through the comics this import rewrites, so ``update_comics`` keeps + # every comic of such a chunk, changed or not. + self.chunk_fks_updated = False # Full set of paths this import touched, captured before the chunking # loop and extract phase zero out ``task.files_*``. Consumed at finish # to stamp ``Comic.metadata_imported_at`` on every comic a forced/lazy diff --git a/codex/librarian/scribe/importer/query/update_comics.py b/codex/librarian/scribe/importer/query/update_comics.py index 727f84431..d330682d2 100644 --- a/codex/librarian/scribe/importer/query/update_comics.py +++ b/codex/librarian/scribe/importer/query/update_comics.py @@ -33,7 +33,8 @@ def _query_update_comic(self, comic: Comic, status: Status) -> None: for key, value in proposed_comic_dict.items() if getattr(comic, key) != value } - # Update even if update_comic_dict is empty to set stat + # Keep it even if update_comic_dict is empty: the update phase still + # writes a changed stat, and skips the comic only if nothing changed. self.metadata[UPDATE_COMICS][comic.pk] = update_comic_dict status.increment_complete() self.status_controller.update(status) diff --git a/codex/librarian/scribe/importer/search/update.py b/codex/librarian/scribe/importer/search/update.py index ce653b922..7ba65991a 100644 --- a/codex/librarian/scribe/importer/search/update.py +++ b/codex/librarian/scribe/importer/search/update.py @@ -10,6 +10,7 @@ FTS_CREATE, FTS_EXISTING_M2MS, FTS_UPDATE, + FTS_UPDATED_M2MS, UNIVERSES_FIELD_NAME, ) from codex.librarian.scribe.importer.search.sync_m2m import ( @@ -270,7 +271,10 @@ def import_search_index(self, cleaned_count: int) -> None: if self.abort_event.is_set(): self.log.info("Search Index update aborted early.") self.abort_event.clear() - self.metadata.pop(FTS_EXISTING_M2MS, None) + # An operate with no entries returns before it consumes its + # inputs; an import that rewrote no comics leaves them empty. + for key in (FTS_CREATE, FTS_UPDATE, FTS_UPDATED_M2MS, FTS_EXISTING_M2MS): + self.metadata.pop(key, None) self.status_controller.finish_many( (ImporterFTSCreateStatus, ImporterFTSUpdateStatus) ) diff --git a/tests/importer/test_noop_update.py b/tests/importer/test_noop_update.py new file mode 100644 index 000000000..634e56846 --- /dev/null +++ b/tests/importer/test_noop_update.py @@ -0,0 +1,198 @@ +""" +Re-importing an unchanged comic writes nothing and reloads no browser. + +Issue 992: a lazy import of a comic with no embedded tags re-read the +archive, found nothing new, and still rewrote the row, purged and rebuilt +its cover and broadcast LIBRARY_CHANGED. Every open browser reloaded. The +update phase now skips a comic whose tags, links, derived values and file +stat are all current, while Force Update still repairs a wrong link. +""" + +import shutil +import zipfile +from pathlib import Path +from threading import Event, Lock +from typing import Final, override +from unittest.mock import MagicMock + +from django.core.cache import cache +from django.test import TestCase +from loguru import logger + +from codex.librarian.covers.tasks import CoverCreateTask +from codex.librarian.notifier.tasks import LIBRARY_CHANGED_TASK +from codex.librarian.scribe.importer.importer import ComicImporter +from codex.librarian.scribe.importer.tasks import ImportTask +from codex.models import Comic, Genre, Library, Publisher, Universe +from codex.startup import codex_init +from tests.tmp_dirs import tmp_dir + +TMP_DIR: Final = tmp_dir(Path(__file__).stem) +_PAGE: Final = b"\xff\xd8\xff\xe0not-a-real-jpeg" +_TAGS: Final = """comicbox: + publisher: + name: P + series: + name: S + genres: + Drama: {} + universes: + Alpha: + designation: '1' +""" + + +class TestNoopUpdate(TestCase): + """An unchanged comic skips the update; a changed one does not.""" + + @override + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cache.clear() + codex_init() + + @override + @classmethod + def tearDownClass(cls): + try: + shutil.rmtree(TMP_DIR, ignore_errors=True) + finally: + super().tearDownClass() + + @override + def setUp(self): + cache.clear() + TMP_DIR.mkdir(exist_ok=True, parents=True) + self.library = Library.objects.create(path=TMP_DIR) # pyright: ignore[reportUninitializedInstanceVariable] + + @override + def tearDown(self): + shutil.rmtree(TMP_DIR, ignore_errors=True) + cache.clear() + + @staticmethod + def _write(name: str, tags: str | None, pages: int = 1) -> str: + path = TMP_DIR / name + with zipfile.ZipFile(path, "w") as zf: + if tags is not None: + zf.writestr("comicbox.yaml", tags) + for page in range(pages): + zf.writestr(f"page{page}.jpg", _PAGE) + return str(path) + + def _import(self, task: ImportTask) -> MagicMock: + """Run every import phase and finish; return the librarian queue.""" + queue = MagicMock() + importer = ComicImporter(task, logger, queue, Lock(), Event()) + assert importer._run_per_comic_phases_chunked() # noqa: SLF001 + importer.fail_imports() + importer.delete() + importer.full_text_search() + importer.finish() + return queue + + def _scan(self, path: str) -> None: + self._import( + ImportTask(library_id=self.library.pk, files_created=frozenset({path})) + ) + + def _force(self, path: str, *, check_metadata_mtime: bool = False) -> MagicMock: + """Import the way Force Update (and, with the mtime check, Lazy) does.""" + return self._import( + ImportTask( + library_id=self.library.pk, + files_modified=frozenset({path}), + force_import_metadata=True, + check_metadata_mtime=check_metadata_mtime, + ) + ) + + @staticmethod + def _queued(queue: MagicMock) -> list: + return [call.args[0] for call in queue.put.call_args_list] + + def _assert_untouched(self, path: str, queue: MagicMock, updated_at) -> None: + queued = self._queued(queue) + assert LIBRARY_CHANGED_TASK not in queued, queued + assert not any(isinstance(task, CoverCreateTask) for task in queued), queued + assert Comic.objects.get(path=path).updated_at == updated_at + + def test_lazy_import_of_tagless_comic_is_silent(self) -> None: + """The reporter's case: hovering a comic with no tags file.""" + path = self._write("tagless.cbz", None) + self._scan(path) + updated_at = Comic.objects.get(path=path).updated_at + + queue = self._force(path, check_metadata_mtime=True) + + self._assert_untouched(path, queue, updated_at) + + def test_force_update_of_unchanged_comic_is_silent(self) -> None: + """Force Update re-reads tags that turn out the same.""" + path = self._write("tagged.cbz", _TAGS) + self._scan(path) + updated_at = Comic.objects.get(path=path).updated_at + + queue = self._force(path) + + self._assert_untouched(path, queue, updated_at) + + def test_changed_file_still_updates(self) -> None: + """New pages with the same tags still rewrite the row and the cover.""" + path = self._write("tagless.cbz", None) + self._scan(path) + updated_at = Comic.objects.get(path=path).updated_at + self._write("tagless.cbz", None, pages=2) + + queue = self._force(path) + + queued = self._queued(queue) + assert LIBRARY_CHANGED_TASK in queued + assert any(isinstance(task, CoverCreateTask) for task in queued), queued + comic = Comic.objects.get(path=path) + assert comic.updated_at > updated_at + assert comic.page_count == 2 # noqa: PLR2004 + + def test_force_update_repairs_a_wrong_fk_link(self) -> None: + """Force Update still relinks a comic whose file is unchanged.""" + path = self._write("tagged.cbz", _TAGS) + self._scan(path) + wrong = Publisher.objects.create(name="Wrong") + Comic.objects.filter(path=path).update(publisher=wrong) + updated_at = Comic.objects.get(path=path).updated_at + + queue = self._force(path) + + assert LIBRARY_CHANGED_TASK in self._queued(queue) + comic = Comic.objects.get(path=path) + assert comic.publisher.name == "P" + assert comic.updated_at > updated_at + + def test_force_update_repairs_a_stray_m2m_link(self) -> None: + """Force Update still unlinks a genre the file does not name.""" + path = self._write("tagged.cbz", _TAGS) + self._scan(path) + comic = Comic.objects.get(path=path) + comic.genres.add(Genre.objects.create(name="Stray")) + + queue = self._force(path) + + assert LIBRARY_CHANGED_TASK in self._queued(queue) + names = set(comic.genres.values_list("name", flat=True)) + assert names == {"Drama"}, names + # The row write is what re-syncs its search entry and timestamp. + assert Comic.objects.get(path=path).updated_at > comic.updated_at + + def test_force_update_of_a_tag_row_keeps_the_comic(self) -> None: + """A corrected tag row reaches the browser through its comic's rewrite.""" + path = self._write("tagged.cbz", _TAGS) + self._scan(path) + Universe.objects.filter(name="Alpha").update(designation="9") + updated_at = Comic.objects.get(path=path).updated_at + + queue = self._force(path) + + assert LIBRARY_CHANGED_TASK in self._queued(queue) + assert Universe.objects.get(name="Alpha").designation == "1" + assert Comic.objects.get(path=path).updated_at > updated_at diff --git a/tests/importer/test_update_none.py b/tests/importer/test_update_none.py index b0d67a01c..69df491fe 100644 --- a/tests/importer/test_update_none.py +++ b/tests/importer/test_update_none.py @@ -69,7 +69,7 @@ DELETE_M2MS: {}, FIS: {}, FTS_EXISTING_M2MS: {}, - FTS_UPDATE: {1: {}}, + FTS_UPDATE: {}, FTS_UPDATED_M2MS: {}, LINK_M2MS: {}, } @@ -78,21 +78,21 @@ { FIS: {}, FTS_EXISTING_M2MS: {}, - FTS_UPDATE: {1: {}}, + FTS_UPDATE: {}, FTS_UPDATED_M2MS: {}, } ) FAILED_IMPORTS_UPDATE_NONE = MappingProxyType( { FTS_EXISTING_M2MS: {}, - FTS_UPDATE: {1: {}}, + FTS_UPDATE: {}, FTS_UPDATED_M2MS: {}, } ) DELETED_COMICS_UPDATE_NONE = MappingProxyType( { FTS_EXISTING_M2MS: {}, - FTS_UPDATE: {1: {}}, + FTS_UPDATE: {}, FTS_UPDATED_M2MS: {}, } )