diff --git a/NEWS.md b/NEWS.md index 7dbae0719..89d69632f 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, 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/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/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/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/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/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/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: {}, } ) 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