diff --git a/NEWS.md b/NEWS.md index 3cd1de184..8bcafbc67 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,6 +10,7 @@ border-radius: 128px; - Features - Series years, status and summary in the metadata pane. + - Online tagging saves series years, status and summary. - Browser title shows series years and a Tags button. - Tag editor edits the MetronInfo series start year. diff --git a/codex/librarian/scribe/importer/read/aggregate_path.py b/codex/librarian/scribe/importer/read/aggregate_path.py index 031697057..4cb63c4c6 100644 --- a/codex/librarian/scribe/importer/read/aggregate_path.py +++ b/codex/librarian/scribe/importer/read/aggregate_path.py @@ -123,6 +123,9 @@ def _aggregate_path(self, path, status) -> None: """Aggregate metadata for one path.""" # Prepare md = self.metadata[EXTRACTED].pop(path) + if series_attrs := self.task.series_attrs.get(path): + # A tag write's values the file couldn't hold. See ImportTask. + md[SERIES_KEY] = {**(md.get(SERIES_KEY) or {}), **series_attrs} self._transform_metadata(md) # Aggregate diff --git a/codex/librarian/scribe/importer/tasks.py b/codex/librarian/scribe/importer/tasks.py index c8e6a8bc9..b52b8a807 100644 --- a/codex/librarian/scribe/importer/tasks.py +++ b/codex/librarian/scribe/importer/tasks.py @@ -2,6 +2,7 @@ from collections.abc import Mapping from dataclasses import dataclass, field +from typing import Any from codex.librarian.scribe.tasks import ScribeTask @@ -31,6 +32,14 @@ class ImportTask(ScribeTask): force_import_metadata: bool = False check_metadata_mtime: bool = True + # path -> series attributes laid over the series that file names. + # + # Set only by the TagWriter's re-read, for values a tag write was + # given but no format it writes can hold. Applying them during the + # import, rather than to the comic's series row beforehand, lands them + # on the row the comic ends up in: a write that names a new publisher + # or series moves the comic to a different row. + series_attrs: Mapping[str, Mapping[str, Any]] = field(default_factory=dict) # Keep vanished rows instead of deleting them. # # Set by the two filesystem scanners, the poller and the watcher, diff --git a/codex/librarian/scribe/tag_writer.py b/codex/librarian/scribe/tag_writer.py index 3c66a69fe..6b99eaf72 100644 --- a/codex/librarian/scribe/tag_writer.py +++ b/codex/librarian/scribe/tag_writer.py @@ -35,6 +35,7 @@ predict_name, will_convert, ) +from codex.librarian.scribe.tagwrite_series_attrs import patch_series_attrs from codex.librarian.scribe.timestamp_update import TimestampUpdater from codex.librarian.worker import WorkerStatusAbortableBase from codex.models.comic import Comic @@ -297,7 +298,7 @@ def write_tags(self, task: BulkTagWriteTask) -> None: ) self._enqueue_rereads( - current_paths, written_paths, converted_paths, post_renamed, lib_of + task, current_paths, written_paths, converted_paths, post_renamed, lib_of ) num_renamed = len(renamed_paths) + len(post_renamed) reason = ( @@ -675,6 +676,7 @@ def _sync_conversions( def _enqueue_rereads( self, + task: BulkTagWriteTask, current_paths: dict[int, Path], written_paths: dict[int, Path], converted_paths: dict[int, Path], @@ -690,19 +692,29 @@ def _enqueue_rereads( eventually re-read from the watcher's own modify event, but only with the import-metadata flag on, so the re-read is requested either way. + + Each comic's patch may carry series attributes the written formats + can't hold. They ride along for the importer to apply (see + ``tagwrite_series_attrs``). """ modified: defaultdict[int, set[str]] = defaultdict(set) created: defaultdict[int, set[str]] = defaultdict(set) + series_attrs: defaultdict[int, dict[str, dict]] = defaultdict(dict) for pk, written_path in written_paths.items(): library_id = lib_of[pk] if pk in converted_paths: - modified[library_id].add(str(converted_paths[pk])) + path = str(converted_paths[pk]) + modified[library_id].add(path) elif written_path != current_paths[pk]: # A kept original's new CBZ is a file the database has # never seen. - created[library_id].add(str(post_renamed.get(pk, written_path))) + path = str(post_renamed.get(pk, written_path)) + created[library_id].add(path) else: - modified[library_id].add(str(written_path)) + path = str(written_path) + modified[library_id].add(path) + if attrs := patch_series_attrs(self._patch_for(task, pk)): + series_attrs[library_id][path] = attrs # A rename with no write changed no metadata, so nothing to re-read. for library_id in modified.keys() | created.keys(): import_task = ImportTask( @@ -711,5 +723,6 @@ def _enqueue_rereads( files_created=frozenset(created.get(library_id, ())), force_import_metadata=True, check_metadata_mtime=False, + series_attrs=series_attrs.get(library_id, {}), ) self.librarian_queue.put(import_task) diff --git a/codex/librarian/scribe/tagwrite_series_attrs.py b/codex/librarian/scribe/tagwrite_series_attrs.py new file mode 100644 index 000000000..fbe693f13 --- /dev/null +++ b/codex/librarian/scribe/tagwrite_series_attrs.py @@ -0,0 +1,57 @@ +""" +Series attributes a tag write saves to the database. + +An online match brings the series' years, status and summary, but no +format codex writes holds the end year, the status or the summary, and +ComicInfo has no start year either. The re-read that follows a write +only sees the file, so on their own these values would be lost. + +The tag writer hands them to that re-read instead, and the importer lays +them over the series the file names. That puts them on the row the comic +lands on, which is a different row whenever the write names a new +publisher or series. The importer's batch rules settle comics of one +series that disagree: the earliest start year, the latest end year and +the first comic's status and summary. A later write's values replace a +stored row's. Empty values are left out, so a write never clears one. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, Final + +from comicbox.enums.comicbox import SeriesStatusEnum +from comicbox.formats.comicbox.schema import SERIES_KEY + +from codex.librarian.scribe.importer.const import ( + END_YEAR_FIELD_NAME, + START_YEAR_FIELD_NAME, + STATUS_FIELD_NAME, + SUMMARY_FIELD_NAME, +) + +if TYPE_CHECKING: + from collections.abc import Mapping + +_SERIES_ATTR_KEYS: Final = ( + START_YEAR_FIELD_NAME, + END_YEAR_FIELD_NAME, + STATUS_FIELD_NAME, + SUMMARY_FIELD_NAME, +) + + +def _status_value(status: Any) -> str | None: + """Return a status as ``SeriesStatusChoices`` stores it, or None.""" + try: + return SeriesStatusEnum(status).value + except ValueError: + return None + + +def patch_series_attrs(patch: Mapping[str, Any] | None) -> dict[str, Any]: + """Return the non-empty series attributes a tag write patch carries.""" + series = (patch or {}).get(SERIES_KEY) or {} + attrs = {key: series.get(key) for key in _SERIES_ATTR_KEYS} + if attrs[STATUS_FIELD_NAME] is not None: + attrs[STATUS_FIELD_NAME] = _status_value(attrs[STATUS_FIELD_NAME]) + return {key: value for key, value in attrs.items() if value not in (None, "")} diff --git a/tests/test_onlinetag_series_attrs.py b/tests/test_onlinetag_series_attrs.py new file mode 100644 index 000000000..222c6dd96 --- /dev/null +++ b/tests/test_onlinetag_series_attrs.py @@ -0,0 +1,231 @@ +""" +Online tagging keeps the series attributes no written format can hold. + +A Metron match brings the series' end year, status and summary, and +ComicInfo has no start year either. The tag write can't put them in the +file, so they ride on the re-read and the importer applies them to the +series the comic lands on. Here the match also names a new publisher and +series, which moves the comic off its filename-derived series row: the +values have to reach the new row, not the one the comic left. +""" + +import shutil +import threading +import zipfile +from pathlib import Path +from threading import Event, Lock +from types import SimpleNamespace +from typing import Any, Final, override +from unittest.mock import MagicMock + +from comicbox.enums.comicbox import SeriesStatusEnum +from django.core.cache import cache +from django.test import SimpleTestCase, TestCase +from loguru import logger + +from codex.librarian.mp_queue import LIBRARIAN_QUEUE +from codex.librarian.onlinetag.tag_pass_runner import TagPassRunner +from codex.librarian.scribe.importer.importer import ComicImporter +from codex.librarian.scribe.importer.tasks import ImportTask +from codex.librarian.scribe.tag_writer import TagWriter +from codex.librarian.scribe.tagwrite_series_attrs import patch_series_attrs +from codex.librarian.scribe.tasks import BulkTagWriteTask +from codex.models import Comic, Library, Series +from codex.startup import codex_init +from tests.importer.test_basic import run_full_import +from tests.onlinetag_session_fakes import FakeQueue, double +from tests.tmp_dirs import tmp_dir + +_TMP_DIR: Final = tmp_dir("codex.tests.onlinetag.series_attrs") +_FILENAME_SERIES: Final = "Spidey" +_SERIES: Final = "The Amazing Spider-Man" +_START_YEAR: Final = 1963 +_END_YEAR: Final = 1998 +_STATUS: Final = "Completed" +_SUMMARY: Final = "The first Spider-Man series." +#: The tags a Metron match hands back, trimmed to what this test reads. +_MATCH_TAGS: Final = { + "publisher": {"name": "Marvel"}, + "series": { + "name": _SERIES, + "start_year": _START_YEAR, + "end_year": _END_YEAR, + "status": _STATUS, + "summary": _SUMMARY, + }, + "issue": {"number": "1"}, +} + + +class _NoopStatusController: + def start(self, _status, **_kwargs) -> None: + pass + + def update(self, _status, **_kwargs) -> None: + pass + + def finish(self, _status, **_kwargs) -> None: + pass + + +class _MatchedSession: + """Matches every comic to ``_MATCH_TAGS``.""" + + def tag_many(self, paths): + for path in paths: + yield SimpleNamespace( + path=path, matched=True, tags=_MATCH_TAGS, error=None, cancelled=False + ) + + +def _untagged_cbz(path: Path) -> None: + """Build an archive with no metadata, so its series comes from its name.""" + with zipfile.ZipFile(path, "w") as zf: + zf.writestr("page1.jpg", b"\xff\xd8\xff\xe0not-a-real-jpeg") + + +def _make_writer(queue: FakeQueue) -> TagWriter: + """Build a TagWriter without the threading machinery.""" + writer = TagWriter.__new__(TagWriter) + writer.log = double(logger) + writer.librarian_queue = double(queue) + writer.status_controller = double(MagicMock()) + writer.abort_event = threading.Event() + return writer + + +def _queued(queue: FakeQueue, task_type: type) -> list[Any]: + return [item for item in queue.items if isinstance(item, task_type)] + + +class PatchSeriesAttrsTestCase(SimpleTestCase): + """Only non-empty attributes leave the patch, status as its choice value.""" + + def test_picks_the_attrs(self) -> None: + assert patch_series_attrs(_MATCH_TAGS) == { + "start_year": _START_YEAR, + "end_year": _END_YEAR, + "status": _STATUS, + "summary": _SUMMARY, + } + + def test_status_enum_member_stores_its_value(self) -> None: + patch = {"series": {"status": SeriesStatusEnum.HIATUS}} + assert patch_series_attrs(patch) == {"status": "Hiatus"} + + def test_drops_empty_and_unknown_values(self) -> None: + patch = {"series": {"end_year": None, "status": "Paused", "summary": ""}} + assert patch_series_attrs(patch) == {} + + def test_no_series(self) -> None: + assert patch_series_attrs({"issue": {"number": "1"}}) == {} + assert patch_series_attrs(None) == {} + + +class OnlineTagSeriesAttrsTestCase(TestCase): + """An online match's series attributes survive its write and re-read.""" + + @override + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cache.clear() + codex_init() + + @override + def setUp(self) -> None: + cache.clear() + _TMP_DIR.mkdir(exist_ok=True, parents=True) + self.path = _TMP_DIR / f"{_FILENAME_SERIES} 001.cbz" # pyright: ignore[reportUninitializedInstanceVariable] + _untagged_cbz(self.path) + self.library = Library.objects.create(path=str(_TMP_DIR), events=False) # pyright: ignore[reportUninitializedInstanceVariable] + self._import( + ImportTask(self.library.pk, files_created=frozenset({str(self.path)})) + ) + self.comic = Comic.objects.select_related("series").get(path=str(self.path)) # pyright: ignore[reportUninitializedInstanceVariable] + + @override + def tearDown(self) -> None: + shutil.rmtree(_TMP_DIR, ignore_errors=True) + + @staticmethod + def _import(task: ImportTask) -> None: + run_full_import(ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event())) + + def _online_tag(self) -> BulkTagWriteTask: + """Run a pass over the comic and return the write it flushed.""" + queue = FakeQueue() + runner = TagPassRunner( + double(logger), + double(queue), + double(_NoopStatusController()), + lambda _state: None, + lambda _state: None, + ) + state = double( + SimpleNamespace( + session=_MatchedSession(), + cancelled=False, + pending_paths=[], + total_comics=0, + completed_comics=0, + path_to_pk={self.path: self.comic.pk}, + collected_tags={}, + effort="balanced", + sources=("metron",), + merge_all_sources=False, + mode="update", + formats=("COMIC_INFO",), + delete_original=False, + rename=False, + ) + ) + runner.collect_results(state, [self.path], flush_writes=True) + (write_task,) = _queued(queue, BulkTagWriteTask) + return write_task + + def _write_and_reread(self, write_task: BulkTagWriteTask) -> ImportTask: + """Write the tags and run the re-read the writer queued.""" + queue = FakeQueue() + _make_writer(queue).write_tags(write_task) + (import_task,) = _queued(queue, ImportTask) + self._import(import_task) + return import_task + + def test_series_attrs_land_on_the_matched_series(self) -> None: + assert self.comic.series.name == _FILENAME_SERIES + old_series_pk = self.comic.series.pk + + import_task = self._write_and_reread(self._online_tag()) + + assert import_task.series_attrs == { + str(self.path): patch_series_attrs(_MATCH_TAGS) + } + self.comic.refresh_from_db() + series = self.comic.series + assert series.pk != old_series_pk + assert series.name == _SERIES + assert series.publisher.name == "Marvel" + # ComicInfo holds none of them, the start year included. + assert series.start_year == _START_YEAR + assert series.end_year == _END_YEAR + assert series.status == _STATUS + assert series.summary == _SUMMARY + + def test_a_file_only_reimport_keeps_them(self) -> None: + self._write_and_reread(self._online_tag()) + + # The watcher's re-read of the same file carries no attributes. + self._import( + ImportTask( + self.library.pk, + files_modified=frozenset({str(self.path)}), + force_import_metadata=True, + check_metadata_mtime=False, + ) + ) + + series = Series.objects.get(name=_SERIES) + assert series.end_year == _END_YEAR + assert series.status == _STATUS + assert series.summary == _SUMMARY