diff --git a/Dockerfile b/Dockerfile index 071b1782d..351c07927 100644 --- a/Dockerfile +++ b/Dockerfile @@ -134,8 +134,11 @@ RUN mkdir -p /home/abc/.config/comicbox \ COPY --from=wheel-installer /opt/codex /opt/codex ENV PATH="/opt/codex/bin:${PATH}" -# Fail the build, not the container start, on a broken venv. -RUN python -B -c "import django, comicbox, pymupdf, PIL.Image, cryptography, granian, rapidfuzz" +# Fail the build, not the container start, on a broken venv. comicbox's +# doctor proves unrar extracts a RAR member, pymupdf loads and every pin is +# satisfied; HOME keeps the config dir it creates out of the image. +RUN python -B -c "import django, cryptography, granian, rapidfuzz" \ + && HOME=/tmp comicbox doctor -q VOLUME /comics VOLUME /config diff --git a/NEWS.md b/NEWS.md index 2acb1b6a0..159343399 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,10 +8,26 @@ border-radius: 128px; ## v2.5.3 -- Fixes +- Features + - Online tagging gains an Ask match mode that prompts for every match + instead of writing any. + - Admin Doctor tab: archive tools, PDF support, image codecs, database, + libraries, watcher limits, config and package problems, each with its fix. + Problems are also logged at startup. +- Fixes + - A missing or broken unrar no longer hides CBR comics from scans or deletes + them from the library; they are listed as failed imports with the cause. + - A broken PDF library disables PDF reading instead of stopping the server. + - Metron logins by username and password tag again; a blank API key no + longer overrides them. + - Credential checks no longer compete with a running tagging job for + Metron's rate limit. + - Re-applying a chosen online match reuses the cached search. - Failed imports no longer vanish from the admin list after other imports. - Size searches accept kb, mb, gb and tb units and the >= and <= operators. - Codex shuts down cleanly on Python 3.12 and 3.13. +- Performance + - Docker image is about 165 MB smaller. ## v2.5.2 diff --git a/README.md b/README.md index 4b186ee65..d382facab 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,8 @@ apk add bsd-compat-headers build-base jpeg-dev libffi-dev libwebp openssl-dev sq ##### Install unrar Runtime Dependency on non-debian Linux -Codex requires unrar to read CBR formatted comic archives. Unrar is often not +Codex requires [RARLAB's unrar](https://www.rarlab.com) to read CBR comic +archives; the bsdtar and 7-Zip fallbacks cannot extract them. Unrar is often not packaged for Linux, but here are some instructions: [How to install unrar in Linux](https://www.unixtutorial.org/how-to-install-unrar-in-linux/) @@ -1106,6 +1107,28 @@ codex like: LOGLEVEL=DEBUG codex ``` +### 🩺 Doctor + +The Admin panel's Doctor tab reports whether this install can read each comic +archive format, has the image codecs cover matching needs, whether comicbox's +config and package versions are in order, and whether the database, config +directory, library folders, filesystem watcher and stored credentials are +usable, with a fix for each problem. Codex logs the problems when it starts, +too. + +The same report runs from the command line, which helps when Codex will not +start. In Codex's Python environment: + +```sh +comicbox doctor +``` + +In Docker: + +```sh +docker exec codex comicbox doctor +``` + ### Watching Filesystem Events with Docker Codex tries to watch for filesystem events to instantly update your comic diff --git a/bun.lock b/bun.lock index 7ab766c57..f473874ea 100644 --- a/bun.lock +++ b/bun.lock @@ -293,7 +293,7 @@ "domutils": ["domutils@3.2.2", "", { "dependencies": { "dom-serializer": "^2.0.0", "domelementtype": "^2.3.0", "domhandler": "^5.0.3" } }, "sha512-6kZKyUajlDuqlHKVX1w7gyslj9MPIXzIFiz/rGu35uC1wMi+kMhQwGhl4lt9unC9Vb9INnY9Z3/ZA3+FhASLaw=="], - "electron-to-chromium": ["electron-to-chromium@1.5.447", "", {}, "sha512-vhYiRwTPq7tvwplP6fOFVRJxfZifavlr6tK8FD1EO5RxRuvZ9qdQM0lr1mvNBeDe1s2XrfL3n5BBwvigXJ7KuA=="], + "electron-to-chromium": ["electron-to-chromium@1.5.448", "", {}, "sha512-c3DEaf2iJbLz7XvmYGZQrzaxy6Azt2/NcWAV1r3GkibEyRNkVer6t3JevvwAQ6gW2JBGGY2N/MGMS/QV+77Ldw=="], "empathic": ["empathic@2.1.0", "", {}, "sha512-AnfC1ATldl49/cvZdLPDjBfrRNwbDO05aibiOtzQu3qtlbJtomNLhF30HEtn/7iBz50dlMECqATo3fG0LrdEgw=="], diff --git a/codex/choices/admin.py b/codex/choices/admin.py index a9596e1b7..3d955a5b5 100644 --- a/codex/choices/admin.py +++ b/codex/choices/admin.py @@ -2,7 +2,7 @@ from types import MappingProxyType -from comicbox.config.online import Effort +from comicbox.config.online import Effort, MatchMode from comicbox.formats.base.online import SOURCE_NAMES from django.db.models.enums import TextChoices @@ -50,12 +50,10 @@ class AdminFlagChoices(TextChoices): TAGGING_CHOICES = MappingProxyType( { + # comicbox's match modes, ask through eager, derived so the two + # cannot drift. Ask prompts for every match instead of writing. "matchMode": MappingProxyType( - { - "careful": "Careful", - "auto": "Auto", - "eager": "Eager", - } + {mode.value: mode.value.capitalize() for mode in MatchMode} ), "promptsMode": MappingProxyType( { diff --git a/codex/doctor/__init__.py b/codex/doctor/__init__.py new file mode 100644 index 000000000..2aa26d784 --- /dev/null +++ b/codex/doctor/__init__.py @@ -0,0 +1,108 @@ +""" +The doctor: comicbox's health checks plus codex's own, for this install. + +comicbox 5.3.0's ``run_checks`` reports whether this host can read each +archive format, whether Pillow has the codecs cover matching needs, whether +the user's comicbox config parses, and whether comicbox's package pins are +satisfied, with a one-line fix for anything that is not. Codex adds the +checks for what it needs on top (``codex.doctor.checks``), shows the rows +to admins under two headings, and logs the problems once at startup, so a +missing unrar or a full disk is explained in one place rather than +discovered as failed imports with no cause. + +comicbox's Online section is left out. Codex keeps the tagging credentials +in its own database and checks them on the Tagging tab, so the doctor's "no +credentials" rows and its ``--online`` advice would describe the CLI, not +this server. + +comicbox caches its tool probes for the life of the process, so a tool +installed while codex runs shows up after a restart. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING + +from comicbox.doctor import CheckResult, Status, run_checks +from comicbox.doctor.online import SECTION as _ONLINE_SECTION + +from codex.doctor.checks import CHECKS, SECTION + +if TYPE_CHECKING: + from loguru import Logger + + from codex.doctor.checks import Check + + +@dataclass(frozen=True, slots=True) +class DoctorReport: + """comicbox's rows and codex's own, with the host they ran on.""" + + #: comicbox version, Python, platform: information only. + header: tuple[str, ...] + comicbox: tuple[CheckResult, ...] + codex: tuple[CheckResult, ...] + + @property + def results(self) -> tuple[CheckResult, ...]: + """Every row, comicbox's first.""" + return (*self.comicbox, *self.codex) + + @property + def problems(self) -> int: + """How many rows say something codex needs is broken.""" + return sum(row.status.is_failure for row in self.results) + + +def _run_check(name: str, check: Check) -> list[CheckResult]: + """Run one codex check; a crash becomes one ERROR row and hides nothing else.""" + rows: list[CheckResult] = [] + try: + rows.extend(check()) + except Exception as exc: # reported as the row + rows.append( + CheckResult( + SECTION, name, Status.ERROR, detail=f"{type(exc).__name__}: {exc}" + ) + ) + return rows + + +def run_doctor() -> DoctorReport: + """Run comicbox's doctor and codex's checks; keep the rows that describe this server.""" + report = run_checks() + comicbox_rows = tuple( + row for row in report.results if row.section != _ONLINE_SECTION + ) + codex_rows = tuple(row for name, check in CHECKS for row in _run_check(name, check)) + return DoctorReport(header=report.header, comicbox=comicbox_rows, codex=codex_rows) + + +def _describe(row: CheckResult) -> str: + """One log line for a row: what, how bad, why, and the fix if there is one.""" + line = f"doctor: {row.name} {row.status}: {row.detail}" + if row.fix: + line += f" Fix: {row.fix}" + return line + + +def log_doctor_problems(log: Logger) -> None: + """ + Log what the doctor found, once, after logging is configured. + + Failures warn and warnings inform. The scanner used to warn about an + unreadable archive format at import time, before any log sink existed, + so the message only ever reached stderr. + """ + report = run_doctor() + for row in report.results: + if row.status.is_failure: + log.warning(_describe(row)) + elif row.status is Status.WARN: + log.info(_describe(row)) + if problems := report.problems: + noun = "problem" if problems == 1 else "problems" + log.warning(f"The doctor found {problems} {noun}. See the Admin Doctor tab.") + else: + log.debug("The doctor found no problems.") diff --git a/codex/doctor/checks.py b/codex/doctor/checks.py new file mode 100644 index 000000000..40603e552 --- /dev/null +++ b/codex/doctor/checks.py @@ -0,0 +1,303 @@ +""" +What codex itself needs, as rows of the doctor report. + +comicbox's doctor covers what comicbox needs. These rows cover the rest: +the database engine, the config directory, each library root, the +filesystem watcher's kernel limits, and the key that decrypts the stored +tagging credentials. Each is a comicbox ``CheckResult`` so the admin page +and the startup log treat them exactly like comicbox's own. + +Paths appear here because this is an admin-only page. Nothing from these +rows is ever sent anywhere; the telemeter has its own, count-only report. +""" + +from __future__ import annotations + +import os +import shutil +import sqlite3 +import sys +from functools import partial +from pathlib import Path +from typing import TYPE_CHECKING + +from comicbox.doctor import CheckResult, Status +from cryptography.fernet import Fernet, InvalidToken +from django.conf import settings +from django.db import connection +from django.db.models import TextField +from django.db.models.functions import Cast +from django.db.utils import OperationalError +from django.template.defaultfilters import filesizeformat + +from codex.librarian.fs.mounted import DOCKER_UNMOUNTED_FN +from codex.models.admin import ComicboxTaggingDefaults +from codex.models.collections import Folder +from codex.models.fields import EncryptedCharField +from codex.models.library import Library +from codex.settings import CONFIG_PATH +from codex.util import is_docker + +if TYPE_CHECKING: + from collections.abc import Callable, Iterable, Iterator + +SECTION = "Codex" + +#: (check name, check). The name labels the ERROR row the runner reports +#: when the check itself crashes. +type Check = Callable[[], Iterable[CheckResult]] + +_row = partial(CheckResult, SECTION) + +# Below this much free space the config dir row warns: the database, its +# nightly backups, the logs and the cover cache all grow here. +_LOW_DISK_BYTES = 1 << 30 +_PERMISSIONS_FIX = "fix its permissions (Docker: PUID and PGID)" + +# The kernel's inotify limits, which the event watcher needs one watch per +# folder against. Only Linux has them; macOS and the BSDs watch by other +# means. Per real user across every process, so other watchers share them. +_INOTIFY_DIR = Path("/proc/sys/fs/inotify") +_HAS_INOTIFY = sys.platform == "linux" +# Warn before the limit is reached: other processes draw on the same count. +_WATCH_HEADROOM = 0.8 +_WATCHES_SUGGESTED_MIN = 524_288 + + +def check_database() -> Iterator[CheckResult]: + """SQLite's version, the FTS5 engine search indexes with, and the journal mode.""" + found = f"sqlite {sqlite3.sqlite_version}" + with connection.cursor() as cursor: + cursor.execute("PRAGMA journal_mode") + mode_row = cursor.fetchone() + journal_mode = str(mode_row[0] if mode_row else "").lower() + try: + # A temp table proves the engine is there; compile options are + # not recorded in every build. + cursor.execute( + "CREATE VIRTUAL TABLE temp.codex_doctor_fts USING fts5(probe)" + ) + cursor.execute("DROP TABLE temp.codex_doctor_fts") + except OperationalError as exc: + yield _row( + "database", + Status.MISSING, + found=found, + detail=f"no FTS5: comics cannot be search indexed ({exc})", + fix="use a Python whose sqlite3 was built with FTS5", + ) + return + if journal_mode != "wal": + yield _row( + "database", + Status.WARN, + found=found, + detail=f"fts5 · journal mode is {journal_mode}, not wal: readers block writers", + fix="keep the config dir on a local filesystem, not a network share", + ) + return + yield _row("database", Status.OK, found=found, detail=f"fts5 · {journal_mode}") + + +def check_config_dir() -> Iterator[CheckResult]: + """Check the config dir is writable with room: the database, backups, logs and covers live here.""" + if not os.access(CONFIG_PATH, os.W_OK): + yield _row( + "config dir", + Status.MISCONFIGURED, + found=CONFIG_PATH, + detail="not writable: the database, logs, backups and covers live here", + fix=_PERMISSIONS_FIX, + ) + return + free = shutil.disk_usage(CONFIG_PATH).free + detail = f"writable · {filesizeformat(free)} free" + if free < _LOW_DISK_BYTES: + yield _row( + "config dir", + Status.WARN, + found=CONFIG_PATH, + detail=detail, + fix="free some space: backups, logs and the cover cache grow here", + ) + return + yield _row("config dir", Status.OK, found=CONFIG_PATH, detail=detail) + + +def _check_library(root: Path, *, read_only: bool) -> CheckResult: + """One library root: there, readable, and writable unless it is read only.""" + name = "library" + if not root.is_dir(): + return _row( + name, + Status.MISSING, + found=root, + detail="not there: its comics look deleted until it is back", + fix="mount it, or remove the library on the Libraries tab", + ) + if (root / DOCKER_UNMOUNTED_FN).exists(): + return _row( + name, + Status.MISCONFIGURED, + found=root, + detail="is the unmounted docker volume, not your comics", + fix="bind mount the comics directory at this path", + ) + if not os.access(root, os.R_OK | os.X_OK): + return _row( + name, + Status.MISCONFIGURED, + found=root, + detail="not readable", + fix=_PERMISSIONS_FIX, + ) + notes = ["readable"] + if read_only: + notes.append("read only") + elif os.access(root, os.W_OK): + notes.append("writable") + else: + return _row( + name, + Status.WARN, + found=root, + detail="readable · not writable: tag writes and renames fail", + fix=f"{_PERMISSIONS_FIX}, or mark it read only on the Libraries tab", + ) + if not any(root.iterdir()): + return _row( + name, + Status.WARN, + found=root, + detail=" · ".join([*notes, "empty: suspect unmounted"]), + fix="mount it, or add comics", + ) + return _row(name, Status.OK, found=root, detail=" · ".join(notes)) + + +def check_libraries() -> Iterator[CheckResult]: + """Every library root, in path order.""" + libraries = list(Library.objects.order_by("path").values_list("path", "read_only")) + if not libraries: + yield _row( + "library", + Status.OFF, + detail="no libraries", + fix="add one on the Libraries tab", + ) + return + for path, read_only in libraries: + yield _check_library(Path(path), read_only=read_only) + + +def _inotify_limit(name: str) -> int: + return int((_INOTIFY_DIR / name).read_text()) + + +def _suggested_watches(folders: int) -> int: + """Suggest a limit with room to grow: the usual recommendation, or the next power of two.""" + return max(_WATCHES_SUGGESTED_MIN, 1 << (2 * folders).bit_length()) + + +def check_watcher() -> Iterator[CheckResult]: + """Whether the kernel lets the event watcher follow every folder it must.""" + roots = Library.objects.filter(events=True).count() + if not roots: + yield _row("watcher", Status.OFF, detail="no library watches for events") + return + # One inotify watch per directory. Folder rows are the directories + # codex knows, so this is a floor; comic-less directories add to it. + folders = Folder.objects.filter(library__events=True).count() + roots + found = f"{folders} folders" + if not _HAS_INOTIFY: + yield _row( + "watcher", Status.OK, found=found, detail="no kernel watch limit here" + ) + return + try: + max_watches = _inotify_limit("max_user_watches") + except (OSError, ValueError) as exc: + yield _row( + "watcher", + Status.WARN, + found=found, + detail=f"inotify limit unreadable: {exc}", + fix="check fs.inotify.max_user_watches by hand", + ) + return + detail = f"inotify allows {max_watches}" + host = "the Docker host" if is_docker() else "this host" + fix = f"sysctl fs.inotify.max_user_watches={_suggested_watches(folders)} on {host}" + if folders > max_watches: + yield _row( + "watcher", + Status.MISCONFIGURED, + found=found, + detail=f"{detail}: changes in the rest go unseen until the next poll", + fix=fix, + ) + elif folders > max_watches * _WATCH_HEADROOM: + yield _row( + "watcher", + Status.WARN, + found=found, + detail=f"{detail}: nearly all of them, shared with every other watcher", + fix=fix, + ) + else: + yield _row("watcher", Status.OK, found=found, detail=detail) + + +def check_credentials() -> Iterator[CheckResult]: + """ + Whether the stored tagging credentials still decrypt. + + The field decrypts on load and, on failure, hands back the ciphertext + as if it were the value, so the Tagging tab would show gibberish and a + run would authenticate with it. A read through ``Cast`` skips the + field's converter and yields what is really stored. + """ + fields = [ + field + for field in ComicboxTaggingDefaults._meta.get_fields() + if isinstance(field, EncryptedCharField) + ] + raw = ( + ComicboxTaggingDefaults.objects.filter(pk=1) + .annotate(**{f"raw_{f.name}": Cast(f.name, TextField()) for f in fields}) + .values(*(f"raw_{f.name}" for f in fields)) + .first() + ) or {} + stored: dict[str, str] = { + f.name: token for f in fields if (token := raw.get(f"raw_{f.name}")) + } + if not stored: + yield _row("credentials", Status.OFF, detail="no tagging credentials stored") + return + fernet = Fernet(settings.FIELD_ENCRYPTION_KEY) + unreadable = [] + for name, token in stored.items(): + try: + fernet.decrypt(token.encode()) + except InvalidToken: + unreadable.append(name) + found = ", ".join(stored) + if unreadable: + yield _row( + "credentials", + Status.MISCONFIGURED, + found=", ".join(unreadable), + detail="cannot be decrypted: the key file in the config dir is not the one they were saved with", + fix="enter them again on the Tagging tab", + ) + return + yield _row("credentials", Status.OK, found=found, detail="decrypt") + + +CHECKS: tuple[tuple[str, Check], ...] = ( + ("database", check_database), + ("config dir", check_config_dir), + ("library", check_libraries), + ("watcher", check_watcher), + ("credentials", check_credentials), +) diff --git a/codex/librarian/fs/filters.py b/codex/librarian/fs/filters.py index 4229cb984..bd1e74fbd 100644 --- a/codex/librarian/fs/filters.py +++ b/codex/librarian/fs/filters.py @@ -4,8 +4,7 @@ from contextlib import suppress from pathlib import Path -from comicbox.box import Comicbox -from loguru import logger +from comicbox.enums.comicbox import FileTypeEnum # Component-level ignore registry consulted by the poller's walker and # the watchfiles filter. Either constant can be extended to add new @@ -68,40 +67,25 @@ def is_ignored_path(path: Path | str, root: Path | str | None = None) -> bool: _IMAGE_REGEX = r"\.(jpe?g|webp|png|gif|bmp)" _IMAGE_MATCHER: re.Pattern = re.compile(_IMAGE_REGEX, re.IGNORECASE) - -def _build_comic_matcher() -> re.Pattern: - comic_regex = r"\.(cb[zt7" - unsupported = [] - if Comicbox.is_unrar_supported(): - comic_regex += r"r" - else: - unsupported.append("CBR") - comic_regex += r"]" - - if Comicbox.is_pdf_supported(): - comic_regex += r"|pdf" - else: - unsupported.append("PDF") - comic_regex += ")$" - if unsupported: - un_str = ", ".join(unsupported) - logger.warning(f"Cannot detect or read from {un_str} archives") - return re.compile(comic_regex, re.IGNORECASE) - - -_COMIC_MATCHER: re.Pattern = _build_comic_matcher() - - -def _match_suffix(pattern: re.Pattern, path: Path) -> bool: - """Match suffix with pattern.""" - return bool(path and path.suffix and pattern.match(path.suffix) is not None) +#: Every comic archive suffix, lowercase with its dot, derived from +#: comicbox's file types rather than from what this host can read today. +#: The scan used to leave out CBR and PDF when comicbox said their tool +#: was missing. That made every existing row of that type look deleted +#: to the poller the day unrar went missing, and comicbox 5.3.0's probe, +#: which test-extracts a member, fails on more hosts than the path check +#: it replaced. A comic the host cannot open now fails its import with +#: comicbox's reason instead ("'unrar' not on path"), where the admin +#: can see it, and its row stays. +COMIC_SUFFIXES: frozenset[str] = frozenset( + f".{file_type.value.lower()}" for file_type in FileTypeEnum +) def match_comic(path: Path) -> bool: """Match comic file.""" - return _match_suffix(_COMIC_MATCHER, path) + return bool(path and path.suffix) and path.suffix.lower() in COMIC_SUFFIXES def match_image(path: Path) -> bool: """Match image file.""" - return _match_suffix(_IMAGE_MATCHER, path) + return bool(path and path.suffix and _IMAGE_MATCHER.match(path.suffix) is not None) diff --git a/codex/librarian/onlinetag/credential_validator.py b/codex/librarian/onlinetag/credential_validator.py index 6d4ee5318..588f147c4 100644 --- a/codex/librarian/onlinetag/credential_validator.py +++ b/codex/librarian/onlinetag/credential_validator.py @@ -1,38 +1,51 @@ """ -Validate online-source credentials by making a tiny authenticated call. +Validate online-source credentials with comicbox's own probe. Used by the admin tagging settings page to give operators an immediate yes/no on whether a saved (or in-the-form) credential set actually works against Metron / Comic Vine — so they don't discover problems only when a real tagging run fails halfway through. +Each source's ``probe()`` is its cheapest authenticated request, sent +once on a private client with no response cache and never retried, so +the verdict is the server's rather than a cached or pooled answer. +Metron's goes through comicbox's process-wide rate gate; Comic Vine's +is counted against the real shared rate-limit bucket under codex's +comicbox cache dir — the one tagging runs draw on — and is refused +outright when that pool is already spent. + A successful Metron check also reports the account's live rate limits, -which mokkari 4 reads off the ``X-RateLimit-*`` headers of the -validation response itself. The daily "sustained" limit varies by -Metron OpenCollective donor tier (5,000-25,000/day), so it is only -discoverable this way. +which the probe reads off the ``X-RateLimit-*`` headers of its own +response. The daily "sustained" limit varies by Metron OpenCollective +donor tier (5,000-25,000/day), so it is only discoverable this way. """ from __future__ import annotations from dataclasses import dataclass -from pathlib import Path -from tempfile import TemporaryDirectory -from typing import TYPE_CHECKING, Any +from types import MappingProxyType +from typing import TYPE_CHECKING, Any, Final +from comicbox.config.online.settings import OnlineSourceCredentials from comicbox.formats.base.online import SOURCE_NAMES +from comicbox.formats.base.online.retry import RetryCategory +from comicbox.formats.comicvine_api.online_source import ComicVineOnlineSource +from comicbox.formats.metron_api.online_source import MetronOnlineSource + +from codex.settings import COMICBOX_ONLINE_CONFIG if TYPE_CHECKING: - from collections.abc import Collection + from collections.abc import Callable, Collection, Mapping + from comicbox.formats.base.online.sources.base import OnlineSource from comicbox.online_session import OnlineCredentials - from mokkari.session import RateLimitStatus # comicbox owns the canonical online-tag source names (metron, comicvine); derive # from it so codex tracks a new source instead of hand-syncing a literal. KNOWN_SOURCES: frozenset[str] = frozenset(SOURCE_NAMES) -_COMICVINE_TIMEOUT_SECS: float = 10.0 +#: What a probe returns: ``{window: {"limit", "remaining", "reset_epoch"}}``. +type _Windows = Mapping[str, Mapping[str, Any]] @dataclass(frozen=True, slots=True) @@ -46,11 +59,12 @@ class RateLimitWindowInfo: @dataclass(frozen=True, slots=True) class RateLimitInfo: """ - A source account's live rate limits, mirrored from mokkari's status. + A source account's live rate limits, mirrored from the probe's windows. - ``reset`` datetimes are deliberately dropped: the burst window resets - within a minute (stale before an admin reads it) and the sustained - reset isn't worth datetime plumbing for a one-shot validation chip. + ``reset_epoch`` values are deliberately dropped: the burst window + resets within a minute (stale before an admin reads it) and the + sustained reset isn't worth datetime plumbing for a one-shot + validation chip. """ burst: RateLimitWindowInfo = RateLimitWindowInfo() @@ -66,104 +80,91 @@ class ValidationResult: rate_limits: RateLimitInfo | None = None -def _extract_rate_limits(status: RateLimitStatus) -> RateLimitInfo | None: - """Codex-shaped rate limits, or None when the source reported nothing.""" - burst = status.burst - sustained = status.sustained - if ( - burst.limit is None - and burst.remaining is None - and sustained.limit is None - and sustained.remaining is None - ): - return None - return RateLimitInfo( - burst=RateLimitWindowInfo(limit=burst.limit, remaining=burst.remaining), - sustained=RateLimitWindowInfo( - limit=sustained.limit, remaining=sustained.remaining - ), +def _metron_credentials(creds: OnlineCredentials) -> OnlineSourceCredentials: + # ``or None`` is load bearing: mokkari sends a Bearer header whenever + # api_token is not None, so an empty string would defeat the legacy + # username & password fallback. + return OnlineSourceCredentials( + key=creds.metron_key or None, + user=creds.metron_user or None, + password=creds.metron_password or None, ) -def _validate_metron(creds: OnlineCredentials) -> ValidationResult: - if not (creds.metron_key or (creds.metron_user and creds.metron_password)): - return ValidationResult(ok=False, error="API key required.") - from mokkari.exceptions import ApiError, AuthenticationError - from mokkari.session import Session +def _comicvine_credentials(creds: OnlineCredentials) -> OnlineSourceCredentials: + return OnlineSourceCredentials( + key=creds.comicvine_key or None, url=creds.comicvine_url or None + ) - # ``or None`` is load bearing: mokkari sends a Bearer header whenever - # api_token is not None, so an empty string would defeat the legacy - # username & password fallback. - session = Session( - username=creds.metron_user, - passwd=creds.metron_password, - api_token=creds.metron_key or None, - cache=None, - user_agent="codex-credential-check", + +def _window_info(window: Mapping[str, Any] | None) -> RateLimitWindowInfo: + if not window: + return RateLimitWindowInfo() + return RateLimitWindowInfo( + limit=window.get("limit"), remaining=window.get("remaining") ) - try: - session.publishers_list({"page": 1}) - except AuthenticationError as err: - return ValidationResult(ok=False, error=str(err) or "Authentication failed.") - except ApiError as err: - return ValidationResult(ok=False, error=str(err) or "API error.") - # The successful response carried the account's X-RateLimit-* headers. - else: - return ValidationResult( - ok=True, rate_limits=_extract_rate_limits(session.rate_limit_status) - ) - finally: - # This runs in the web process, once per validation, and the - # session is not shared with anything. Since mokkari 4.8.0 it - # holds a pooled TLS connection until closed. - session.close() - - -def _validate_comicvine(creds: OnlineCredentials) -> ValidationResult: - if not creds.comicvine_key: + + +def _metron_rate_limits(windows: _Windows) -> RateLimitInfo | None: + """Codex-shaped rate limits, or None when the response carried no headers.""" + info = RateLimitInfo( + burst=_window_info(windows.get("burst")), + sustained=_window_info(windows.get("daily")), + ) + return None if info == RateLimitInfo() else info + + +def _no_rate_limits(_windows: _Windows) -> None: + """Comic Vine's pools are hourly; the UI chip speaks per-minute / per-day.""" + + +@dataclass(frozen=True, slots=True) +class _Source: + """How codex drives one comicbox online source for a credential check.""" + + cls: type[OnlineSource] + credentials: Callable[[OnlineCredentials], OnlineSourceCredentials] + rate_limits: Callable[[_Windows], RateLimitInfo | None] + + +_SOURCES: Final[Mapping[str, _Source]] = MappingProxyType( + { + "metron": _Source(MetronOnlineSource, _metron_credentials, _metron_rate_limits), + "comicvine": _Source( + ComicVineOnlineSource, _comicvine_credentials, _no_rate_limits + ), + } +) + +# Shown when the client's exception carries no message of its own. +_FALLBACK_ERRORS: Final[Mapping[RetryCategory | None, str]] = MappingProxyType( + { + RetryCategory.AUTH: "Authentication failed.", + RetryCategory.RATE_LIMIT: "Rate limited.", + } +) + + +def _error_message(source: OnlineSource, exc: Exception) -> str: + """Name what went wrong: the exception's first line, else its category.""" + # simyan raises ``RateLimitError(None)`` for a 429 with no error body, + # and str() of that is the word "None". Some upstream errors carry a + # whole HTML page, of which only the first line is the message. + text = str(exc) if exc.args and exc.args[0] is not None else "" + first_line = text.partition("\n")[0].strip() + category = source.classify_retry_exception(exc) + return first_line or _FALLBACK_ERRORS.get(category, type(exc).__name__) + + +def _validate(spec: _Source, creds: OnlineCredentials) -> ValidationResult: + source = spec.cls(spec.credentials(creds), COMICBOX_ONLINE_CONFIG.online) + if not source.is_configured(): return ValidationResult(ok=False, error="API key required.") - from requests_cache import DO_NOT_CACHE - from simyan.comicvine import Comicvine - from simyan.errors import AuthenticationError, ServiceError - - # A credential check must always hit the network — api_key is - # excluded from simyan's cache key, so a cached response would - # validate any key — so responses are never cached (DO_NOT_CACHE) - # and both sqlite files land in a throwaway dir. - # - # The AuthenticationError below only started firing with simyan 4: - # 3.x returned Comic Vine's error body verbatim, so a rejected key - # came back as an ordinary empty result and validated as good. - with TemporaryDirectory(prefix="codex-credential-check-") as tmp: - tmp_path = Path(tmp) - # dict[str, Any] expansion because DO_NOT_CACHE is an int sentinel - # that simyan's timedelta-typed cache_expiry accepts at runtime. - kwargs: dict[str, Any] = { - "api_key": creds.comicvine_key, - "user_agent": "codex-credential-check", - "timeout": _COMICVINE_TIMEOUT_SECS, - "cache_path": tmp_path / "cache.sqlite", - "cache_expiry": DO_NOT_CACHE, - "ratelimit_path": tmp_path / "ratelimits.sqlite", - } - if creds.comicvine_url: - kwargs["base_url"] = creds.comicvine_url - cv = Comicvine(**kwargs) - try: - cv.list_publishers(params={"limit": "1"}, max_results=1) - except AuthenticationError as err: - return ValidationResult( - ok=False, error=str(err) or "Authentication failed." - ) - except ServiceError as err: - return ValidationResult(ok=False, error=str(err) or "Service error.") - return ValidationResult(ok=True) - - -_VALIDATORS = { - "metron": _validate_metron, - "comicvine": _validate_comicvine, -} + try: + windows = source.probe() + except Exception as exc: + return ValidationResult(ok=False, error=_error_message(source, exc)) + return ValidationResult(ok=True, rate_limits=spec.rate_limits(windows or {})) def validate_credentials( @@ -177,4 +178,4 @@ def validate_credentials( constrained the inputs. """ targets = KNOWN_SOURCES if sources is None else (set(sources) & KNOWN_SOURCES) - return {name: _VALIDATORS[name](creds) for name in sorted(targets)} + return {name: _validate(_SOURCES[name], creds) for name in sorted(targets)} diff --git a/codex/librarian/onlinetag/prompt_apply.py b/codex/librarian/onlinetag/prompt_apply.py index 59cda2f03..6285ad896 100644 --- a/codex/librarian/onlinetag/prompt_apply.py +++ b/codex/librarian/onlinetag/prompt_apply.py @@ -44,6 +44,7 @@ from codex.librarian.scribe.tagwrite_errors import add_tag_write_error from codex.librarian.scribe.tasks import BulkTagWriteTask from codex.models.comic import Comic +from codex.settings import COMICBOX_ONLINE_CONFIG if TYPE_CHECKING: from collections.abc import Callable @@ -319,6 +320,10 @@ def _apply_replayed_search( # PROMPT_VERSION; only the comicbox kwarg moved. match=MatchMode(prompt.get("mode") or "auto"), defer_prompts=True, + # The same settings the search ran with, so the replay reads + # the response cache that search filled under /config rather + # than comicbox's own default cache dir. + config=COMICBOX_ONLINE_CONFIG, client_name=CLIENT_NAME, ) session.preload_resolution( diff --git a/codex/librarian/onlinetag/session_manager.py b/codex/librarian/onlinetag/session_manager.py index b46c40c07..5dcbb952a 100644 --- a/codex/librarian/onlinetag/session_manager.py +++ b/codex/librarian/onlinetag/session_manager.py @@ -217,12 +217,16 @@ def _build_credentials(self) -> OnlineCredentials | None: and not defaults.comicvine_key ): return None + # ``or None``, not ``or ""``: comicbox hands the key to mokkari as its + # api_token, and mokkari sends a Bearer header for any token that is + # not None, an empty one included, in place of the username and + # password. An unset key spelled "" shut off every legacy login. return OnlineCredentials( - metron_key=defaults.metron_key or "", - metron_user=defaults.metron_user or "", - metron_password=defaults.metron_password or "", - comicvine_key=defaults.comicvine_key or "", - comicvine_url=defaults.comicvine_url or "", + metron_key=defaults.metron_key or None, + metron_user=defaults.metron_user or None, + metron_password=defaults.metron_password or None, + comicvine_key=defaults.comicvine_key or None, + comicvine_url=defaults.comicvine_url or None, ) @staticmethod diff --git a/codex/librarian/onlinetag/session_state.py b/codex/librarian/onlinetag/session_state.py index 798ff916d..cd11bd833 100644 --- a/codex/librarian/onlinetag/session_state.py +++ b/codex/librarian/onlinetag/session_state.py @@ -165,7 +165,7 @@ def serialize_prompt( "comics": comics, "source": dp.source, "candidates": [serialize_candidate(c) for c in dp.candidates], - "mode": getattr(dp.match, "value", str(dp.match)), + "mode": str(dp.match), "formats": list(formats), "delete_original": delete_original, "rename": rename, diff --git a/codex/librarian/scribe/force_updater.py b/codex/librarian/scribe/force_updater.py index 62cfb781c..64a8abda9 100644 --- a/codex/librarian/scribe/force_updater.py +++ b/codex/librarian/scribe/force_updater.py @@ -20,7 +20,7 @@ def force_update(self, task: ForceUpdateComicsTask) -> None: comics = Comic.objects.filter(pk__in=task.comic_pks).only("path", "library_id") library_path_map: defaultdict[int, set[str]] = defaultdict(set) for comic in comics: - library_path_map[comic.library_id].add(comic.path) # pyright: ignore[reportAttributeAccessIssue] + library_path_map[comic.library_id].add(comic.path) # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] total = 0 for library_id, paths in library_path_map.items(): diff --git a/codex/librarian/scribe/importer/moved/covers.py b/codex/librarian/scribe/importer/moved/covers.py index 5c13a75c6..c0641e505 100644 --- a/codex/librarian/scribe/importer/moved/covers.py +++ b/codex/librarian/scribe/importer/moved/covers.py @@ -50,10 +50,10 @@ def _bulk_covers_moved_unlink(self, unlink_pks) -> None: collections = model.objects.filter(custom_cover__in=unlink_pks) unlink_collections = [] for collection in collections: - collection.custom_cover = None + collection.custom_cover = None # ty: ignore[invalid-assignment] unlink_collections.append(collection) if unlink_collections: - model.objects.bulk_update(unlink_collections, ["custom_cover"]) + model.objects.bulk_update(unlink_collections, ["custom_cover"]) # ty: ignore[invalid-argument-type] self.log.debug( f"Unlinked {len(unlink_collections)} {model.__name__} moved custom covers." ) diff --git a/codex/librarian/scribe/importer/moved/folders.py b/codex/librarian/scribe/importer/moved/folders.py index 927628e41..4570c9cfb 100644 --- a/codex/librarian/scribe/importer/moved/folders.py +++ b/codex/librarian/scribe/importer/moved/folders.py @@ -49,7 +49,7 @@ def _bulk_move_folders( folder.name = Path(new_path).name folder.path = new_path parent_path_str = str(Path(new_path).parent) - folder.parent_folder = dest_parent_folders_map.get(parent_path_str) + folder.parent_folder = dest_parent_folders_map.get(parent_path_str) # ty: ignore[invalid-assignment] folder.presave() folder.updated_at = Now() update_folders.append(folder) diff --git a/codex/librarian/scribe/importer/query/foreign_keys.py b/codex/librarian/scribe/importer/query/foreign_keys.py index 5a22bf788..010588395 100644 --- a/codex/librarian/scribe/importer/query/foreign_keys.py +++ b/codex/librarian/scribe/importer/query/foreign_keys.py @@ -38,7 +38,9 @@ def query_existing_mds( ) rels = MODEL_REL_MAP[model] select_related = MODEL_SELECT_RELATED.get(model, ()) - fields = tuple(filter(bool, flatten(rels))) + # A comprehension, not filter(bool, ...): bool's signature widens the + # element type to object, which values_list refuses. + fields = tuple(field for field in flatten(rels) if field) qs = model.objects qs = qs.select_related(*select_related) qs = qs.filter(fk_filter).distinct().values_list(*fields) diff --git a/codex/librarian/scribe/janitor/integrity/foreign_keys.py b/codex/librarian/scribe/janitor/integrity/foreign_keys.py index 47f629322..0f23d001d 100644 --- a/codex/librarian/scribe/janitor/integrity/foreign_keys.py +++ b/codex/librarian/scribe/janitor/integrity/foreign_keys.py @@ -6,12 +6,12 @@ from pathlib import Path from typing import TYPE_CHECKING, cast -from comicbox.enums.comicbox import FileTypeEnum from django.apps import apps from django.core.exceptions import FieldDoesNotExist from django.db import DEFAULT_DB_ALIAS, connections, transaction from django.db.models.functions import Now +from codex.librarian.fs.filters import COMIC_SUFFIXES from codex.models.util import get_sort_name if TYPE_CHECKING: @@ -22,25 +22,16 @@ # Comic file extensions we'll consider — phantom directory-as-comic # rows (older bug) are out of scope for parent-folder drift repair. -# -# Derived from comicbox rather than written out, so a new archive format -# cannot leave rows behind here, and compared case-insensitively because -# that is how the scanner matched them on the way in -# (``filters`` compiles its regex with ``re.IGNORECASE`` and the importer -# stores the path verbatim, so ``Foo.CBZ`` is a real, importable comic). -# A case-sensitive test dropped those rows from ``needed`` and pruned the +# The scanner's own list, compared case-insensitively because that is +# how the scanner matched them on the way in (the importer stores the +# path verbatim, so ``Foo.CBZ`` is a real, importable comic). A +# case-sensitive test dropped those rows from ``needed`` and pruned the # folders they live in, cascading the comics and their bookmarks away. -# -# Deliberately *not* ``filters.match_comic``: that consults -# ``Comicbox.is_unrar_supported()`` / ``is_pdf_supported()``, so on a host -# without unrar every ``.cbr`` row would drop out of ``needed`` and be -# pruned — the same bug from the other side. -_COMIC_SUFFIXES = frozenset(f".{file_type.value.lower()}" for file_type in FileTypeEnum) def _is_comic_path(path: str) -> bool: """Whether a stored path is a comic archive, however it is cased.""" - return Path(path).suffix.lower() in _COMIC_SUFFIXES + return Path(path).suffix.lower() in COMIC_SUFFIXES # SQLite's parameter cap is 32766; leave headroom for the rare case diff --git a/codex/librarian/scribe/lazy_importer.py b/codex/librarian/scribe/lazy_importer.py index 9e0e119c9..bdc328e3e 100644 --- a/codex/librarian/scribe/lazy_importer.py +++ b/codex/librarian/scribe/lazy_importer.py @@ -36,7 +36,7 @@ def lazy_import(self, task) -> None: # Map comics to libraries. library_path_map: defaultdict[int, set[str]] = defaultdict(set) for comic in comics: - library_id = comic.library_id # pyright: ignore[reportAttributeAccessIssue] + library_id = comic.library_id # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] library_path_map[library_id].add(comic.path) for library_id, paths in library_path_map.items(): diff --git a/codex/librarian/scribe/tag_writer.py b/codex/librarian/scribe/tag_writer.py index 88c772446..3c66a69fe 100644 --- a/codex/librarian/scribe/tag_writer.py +++ b/codex/librarian/scribe/tag_writer.py @@ -241,7 +241,7 @@ def _resolve_comics( self._file_type_of = {} for comic in comics: comic_paths[comic.pk] = Path(comic.path) - lib_of[comic.pk] = comic.library_id # pyright: ignore[reportAttributeAccessIssue] + lib_of[comic.pk] = comic.library_id # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] self._file_type_of[comic.pk] = comic.file_type or "" return comic_paths, lib_of diff --git a/codex/migrations/0056_match_mode_ask.py b/codex/migrations/0056_match_mode_ask.py new file mode 100644 index 000000000..f1e57c820 --- /dev/null +++ b/codex/migrations/0056_match_mode_ask.py @@ -0,0 +1,35 @@ +""" +Online tagging's match mode gains comicbox's ``ask``. + +Ask auto-writes nothing and prompts for every match. comicbox 5.3.1 accepts +it from a session with a prompt handler, which codex always supplies, so the +choices now come from comicbox's enum instead of a hand-written copy. The +column itself is unchanged; only the choices Django validates against grow. +""" + +from django.db import migrations, models + + +class Migration(migrations.Migration): + """Add ask to the match mode choices.""" + + dependencies = [ + ("codex", "0055_settings_defaults"), + ] + + operations = [ + migrations.AlterField( + model_name="comicboxtaggingdefaults", + name="default_match_mode", + field=models.CharField( + choices=[ + ("ask", "Ask"), + ("careful", "Careful"), + ("auto", "Auto"), + ("eager", "Eager"), + ], + default="auto", + max_length=32, + ), + ), + ] diff --git a/codex/models/admin.py b/codex/models/admin.py index 8f4b41fd2..e5c9e1535 100644 --- a/codex/models/admin.py +++ b/codex/models/admin.py @@ -2,6 +2,7 @@ from typing import override +from comicbox.config.online import MatchMode from django.db.models import ( SET_NULL, BooleanField, @@ -31,6 +32,7 @@ from codex.models.choices import ( ReadingDirectionChoices, max_choices_len, + text_choices_from_enum, text_choices_from_map, ) from codex.models.fields import EncryptedCharField @@ -103,12 +105,11 @@ class Meta(BaseModel.Meta): class ComicboxTaggingDefaults(BaseModel): """Singleton model for default comicbox tagging options and credentials.""" - class MatchModeChoices(TextChoices): - """Match mode options for online tagging (mirrors comicbox.MatchMode).""" - - CAREFUL = "careful", _("Careful") - AUTO = "auto", _("Auto") - EAGER = "eager", _("Eager") + # comicbox's match modes, ask through eager, so a mode it adds reaches + # the admin page without a hand-synced copy. Ask auto-writes nothing + # and prompts for every match; comicbox 5.3.1 accepts it from a session + # with a prompt handler, which codex always supplies. + MatchModeChoices = text_choices_from_enum(MatchMode, "MatchModeChoices") class PromptsModeChoices(TextChoices): """Prompt behavior options for online tagging.""" @@ -143,7 +144,7 @@ class EffortChoices(TextChoices): default_match_mode = CharField( max_length=MAX_FIELD_LEN, choices=MatchModeChoices.choices, - default=MatchModeChoices.AUTO, + default=MatchMode.AUTO.value, ) default_prompts_mode = CharField( max_length=MAX_FIELD_LEN, diff --git a/codex/serializers/admin/doctor.py b/codex/serializers/admin/doctor.py new file mode 100644 index 000000000..b2be30b86 --- /dev/null +++ b/codex/serializers/admin/doctor.py @@ -0,0 +1,30 @@ +"""Admin doctor report serializers.""" + +from rest_framework.serializers import ( + CharField, + IntegerField, + ListField, + Serializer, +) + + +class DoctorRowSerializer(Serializer): + """One row of the doctor report.""" + + section = CharField(read_only=True) + name = CharField(read_only=True) + #: OK, WARN, OFF, MISSING, WRONG VERSION, MISCONFIGURED or ERROR. + status = CharField(read_only=True) + found = CharField(read_only=True) + detail = CharField(read_only=True) + fix = CharField(read_only=True) + + +class DoctorReportSerializer(Serializer): + """The doctor report for this install: comicbox's rows and codex's own.""" + + #: comicbox version, Python, platform, and "Docker" inside a container. + header = ListField(child=CharField(), read_only=True) + comicbox = DoctorRowSerializer(many=True, read_only=True) + codex = DoctorRowSerializer(many=True, read_only=True) + problems = IntegerField(read_only=True) diff --git a/codex/serializers/admin/tagging.py b/codex/serializers/admin/tagging.py index eebf84dd7..02f0cb61a 100644 --- a/codex/serializers/admin/tagging.py +++ b/codex/serializers/admin/tagging.py @@ -61,7 +61,11 @@ class OnlineTagStartSerializer(Serializer): collection = CharField() pks = ListField(child=CharField()) sources = ListField(child=CharField(), required=False, default=list(SOURCE_NAMES)) - mode = CharField(required=False, default="auto") + mode = ChoiceField( + choices=ComicboxTaggingDefaults.MatchModeChoices.values, + required=False, + default="auto", + ) # None falls back to the admin default, like the flags below. effort = CharField(required=False, default=None, allow_null=True) prompts_mode = CharField(required=False, default="ask") diff --git a/codex/startup/__init__.py b/codex/startup/__init__.py index 2ba0dd6bd..2ff3beba7 100644 --- a/codex/startup/__init__.py +++ b/codex/startup/__init__.py @@ -8,6 +8,7 @@ from rest_framework.authtoken.models import Token from codex.choices.admin import AdminFlagChoices +from codex.doctor import log_doctor_problems from codex.librarian.status_controller import STATUS_DEFAULTS from codex.models import ( AdminFlag, @@ -257,4 +258,5 @@ def codex_init() -> bool: if AUTH_REMOTE_USER: logger.info("Remote User authorization enabled.") log_docker_hub_deprecation(logger) + log_doctor_problems(logger) return True diff --git a/codex/urls/api/v4/admin.py b/codex/urls/api/v4/admin.py index 3ff54890e..c902d33be 100644 --- a/codex/urls/api/v4/admin.py +++ b/codex/urls/api/v4/admin.py @@ -14,6 +14,7 @@ AdminCustomCoverRemoveView, AdminCustomCoverUploadView, ) +from codex.views.admin.doctor import AdminDoctorView from codex.views.admin.dump_user_data import AdminDumpUserDataView from codex.views.admin.email import AdminEmailSettingsView, AdminEmailTestSendView from codex.views.admin.failed_imports_seen import AdminFailedImportsSeenView @@ -225,6 +226,11 @@ AdminStatsView.as_view(), name="stats", ), + path( + "doctor", + AdminDoctorView.as_view(), + name="doctor", + ), path( "api-key", AdminAPIKey.as_view(), diff --git a/codex/views/admin/doctor.py b/codex/views/admin/doctor.py new file mode 100644 index 000000000..bff7331cd --- /dev/null +++ b/codex/views/admin/doctor.py @@ -0,0 +1,25 @@ +"""Admin doctor view.""" + +from adrf.mixins import get_data +from channels.db import database_sync_to_async +from rest_framework.response import Response + +from codex.doctor import run_doctor +from codex.serializers.admin.doctor import DoctorReportSerializer +from codex.views.admin.auth import AsyncAdminGenericAPIView + + +class AdminDoctorView(AsyncAdminGenericAPIView): + """GET: the doctor report for this install.""" + + serializer_class = DoctorReportSerializer + + async def get(self, *_args, **_kwargs) -> Response: + """Run the doctor and serialize its rows.""" + # About a second: a RAR extraction subprocess, config files, + # package metadata and a few library queries. Off the event loop + # and off the one thread every sync view shares; channels' + # wrapper closes the connection the executor thread opened. + report = await database_sync_to_async(run_doctor, thread_sensitive=False)() + serializer = self.get_serializer(report) + return Response(await get_data(serializer)) diff --git a/codex/views/admin/stats.py b/codex/views/admin/stats.py index 24bae0ee4..0ef7e86ca 100644 --- a/codex/views/admin/stats.py +++ b/codex/views/admin/stats.py @@ -36,7 +36,10 @@ class AdminStatsView(AsyncAdminGenericAPIView): """Admin Flag Viewset.""" - permission_classes = (HasAPIKeyOrIsAdminUser,) + # DRF's stubs declare this an instance variable and AdminAuthMixin a + # ClassVar; no annotation satisfies both checkers from a class that + # inherits the two, so the override says which rule it is waiving. + permission_classes = (HasAPIKeyOrIsAdminUser,) # ty: ignore[invalid-attribute-override] serializer_class = StatsSerializer input_serializer_class = AdminStatsRequestSerializer diff --git a/codex/views/browser/saved_settings.py b/codex/views/browser/saved_settings.py index a7a56f245..93a90cc19 100644 --- a/codex/views/browser/saved_settings.py +++ b/codex/views/browser/saved_settings.py @@ -226,7 +226,7 @@ def post(self, *args, **kwargs) -> Response: **direct_kwargs, ) # Clone filters - src_filters = current.filters # pyright: ignore[reportAttributeAccessIssue] + src_filters = current.filters # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] new_filters = SettingsBrowserFilters(browser=new_sb) for key in SettingsBrowserFilters.FILTER_KEYS: val = getattr(src_filters, key) @@ -236,7 +236,7 @@ def post(self, *args, **kwargs) -> Response: new_filters.save() # Clone last_route - src_route = current.last_route # pyright: ignore[reportAttributeAccessIssue] + src_route = current.last_route # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] SettingsBrowserLastRoute.objects.create( browser=new_sb, collection=src_route.collection, @@ -286,7 +286,7 @@ def get(self, *args, **kwargs) -> Response: data = self.browser_instance_to_dict(saved) # Validate filter PKs, persist cleaned data, and collect warnings. - filters_obj = saved.filters # pyright: ignore[reportAttributeAccessIssue] + filters_obj = saved.filters # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] filter_warnings = _validate_filter_pks(data.get("filters", {}), filters_obj) result = { diff --git a/codex/views/opds/authentication/v1.py b/codex/views/opds/authentication/v1.py index 9293bf451..30c58a74e 100644 --- a/codex/views/opds/authentication/v1.py +++ b/codex/views/opds/authentication/v1.py @@ -77,7 +77,7 @@ def _absolute_doc(request) -> dict: def static_get(cls, request, status_code=status.HTTP_200_OK) -> JsonResponse: """Serialize the authentication dict with absolute URLs.""" doc = cls._absolute_doc(request) - serializer = cls.serializer_class(doc) # pyright: ignore[reportOptionalCall] + serializer = cls.serializer_class(doc) # pyright: ignore[reportOptionalCall], # ty: ignore[call-non-callable] response = JsonResponse( serializer.data, status=status_code, diff --git a/codex/views/opds/v2/manifest.py b/codex/views/opds/v2/manifest.py index a3ee09a23..bd7770a68 100644 --- a/codex/views/opds/v2/manifest.py +++ b/codex/views/opds/v2/manifest.py @@ -77,7 +77,7 @@ def _publication_identifier(self, obj) -> str: .order_by("source_name", "key") ) return ",".join( - f"{identifier.source_name}:{identifier.id_type}:{identifier.key}" # pyright: ignore[reportAttributeAccessIssue] + f"{identifier.source_name}:{identifier.id_type}:{identifier.key}" # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] for identifier in identifiers ) diff --git a/codex/views/reader/books.py b/codex/views/reader/books.py index c121d0138..2380dc582 100644 --- a/codex/views/reader/books.py +++ b/codex/views/reader/books.py @@ -66,7 +66,7 @@ def _batch_settings_and_bookmarks( **reader_auth, comic_id__in=book_pks ) settings_by_pk: dict[int, SettingsReader] = { - s.comic_id: s # pyright: ignore[reportAttributeAccessIssue] + s.comic_id: s # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] for s in settings_qs } bookmark_auth = self.get_bookmark_auth_filter() @@ -74,7 +74,7 @@ def _batch_settings_and_bookmarks( **bookmark_auth, comic_id__in=book_pks ).only("page", "finished", "comic_id") bookmarks_by_pk: dict[int, Bookmark] = { - b.comic_id: b # pyright: ignore[reportAttributeAccessIssue] + b.comic_id: b # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] for b in bookmark_qs } return settings_by_pk, bookmarks_by_pk diff --git a/codex/views/reader/page.py b/codex/views/reader/page.py index 01ef280b1..e13db8ca0 100644 --- a/codex/views/reader/page.py +++ b/codex/views/reader/page.py @@ -5,12 +5,12 @@ import time from typing import TYPE_CHECKING, Final +from comicbox.box import Comicbox from comicbox.exceptions import ComicboxError from django.http import HttpResponse from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema from loguru import logger -from pdffile import PageMode, PDFFile from rest_framework.exceptions import NotFound from codex.librarian.bookmark.tasks import BookmarkUpdateTask @@ -22,7 +22,7 @@ from codex.views.reader._archive_cache import archive_cache, page_acl_cache if TYPE_CHECKING: - from pdffile import PageVerdict + from pdffile import PageVerdict, PDFFile from codex.views.reader._archive_cache import _ArchiveEntry @@ -132,6 +132,16 @@ def _try_pdf_image_serve( ``None`` when the caller should fall back to the legacy single-page-PDF path. """ + if not Comicbox.is_pdf_supported(): + # Since comicbox 5.3.0 a pdffile that is installed but will not + # import (pymupdf against the wrong libmupdf, say) disables PDFs + # instead of breaking comicbox. Importing pdffile at module + # level here undid that: it failed the reader's import and with + # it the server. The legacy path below reports the same comic + # as unreadable, which is the truth. + return None + from pdffile import PageMode, PDFFile + with archive_cache.open_entry(path) as entry: # ``Comicbox._get_archive`` returns the underlying archive # union (zip / rar / 7z / tar / pdf). Caller has gated on diff --git a/codex/views/settings.py b/codex/views/settings.py index 97644112e..bd852f252 100644 --- a/codex/views/settings.py +++ b/codex/views/settings.py @@ -116,13 +116,13 @@ def _get_or_create_settings_user( if instance is None: return None - if session_key and instance.session_id != session_key: # pyright: ignore[reportAttributeAccessIssue] + if session_key and instance.session_id != session_key: # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] # Discard any anonymous row that owns the new session so # the unique constraint isn't violated. model.objects.filter( session_id=session_key, user__isnull=True, **base_filter ).delete() - instance.session_id = session_key # pyright: ignore[reportAttributeAccessIssue] + instance.session_id = session_key # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[invalid-assignment] instance.save(update_fields=("session_id", "updated_at")) return instance @@ -143,7 +143,7 @@ def _get_or_create_settings_session( if instance is None: return None - if user and instance.user_id is None: + if user and instance.user_id is None: # ty: ignore[unresolved-attribute] # Promote anonymous row to a user row (first login). instance.user = user instance.save(update_fields=("user_id", "updated_at")) diff --git a/frontend/src/api/v4/admin.js b/frontend/src/api/v4/admin.js index 9e51b0647..58e6835e3 100644 --- a/frontend/src/api/v4/admin.js +++ b/frontend/src/api/v4/admin.js @@ -182,6 +182,10 @@ export const getAllLibrarianStatuses = () => export const getStats = () => HTTP.get("/admin/stats", { params: { ts: Date.now() } }); +// comicbox's doctor report: one row per environment check. +export const getDoctorReport = () => + HTTP.get("/admin/doctor", { params: { ts: Date.now() } }); + export const getAPIKey = () => HTTP.get("/admin/api-key", { params: { ts: Date.now() } }); diff --git a/frontend/src/components/admin/tabs/doctor-tab.vue b/frontend/src/components/admin/tabs/doctor-tab.vue new file mode 100644 index 000000000..f229585c0 --- /dev/null +++ b/frontend/src/components/admin/tabs/doctor-tab.vue @@ -0,0 +1,106 @@ + + + + + + diff --git a/frontend/src/components/admin/tabs/doctor-table.vue b/frontend/src/components/admin/tabs/doctor-table.vue new file mode 100644 index 000000000..e67f5159f --- /dev/null +++ b/frontend/src/components/admin/tabs/doctor-table.vue @@ -0,0 +1,139 @@ + + + + + + diff --git a/frontend/src/components/admin/tabs/tagging-tab.vue b/frontend/src/components/admin/tabs/tagging-tab.vue index b41a0d0b0..00ca0fb63 100644 --- a/frontend/src/components/admin/tabs/tagging-tab.vue +++ b/frontend/src/components/admin/tabs/tagging-tab.vue @@ -487,7 +487,7 @@ export default { renameFilesHint: "Rename each comic file to the comicbox scheme derived from its tags after writing. Can be overridden per operation when editing tags or tagging online.", matchModeHint: - "How aggressively to accept online matches. Careful writes only near-certain matches, Auto also writes confident ones, and Eager also writes weaker best guesses.", + "How aggressively to accept online matches. Ask writes nothing and prompts for every match, Careful writes only near-certain matches, Auto also writes confident ones, and Eager also writes weaker best guesses.", effortHint: "How many Comic Vine requests a comic may spend searching. Thorough finds the most and costs the most against the hourly limit; Metron answers in one call and ignores this.", promptsModeHint: diff --git a/frontend/src/components/online-tag/launcher-dialog.vue b/frontend/src/components/online-tag/launcher-dialog.vue index 12245d9a0..14787dc46 100644 --- a/frontend/src/components/online-tag/launcher-dialog.vue +++ b/frontend/src/components/online-tag/launcher-dialog.vue @@ -187,6 +187,7 @@ import { useOnlineTagStore } from "@/stores/online-tag"; // so it changes no request count. Effort is what buys calls, and only // against Comic Vine: Metron answers in a flat two-step whatever you ask. const MATCH_MODE_HINTS = { + ask: "Writes nothing on its own. Every match becomes a prompt to review, so Prompts must be set to Ask.", careful: "Only accepts high-confidence, unambiguous matches. Defers anything uncertain for manual review.", auto: "Balances accuracy and speed. Accepts confident matches automatically and defers uncertain ones.", diff --git a/frontend/src/plugins/router.js b/frontend/src/plugins/router.js index 9e340374b..9db8f153f 100644 --- a/frontend/src/plugins/router.js +++ b/frontend/src/plugins/router.js @@ -19,6 +19,7 @@ const AdminCustomCoversTab = () => const AdminJobsTab = () => import("@/components/admin/tabs/job-tab.vue"); const AdminRestoreTab = () => import("@/components/admin/tabs/restore-tab.vue"); const AdminStatsTab = () => import("@/components/admin/tabs/stats-tab.vue"); +const AdminDoctorTab = () => import("@/components/admin/tabs/doctor-tab.vue"); const AdminTaggingTab = () => import("@/components/admin/tabs/tagging-tab.vue"); const AdminEmailTab = () => import("@/components/admin/tabs/email-tab.vue"); const AdminAuthTab = () => import("@/components/admin/tabs/auth-tab.vue"); @@ -93,6 +94,7 @@ const routes = [ { name: "admin-jobs", path: "jobs", component: AdminJobsTab }, { name: "admin-stats", path: "stats", component: AdminStatsTab }, { name: "admin-restore", path: "restore", component: AdminRestoreTab }, + { name: "admin-doctor", path: "doctor", component: AdminDoctorTab }, ], }, { diff --git a/frontend/src/stores/admin.js b/frontend/src/stores/admin.js index 28539f1ef..f790ba68e 100644 --- a/frontend/src/stores/admin.js +++ b/frontend/src/stores/admin.js @@ -63,6 +63,7 @@ export const TABS = Object.freeze([ "Jobs", "Restore", "Stats", + "Doctor", ]); export const UNRESTRICTED_LABEL = "Adult"; @@ -87,6 +88,7 @@ export const useAdminStore = defineStore("admin", { }, timestamps: {}, stats: undefined, + doctor: undefined, taggingDefaults: undefined, emailSettings: undefined, oidcSettings: undefined, @@ -298,6 +300,15 @@ export const useAdminStore = defineStore("admin", { console.warn(error); } }, + async loadDoctor() { + if (this._requireAdmin()) return false; + try { + const response = await API.getDoctorReport(); + this.doctor = response.data; + } catch (error) { + console.warn(error); + } + }, async loadAllStatuses() { if (this._requireAdmin()) return false; try { diff --git a/frontend/tests/unit/doctor-tab.test.js b/frontend/tests/unit/doctor-tab.test.js new file mode 100644 index 000000000..77c0543ba --- /dev/null +++ b/frontend/tests/unit/doctor-tab.test.js @@ -0,0 +1,224 @@ +/* + * Tests for the admin Doctor tab. + * + * The tab is what an administrator sees of the doctor report, so behavior + * locked in here: + * - Every status the doctor can report wears the chip colour the + * component's own map says, so a status the backend adds without a + * colour fails here rather than rendering grey by accident. + * - comicbox's rows and codex's own sit under their own headings; a + * section header renders once among comicbox's rows, ahead of its + * first row, and the rows stay in report order. + * - The heading is the verdict, counting the problems in plain words and + * coloured by it; the hint carries the host line. + * - A fix is a shell command or a setting path, so it renders as code. + * - A report already in the store shows at once without a refetch; an + * empty store fetches on mount; Re-check always fetches. + */ +import { createTestingPinia } from "@pinia/testing"; +import { mount } from "@vue/test-utils"; +import { describe, expect, test } from "vitest"; + +import DoctorTab from "@/components/admin/tabs/doctor-tab.vue"; +import { STATUS_COLORS } from "@/components/admin/tabs/doctor-table.vue"; +import vuetify from "@/plugins/vuetify"; +import { useAdminStore } from "@/stores/admin"; + +const HEADER = ["comicbox 5.3.0", "Python 3.14.4", "Linux-6.1", "Docker"]; +const PDF_FIX = "pip install --force-reinstall 'comicbox-pdffile~=1.0'"; + +const COMICBOX_RESULTS = [ + { + section: "Archives", + name: "CBR", + status: "OK", + found: "rarfile 4.5", + detail: "via unrar · probe read OK · crypto: cryptography", + fix: "", + }, + { + section: "Archives", + name: "PDF", + status: "MISCONFIGURED", + found: "comicbox-pdffile 1.0.0", + detail: "pdffile fails to import: ImportError(...)", + fix: PDF_FIX, + }, + { + section: "Images (online cover matching)", + name: "Pillow", + status: "OK", + found: "Pillow 12.3.0", + detail: "jpeg webp png gif · no jxl codec", + fix: "", + }, + { + section: "Config", + name: "user config", + status: "OK", + found: "~/.config/comicbox/config.yaml", + detail: "parsed", + fix: "", + }, + { + section: "Config", + name: "unknown key", + status: "WARN", + found: "~/.config/comicbox/config.yaml", + detail: "general.loglevl is ignored", + fix: "did you mean general.loglevel?", + }, +]; +const CODEX_RESULTS = [ + { + section: "Codex", + name: "library", + status: "OK", + found: "/comics", + detail: "readable · writable", + fix: "", + }, + { + section: "Codex", + name: "library", + status: "WARN", + found: "/comics-empty", + detail: "readable · read only · empty: suspect unmounted", + fix: "mount it, or add comics", + }, +]; +const RESULTS = [...COMICBOX_RESULTS, ...CODEX_RESULTS]; + +const REPORT = { + header: HEADER, + comicbox: COMICBOX_RESULTS, + codex: CODEX_RESULTS, + problems: 1, +}; + +// One row per status the component knows, so the chip test enumerates the +// vocabulary from the component instead of keeping a second copy here. +const EVERY_STATUS_REPORT = { + header: HEADER, + comicbox: Object.keys(STATUS_COLORS).map((status) => ({ + section: "Statuses", + name: status.toLowerCase(), + status, + found: "", + detail: "", + fix: "", + })), + codex: [], + problems: 4, +}; + +const COLOR_CLASSES = new Set( + Object.values(STATUS_COLORS) + .filter(Boolean) + .map((color) => `text-${color}`), +); + +function mountTab(doctor) { + const pinia = createTestingPinia({ initialState: { admin: { doctor } } }); + const wrapper = mount(DoctorTab, { + global: { plugins: [pinia, vuetify] }, + }); + return { wrapper, store: useAdminStore() }; +} + +describe("AdminDoctorTab", () => { + test("every status wears its chip colour", () => { + const chips = mountTab(EVERY_STATUS_REPORT).wrapper.findAll(".v-chip"); + expect(chips).toHaveLength(Object.keys(STATUS_COLORS).length); + for (const chip of chips) { + const status = chip.text(); + const color = STATUS_COLORS[status]; + const colorClasses = chip + .classes() + .filter((cls) => COLOR_CLASSES.has(cls)); + // A tonal chip carries exactly its colour as text-; an + // uncoloured one carries none, which is the default grey. + expect(colorClasses).toStrictEqual(color ? [`text-${color}`] : []); + } + }); + + test("comicbox and codex rows sit under their own headings", () => { + const { wrapper } = mountTab(REPORT); + const headings = wrapper.findAll("h4").map((el) => el.text()); + expect(headings).toStrictEqual(["comicbox", "Codex"]); + const tables = wrapper.findAll(".doctorTable"); + expect(tables).toHaveLength(2); + // comicbox's rows are grouped by section, each header once, ahead of + // its first row. + const [comicbox, codex] = tables; + expect( + comicbox.findAll(".doctorSectionRow").map((row) => row.text()), + ).toStrictEqual(["Archives", "Images (online cover matching)", "Config"]); + expect(comicbox.find("tbody tr").classes()).toContain("doctorSectionRow"); + // Codex's rows run flat under their heading: no "Codex" row repeating it. + expect(codex.findAll(".doctorSectionRow")).toHaveLength(0); + // Rows stay in report order, repeated names included. + const names = wrapper.findAll(".doctorName").map((cell) => cell.text()); + expect(names).toStrictEqual(RESULTS.map((result) => result.name)); + }); + + test.each([ + { problems: 0, verdict: "No problems", cls: "doctorOk" }, + { problems: 1, verdict: "1 problem", cls: "doctorProblems" }, + { problems: 3, verdict: "3 problems", cls: "doctorProblems" }, + ])( + "the heading is the verdict for $problems problems", + ({ problems, verdict, cls }) => { + const { wrapper } = mountTab({ ...REPORT, problems }); + expect(wrapper.find("h3").text()).toBe(verdict); + expect(wrapper.find(".adminGroup").classes()).toContain(cls); + }, + ); + + test("the hint is the host line, not the verdict", () => { + const hint = mountTab(REPORT).wrapper.find(".adminHint").text(); + expect(hint).toBe(HEADER.join(" · ")); + }); + + test("renders a fix as code and an empty fix as nothing", () => { + const { wrapper } = mountTab(REPORT); + const fixes = wrapper.findAll(".doctorFix"); + expect(fixes).toHaveLength(RESULTS.length); + const codes = wrapper.findAll(".doctorFix code").map((el) => el.text()); + expect(codes).toStrictEqual([ + PDF_FIX, + "did you mean general.loglevel?", + "mount it, or add comics", + ]); + expect(fixes[0].find("code").exists()).toBe(false); + expect(fixes[0].text()).toBe(""); + }); + + test("renders found and detail", () => { + const text = mountTab(REPORT).wrapper.text(); + expect(text).toContain("rarfile 4.5"); + expect(text).toContain("general.loglevl is ignored"); + expect(text).toContain("/comics-empty"); + }); + + test("shows Checking… and no table before the report arrives", () => { + const { wrapper } = mountTab(); + expect(wrapper.find("h3").text()).toBe("Checking…"); + expect(wrapper.find(".adminGroup").classes()).not.toContain("doctorOk"); + expect(wrapper.find(".adminHint").exists()).toBe(false); + expect(wrapper.find(".doctorTable").exists()).toBe(false); + expect(wrapper.find("h4").exists()).toBe(false); + expect(wrapper.find(".v-btn").exists()).toBe(true); + }); + + test("fetches on mount only when the store is empty", () => { + expect(mountTab(REPORT).store.loadDoctor).not.toHaveBeenCalled(); + expect(mountTab().store.loadDoctor).toHaveBeenCalledTimes(1); + }); + + test("Re-check always fetches", async () => { + const { wrapper, store } = mountTab(REPORT); + await wrapper.find(".v-btn").trigger("click"); + expect(store.loadDoctor).toHaveBeenCalledTimes(1); + }); +}); diff --git a/pyproject.toml b/pyproject.toml index f3078c0a5..b49302720 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ dependencies = [ "asgiref~=3.12", "bidict~=0.23", "channels~=4.2", - "comicbox[pdf]~=5.3.0", + "comicbox[pdf]~=5.3.1", "cryptography>=48.0.0", "dateparser~=1.2", "django-allauth[socialaccount]~=65.13", diff --git a/tests/test_admin_custom_cover.py b/tests/test_admin_custom_cover.py index b7cc3e5c7..857acdfd4 100644 --- a/tests/test_admin_custom_cover.py +++ b/tests/test_admin_custom_cover.py @@ -80,7 +80,7 @@ def test_upload_happy_path(self, mock_queue) -> None: pk = _v4(response)["customCoverPk"] cover = CustomCover.objects.get(pk=pk) assert cover.collection == "publishers" - assert cover.library_id is None # pyright: ignore[reportAttributeAccessIssue] + assert cover.library_id is None # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] assert cover.path.startswith(str(CUSTOM_COVERS_UPLOADS_DIR)) # Naming convention: ``{collection}-{pk}-{slug}.{ext}``. Sortable by # collection on disk and trivially scannable for a given linked diff --git a/tests/test_admin_doctor_endpoint.py b/tests/test_admin_doctor_endpoint.py new file mode 100644 index 000000000..d09773962 --- /dev/null +++ b/tests/test_admin_doctor_endpoint.py @@ -0,0 +1,106 @@ +"""Integration tests for the /admin/doctor endpoint.""" + +from pathlib import Path +from typing import Final, override +from unittest.mock import patch + +from comicbox.doctor import CheckResult, Status +from django.contrib.auth.models import User +from django.test import Client, TestCase + +from codex.doctor import DoctorReport + +_TEST_PASSWORD: Final = "test-pw-hush-S106" # noqa: S105 +_URL: Final = "/api/v4/admin/doctor" +_HTTP_OK: Final = 200 +_HTTP_FORBIDDEN: Final = 403 + +_REPORT = DoctorReport( + header=("comicbox 5.3.0", "Python 3.14.4", "Linux-6.1", "Docker"), + comicbox=( + CheckResult( + "Archives", "CBR", Status.OK, found="rarfile 4.5", detail="via unrar" + ), + CheckResult( + "Archives", + "PDF", + Status.WRONG_VERSION, + found="comicbox-pdffile 0.6.3", + detail="comicbox requires comicbox-pdffile~=1.0", + fix="pip install 'comicbox-pdffile~=1.0'", + ), + CheckResult( + "Config", + "user config", + Status.OK, + found=Path("/home/abc/.config/comicbox/config.yaml"), + detail="parsed", + ), + ), + codex=( + CheckResult( + "Codex", + "library", + Status.MISSING, + found=Path("/comics"), + detail="not there: its comics look deleted until it is back", + fix="mount it, or remove the library on the Libraries tab", + ), + ), +) + + +def _data(response) -> dict: + """Unwrap the v4 ``{data, meta, errors}`` envelope.""" + return response.json()["data"] + + +class DoctorAuthTestCase(TestCase): + """The report names paths and versions, so only admins read it.""" + + def test_anonymous_blocked(self) -> None: + assert Client().get(_URL).status_code == _HTTP_FORBIDDEN + + def test_non_admin_blocked(self) -> None: + User.objects.create_user(username="regular", password=_TEST_PASSWORD) + client = Client() + client.login(username="regular", password=_TEST_PASSWORD) + assert client.get(_URL).status_code == _HTTP_FORBIDDEN + + +class DoctorReportTestCase(TestCase): + """The rows arrive as plain strings with a problem count.""" + + @override + def setUp(self) -> None: + User.objects.create_user( + username="doctor_admin", + password=_TEST_PASSWORD, + is_staff=True, + is_superuser=True, + ) + self.client.login(username="doctor_admin", password=_TEST_PASSWORD) + + def test_report(self) -> None: + with patch("codex.views.admin.doctor.run_doctor", return_value=_REPORT): + response = self.client.get(_URL) + assert response.status_code == _HTTP_OK + data = _data(response) + assert data["header"] == list(_REPORT.header) + assert data["problems"] == _REPORT.problems + pdf = data["comicbox"][1] + assert pdf == { + "section": "Archives", + "name": "PDF", + # The enum's spelling, space included, not its member name. + "status": "WRONG VERSION", + "found": "comicbox-pdffile 0.6.3", + "detail": "comicbox requires comicbox-pdffile~=1.0", + "fix": "pip install 'comicbox-pdffile~=1.0'", + } + # A Path renders as its string. + assert data["comicbox"][2]["found"] == "/home/abc/.config/comicbox/config.yaml" + assert data["comicbox"][0]["fix"] == "" + # Codex's own rows arrive apart, for their own heading. + assert [row["name"] for row in data["codex"]] == ["library"] + assert data["codex"][0]["found"] == "/comics" diff --git a/tests/test_credential_validator.py b/tests/test_credential_validator.py index 79cb0344a..4b8aef846 100644 --- a/tests/test_credential_validator.py +++ b/tests/test_credential_validator.py @@ -1,8 +1,13 @@ """Unit tests for the online-source credential validator.""" -from typing import Final +from dataclasses import replace +from pathlib import Path +from typing import Any, Final from unittest.mock import MagicMock, patch +from comicbox.config.online.settings import OnlineSourceCredentials +from comicbox.formats.comicvine_api.online_source import ComicVineOnlineSource +from comicbox.formats.metron_api.online_source import MetronOnlineSource from comicbox.online_session import OnlineCredentials from codex.librarian.onlinetag.credential_validator import ( @@ -12,6 +17,23 @@ ValidationResult, validate_credentials, ) +from codex.settings import COMICBOX_ONLINE_CONFIG + +# What Metron's probe returns off the validation response's X-RateLimit-* +# headers. The reset epochs are what codex deliberately drops. +_METRON_WINDOWS: Final = { + "burst": {"limit": 20, "remaining": 19, "reset_epoch": None}, + "daily": {"limit": 25_000, "remaining": 24_987, "reset_epoch": 1_784_419_200.0}, +} +# A response that carried no X-RateLimit-* headers at all. +_EMPTY_METRON_WINDOWS: Final = { + "burst": {"limit": None, "remaining": None, "reset_epoch": None}, + "daily": {"limit": None, "remaining": None, "reset_epoch": None}, +} +# Comic Vine's per-endpoint hourly pools; never shown, whatever they hold. +_COMICVINE_WINDOWS: Final = { + "origins": {"limit": 200, "remaining": 199, "reset_epoch": 1_784_419_200.0}, +} def _full_creds() -> OnlineCredentials: @@ -26,6 +48,17 @@ def _legacy_creds() -> OnlineCredentials: ) +def _probe(source_cls: type, **kwargs: Any): + """Patch ``source_cls.probe``; autospec so the mock is called with ``self``.""" + return patch.object(source_cls, "probe", autospec=True, **kwargs) + + +def _probed_source(probe: MagicMock) -> Any: + """Return the source instance the (autospecced) probe ran on.""" + probe.assert_called_once() + return probe.call_args.args[0] + + class TestValidateCredentials: """Cover success, auth-error, transport-error, and missing-creds paths.""" @@ -33,76 +66,21 @@ def test_known_sources_set(self) -> None: assert frozenset({"metron", "comicvine"}) == KNOWN_SOURCES def test_metron_success(self) -> None: - from mokkari.session import RateLimitStatus - - with patch("mokkari.session.Session") as mocked: - instance = MagicMock() - instance.publishers_list.return_value = [] - # A session that saw no X-RateLimit-* headers -> rate_limits None. - instance.rate_limit_status = RateLimitStatus() - mocked.return_value = instance + with _probe(MetronOnlineSource, return_value=_EMPTY_METRON_WINDOWS) as probe: results = validate_credentials(_full_creds(), {"metron"}) assert results == {"metron": ValidationResult(ok=True)} - instance.publishers_list.assert_called_once_with({"page": 1}) - assert mocked.call_args.kwargs["api_token"] == "token" # noqa: S105 - # Since mokkari 4.8.0 the session holds a pooled TLS connection - # until closed, and this one is used once in the web process. - instance.close.assert_called_once_with() + source = _probed_source(probe) + assert source._credentials == OnlineSourceCredentials(key="token") # noqa: SLF001 - def test_metron_legacy_login_still_validates(self) -> None: - """A stored username & password authenticates when no API key is set.""" - from mokkari.session import RateLimitStatus - - with patch("mokkari.session.Session") as mocked: - instance = MagicMock() - instance.publishers_list.return_value = [] - instance.rate_limit_status = RateLimitStatus() - mocked.return_value = instance - results = validate_credentials(_legacy_creds(), {"metron"}) + def test_metron_success_without_windows(self) -> None: + """A source that keeps no windows returns None; that is still a pass.""" + with _probe(MetronOnlineSource, return_value=None): + results = validate_credentials(_full_creds(), {"metron"}) assert results == {"metron": ValidationResult(ok=True)} - kwargs = mocked.call_args.kwargs - # None, not "" — mokkari sends a Bearer header for any non-None token, - # which would shut off the username & password fallback. - assert kwargs["api_token"] is None - assert kwargs["username"] == "user" - - def test_metron_key_wins_over_legacy_login(self) -> None: - """Both stored: the token is what mokkari gets told to prefer.""" - from mokkari.session import RateLimitStatus - - creds = OnlineCredentials( - metron_key="token", - metron_user="user", - metron_password="pw", # noqa: S106 - ) - with patch("mokkari.session.Session") as mocked: - instance = MagicMock() - instance.publishers_list.return_value = [] - instance.rate_limit_status = RateLimitStatus() - mocked.return_value = instance - results = validate_credentials(creds, {"metron"}) - assert results["metron"].ok is True - assert mocked.call_args.kwargs["api_token"] == "token" # noqa: S105 def test_metron_success_reports_rate_limits(self) -> None: """The validation response's live account limits ride along, sans reset.""" - from datetime import UTC, datetime - - from mokkari.session import RateLimitStatus, RateLimitWindow - - status = RateLimitStatus( - burst=RateLimitWindow(limit=20, remaining=19), - sustained=RateLimitWindow( - limit=25_000, - remaining=24_987, - reset=datetime(2026, 7, 19, tzinfo=UTC), - ), - ) - with patch("mokkari.session.Session") as mocked: - instance = MagicMock() - instance.publishers_list.return_value = [] - instance.rate_limit_status = status - mocked.return_value = instance + with _probe(MetronOnlineSource, return_value=_METRON_WINDOWS): results = validate_credentials(_full_creds(), {"metron"}) assert results == { "metron": ValidationResult( @@ -114,95 +92,142 @@ def test_metron_success_reports_rate_limits(self) -> None: ) } + def test_metron_partial_windows_keep_what_was_reported(self) -> None: + windows = {"daily": {"limit": 5_000, "remaining": None, "reset_epoch": None}} + with _probe(MetronOnlineSource, return_value=windows): + results = validate_credentials(_full_creds(), {"metron"}) + assert results["metron"].rate_limits == RateLimitInfo( + sustained=RateLimitWindowInfo(limit=5_000) + ) + + def test_metron_legacy_login_still_validates(self) -> None: + """A stored username & password authenticates when no API key is set.""" + with _probe(MetronOnlineSource, return_value=_EMPTY_METRON_WINDOWS) as probe: + results = validate_credentials(_legacy_creds(), {"metron"}) + assert results == {"metron": ValidationResult(ok=True)} + # key is None, not "" — mokkari sends a Bearer header for any non-None + # token, which would shut off the username & password fallback. + assert _probed_source(probe)._credentials == OnlineSourceCredentials( # noqa: SLF001 + user="user", + password="pw", # noqa: S106 + ) + + def test_metron_key_wins_over_legacy_login(self) -> None: + """Both stored: the token reaches mokkari, which prefers it.""" + creds = OnlineCredentials( + metron_key="token", + metron_user="user", + metron_password="pw", # noqa: S106 + ) + with _probe(MetronOnlineSource, return_value=_EMPTY_METRON_WINDOWS) as probe: + results = validate_credentials(creds, {"metron"}) + assert results["metron"].ok is True + assert _probed_source(probe)._credentials == OnlineSourceCredentials( # noqa: SLF001 + key="token", + user="user", + password="pw", # noqa: S106 + ) + def test_metron_auth_failure(self) -> None: from mokkari.exceptions import AuthenticationError - with patch("mokkari.session.Session") as mocked: - instance = MagicMock() - instance.publishers_list.side_effect = AuthenticationError() - mocked.return_value = instance + with _probe(MetronOnlineSource, side_effect=AuthenticationError()): results = validate_credentials(_full_creds(), {"metron"}) assert results["metron"].ok is False assert "authoriz" in (results["metron"].error or "").lower() - # A rejected key leaves a connection open just the same. - instance.close.assert_called_once_with() def test_metron_api_error(self) -> None: from mokkari.exceptions import ApiError - with patch("mokkari.session.Session") as mocked: - instance = MagicMock() - instance.publishers_list.side_effect = ApiError("boom") - mocked.return_value = instance + with _probe(MetronOnlineSource, side_effect=ApiError("boom")): results = validate_credentials(_full_creds(), {"metron"}) assert results["metron"] == ValidationResult(ok=False, error="boom") + def test_error_text_is_the_exceptions_first_line(self) -> None: + """Some upstream errors carry a whole HTML page; the admin sees one line.""" + from mokkari.exceptions import ApiError + + err = ApiError("502 Bad Gateway\nnginx") + with _probe(MetronOnlineSource, side_effect=err): + results = validate_credentials(_full_creds(), {"metron"}) + assert results["metron"] == ValidationResult(ok=False, error="502 Bad Gateway") + def test_metron_missing_creds(self) -> None: creds = OnlineCredentials(metron_key="", metron_user="", metron_password="") - results = validate_credentials(creds, {"metron"}) + with _probe(MetronOnlineSource) as probe: + results = validate_credentials(creds, {"metron"}) assert results["metron"].ok is False assert "api key required" in (results["metron"].error or "").lower() + probe.assert_not_called() def test_metron_half_a_legacy_login_is_not_enough(self) -> None: creds = OnlineCredentials(metron_user="user", metron_password="") - results = validate_credentials(creds, {"metron"}) + with _probe(MetronOnlineSource) as probe: + results = validate_credentials(creds, {"metron"}) assert results["metron"].ok is False assert "api key required" in (results["metron"].error or "").lower() + probe.assert_not_called() def test_comicvine_success(self) -> None: - with patch("simyan.comicvine.Comicvine") as mocked: - instance = MagicMock() - instance.list_publishers.return_value = [] - mocked.return_value = instance + with _probe(ComicVineOnlineSource, return_value=_COMICVINE_WINDOWS) as probe: results = validate_credentials(_full_creds(), {"comicvine"}) + # Comic Vine's hourly pools never become the per-minute/per-day chip. assert results == {"comicvine": ValidationResult(ok=True)} - instance.list_publishers.assert_called_once_with( - params={"limit": "1"}, max_results=1 - ) + source = _probed_source(probe) # No override means simyan's own default endpoint. - assert "base_url" not in mocked.call_args.kwargs + assert source._credentials == OnlineSourceCredentials(key="key") # noqa: SLF001 + # Codex's own online settings, so the probe is counted against the + # same shared rate-limit bucket tagging runs draw on. + assert source._settings is COMICBOX_ONLINE_CONFIG.online # noqa: SLF001 def test_comicvine_custom_url_overrides_the_endpoint(self) -> None: - """A saved custom URL becomes simyan's base_url.""" + """A saved custom URL reaches the source as its base url.""" creds = OnlineCredentials( comicvine_key="key", comicvine_url="https://cv.example.com/api" ) - with patch("simyan.comicvine.Comicvine") as mocked: - instance = MagicMock() - instance.list_publishers.return_value = [] - mocked.return_value = instance + with _probe(ComicVineOnlineSource, return_value=_COMICVINE_WINDOWS) as probe: results = validate_credentials(creds, {"comicvine"}) assert results == {"comicvine": ValidationResult(ok=True)} - assert mocked.call_args.kwargs["base_url"] == "https://cv.example.com/api" + assert _probed_source(probe)._credentials == OnlineSourceCredentials( # noqa: SLF001 + key="key", url="https://cv.example.com/api" + ) def test_comicvine_url_alone_is_not_credentials(self) -> None: """A custom URL cannot authenticate; the key is still required.""" creds = OnlineCredentials(comicvine_url="https://cv.example.com/api") - results = validate_credentials(creds, {"comicvine"}) + with _probe(ComicVineOnlineSource) as probe: + results = validate_credentials(creds, {"comicvine"}) assert results["comicvine"].ok is False assert "required" in (results["comicvine"].error or "").lower() + probe.assert_not_called() def test_comicvine_auth_failure(self) -> None: from simyan.errors import AuthenticationError - with patch("simyan.comicvine.Comicvine") as mocked: - instance = MagicMock() - instance.list_publishers.side_effect = AuthenticationError("bad key") - mocked.return_value = instance + with _probe(ComicVineOnlineSource, side_effect=AuthenticationError("bad key")): results = validate_credentials(_full_creds(), {"comicvine"}) assert results["comicvine"] == ValidationResult(ok=False, error="bad key") - def test_comicvine_rejected_key_fails_through_the_real_simyan(self) -> None: + def test_comicvine_rejected_key_fails_through_the_real_simyan( + self, tmp_path: Path + ) -> None: """ A bad key must fail validation, not pass it. Comic Vine answers a rejected key with HTTP 200 and status_code 100 in the body. simyan 3.x returned that body verbatim, so the - publisher list came back empty and the key validated as good. - simyan 4 maps the status code to an exception, so this patches - the HTTP session rather than the client, and lets simyan's own - code decide. + listing came back empty and the key validated as good. simyan 4 + maps the status code to an exception, so this patches the HTTP + session rather than the client, and lets comicbox's real probe + and simyan's own code decide. The cache dir is pointed at a + scratch path so the probe's bucket and cache files stay out of + the developer's config dir. """ + online = COMICBOX_ONLINE_CONFIG.online + config = replace( + COMICBOX_ONLINE_CONFIG, + online=replace(online, cache=replace(online.cache, dir=tmp_path)), + ) response = MagicMock() response.raise_for_status.return_value = None response.json.return_value = { @@ -210,23 +235,55 @@ def test_comicvine_rejected_key_fails_through_the_real_simyan(self) -> None: "error": "Invalid API Key", "results": [], } - with patch("simyan.comicvine.CachedLimiterSession.request") as request: + with ( + patch( + "codex.librarian.onlinetag.credential_validator.COMICBOX_ONLINE_CONFIG", + config, + ), + patch("simyan.comicvine.CachedLimiterSession.request") as request, + ): request.return_value = response results = validate_credentials(_full_creds(), {"comicvine"}) assert results == { "comicvine": ValidationResult(ok=False, error="Invalid API Key") } + # The probe spent one request, on the origins pool tagging never uses. + request.assert_called_once() + assert "/origins" in request.call_args.kwargs["url"] + + def test_comicvine_rate_limit_exhausted(self) -> None: + """The probe refuses to block on a spent pool; the admin learns why.""" + from simyan.errors import RateLimitError + + err = RateLimitError("the hourly origins budget is spent") + with _probe(ComicVineOnlineSource, side_effect=err): + results = validate_credentials(_full_creds(), {"comicvine"}) + assert results["comicvine"] == ValidationResult( + ok=False, error="the hourly origins budget is spent" + ) + + def test_messageless_rate_limit_error_names_its_category(self) -> None: + """A 429 with no error body is ``RateLimitError(None)``, not "None".""" + from simyan.errors import RateLimitError + + with _probe(ComicVineOnlineSource, side_effect=RateLimitError(None)): + results = validate_credentials(_full_creds(), {"comicvine"}) + assert results["comicvine"] == ValidationResult(ok=False, error="Rate limited.") def test_comicvine_service_error(self) -> None: from simyan.errors import ServiceError - with patch("simyan.comicvine.Comicvine") as mocked: - instance = MagicMock() - instance.list_publishers.side_effect = ServiceError("upstream down") - mocked.return_value = instance + with _probe(ComicVineOnlineSource, side_effect=ServiceError("upstream down")): results = validate_credentials(_full_creds(), {"comicvine"}) assert results["comicvine"] == ValidationResult(ok=False, error="upstream down") + def test_unclassified_messageless_error_names_its_type(self) -> None: + with _probe(ComicVineOnlineSource, side_effect=ConnectionResetError()): + results = validate_credentials(_full_creds(), {"comicvine"}) + assert results["comicvine"] == ValidationResult( + ok=False, error="ConnectionResetError" + ) + def test_comicvine_missing_creds(self) -> None: creds = OnlineCredentials(comicvine_key="") results = validate_credentials(creds, {"comicvine"}) @@ -234,15 +291,10 @@ def test_comicvine_missing_creds(self) -> None: assert "required" in (results["comicvine"].error or "").lower() def test_default_targets_all_known_sources(self) -> None: - from mokkari.session import RateLimitStatus - with ( - patch("mokkari.session.Session") as metron_mock, - patch("simyan.comicvine.Comicvine") as cv_mock, + _probe(MetronOnlineSource, return_value=_EMPTY_METRON_WINDOWS), + _probe(ComicVineOnlineSource, return_value=_COMICVINE_WINDOWS), ): - metron_mock.return_value.publishers_list.return_value = [] - metron_mock.return_value.rate_limit_status = RateLimitStatus() - cv_mock.return_value.list_publishers.return_value = [] results = validate_credentials(_full_creds()) assert set(results) == KNOWN_SOURCES assert all(r.ok for r in results.values()) diff --git a/tests/test_dockerfile.py b/tests/test_dockerfile.py index 45b521b28..d751cbefe 100644 --- a/tests/test_dockerfile.py +++ b/tests/test_dockerfile.py @@ -110,3 +110,11 @@ def test_bytecode_is_kept(stages: dict[str, list[tuple[str, str]]]) -> None: ] assert installs assert all("--compile-bytecode" in command for command in installs) + + +def test_final_smoke_test_runs_the_doctor( + stages: dict[str, list[tuple[str, str]]], +) -> None: + """The doctor proves the image reads every archive format before it ships.""" + runs = _runs(stages["final"]) + assert [run for run in runs if "comicbox doctor -q" in run] diff --git a/tests/test_doctor.py b/tests/test_doctor.py new file mode 100644 index 000000000..4011f4caf --- /dev/null +++ b/tests/test_doctor.py @@ -0,0 +1,133 @@ +""" +The doctor as codex runs it. + +comicbox's report is comicbox's; what codex adds is dropping the Online +section, appending its own rows, counting the problems, and logging them +once at startup. +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from comicbox.doctor import CheckResult, DoctorReport, Status +from comicbox.doctor.online import SECTION as ONLINE_SECTION +from django.test import TestCase + +from codex.doctor import log_doctor_problems, run_doctor +from codex.doctor.checks import SECTION as CODEX_SECTION + +_HEADER = ("comicbox 5.3.0", "Python 3.14.4", "Linux-6.1") +_CBR_MISSING = CheckResult( + "Archives", + "CBR", + Status.MISSING, + found="rarfile 4.5", + detail="no RAR tool found: 'unrar' not on path", + fix="apt install unrar (Debian: enable non-free)", +) +_PDF_OK = CheckResult( + "Archives", "PDF", Status.OK, found="comicbox-pdffile 1.0.0", detail="pymupdf" +) +_UNKNOWN_KEY = CheckResult( + "Config", + "unknown key", + Status.WARN, + found=Path("/config/comicbox.yaml"), + detail="general.loglevl is ignored", + fix="did you mean general.loglevel?", +) +_METRON = CheckResult(ONLINE_SECTION, "metron", Status.OFF, detail="no credentials") +_REPORT = DoctorReport( + header=_HEADER, results=(_CBR_MISSING, _PDF_OK, _UNKNOWN_KEY, _METRON) +) +_DATABASE_OK = CheckResult(CODEX_SECTION, "database", Status.OK, detail="fts5 · wal") + + +def _patched(report: DoctorReport = _REPORT): + return patch("codex.doctor.run_checks", return_value=report) + + +def _codex_rows(*rows: CheckResult): + """Stand in for every codex check with fixed rows.""" + return patch("codex.doctor.CHECKS", (("codex", lambda: rows),)) + + +class RunDoctorTests(TestCase): + """What codex keeps of comicbox's report, and what it adds.""" + + def test_drops_the_online_section_and_keeps_codex_rows_apart(self) -> None: + """Codex checks credentials on the Tagging tab, from its own database.""" + with _patched(), _codex_rows(_DATABASE_OK): + report = run_doctor() + assert report.header == _HEADER + assert report.comicbox == (_CBR_MISSING, _PDF_OK, _UNKNOWN_KEY) + assert report.codex == (_DATABASE_OK,) + assert report.results == (_CBR_MISSING, _PDF_OK, _UNKNOWN_KEY, _DATABASE_OK) + + def test_problems_are_failures_not_warnings(self) -> None: + with _patched(), _codex_rows(_DATABASE_OK): + report = run_doctor() + assert report.problems == 1 + + def test_a_crashing_codex_check_is_one_error_row(self) -> None: + def explode(): + msg = "boom" + raise RuntimeError(msg) + + with _patched(), patch("codex.doctor.CHECKS", (("watcher", explode),)): + report = run_doctor() + assert report.codex == ( + CheckResult( + CODEX_SECTION, "watcher", Status.ERROR, detail="RuntimeError: boom" + ), + ) + + def test_the_real_doctor_runs(self) -> None: + """Unpatched: comicbox's checks and codex's run here, whatever they find.""" + report = run_doctor() + assert report.header + assert {"CBZ", "CBR", "PDF", "Pillow", "cover hash"} <= { + row.name for row in report.comicbox + } + assert {"database", "config dir", "library", "watcher", "credentials"} <= { + row.name for row in report.codex + } + assert not any(row.section == ONLINE_SECTION for row in report.comicbox) + + +class LogDoctorProblemsTests(TestCase): + """Failures warn with their fix, warnings inform, and a verdict closes.""" + + def test_failures_warn_with_the_fix(self) -> None: + log = MagicMock() + with _patched(), _codex_rows(_DATABASE_OK): + log_doctor_problems(log) + warnings = [call.args[0] for call in log.warning.call_args_list] + assert warnings == [ + ( + "doctor: CBR MISSING: no RAR tool found: 'unrar' not on path" + " Fix: apt install unrar (Debian: enable non-free)" + ), + "The doctor found 1 problem. See the Admin Doctor tab.", + ] + + def test_warnings_inform(self) -> None: + log = MagicMock() + with _patched(), _codex_rows(_DATABASE_OK): + log_doctor_problems(log) + infos = [call.args[0] for call in log.info.call_args_list] + assert infos == [ + ( + "doctor: unknown key WARN: general.loglevl is ignored" + " Fix: did you mean general.loglevel?" + ) + ] + + def test_a_clean_report_is_quiet(self) -> None: + log = MagicMock() + clean = DoctorReport(header=_HEADER, results=(_PDF_OK, _METRON)) + with _patched(clean), _codex_rows(_DATABASE_OK): + log_doctor_problems(log) + log.warning.assert_not_called() + log.info.assert_not_called() + log.debug.assert_called_once() diff --git a/tests/test_doctor_codex_checks.py b/tests/test_doctor_codex_checks.py new file mode 100644 index 000000000..ef0fd4d0b --- /dev/null +++ b/tests/test_doctor_codex_checks.py @@ -0,0 +1,259 @@ +""" +Codex's own doctor rows. + +Each check reads the host or the database and answers with comicbox's row +shape. These pin the verdicts: what counts as a failure, a warning, and an +optional feature that is simply off. +""" + +import os +import shutil +from pathlib import Path +from typing import override +from unittest.mock import patch + +from comicbox.doctor import Status +from cryptography.fernet import Fernet +from django.test import TestCase, override_settings + +from codex.doctor import checks +from codex.doctor.checks import ( + check_config_dir, + check_credentials, + check_database, + check_libraries, + check_watcher, +) +from codex.librarian.fs.mounted import DOCKER_UNMOUNTED_FN +from codex.models import ComicboxTaggingDefaults, Folder, Library +from tests.tmp_dirs import tmp_dir + +_ROOT = tmp_dir("codex.tests.doctor_checks") + + +class _DoctorCheckTestCase(TestCase): + """A fresh scratch directory per test.""" + + @override + def setUp(self) -> None: + self.scratch = _ROOT / self._testMethodName # pyright: ignore[reportUninitializedInstanceVariable] + self.scratch.mkdir(parents=True, exist_ok=True) + + @override + def tearDown(self) -> None: + shutil.rmtree(self.scratch, ignore_errors=True) + + def _library(self, name: str = "lib", *, comics: bool = True, **fields) -> Path: + root = self.scratch / name + root.mkdir(exist_ok=True) + if comics: + (root / "a.cbz").write_bytes(b"") + Library.objects.create(path=str(root), **fields) + return root + + +class DatabaseCheckTests(TestCase): + """The test database is the same engine the server runs on.""" + + def test_reports_the_engine(self) -> None: + (row,) = check_database() + assert row.name == "database" + assert str(row.found).startswith("sqlite ") + # FTS5 is what the search index is built on; the suite needs it too. + assert row.status is not Status.MISSING + assert "fts5" in row.detail + + +class ConfigDirCheckTests(_DoctorCheckTestCase): + """Writable with room is OK; unwritable is a failure.""" + + def test_writable_with_room(self) -> None: + with patch.object(checks, "CONFIG_PATH", self.scratch): + (row,) = check_config_dir() + assert row.status is Status.OK + assert row.found == self.scratch + assert "free" in row.detail + + def test_low_disk_warns(self) -> None: + with ( + patch.object(checks, "CONFIG_PATH", self.scratch), + patch.object(checks, "_LOW_DISK_BYTES", 1 << 62), + ): + (row,) = check_config_dir() + assert row.status is Status.WARN + assert "free some space" in row.fix + + def test_unwritable_is_misconfigured(self) -> None: + if os.geteuid() == 0: + self.skipTest("root writes anywhere") + self.scratch.chmod(0o500) + try: + with patch.object(checks, "CONFIG_PATH", self.scratch): + (row,) = check_config_dir() + finally: + self.scratch.chmod(0o700) + assert row.status is Status.MISCONFIGURED + assert "PUID" in row.fix + + +class LibrariesCheckTests(_DoctorCheckTestCase): + """One row per library root, in path order.""" + + def test_no_libraries_is_off(self) -> None: + (row,) = check_libraries() + assert row.status is Status.OFF + assert "Libraries tab" in row.fix + + def test_a_readable_writable_library(self) -> None: + root = self._library() + (row,) = check_libraries() + assert row.status is Status.OK + assert row.found == root + assert row.detail == "readable · writable" + + def test_read_only_is_not_asked_to_be_writable(self) -> None: + self._library(read_only=True) + (row,) = check_libraries() + assert row.status is Status.OK + assert row.detail == "readable · read only" + + def test_unwritable_warns_about_tag_writes(self) -> None: + if os.geteuid() == 0: + self.skipTest("root writes anywhere") + root = self._library() + root.chmod(0o500) + try: + (row,) = check_libraries() + finally: + root.chmod(0o700) + assert row.status is Status.WARN + assert "tag writes" in row.detail + assert "read only" in row.fix + + def test_missing_root(self) -> None: + Library.objects.create(path=str(self.scratch / "gone")) + (row,) = check_libraries() + assert row.status is Status.MISSING + assert "deleted" in row.detail + + def test_unmounted_docker_volume(self) -> None: + root = self._library(comics=False) + (root / DOCKER_UNMOUNTED_FN).touch() + (row,) = check_libraries() + assert row.status is Status.MISCONFIGURED + assert "bind mount" in row.fix + + def test_empty_root_warns(self) -> None: + self._library(comics=False) + (row,) = check_libraries() + assert row.status is Status.WARN + assert "empty" in row.detail + + def test_rows_follow_path_order(self) -> None: + self._library("b") + self._library("a") + rows = list(check_libraries()) + assert [row.found for row in rows] == [self.scratch / "a", self.scratch / "b"] + + +class WatcherCheckTests(_DoctorCheckTestCase): + """Folder count against the kernel's inotify watch limit, on Linux.""" + + def _watched_library(self, folders: int) -> None: + root = self._library() + library = Library.objects.get(path=str(root)) + for index in range(folders): + # Folder rows stat their path on save, so the directory must exist. + (root / str(index)).mkdir() + Folder.objects.create( + library=library, path=f"{root}/{index}", name=str(index) + ) + + def _limits(self, max_watches: int) -> Path: + inotify = self.scratch / "inotify" + inotify.mkdir() + (inotify / "max_user_watches").write_text(f"{max_watches}\n") + return inotify + + def test_no_watched_libraries_is_off(self) -> None: + self._library(events=False) + (row,) = check_watcher() + assert row.status is Status.OFF + + def test_other_platforms_have_no_limit(self) -> None: + self._watched_library(3) + with patch.object(checks, "_HAS_INOTIFY", new=False): + (row,) = check_watcher() + assert row.status is Status.OK + assert row.found == "4 folders" + + def test_within_the_limit(self) -> None: + self._watched_library(3) + inotify = self._limits(8192) + with ( + patch.object(checks, "_HAS_INOTIFY", new=True), + patch.object(checks, "_INOTIFY_DIR", inotify), + ): + (row,) = check_watcher() + assert row.status is Status.OK + assert row.detail == "inotify allows 8192" + + def test_near_the_limit_warns(self) -> None: + self._watched_library(9) + inotify = self._limits(12) + with ( + patch.object(checks, "_HAS_INOTIFY", new=True), + patch.object(checks, "_INOTIFY_DIR", inotify), + ): + (row,) = check_watcher() + assert row.status is Status.WARN + assert row.fix.startswith("sysctl fs.inotify.max_user_watches=524288 on ") + + def test_over_the_limit_is_misconfigured(self) -> None: + self._watched_library(9) + inotify = self._limits(4) + with ( + patch.object(checks, "_HAS_INOTIFY", new=True), + patch.object(checks, "_INOTIFY_DIR", inotify), + patch.object(checks, "is_docker", return_value=True), + ): + (row,) = check_watcher() + assert row.status is Status.MISCONFIGURED + assert row.fix.endswith(" on the Docker host") + + def test_unreadable_limit_warns(self) -> None: + self._watched_library(1) + with ( + patch.object(checks, "_HAS_INOTIFY", new=True), + patch.object(checks, "_INOTIFY_DIR", self.scratch / "nope"), + ): + (row,) = check_watcher() + assert row.status is Status.WARN + assert "unreadable" in row.detail + + +class CredentialsCheckTests(TestCase): + """Stored credentials must decrypt with the key file in the config dir.""" + + def test_none_stored_is_off(self) -> None: + ComicboxTaggingDefaults.objects.update_or_create(pk=1) + (row,) = check_credentials() + assert row.status is Status.OFF + + def test_stored_credentials_decrypt(self) -> None: + ComicboxTaggingDefaults.objects.update_or_create( + pk=1, defaults={"metron_key": "token", "comicvine_key": "cv"} + ) + (row,) = check_credentials() + assert row.status is Status.OK + assert row.found == "metron_key, comicvine_key" + + def test_a_changed_key_file_is_misconfigured(self) -> None: + ComicboxTaggingDefaults.objects.update_or_create( + pk=1, defaults={"metron_key": "token", "metron_user": "u"} + ) + with override_settings(FIELD_ENCRYPTION_KEY=Fernet.generate_key()): + (row,) = check_credentials() + assert row.status is Status.MISCONFIGURED + assert row.found == "metron_key, metron_user" + assert "Tagging tab" in row.fix diff --git a/tests/test_fs_filters_comic_suffixes.py b/tests/test_fs_filters_comic_suffixes.py new file mode 100644 index 000000000..7ae698c7e --- /dev/null +++ b/tests/test_fs_filters_comic_suffixes.py @@ -0,0 +1,45 @@ +""" +The scanner's comic suffixes come from comicbox, not from this host's tools. + +Leaving a suffix out of the scan because its tool is missing made every +existing row of that type look deleted to the poller. comicbox 5.3.0's +RAR probe test-extracts a member, so it fails on more hosts than the path +check it replaced, and this is where that must not cost anyone a library. +""" + +from pathlib import Path +from unittest.mock import patch + +import pytest +from comicbox.box import Comicbox +from comicbox.enums.comicbox import FileTypeEnum + +from codex.librarian.fs.filters import COMIC_SUFFIXES, match_comic + + +def test_suffixes_are_every_comicbox_file_type() -> None: + """A format comicbox adds is scanned without anyone editing a list here.""" + assert {f".{ft.value.lower()}" for ft in FileTypeEnum} == COMIC_SUFFIXES + assert {".cbz", ".cbr", ".cb7", ".cbt", ".pdf"} == COMIC_SUFFIXES + + +@pytest.mark.parametrize("name", ["a.cbz", "a.CBR", "a.cb7", "a.Cbt", "a.PDF"]) +def test_every_file_type_matches_in_any_case(name: str) -> None: + """The importer stores whatever case the filesystem had.""" + assert match_comic(Path(name)) + + +@pytest.mark.parametrize("name", ["a.jpg", "a.zip", "a.cbz.part", "noext", ".cbz"]) +def test_other_files_do_not_match(name: str) -> None: + """Only a whole, known suffix is a comic.""" + assert not match_comic(Path(name)) + + +def test_a_missing_tool_does_not_hide_its_comics() -> None: + """Unreadable archives fail their import with the reason; they are still scanned.""" + with ( + patch.object(Comicbox, "is_unrar_supported", return_value=False), + patch.object(Comicbox, "is_pdf_supported", return_value=False), + ): + assert match_comic(Path("/comics/a.cbr")) + assert match_comic(Path("/comics/a.pdf")) diff --git a/tests/test_janitor_folder_relations.py b/tests/test_janitor_folder_relations.py index c27c4e1f0..be68a7a3a 100644 --- a/tests/test_janitor_folder_relations.py +++ b/tests/test_janitor_folder_relations.py @@ -17,8 +17,8 @@ from django.test import TestCase from loguru import logger +from codex.librarian.fs.filters import COMIC_SUFFIXES from codex.librarian.scribe.janitor.integrity.foreign_keys import ( - _COMIC_SUFFIXES, _is_comic_path, fix_folder_relations, ) @@ -235,8 +235,8 @@ class ComicSuffixTests(TestCase): def test_suffixes_are_derived_from_comicbox(self) -> None: """A new archive format in comicbox must not be left behind here.""" - assert frozenset({".cbz", ".cbr", ".cb7", ".cbt", ".pdf"}) == _COMIC_SUFFIXES, ( - _COMIC_SUFFIXES + assert frozenset({".cbz", ".cbr", ".cb7", ".cbt", ".pdf"}) == COMIC_SUFFIXES, ( + COMIC_SUFFIXES ) def test_comic_paths_are_recognized_in_any_case(self) -> None: diff --git a/tests/test_onlinetag_credentials.py b/tests/test_onlinetag_credentials.py index f7f2b28ff..7fdbe42be 100644 --- a/tests/test_onlinetag_credentials.py +++ b/tests/test_onlinetag_credentials.py @@ -22,6 +22,9 @@ def test_metron_credentials_accept_a_key_or_a_legacy_login(self) -> None: legacy = manager._build_credentials() # noqa: SLF001 assert legacy is not None assert legacy.metron_user == "u" + # None, not "": mokkari sends a Bearer header for any token that is + # not None, which would replace the username and password. + assert legacy.metron_key is None assert manager._source_has_credentials(legacy, "metron") is True # noqa: SLF001 ComicboxTaggingDefaults.objects.update_or_create( @@ -31,6 +34,7 @@ def test_metron_credentials_accept_a_key_or_a_legacy_login(self) -> None: keyed = manager._build_credentials() # noqa: SLF001 assert keyed is not None assert keyed.metron_key == "t" + assert keyed.metron_user is None assert manager._source_has_credentials(keyed, "metron") is True # noqa: SLF001 def test_no_metron_credentials_at_all_is_unconfigured(self) -> None: diff --git a/tests/test_onlinetag_match_mode_ask.py b/tests/test_onlinetag_match_mode_ask.py new file mode 100644 index 000000000..2fb0a809d --- /dev/null +++ b/tests/test_onlinetag_match_mode_ask.py @@ -0,0 +1,86 @@ +""" +The match modes codex offers are comicbox's, ask included. + +comicbox 5.3.1 accepts ``MatchMode.ASK`` from a session with a prompt +handler, which codex always supplies, so the mode that prompts for every +match is offered end to end: the admin default, the launcher, the start +endpoint and the session comicbox runs. +""" + +from unittest.mock import patch + +from comicbox.config.online import MatchMode +from django.test import SimpleTestCase + +from codex.choices.admin import TAGGING_CHOICES +from codex.librarian.onlinetag.tasks import BulkOnlineTagTask +from codex.models import ComicboxTaggingDefaults +from codex.serializers.admin.tagging import OnlineTagStartSerializer +from tests.onlinetag_session_fakes import ( + PATCH_TARGET, + FakePassRunner, + FakeSession, + OnlineTagSessionTestCase, + double, + make_comic, +) + + +class MatchModeChoicesTests(SimpleTestCase): + """Every mode comicbox has, in its order, and nothing comicbox lacks.""" + + def test_the_model_choices_are_comicboxs_enum(self) -> None: + values = [choice.value for choice in ComicboxTaggingDefaults.MatchModeChoices] + assert values == [mode.value for mode in MatchMode] + assert "ask" in values + + def test_the_ui_choices_are_comicboxs_enum(self) -> None: + assert list(TAGGING_CHOICES["matchMode"]) == [mode.value for mode in MatchMode] + assert TAGGING_CHOICES["matchMode"] == { + mode.value: mode.value.capitalize() for mode in MatchMode + } + + def test_the_default_is_still_auto(self) -> None: + field = ComicboxTaggingDefaults._meta.get_field("default_match_mode") + assert field.default == "auto" + + +class StartSerializerModeTests(SimpleTestCase): + """The start endpoint refuses a mode comicbox would reject in the librarian.""" + + @staticmethod + def _validate(mode: str) -> OnlineTagStartSerializer: + serializer = OnlineTagStartSerializer( + data={"collection": "comics", "pks": ["1"], "mode": mode} + ) + serializer.is_valid() + return serializer + + def test_ask_is_accepted(self) -> None: + serializer = self._validate("ask") + assert not serializer.errors + assert serializer.validated_data["mode"] == "ask" + + def test_an_unknown_mode_is_rejected(self) -> None: + assert "mode" in self._validate("reckless").errors + + +class SessionMatchModeTests(OnlineTagSessionTestCase): + """The task's mode reaches comicbox as its enum member.""" + + def test_ask_reaches_the_session(self) -> None: + comic = make_comic() + self.manager._pass_runner = double(FakePassRunner()) # noqa: SLF001 + task = BulkOnlineTagTask( + comic_pks=frozenset({comic.pk}), + session_id="scan-ask", + sources=("metron",), + mode="ask", + ) + + with patch(PATCH_TARGET, FakeSession): + self.manager.run_session(task) + + assert FakeSession.last_kwargs["match"] is MatchMode.ASK + # The resolver comicbox requires for ask: codex's own prompt handler. + assert FakeSession.last_kwargs["prompt_handler"] is not None diff --git a/tests/test_onlinetag_prompt_resolution.py b/tests/test_onlinetag_prompt_resolution.py index 7ceb496ba..7d62b762b 100644 --- a/tests/test_onlinetag_prompt_resolution.py +++ b/tests/test_onlinetag_prompt_resolution.py @@ -24,6 +24,7 @@ from codex.librarian.onlinetag.statuses import USER_MATCHED from codex.librarian.onlinetag.tasks import OnlineTagPromptResponseTask from codex.librarian.scribe.tagwrite_errors import get_tag_write_errors +from codex.settings import COMICBOX_ONLINE_CONFIG from tests.onlinetag_session_fakes import ( APPLY_FETCH_TARGET, APPLY_SESSION_TARGET, @@ -215,6 +216,30 @@ def test_resolve_unmatched_result_does_not_write(self) -> None: assert not self.write_tasks() + def test_replay_runs_with_codex_online_config(self) -> None: + """ + The replay reads the cache the search filled. + + Built without ``config``, the session fell back to comicbox's own + defaults: the user's config file and a cache dir under platformdirs, + not the one under /config the search wrote to. + """ + comic = make_comic() + set_pending_prompts({"fp1": _prompt(comic, candidates=[{"source": "metron"}])}) + FakeSession.tag_results = [ + SimpleNamespace( + path=Path(comic.path), + tags={"series": "Existing"}, + error=None, + matched=False, + ) + ] + + with patch(APPLY_SESSION_TARGET, FakeSession): + self.manager.resolve_prompt("fp1", "choose", 0, None) + + assert FakeSession.last_kwargs["config"] is COMICBOX_ONLINE_CONFIG + def test_resolve_skip_drops_prompt_without_writing(self) -> None: set_pending_prompts( {"fp1": {"fingerprint": "fp1", "pk": 1, "path": "/c/1.cbz", "source": "x"}} diff --git a/tests/test_settings_defaults_seeding.py b/tests/test_settings_defaults_seeding.py index ea9c63230..a63bbafa8 100644 --- a/tests/test_settings_defaults_seeding.py +++ b/tests/test_settings_defaults_seeding.py @@ -121,7 +121,7 @@ def test_folders_default_lands_without_a_redirect(self) -> None: row = SettingsBrowser.objects.select_related("last_route").get( session_id=client.session.session_key ) - assert row.last_route.collection == "folders" # pyright: ignore[reportAttributeAccessIssue] + assert row.last_route.collection == "folders" # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] response = client.get("/api/v4/browse/folders") assert response.status_code == HTTPStatus.OK, response.content diff --git a/uv.lock b/uv.lock index 49c006740..ccb6ba86c 100644 --- a/uv.lock +++ b/uv.lock @@ -724,7 +724,7 @@ requires-dist = [ { name = "asgiref", specifier = "~=3.12" }, { name = "bidict", specifier = "~=0.23" }, { name = "channels", specifier = "~=4.2" }, - { name = "comicbox", extras = ["pdf"], specifier = "~=5.3.0" }, + { name = "comicbox", extras = ["pdf"], specifier = "~=5.3.1" }, { name = "cryptography", specifier = ">=48.0.0" }, { name = "dateparser", specifier = "~=1.2" }, { name = "django", specifier = "~=6.0" }, @@ -811,7 +811,7 @@ wheels = [ [[package]] name = "comicbox" -version = "5.3.0" +version = "5.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "bidict" }, @@ -846,9 +846,9 @@ dependencies = [ { name = "xmltodict" }, { name = "zipremove" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/ec/a0/d6f6e64d34b5affbdc19f03c83ffc5cf0d1622ccd5ba37919e10e4df8275/comicbox-5.3.0.tar.gz", hash = "sha256:c0fe6aef16156feb32593a5685fdc87aad49bea144cdffe0e7486b701cc9ff0b", size = 1291092, upload-time = "2026-10-07T01:23:14.326Z" } +sdist = { url = "https://files.pythonhosted.org/packages/c7/06/ca3834b40102d4c366951cb249d86a04363532acb767d67bfb9d5d432767/comicbox-5.3.1.tar.gz", hash = "sha256:7991b868c69325e90b35b5a76658a953c9c290178e2d10d6f995be67a70ec54e", size = 1292543, upload-time = "2026-10-07T04:18:05.074Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/9d/9b/6f30886090ae33172606738c89f3ba3c8d4d092a645576b7dadb8647e22c/comicbox-5.3.0-py3-none-any.whl", hash = "sha256:13a06b06d299cce28510c8b73dc68656a87ed30faf1d206dca6793f93c7cb1a6", size = 483746, upload-time = "2026-10-07T01:23:13.052Z" }, + { url = "https://files.pythonhosted.org/packages/f8/d5/a25d912358f6b2899e35216716d31dcfce539d25a2045f5aa28c33751fdb/comicbox-5.3.1-py3-none-any.whl", hash = "sha256:871950bf0baa664778cce0a119f7b074c007f800910e4ed229969dac4f1d15e3", size = 484073, upload-time = "2026-10-07T04:18:03.367Z" }, ] [package.optional-dependencies]