From 05bf29fe09660ea164dfe9a13f61a710a74debfc Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi <3247106+gantoine@users.noreply.github.com> Date: Fri, 2 Oct 2026 13:49:40 -0400 Subject: [PATCH] feat(converto): read per-file title ids with rom-converto during scans (#5031) * feat(converto): read per-file title ids with rom-converto during scans The title-id half of #5002: rom-converto's info runs fill rom_files.title_id and title_version, sigil fills what rom-converto can't read and keeps owning save targets, and roms in a scan batch share info runs of up to 64 files. A converto_read_at marker lets quick scans backfill files rom-converto has not read yet. converto.scan_metadata (config.yml only) turns it off. The other per-file fields and embedded images stay out until a UI reads them. Co-Authored-By: Claude Opus 5.5 (1M context) * refactor(converto): name the same-content check that keeps unread title ids Co-Authored-By: Claude Opus 5.5 (1M context) * refactor(converto): queue unread files in one pass over the built rows Co-Authored-By: Claude Opus 5.5 (1M context) * fix(converto): address Greptile review on #5031 - A file refresh now persists converto_read_at when rom-converto's read confirms a reused row's stored title id, so the file isn't re-read on every refresh. - RomConvertoService.read_infos splits its paths into info runs of at most INFO_BATCH_SIZE, so a rom with many files can't send them all through one run. Co-Authored-By: Claude Opus 5.5 (1M context) * fix(converto): apply code-review fixes - A stored file title id counts as rom-converto's only once rom-converto has read the row, so a stale sigil id can't override a fresh extraction. - Fresh hashes decide whether unread title ids still describe a file, ahead of a size and mtime a copy can preserve. - One rom's failed candidate listing no longer aborts its scan batch. - The info paths file's cleanup can't raise out of read_infos. - _rom_file_order sorts by Path like _TitleIdSource.order. - Restored the in-place reconcile test for _identify_rom. Co-Authored-By: Claude Opus 5.5 (1M context) * refactor(converto): apply simplify pass - sync_rom_files reads 'rom-converto read this pass' off converto_read_at instead of an inspected set threaded through ParsedRomFiles, FSRom, the scan socket and the refresh. - One _converto_may_read check serves the scan and the batch candidates. - converto_candidates reuses _list_rom_dir, validates paths off the event loop and is best-effort itself, so the scan socket gathers it plainly. - get_rom_files builds rom-converto paths from the validated rom dir instead of validating each file again. - _rom_level_identity returns early on sigil's extraction. Co-Authored-By: Claude Opus 5.5 (1M context) * chore(converto): apply review-polish pass Co-Authored-By: Claude Opus 5.5 (1M context) * refactor(converto): drop the shared info batch Sharing rom-converto info runs across a scan batch saved about 2 ms per rom (one process start) in exchange for a second directory walk per rebuilt rom, the RomConvertoInfoBatch bookkeeping and the scan socket wiring. Each rom now reads its own files; read_infos still caps a run at INFO_RUN_MAX_FILES. Co-Authored-By: Claude Opus 5.5 (1M context) * refactor(converto): name the rom-converto columns inline Drops ROM_FILE_CONVERTO_COLUMNS and ROM_FILE_CONVERTO_KEPT_COLUMNS, so _copy_scanned_columns keeps master's frozenset keep_when_unset. Co-Authored-By: Claude Opus 5.5 (1M context) --------- Co-authored-by: Claude Opus 5.5 (1M context) --- backend/adapters/services/rom_converto.py | 202 ++++- .../versions/0147_rom_file_title_ids.py | 37 + backend/config/config_manager.py | 5 + backend/endpoints/configs.py | 6 +- backend/handler/database/roms_handler.py | 37 +- backend/handler/filesystem/roms_handler.py | 129 +++- backend/handler/rom_files.py | 21 +- backend/models/rom.py | 7 + .../adapters/services/test_rom_converto.py | 577 +++++++++++++++ backend/tests/config/test_config_loader.py | 4 + backend/tests/endpoints/test_config.py | 5 +- .../handler/database/test_roms_handler.py | 132 ++++ .../handler/filesystem/test_roms_handler.py | 4 +- .../handler/filesystem/test_roms_title_ids.py | 687 ++++++++++++++++++ backend/tests/handler/test_rom_files.py | 115 +++ backend/tests/test_migrations.py | 18 + backend/tests/utils/test_switch.py | 46 ++ backend/utils/switch.py | 2 +- docs/BACKEND_ARCHITECTURE.md | 20 +- examples/config.example.yml | 3 + .../__generated__/models/ConvertoConfig.ts | 1 + .../models/ConvertoSettingsPayload.ts | 2 +- frontend/src/stores/config.ts | 1 + 23 files changed, 2017 insertions(+), 44 deletions(-) create mode 100644 backend/alembic/versions/0147_rom_file_title_ids.py create mode 100644 backend/tests/handler/filesystem/test_roms_title_ids.py diff --git a/backend/adapters/services/rom_converto.py b/backend/adapters/services/rom_converto.py index c37b0881c..29e44d56a 100644 --- a/backend/adapters/services/rom_converto.py +++ b/backend/adapters/services/rom_converto.py @@ -2,20 +2,44 @@ import asyncio import contextlib import json import os +import re import shutil import tempfile from dataclasses import dataclass +from itertools import batched from pathlib import Path -from typing import Final +from typing import Any, Final from config import ROM_CONVERTO_ENABLED, ROM_CONVERTO_TIMEOUT from logger.formatter import LIGHTMAGENTA from logger.formatter import highlight as hl from logger.logger import log +from models.rom import TITLE_ID_MAX_LENGTH from utils.filesystem import SERVED_FILE_MODE from utils.platform_slugs import UniversalPlatformSlug as UPS -# The capabilities probe must never hang download paths; a real +# Wider than the conversion targets: `info` only reads headers, so every +# platform it can pull a title id or serial from counts. +CONVERTO_PLATFORM_SLUGS: Final[frozenset[str]] = frozenset( + { + UPS.N3DS, + UPS.NDS, + UPS.PSP, + UPS.PSVITA, + UPS.PSX, + UPS.PS2, + UPS.PS3, + UPS.NGC, + UPS.WII, + UPS.WIIU, + UPS.SWITCH, + UPS.SWITCH_2, + UPS.XBOX, + UPS.XBOX360, + } +) + +# The capabilities probe must never hang scan/download paths; a real # manifest print is instant. _PROBE_TIMEOUT_SECONDS = 30 @@ -25,6 +49,11 @@ _BINARY: Final = "rom-converto" CUE_EXT: Final = ".cue" +# Bounds one run's output, which carries every file's embedded images as JSON. +INFO_RUN_MAX_FILES: Final = 64 + +_PSP_TITLE_ID_PATTERN: Final[re.Pattern[str]] = re.compile(r"[A-Z]{4}[0-9]{5}") + class RomConvertoError(Exception): ... @@ -47,6 +76,14 @@ class RomConvertoUnsafeSourceError(RomConvertoError): _LOSSY_ARGV: Final[frozenset[tuple[str, ...]]] = frozenset({("xbox", "convert")}) +@dataclass(frozen=True) +class RomConvertoInfo: + # Rendered the way sigil renders the same platform's id, so either + # extractor can fill `Rom.title_id` interchangeably. + title_id: str | None = None + title_version: int | None = None + + @dataclass(frozen=True) class Operation: """A subcommand bringing `input_exts` files to `target`; source and output are appended to `argv`.""" @@ -265,6 +302,24 @@ def _convert_argv(operation: Operation, src: Path, out: Path) -> list[str]: return [*operation.argv, os.path.abspath(src), os.path.abspath(out)] +def _listable(path: Path) -> bool: + """Whether `info` may read `path` from a paths file, which the CLI decodes as UTF-8 and trims per line.""" + name = str(path) + if "\n" in name or name != name.strip(): + return False + try: + name.encode("utf-8") + except UnicodeEncodeError: + return False + if name.lower().endswith(CUE_EXT): + try: + cue_tracks(path) + except (RomConvertoUnsafeSourceError, OSError) as exc: + log.warning(f"Skipping rom-converto info on {path}: {exc}") + return False + return True + + def _make_sandbox() -> tuple[Path, Path]: """A new empty dir to run the CLI from, and the empty config file inside it.""" sandbox = Path(tempfile.mkdtemp(prefix="rom-converto-")) @@ -354,11 +409,81 @@ def _kill(proc: asyncio.subprocess.Process) -> None: proc.kill() +def _int(value: Any) -> int | None: + """The value when it is an int a signed BIGINT column holds, else None.""" + # A corrupt header's u64 would otherwise overflow the column and fail the row. + if isinstance(value, bool) or not isinstance(value, int): + return None + return value if 0 <= value < 2**63 else None + + +def _text(value: Any, max_length: int) -> str | None: + """The text on one line, clipped to `max_length`, or None if empty.""" + if not isinstance(value, str): + return None + text = " ".join(value.replace("\x00", "").split()) + return text[:max_length].rstrip() or None + + +def _first_id(data: dict[str, Any], *keys: str) -> str | None: + """The first of `keys` holding text, clipped to the title id column.""" + for key in keys: + if title_id := _text(data.get(key), TITLE_ID_MAX_LENGTH): + return title_id + return None + + +def _title_id(kind: str, flat: dict[str, Any]) -> str | None: + if kind in ("dol", "rvl"): + # Sigil keys GameCube and Wii by the hex-encoded 4-char game id. + game_id = flat.get("game_id") + if isinstance(game_id, str) and len(game_id) >= 4: + return game_id[:4].encode("ascii", "replace").hex().upper() + return None + if kind == "wup": + title_id_hex = _first_id(flat, "title_id_hex") + return title_id_hex[-8:] if title_id_hex else None + if kind == "xbox": + # An Xbox 360 disc image carries only the `xex` header, keyed by hex. + return _first_id(flat, "title_id_code", "title_id_hex") + if kind == "xenon": + return _first_id(flat, "title_id_hex") + title_id = _first_id(flat, "application_title_id_hex", "title_id", "game_code") + if kind in ("psp", "pbp"): + title_id = title_id or _first_id(flat, "disc_id") + if title_id and _PSP_TITLE_ID_PATTERN.fullmatch(title_id): + return f"{title_id[:4]}-{title_id[4:]}" + return title_id + + +def _parse_info(payload: dict[str, Any]) -> RomConvertoInfo: + kind = str(payload.get("kind") or "") + if kind in ("chd", "cso"): + # rom-converto only nests a PS1/PS2 or PSP disc here. + content = payload.get("content") + if isinstance(content, dict) and content.get("kind") not in ("chd", "cso"): + return _parse_info(content) + return RomConvertoInfo() + # Consoles nest their header (Xbox `xbe`, 360 `xex`, Switch `full`, the Wii + # `tmd` holding its title version); top-level keys win on conflict. + flat = dict(payload) + for key in ("xbe", "xex", "full", "tmd"): + nested = payload.get(key) + if isinstance(nested, dict): + flat = {**nested, **flat} + return RomConvertoInfo( + title_id=_title_id(kind, flat), + title_version=_int(flat.get("title_version")), + ) + + class RomConvertoService: - """Service to convert ROMs using the rom-converto CLI.""" + """Service to inspect and convert ROMs using the rom-converto CLI.""" def __init__(self) -> None: self._available: bool | None = None + # None when the manifest predates `info_extensions`: inspect everything. + self._info_extensions: frozenset[str] | None = None self._probe_lock = asyncio.Lock() async def is_enabled(self) -> bool: @@ -391,15 +516,80 @@ class RomConvertoService: manifest = json.loads(stdout) except json.JSONDecodeError: manifest = None - version = ( - manifest.get("version") if isinstance(manifest, dict) else None - ) or "unknown version" + if not isinstance(manifest, dict): + manifest = {} + version = manifest.get("version") or "unknown version" + extensions = manifest.get("info_extensions") + if isinstance(extensions, list): + self._info_extensions = frozenset( + f".{ext.lower()}" for ext in extensions if isinstance(ext, str) + ) log.info( f"Detected {hl('rom-converto', color=LIGHTMAGENTA)} {hl(str(version))}" ) self._available = True return True + def can_inspect(self, path: Path) -> bool: + """Whether `info` recognizes this file's extension.""" + if self._info_extensions is None: + return True + return path.suffix.lower() in self._info_extensions + + async def read_infos(self, paths: list[Path]) -> dict[Path, RomConvertoInfo]: + """Inspect files in `info` runs of up to `INFO_RUN_MAX_FILES`, keyed by the paths it recognized; never raises.""" + listed = await asyncio.to_thread(lambda: [p for p in paths if _listable(p)]) + infos: dict[Path, RomConvertoInfo] = {} + for chunk in batched(listed, INFO_RUN_MAX_FILES, strict=False): + infos.update(await self._read_run(list(chunk))) + return infos + + async def _read_run(self, listed: list[Path]) -> dict[Path, RomConvertoInfo]: + log.debug( + f"Executing {hl('rom-converto', color=LIGHTMAGENTA)} info on {len(listed)} file(s)" + ) + paths_file: str | None = None + try: + with tempfile.NamedTemporaryFile( + "w", encoding="utf-8", suffix=".txt", delete=False + ) as fh: + paths_file = fh.name + fh.write("".join(f"{p}\n" for p in listed)) + code, stdout, stderr = await _run( + ["info", "--json", "--paths-file", paths_file], ROM_CONVERTO_TIMEOUT + ) + except (RomConvertoError, OSError) as exc: + log.warning(f"rom-converto info failed: {exc}") + return {} + finally: + if paths_file is not None: + with contextlib.suppress(OSError): + os.unlink(paths_file) + if code != 0: + log.warning(f"rom-converto info failed (code {code}): {_tail(stderr)}") + return {} + try: + records = json.loads(stdout) + except json.JSONDecodeError: + log.warning("rom-converto info returned non-JSON output") + return {} + if not isinstance(records, list): + return {} + + infos: dict[Path, RomConvertoInfo] = {} + for record in records: + if not isinstance(record, dict): + continue + payload = record.get("info") + if record.get("ok") and isinstance(payload, dict): + infos[Path(str(record.get("path")))] = _parse_info(payload) + else: + log.debug( + f"rom-converto did not recognize {record.get('path')}: " + f"{record.get('error')}" + ) + return infos + async def convert(self, operation: Operation, src: Path, out: Path) -> None: """Run `operation` on `src`, writing `out`, which must be alone in its directory. diff --git a/backend/alembic/versions/0147_rom_file_title_ids.py b/backend/alembic/versions/0147_rom_file_title_ids.py new file mode 100644 index 000000000..aa06efc22 --- /dev/null +++ b/backend/alembic/versions/0147_rom_file_title_ids.py @@ -0,0 +1,37 @@ +"""Add the per-file title id columns rom-converto fills on rom_files. + +Revision ID: 0147_rom_file_title_ids +Revises: 0146_roms_search_aliases +Create Date: 2026-10-02 00:00:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op # type: ignore[attr-defined] + +# revision identifiers, used by Alembic. +revision = "0147_rom_file_title_ids" +down_revision = "0146_roms_search_aliases" +branch_labels = None +depends_on = None + + +def _columns() -> list[sa.Column]: + return [ + sa.Column("title_id", sa.String(length=100), nullable=True), + # BigInteger: Switch title versions are u32 and can exceed signed int32. + sa.Column("title_version", sa.BigInteger(), nullable=True), + sa.Column("converto_read_at", sa.TIMESTAMP(timezone=True), nullable=True), + ] + + +def upgrade() -> None: + with op.batch_alter_table("rom_files", schema=None) as batch_op: + for column in _columns(): + batch_op.add_column(column, if_not_exists=True) + + +def downgrade() -> None: + with op.batch_alter_table("rom_files", schema=None) as batch_op: + for column in reversed(_columns()): + batch_op.drop_column(column.name, if_exists=True) diff --git a/backend/config/config_manager.py b/backend/config/config_manager.py index 5e4e59763..20c47431b 100644 --- a/backend/config/config_manager.py +++ b/backend/config/config_manager.py @@ -406,6 +406,7 @@ VALID_SCAN_REGION_MODES = frozenset({"prefer_rom_tags", "prefer_config"}) @dataclasses.dataclass class ConvertoConfig: download_conversion_enabled: bool = False + scan_metadata: bool = True cache_ttl_hours: int = 24 # 0 leaves the cache unbounded. cache_max_size_gb: int = 20 @@ -1312,6 +1313,10 @@ class ConfigManager: ) sys.exit(3) + if not isinstance(self.config.CONVERTO.scan_metadata, bool): + log.critical("Invalid config.yml: converto.scan_metadata must be a boolean") + sys.exit(3) + for key, minimum in CONVERTO_INT_MINIMUMS.items(): value = getattr(self.config.CONVERTO, key) if isinstance(value, bool) or not isinstance(value, int) or value < minimum: diff --git a/backend/endpoints/configs.py b/backend/endpoints/configs.py index 98937a39b..dacae0c2b 100644 --- a/backend/endpoints/configs.py +++ b/backend/endpoints/configs.py @@ -124,7 +124,7 @@ class ScanSettingsPayload(BaseModel): class ConvertoSettingsPayload(BaseModel): - """The converto.* settings editable at runtime; `cache_ttl_hours` stays config.yml-only.""" + """Runtime-editable converto.* settings; `cache_ttl_hours` and `scan_metadata` stay in config.yml.""" download_conversion_enabled: bool cache_max_size_gb: int = Field(ge=CONVERTO_INT_MINIMUMS["cache_max_size_gb"]) @@ -374,7 +374,9 @@ async def update_converto_settings( try: cm.update_converto_settings( ConvertoConfig( - **payload.model_dump(), cache_ttl_hours=current.cache_ttl_hours + **payload.model_dump(), + cache_ttl_hours=current.cache_ttl_hours, + scan_metadata=current.scan_metadata, ) ) except ConfigNotWritableException as exc: diff --git a/backend/handler/database/roms_handler.py b/backend/handler/database/roms_handler.py index fd137115b..5f3f71b82 100644 --- a/backend/handler/database/roms_handler.py +++ b/backend/handler/database/roms_handler.py @@ -53,6 +53,7 @@ from handler.database.rom_filters import ( RomFiltersDict, RomFilterSpec, ) +from handler.filesystem.roms_handler import rom_file_unchanged from handler.redis_handler import sync_cache from logger.logger import log from models.assets import Save, Screenshot, State @@ -337,6 +338,9 @@ ROM_FILE_SCANNED_COLUMNS = ( "sha1_hash", "ra_hash", "chd_sha1_hash", + "title_id", + "title_version", + "converto_read_at", "archive_members", "category", ) @@ -401,6 +405,21 @@ def _rom_file_content_key(rom_file: RomFile) -> tuple[str, str, str] | None: return (rom_file.crc_hash, rom_file.md5_hash, rom_file.sha1_hash) +def _same_content(row: RomFile, scanned: RomFile) -> bool: + """Whether a rescanned file still holds the bytes its row was written from.""" + content_key = _rom_file_content_key(scanned) + stored_key = _rom_file_content_key(row) + # Fresh hashes outrank a size and mtime a copy can preserve. + if content_key is not None and stored_key is not None: + return content_key == stored_key + return scanned.last_modified is not None and rom_file_unchanged( + row, + size=scanned.file_size_bytes, + mtime=scanned.last_modified, + hashable=False, + ) + + def _cache_value_to_str(value: Any) -> str | None: if value is None: return None @@ -2614,7 +2633,20 @@ class DBRomsHandler(DBBaseHandler): Returns the cover path this update orphaned, if any. """ - _copy_scanned_columns(scanned, row, ROM_FILE_SCANNED_COLUMNS, RomFile) + # Title ids rom-converto didn't read this pass are reusable only when + # they still describe the same content. + keep_converto = scanned.converto_read_at is None and _same_content(row, scanned) + _copy_scanned_columns( + scanned, + row, + ROM_FILE_SCANNED_COLUMNS, + RomFile, + keep_when_unset=( + frozenset({"title_id", "title_version", "converto_read_at"}) + if keep_converto + else frozenset() + ), + ) if row.missing_from_fs: row.missing_from_fs = False @@ -2678,6 +2710,9 @@ class DBRomsHandler(DBBaseHandler): for scanned in scanned_files: row = by_path.get((scanned.file_path, scanned.file_name)) + if row is None and scanned.id is not None and scanned.id in unmatched: + # A reused row renamed in place is still that row. + row = unmatched[scanned.id] if row is not None and row.id in unmatched: del unmatched[row.id] pairs.append((scanned, row)) diff --git a/backend/handler/filesystem/roms_handler.py b/backend/handler/filesystem/roms_handler.py index 0442960ea..af1072a36 100644 --- a/backend/handler/filesystem/roms_handler.py +++ b/backend/handler/filesystem/roms_handler.py @@ -7,10 +7,15 @@ import re import struct import zlib from collections.abc import Mapping, Sequence -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace +from datetime import datetime, timezone from pathlib import Path from typing import Any, Final, NotRequired, TypedDict +from adapters.services.rom_converto import ( + CONVERTO_PLATFORM_SLUGS, + rom_converto_service, +) from adapters.services.sigil import ( SIGIL_PLATFORM_SLUGS, SWITCH_PLATFORM_SLUGS, @@ -329,6 +334,14 @@ def _may_hold_title_id(path: Path, category: RomFileCategory | None) -> bool: ) +def _converto_may_read(path: Path, category: RomFileCategory | None) -> bool: + """Whether rom-converto can read a title id from this file.""" + return ( + category not in NON_BINARY_FILE_CATEGORIES + and rom_converto_service.can_inspect(path) + ) + + @dataclass(frozen=True) class _TitleIdSource: path: Path @@ -342,6 +355,12 @@ class _TitleIdSource: return self.path.parent, compute_name_sort_key(self.path.name), self.path.name +def _rom_file_order(rom_file: RomFile) -> tuple[Path, str, str]: + """`_TitleIdSource.order` for a row, so a multi-disc ROM falls back to its first disc.""" + name = rom_file.file_name + return Path(rom_file.file_path), compute_name_sort_key(name), name + + # Exclusion patterns holding one of these need fnmatch; the rest match literally. _GLOB_CHARS_RE: Final = re.compile(r"[*?\[]") @@ -357,22 +376,41 @@ def _parse_save_target_layout(usage: str) -> SaveTargetLayout | None: def _rom_level_identity( platform_slug: str, extractions: list[SigilExtractionResult], + rom_files: list[RomFile], + stored: RomIdentity, ) -> RomIdentity: - """The rom's identity, from the base game's file where the family has one.""" - if not extractions: - return RomIdentity() + """The rom's identity: sigil's extraction (holding rom-converto's id where it read one), else rom-converto's file ids.""" + is_switch = platform_slug in SWITCH_PLATFORM_SLUGS + if extractions: + chosen = next( + (e for e in extractions if switch.is_base_title_id(e.title_id)), + extractions[0], + ) + return switch.normalize_identity( + is_switch, + RomIdentity( + title_id=chosen.title_id, + save_target=chosen.save_target, + save_target_layout=_parse_save_target_layout(chosen.usage), + ), + ) - chosen = next( - (e for e in extractions if switch.is_base_title_id(e.title_id)), extractions[0] - ) - return switch.normalize_identity( - platform_slug in SWITCH_PLATFORM_SLUGS, - RomIdentity( - title_id=chosen.title_id, - save_target=chosen.save_target, - save_target_layout=_parse_save_target_layout(chosen.usage), - ), + title_ids = [ + f.title_id for f in sorted(rom_files, key=_rom_file_order) if f.title_id + ] + title_id = next( + (t for t in title_ids if is_switch and switch.is_base_title_id(t)), + title_ids[0] if title_ids else None, ) + identity = switch.normalize_identity(is_switch, RomIdentity(title_id=title_id)) + # rom-converto reads no save target, so the one sigil stored for the same id stands. + if ( + identity.title_id + and identity.title_id == stored.title_id + and (stored.save_target or not identity.save_target) + ): + return stored + return identity class FSRomsHandler(FSHandler): @@ -607,6 +645,28 @@ class FSRomsHandler(FSHandler): log.warning(f"Skipping unreadable file {f_path / file_name}: {exc}") return entries + async def converto_active(self, platform_slug: str) -> bool: + """Whether rom-converto should inspect a platform's files during scan.""" + return ( + platform_slug in CONVERTO_PLATFORM_SLUGS + and cm.get_config().CONVERTO.scan_metadata + and await rom_converto_service.is_enabled() + ) + + async def _read_converto_infos(self, sources: list[tuple[Path, RomFile]]) -> None: + """Write title ids and the read time onto the files rom-converto recognized.""" + if not sources: + return + infos = await rom_converto_service.read_infos([p for p, _ in sources]) + read_at = datetime.now(timezone.utc) + for path, rom_file in sources: + info = infos.get(path) + if info is None: + continue + rom_file.title_id = info.title_id + rom_file.title_version = info.title_version + rom_file.converto_read_at = read_at + async def get_rom_files( self, rom: Rom, @@ -642,6 +702,9 @@ class FSRomsHandler(FSHandler): # non-hashable platforms like Switch. sigil_platform = extract_title_ids and rom.platform_slug in SIGIL_PLATFORM_SLUGS is_switch = rom.platform_slug in SWITCH_PLATFORM_SLUGS + # rom-converto reads new or changed files' title ids even when sigil + # extraction is skipped; sigil owns save targets. + converto_active = await self.converto_active(rom.platform_slug) is_multi_part = await self.directory_exists(rom.full_path) sigil_extractions: list[SigilExtractionResult] = [] embed_candidates: list[TitleIdEmbedCandidate] = [] @@ -660,6 +723,20 @@ class FSRomsHandler(FSHandler): ) if extraction is None: return + if source.rom_file.title_id and source.rom_file.converto_read_at: + # rom-converto's id wins; sigil keeps its save target and content type. + extraction = replace( + extraction, + title_id=source.rom_file.title_id, + version=( + source.rom_file.title_version + if source.rom_file.title_version is not None + else extraction.version + ), + ) + else: + source.rom_file.title_id = extraction.title_id or None + source.rom_file.title_version = extraction.version if extraction.content_type is not None: category = switch.CONTENT_TYPE_CATEGORIES.get(extraction.content_type) if category is not None: @@ -955,6 +1032,21 @@ class FSRomsHandler(FSHandler): rom_files.append(rom_file) _record_title_id_source(rom_dir, rom_file) + # New and changed rows were never read; reused ones only if rom-converto + # hasn't recognized them yet. Every row sits under the validated rom dir. + converto_sources = [ + (path, rom_file) + for rom_file in rom_files + if converto_active + and rom_file.converto_read_at is None + and _converto_may_read( + path := abs_fs_path + / Path(rom_file.full_path).relative_to(rel_roms_path), + rom_file.category, + ) + ] + await self._read_converto_infos(converto_sources) + # Listings come in no fixed order; a ROM is identified by its first disc, # and only Switch reads past it for each file's content type. for source in sorted(title_id_sources, key=_TitleIdSource.order): @@ -989,7 +1081,14 @@ class FSRomsHandler(FSHandler): sha1_hash=sha1_hash, ra_hash=ra_hash, top_level_changed=top_level_changed, - identity=_rom_level_identity(rom.platform_slug, sigil_extractions), + # rom-converto's file ids stand in only when extraction ran, so a + # pass that skipped it can't blank the save target sigil wrote. + identity=_rom_level_identity( + rom.platform_slug, + sigil_extractions, + rom_files if converto_active and extract_title_ids else [], + RomIdentity.from_rom(rom), + ), embed_candidates=embed_candidates, ) diff --git a/backend/handler/rom_files.py b/backend/handler/rom_files.py index 007178f5e..a9140a885 100644 --- a/backend/handler/rom_files.py +++ b/backend/handler/rom_files.py @@ -40,6 +40,16 @@ class RomFilesRefresh: ) +def _extracted_state(rom_file: RomFile) -> tuple[Any, ...]: + """The columns title id extraction and rom-converto write onto a reused row.""" + return ( + rom_file.category, + rom_file.title_id, + rom_file.title_version, + rom_file.converto_read_at, + ) + + def loaded_rom_files(rom: Rom) -> list[RomFile]: """The ROM's file rows, fetched on demand when the relationship is unloaded.""" if "files" not in sa_inspect(rom).unloaded: @@ -64,9 +74,9 @@ async def refresh_rom_files(rom: Rom) -> RomFilesRefresh: async def _refresh(rom: Rom) -> RomFilesRefresh: existing = loaded_rom_files(rom) - # Extraction can settle a reused row's category in place, so that row is - # compared against the category it was loaded with. - reused_categories = {id(f): f.category for f in existing} + # Extraction can settle a reused row's category, title id and read time in + # place, so that row is compared against the values it was loaded with. + reused_state = {id(f): _extracted_state(f) for f in existing} cnfg = cm.get_config() calculate_hashes = not cnfg.SKIP_HASH_CALCULATION parsed = await fs_rom_handler.get_rom_files( @@ -87,10 +97,7 @@ async def _refresh(rom: Rom) -> RomFilesRefresh: new_keys: set[RomFileKey] = set() updated_keys: set[RomFileKey] = set() for scanned in parsed.rom_files: - if ( - id(scanned) in reused_categories - and reused_categories[id(scanned)] == scanned.category - ): + if reused_state.get(id(scanned)) == _extracted_state(scanned): continue key = rom_file_key(scanned) (updated_keys if key in existing_keys else new_keys).add(key) diff --git a/backend/models/rom.py b/backend/models/rom.py index 1c9370141..38f8e36f9 100644 --- a/backend/models/rom.py +++ b/backend/models/rom.py @@ -369,6 +369,13 @@ class RomFile(BaseModel): sha1_hash: Mapped[str | None] = mapped_column(String(100)) ra_hash: Mapped[str | None] = mapped_column(String(100)) chd_sha1_hash: Mapped[str | None] = mapped_column(String(100)) + title_id: Mapped[str | None] = mapped_column(String(length=TITLE_ID_MAX_LENGTH)) + # BigInteger because Switch title versions exceed int32 + title_version: Mapped[int | None] = mapped_column(BigInteger, default=None) + # When rom-converto last recognized the file; NULL queues it on the next scan. + converto_read_at: Mapped[datetime | None] = mapped_column( + TIMESTAMP(timezone=True), default=None + ) archive_members: Mapped[list[RomArchiveMember] | None] = mapped_column( CustomJSON(), default=None, nullable=True ) diff --git a/backend/tests/adapters/services/test_rom_converto.py b/backend/tests/adapters/services/test_rom_converto.py index 90ee2ac0c..ea2f3bdbd 100644 --- a/backend/tests/adapters/services/test_rom_converto.py +++ b/backend/tests/adapters/services/test_rom_converto.py @@ -1,10 +1,13 @@ import asyncio +import json import os import stat import threading from pathlib import Path +from typing import Any from unittest.mock import patch +import anyio import pytest from adapters.services import rom_converto @@ -13,6 +16,8 @@ from adapters.services.rom_converto import ( OPERATIONS, Operation, RomConvertoBinaryNotFoundError, + RomConvertoError, + RomConvertoInfo, RomConvertoOperationError, RomConvertoService, RomConvertoTimeoutError, @@ -23,8 +28,12 @@ from adapters.services.rom_converto import ( file_format, resolve_operation, ) +from models.rom import TITLE_ID_MAX_LENGTH from utils.filesystem import SERVED_FILE_MODE +# rom-converto carries the images a file embeds in its info output. +_FAKE_PNG = b"\x89PNG\r\n\x1a\nfake" + class FakeProc: def __init__( @@ -172,6 +181,179 @@ def _capture_spawn(proc: FakeProc, calls: list[tuple[str, ...]]): return spawn +class TestReadInfos: + async def test_batches_paths_and_keys_recognized_files( + self, service: RomConvertoService + ): + records = [ + { + "path": "/roms/game.nsp", + "ok": True, + "info": { + "kind": "nx", + "full": { + "application_title_id_hex": "0100ABCD12345000", + "title_version": 65536, + }, + }, + }, + {"path": "/roms/junk.bin", "ok": False, "error": "could not detect"}, + ] + calls: list[tuple[str, ...]] = [] + listed: list[str] = [] + proc = FakeProc(stdout=json.dumps(records).encode()) + + async def spawn(*args, **kwargs): + calls.append(args) + paths_file = anyio.Path(args[args.index("--paths-file") + 1]) + listed.extend((await paths_file.read_text()).split()) + return proc + + with ( + patch("shutil.which", return_value="rc"), + patch("asyncio.create_subprocess_exec", spawn), + ): + infos = await service.read_infos( + [Path("/roms/game.nsp"), Path("/roms/junk.bin")] + ) + + assert len(calls) == 1 + assert listed == ["/roms/game.nsp", "/roms/junk.bin"] + assert infos == { + Path("/roms/game.nsp"): RomConvertoInfo( + title_id="0100ABCD12345000", title_version=65536 + ) + } + + async def test_splits_paths_into_runs_of_the_run_limit( + self, service: RomConvertoService + ): + runs: list[list[str]] = [] + + async def spawn(*args, **kwargs): + paths_file = anyio.Path(args[args.index("--paths-file") + 1]) + listed = (await paths_file.read_text()).split() + runs.append(listed) + records = [ + {"path": p, "ok": True, "info": {"kind": "psx", "title_id": p}} + for p in listed + ] + return FakeProc(stdout=json.dumps(records).encode()) + + paths = [Path("/roms/a.chd"), Path("/roms/b.chd"), Path("/roms/c.chd")] + with ( + patch("shutil.which", return_value="rc"), + patch("asyncio.create_subprocess_exec", spawn), + patch.object(rom_converto, "INFO_RUN_MAX_FILES", 2), + ): + infos = await service.read_infos(paths) + + assert runs == [["/roms/a.chd", "/roms/b.chd"], ["/roms/c.chd"]] + assert set(infos) == set(paths) + + async def test_paths_file_is_removed(self, service: RomConvertoService): + calls: list[tuple[str, ...]] = [] + with ( + patch("shutil.which", return_value="rc"), + patch( + "asyncio.create_subprocess_exec", + _capture_spawn(FakeProc(stdout=b"[]"), calls), + ), + ): + await service.read_infos([Path("/roms/game.iso")]) + + paths_file = calls[0][calls[0].index("--paths-file") + 1] + assert not await anyio.Path(paths_file).exists() + + async def test_nonzero_returns_empty(self, service: RomConvertoService): + proc = FakeProc(returncode=2, stderr=b"error: bad arguments") + with ( + patch("shutil.which", return_value="rc"), + patch("asyncio.create_subprocess_exec", return_value=proc), + ): + assert await service.read_infos([Path("/roms/game.iso")]) == {} + + @pytest.mark.parametrize( + "stdout", + [ + pytest.param(b"not json at all", id="non-json"), + pytest.param(b"{}", id="object"), + pytest.param(b"null", id="null"), + ], + ) + async def test_invalid_output_returns_empty( + self, service: RomConvertoService, stdout: bytes + ): + proc = FakeProc(stdout=stdout) + with ( + patch("shutil.which", return_value="rc"), + patch("asyncio.create_subprocess_exec", return_value=proc), + ): + assert await service.read_infos([Path("/roms/game.iso")]) == {} + + async def test_unrecognized_files_return_an_empty_dict( + self, service: RomConvertoService + ): + records = [{"path": "/roms/game.iso", "ok": False, "error": "could not detect"}] + with patch.object( + rom_converto, "_run", return_value=(0, json.dumps(records), "") + ): + assert await service.read_infos([Path("/roms/game.iso")]) == {} + + @pytest.mark.parametrize( + "path", + [ + pytest.param(Path("/roms/bad\nname.iso"), id="newline"), + pytest.param(Path(os.fsdecode(b"/roms/\xff.iso")), id="not-utf8"), + pytest.param(Path("/roms/game.iso "), id="trailing-space"), + ], + ) + async def test_no_listable_paths_skips_the_subprocess( + self, service: RomConvertoService, path: Path + ): + with patch("asyncio.create_subprocess_exec") as spawn: + assert await service.read_infos([path]) == {} + spawn.assert_not_called() + + async def test_a_cue_reaching_outside_its_folder_is_not_listed( + self, service: RomConvertoService, tmp_path: Path + ): + unsafe = tmp_path / "unsafe.cue" + unsafe.write_text('FILE "../secret.bin" BINARY\n') + safe = tmp_path / "safe.cue" + safe.write_text('FILE "safe.bin" BINARY\n') + listed: list[str] = [] + + async def spawn(*args, **kwargs): + paths_file = anyio.Path(args[args.index("--paths-file") + 1]) + listed.extend((await paths_file.read_text()).split()) + return FakeProc(stdout=b"[]") + + with ( + patch("shutil.which", return_value="rc"), + patch("asyncio.create_subprocess_exec", spawn), + ): + await service.read_infos([unsafe, safe]) + + assert listed == [str(safe)] + + +class TestReadInfosFailures: + @pytest.mark.parametrize( + "error", + [ + pytest.param(RomConvertoError("failed"), id="run-error"), + pytest.param(RomConvertoTimeoutError("slow"), id="timeout"), + pytest.param(OSError(8, "Exec format error"), id="os-error"), + ], + ) + async def test_run_failure_returns_empty( + self, service: RomConvertoService, error: Exception + ): + with patch.object(rom_converto, "_run", side_effect=error): + assert await service.read_infos([Path("/roms/game.iso")]) == {} + + class TestIsolation: async def test_every_run_skips_the_update_check_config_and_cache(self): calls: list[tuple[str, ...]] = [] @@ -241,6 +423,401 @@ class TestIsolation: assert not await asyncio.to_thread(made[0].exists) +class TestCanInspect: + async def test_manifest_extensions_gate_inspection( + self, service: RomConvertoService + ): + proc = FakeProc( + stdout=b'{"version": "0.21.0", "info_extensions": ["ISO", "nsp"]}' + ) + with ( + patch.object(rom_converto, "ROM_CONVERTO_ENABLED", True), + patch("shutil.which", return_value="/usr/bin/rom-converto"), + patch("asyncio.create_subprocess_exec", return_value=proc), + ): + assert await service.is_enabled() is True + + assert service.can_inspect(Path("/roms/Game.iso")) is True + assert service.can_inspect(Path("/roms/game.NSP")) is True + assert service.can_inspect(Path("/roms/readme.nfo")) is False + + def test_unknown_manifest_inspects_everything(self, service: RomConvertoService): + assert service.can_inspect(Path("/roms/anything.xyz")) is True + + +class TestParseInfo: + + @pytest.mark.parametrize( + ("payload", "expected"), + [ + pytest.param( + { + "kind": "nx", + "container_kind": "nsz", + "is_compressed": True, + "full": { + "title_kind": "application", + "required_system_version": 1073741824, + "title_version": 65536, + "application_title_id_hex": "0100000000010000", + "control": { + "icon": {"png_bytes": list(_FAKE_PNG)}, + "display_version": "1.0.0", + "supported_languages": [ + "AmericanEnglish", + "CanadianFrench", + ], + "titles": [ + { + "language": "Japanese", + "name": "ゼルダの伝説", + "publisher": "任天堂", + }, + { + "language": "AmericanEnglish", + "name": "The Legend of Zelda", + "publisher": "Nintendo", + }, + ], + }, + }, + }, + RomConvertoInfo(title_id="0100000000010000", title_version=65536), + id="nx-reads-the-application-id", + ), + pytest.param( + {"kind": "nx", "container_kind": "nsp"}, + RomConvertoInfo(), + id="nx-without-prod-keys-has-no-title-id", + ), + pytest.param( + { + "kind": "ctr", + "title_id": "0004000000123456", + "product_code": "CTR-P-AZRE", + "content_kind": "game", + "compressed": True, + "format": "cia", + "icon": {"png_bytes": list(_FAKE_PNG)}, + "small_icon": {"png_bytes": list(_FAKE_PNG + b"2")}, + "smdh": { + "region_names": ["North America", "Japan"], + "titles": [ + { + "language": "Japanese", + "short_description": "ロックマンゼロ", + "publisher": "カプコン", + }, + { + "language": "English", + "short_description": "Mega Man Zero", + "long_description": "Mega Man Zero", + "publisher": "Capcom", + }, + ], + }, + }, + RomConvertoInfo(title_id="0004000000123456"), + id="ctr-reads-the-title-id", + ), + pytest.param( + { + "kind": "wup", + "title_id_hex": "0005000010143500", + "title_version": 16, + "content_kind": "game", + "image": {"png_bytes": list(_FAKE_PNG)}, + "source_kind": "disc (GM0005000010143500)", + "meta": { + "product_code": "WUP-P-ARZE", + "company_name": "Nintendo", + "region_names": ["Europe", "Australia"], + "long_names": { + "entries": [ + ["japanese", "ゼルダの伝説"], + ["english", "The Legend of Zelda"], + ] + }, + "publishers": { + "entries": [["japanese", "任天堂"], ["english", "Nintendo"]] + }, + }, + }, + RomConvertoInfo(title_id="10143500", title_version=16), + id="wup-reads-the-title-id", + ), + pytest.param( + { + "kind": "wup", + "title_id_hex": "0005000010143500", + "title_version": 16, + }, + RomConvertoInfo(title_id="10143500", title_version=16), + id="wup-last-8-of-title-id", + ), + pytest.param( + { + "kind": "dol", + "game_id": "GZLE01", + "game_name": "The Legend of Zelda", + "disc_version": 2, + "region": "Usa", + "container": "RVZ", + "banner_image": {"png_bytes": list(_FAKE_PNG)}, + "banner": { + "titles": [ + {"language": "Default", "long_game_name": "ZELDA"}, + {"language": "German", "long_game_name": ""}, + { + "language": "English", + "long_game_name": "The Legend of Zelda", + "short_game_name": "Zelda", + "long_maker": "Nintendo", + }, + ] + }, + }, + RomConvertoInfo(title_id="475A4C45"), + id="dol-reads-the-game-id", + ), + pytest.param( + {"kind": "dol", "game_id": "GZLE01"}, + RomConvertoInfo(title_id="475A4C45"), + id="dol-hex-encodes-game-id", + ), + pytest.param( + { + "kind": "rvl", + "game_id": "SMNE01", + "game_name": "New Super Mario Bros. Wii", + "disc_version": 1, + "region": "PAL", + "container": "iso", + "tmd": {"title_version": 3}, + "imet_names": { + "entries": [ + ["Japanese", "ニュースーパーマリオブラザーズWii"], + ["English", "New Super Mario Bros. Wii"], + ] + }, + "maker_name": "Nintendo", + "image": {"png_bytes": list(_FAKE_PNG)}, + }, + RomConvertoInfo(title_id="534D4E45", title_version=3), + id="rvl-prefers-the-tmd-title-version", + ), + pytest.param( + {"kind": "rvl", "game_id": "RZTE01"}, + RomConvertoInfo(title_id="525A5445"), + id="rvl-hex-encodes-game-id", + ), + pytest.param( + { + "kind": "ntr", + "game_code": "ARZE", + "game_title": "Single Line", + "rom_version": 2, + "banner": { + "titles": { + "entries": [ + ["Japanese", "ホームブルー\n作者不明"], + ["English", "Homebrew Game\nby TestDev\nfinal release"], + ] + }, + "icon": {"png_bytes": list(_FAKE_PNG)}, + }, + }, + RomConvertoInfo(title_id="ARZE"), + id="ntr-reads-the-game-code", + ), + pytest.param( + { + "kind": "xbox", + "xbe": { + "title_id_code": "TT-027", + "title_name": "Stubbs the Zombie", + "version": 1, + "region_names": ["NTSC-U"], + "icon": {"png_bytes": list(_FAKE_PNG)}, + }, + "xex": {"icon": {"png_bytes": list(_FAKE_PNG + b"2")}}, + }, + RomConvertoInfo(title_id="TT-027"), + id="xbox-reads-the-xbe-title-id", + ), + pytest.param( + { + "kind": "xbox", + "xex": { + "title_id_hex": "4D5307E6", + "title_name": "Halo 3", + "version": "1.0.0.0", + "region_names": ["NTSC-U", "PAL"], + "icon": {"png_bytes": list(_FAKE_PNG)}, + }, + }, + RomConvertoInfo(title_id="4D5307E6"), + id="xbox-360-iso-reads-the-xex-title-id", + ), + pytest.param( + { + "kind": "xbox", + "xbe": {"title_id_code": "TT-027", "title_id_hex": "5454001B"}, + }, + RomConvertoInfo(title_id="TT-027"), + id="xbox-nested-xbe-code", + ), + pytest.param( + { + "kind": "xenon", + "compressed_size": 725614592, + "logical_size": 786432000, + "xex": { + "title_id_hex": "4D5307DC", + "title_name": "Halo 3", + "version": "2.0.4552.0", + "region_names": ["RegionFree"], + "icon": {"png_bytes": list(_FAKE_PNG)}, + }, + }, + RomConvertoInfo(title_id="4D5307DC"), + id="xenon-reads-the-xex-title-id", + ), + pytest.param( + {"kind": "xenon", "xex": {"title_id_hex": "4D5307DC"}}, + RomConvertoInfo(title_id="4D5307DC"), + id="xenon-nested-xex-hex", + ), + pytest.param( + {"kind": "psx", "title_id": "SCUS-94163", "version": "1.1"}, + RomConvertoInfo(title_id="SCUS-94163"), + id="psx-reads-the-disc-serial", + ), + pytest.param( + { + "kind": "psp", + "title": "Patapon", + "title_id": "UCUS98696", + "content_kind": "game", + "version": "1.0", + "firmware": "5.00", + }, + RomConvertoInfo(title_id="UCUS-98696"), + id="psp-reads-the-disc-serial", + ), + pytest.param( + { + "kind": "pbp", + "title": "Patapon", + "disc_id": "UCUS98696", + "content_kind": "update", + "disc_version": "1.00", + "psp_system_ver": "5.55", + "icon": {"png_bytes": list(_FAKE_PNG)}, + }, + RomConvertoInfo(title_id="UCUS-98696"), + id="pbp-reads-the-disc-id", + ), + pytest.param( + {"kind": "psp", "title_id": "HOMEBREW"}, + RomConvertoInfo(title_id="HOMEBREW"), + id="psp-keeps-nonmatching-title-id", + ), + pytest.param( + {"kind": "psp", "title_id": "ulus10041"}, + RomConvertoInfo(title_id="ulus10041"), + id="psp-keeps-lowercase-title-id", + ), + pytest.param( + { + "kind": "ps3", + "title": "Gran Turismo 5", + "title_id": "BCUS98114", + "content_kind": "dlc", + "version": "01.02", + "region": "USA", + "firmware": "3.50", + }, + RomConvertoInfo(title_id="BCUS98114"), + id="ps3-reads-the-disc-serial", + ), + pytest.param( + { + "kind": "chd", + "compressors": ["zstd"], + "logical_bytes": 1234567890, + "content": { + "kind": "psp", + "title": "Daxter", + "title_id": "UCUS98718", + "content_kind": "game", + "version": "1.00", + "icon": {"png_bytes": list(_FAKE_PNG)}, + "background": {"png_bytes": list(_FAKE_PNG)}, + }, + }, + RomConvertoInfo(title_id="UCUS-98718"), + id="chd-reads-the-inner-psp-disc", + ), + pytest.param( + {"kind": "chd", "compressors": [], "logical_bytes": 786432000}, + RomConvertoInfo(), + id="chd-without-an-inner-disc-has-no-title-id", + ), + pytest.param( + { + "kind": "cso", + "format": "zso", + "uncompressed_size": 456789012, + "content": { + "kind": "psx", + "title_id": "SCUS-94163", + "version": "1.1", + }, + }, + RomConvertoInfo(title_id="SCUS-94163"), + id="cso-reads-the-inner-psx-disc", + ), + pytest.param( + {"kind": "nds", "game_code": "AXXE"}, + RomConvertoInfo(title_id="AXXE", title_version=None), + id="nds-falls-back-to-game-code", + ), + pytest.param( + {"kind": "ps3", "title_id": "BLUS31426\x00\x00" + "X" * 150}, + RomConvertoInfo( + title_id=("BLUS31426" + "X" * 150)[:TITLE_ID_MAX_LENGTH] + ), + id="title-id-drops-nul-bytes-and-clips-at-its-column-limit", + ), + pytest.param( + {"kind": "psp", "title_id": "UCUS98696\x00"}, + RomConvertoInfo(title_id="UCUS-98696"), + id="psp-title-id-is-cleaned-before-it-is-formatted", + ), + pytest.param( + {"kind": "wup", "title_id_hex": ""}, + RomConvertoInfo(), + id="wup-empty-title-id-is-none", + ), + pytest.param( + { + "kind": "nx", + "is_compressed": "yes", + "full": { + "title_kind": ["patch"], + "control": {"titles": {"language": "English"}}, + }, + }, + RomConvertoInfo(), + id="malformed-shapes-yield-nothing", + ), + ], + ) + def test_parse_info(self, payload: dict[str, Any], expected: RomConvertoInfo): + assert rom_converto._parse_info(payload) == expected + + class TestConvert: def _op(self) -> Operation: return Operation( diff --git a/backend/tests/config/test_config_loader.py b/backend/tests/config/test_config_loader.py index 4e43f6d1e..96f70f802 100644 --- a/backend/tests/config/test_config_loader.py +++ b/backend/tests/config/test_config_loader.py @@ -190,6 +190,7 @@ def test_empty_config_loader(): assert loader.config.GAMELIST_MEDIA_THUMBNAIL == "box2d" assert loader.config.GAMELIST_MEDIA_IMAGE == "screenshot" assert not loader.config.CONVERTO.download_conversion_enabled + assert loader.config.CONVERTO.scan_metadata assert loader.config.CONVERTO.cache_ttl_hours == 24 assert loader.config.CONVERTO.platform_formats == {} assert loader.config.STRUCTURE_TEMPLATES == {} @@ -686,6 +687,7 @@ def test_converto_config_from_yaml(tmp_path): config_file.write_text( "converto:\n" " download_conversion_enabled: true\n" + " scan_metadata: false\n" " cache_ttl_hours: 48\n" " cache_max_size_gb: 0\n" " platform_formats:\n" @@ -697,6 +699,7 @@ def test_converto_config_from_yaml(tmp_path): # Slugs and targets are normalized to lowercase. assert loader.config.CONVERTO == ConvertoConfig( download_conversion_enabled=True, + scan_metadata=False, cache_ttl_hours=48, cache_max_size_gb=0, platform_formats={"psp": "iso", "ngc": "rvz"}, @@ -716,6 +719,7 @@ def test_update_converto_settings_round_trip(tmp_path): loader = ConfigManager(str(config_file)) converto = ConvertoConfig( download_conversion_enabled=True, + scan_metadata=True, cache_ttl_hours=72, cache_max_size_gb=50, platform_formats={"psp": "iso", "ngc": "rvz"}, diff --git a/backend/tests/endpoints/test_config.py b/backend/tests/endpoints/test_config.py index ef4415e5b..d49eb6327 100644 --- a/backend/tests/endpoints/test_config.py +++ b/backend/tests/endpoints/test_config.py @@ -279,7 +279,9 @@ def _converto_payload(**overrides): def test_update_converto_settings_normalizes_formats_and_keeps_the_ttl( client, access_token: str ): - current = SimpleNamespace(CONVERTO=ConvertoConfig(cache_ttl_hours=72)) + current = SimpleNamespace( + CONVERTO=ConvertoConfig(cache_ttl_hours=72, scan_metadata=False) + ) with ( patch.object(cm, "get_config", return_value=current), patch.object(cm, "update_converto_settings") as update_converto_settings, @@ -294,6 +296,7 @@ def test_update_converto_settings_normalizes_formats_and_keeps_the_ttl( update_converto_settings.assert_called_once_with( ConvertoConfig( download_conversion_enabled=True, + scan_metadata=False, cache_ttl_hours=72, cache_max_size_gb=20, platform_formats={"psp": "cso"}, diff --git a/backend/tests/handler/database/test_roms_handler.py b/backend/tests/handler/database/test_roms_handler.py index c3ba78035..eed71baf8 100644 --- a/backend/tests/handler/database/test_roms_handler.py +++ b/backend/tests/handler/database/test_roms_handler.py @@ -5,7 +5,9 @@ the columns derived from `name` / `fs_name` / `fs_path` in sync explicitly. """ import re +import struct from collections.abc import Iterator +from datetime import datetime, timezone from typing import Any from unittest.mock import MagicMock @@ -401,6 +403,125 @@ class TestSyncRomFiles: assert reloaded.file_size_bytes == 200 assert reloaded.sha1_hash == "new-sha1" + @pytest.mark.parametrize( + ("inspected", "size", "mtime", "sigil_title_id", "keep_metadata"), + [ + pytest.param(False, 200, 1000.0, None, True, id="unchanged-keeps-metadata"), + pytest.param(False, 300, 1000.0, None, False, id="size-change-clears"), + pytest.param(False, 200, 2000.0, None, False, id="mtime-change-clears"), + pytest.param(True, 200, 1000.0, None, False, id="inspected-clears"), + pytest.param( + False, + 300, + 1000.0, + "0100ABCD12345000", + False, + id="changed-writes-sigil-id-and-clears-metadata", + ), + pytest.param( + False, + 200, + 1000.0, + "0100ABCD12345000", + True, + id="unchanged-writes-sigil-id-and-keeps-metadata", + ), + ], + ) + def test_converto_metadata_tracks_content_and_inspection( + self, + rom: Rom, + inspected: bool, + size: int, + mtime: float, + sigil_title_id: str | None, + keep_metadata: bool, + ): + metadata = {"title_id": "0100ABCD12340000", "title_version": 65536} + (first,) = _sync(rom, [_scanned_file(rom, "a.bin")]) + scanned = _scanned_file(rom, "a.bin", size=200) + scanned.last_modified = 1000.0 + for column, value in metadata.items(): + setattr(scanned, column, value) + scanned.converto_read_at = datetime(2026, 1, 1, tzinfo=timezone.utc) + + (second,) = _sync(rom, [scanned]) + + assert second.id == first.id + stored = db_rom_handler.get_rom_file_by_id(first.id) + assert stored is not None + assert {column: getattr(stored, column) for column in metadata} == metadata + + scanned = _scanned_file(rom, "a.bin", size=size, sha1=None) + scanned.last_modified = mtime + scanned.title_id = sigil_title_id + if inspected: + scanned.converto_read_at = datetime(2026, 2, 1, tzinfo=timezone.utc) + db_rom_handler.sync_rom_files(rom.id, [scanned]) + + stored = db_rom_handler.get_rom_file_by_id(first.id) + assert stored is not None + assert stored.file_size_bytes == size + assert stored.last_modified == mtime + expected = dict(metadata) if keep_metadata else dict.fromkeys(metadata) + if sigil_title_id is not None: + expected["title_id"] = sigil_title_id + assert {column: getattr(stored, column) for column in metadata} == expected + assert (stored.converto_read_at is not None) == (keep_metadata or inspected) + + @pytest.mark.parametrize( + ("stored_mtime", "scanned_mtime", "matching_hashes"), + [ + pytest.param( + struct.unpack("f", struct.pack("f", 1700000000.123))[0], + 1700000000.123, + False, + id="legacy-single-precision-mtime", + ), + pytest.param(1000.0, 2000.0, True, id="matching-content-with-new-mtime"), + ], + ) + def test_unread_title_ids_survive_equivalent_content( + self, + rom: Rom, + stored_mtime: float, + scanned_mtime: float, + matching_hashes: bool, + ): + metadata = {"title_id": "SCUS-94163", "title_version": 1} + scanned = _scanned_file( + rom, "game.chd", sha1="sha1" if matching_hashes else None + ) + scanned.last_modified = stored_mtime + for column, value in metadata.items(): + setattr(scanned, column, value) + (first,) = _sync(rom, [scanned]) + scanned = _scanned_file( + rom, "game.chd", sha1="sha1" if matching_hashes else None + ) + scanned.last_modified = scanned_mtime + + db_rom_handler.sync_rom_files(rom.id, [scanned]) + + stored = db_rom_handler.get_rom_file_by_id(first.id) + assert stored is not None + assert stored.last_modified == scanned_mtime + assert {column: getattr(stored, column) for column in metadata} == metadata + + def test_new_hashes_clear_unread_title_ids_despite_a_kept_mtime(self, rom: Rom): + scanned = _scanned_file(rom, "game.chd") + scanned.last_modified = 1000.0 + scanned.title_id = "SCUS-94163" + (first,) = _sync(rom, [scanned]) + rehashed = _scanned_file(rom, "game.chd", sha1="other-sha1") + rehashed.last_modified = 1000.0 + + db_rom_handler.sync_rom_files(rom.id, [rehashed]) + + stored = db_rom_handler.get_rom_file_by_id(first.id) + assert stored is not None + assert stored.title_id is None + def test_retagged_track_meta_is_updated_in_place(self, rom: Rom): def scanned(title: str, year: int) -> RomFile: return _scanned_file( @@ -439,6 +560,17 @@ class TestSyncRomFiles: assert second.id == first.id assert second.file_name == "b.bin" + def test_renamed_reused_row_keeps_its_id(self, rom: Rom): + (first,) = _sync(rom, [_scanned_file(rom, "old.nsp")]) + first.file_name = "new.nsp" + + synced = db_rom_handler.sync_rom_files(rom.id, [first]) + + stored = db_rom_handler.get_rom_file_by_id(first.id) + assert stored is not None + assert synced.files[0].id == first.id + assert stored.file_name == "new.nsp" + def test_moved_file_is_matched_by_content(self, rom: Rom): (first,) = _sync(rom, [_scanned_file(rom, "a.bin")]) (second,) = _sync( diff --git a/backend/tests/handler/filesystem/test_roms_handler.py b/backend/tests/handler/filesystem/test_roms_handler.py index 01e1f8d3b..80a9a89a7 100644 --- a/backend/tests/handler/filesystem/test_roms_handler.py +++ b/backend/tests/handler/filesystem/test_roms_handler.py @@ -15,7 +15,7 @@ from tests._zipfile_shim import reload_zipfile from adapters.services.sigil import SigilExtractionResult from config import LIBRARY_BASE_PATH -from config.config_manager import DEFAULT_EXCLUDED_EXTENSIONS, Config +from config.config_manager import DEFAULT_EXCLUDED_EXTENSIONS, Config, ConvertoConfig from handler.filesystem.base_handler import ( LANGUAGES_BY_SHORTCODE, REGIONS_BY_SHORTCODE, @@ -1949,6 +1949,7 @@ def sigil_config(monkeypatch): "default": "{platform}/roms/{game}", "firmware": "{platform}/bios", }, + CONVERTO=ConvertoConfig(), ) monkeypatch.setattr("handler.filesystem.roms_handler.cm.get_config", lambda: cnfg) return cnfg @@ -3248,6 +3249,7 @@ class TestIncrementalRomFiles: "default": "{platform}/roms/{game}", "firmware": "{platform}/bios", }, + CONVERTO=ConvertoConfig(), ) mocker.patch( "handler.filesystem.roms_handler.cm.get_config", return_value=config diff --git a/backend/tests/handler/filesystem/test_roms_title_ids.py b/backend/tests/handler/filesystem/test_roms_title_ids.py new file mode 100644 index 000000000..61dae8ae6 --- /dev/null +++ b/backend/tests/handler/filesystem/test_roms_title_ids.py @@ -0,0 +1,687 @@ +"""Tests for rom-converto's per-file title ids during scan.""" + +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import pytest + +from adapters.services.rom_converto import RomConvertoInfo, rom_converto_service +from adapters.services.sigil import SigilExtractionResult +from config.config_manager import Config, ConvertoConfig +from handler.filesystem.roms_handler import FSRom, FSRomsHandler, _rom_level_identity +from handler.scan_handler import ScanType, scan_rom +from models.platform import Platform +from models.rom import Rom, RomFile, RomIdentity, SaveTargetLayout +from utils import switch + + +def _info(**overrides) -> RomConvertoInfo: + defaults: dict[str, Any] = { + "title_id": "SCUS-94163", + "title_version": None, + } + defaults.update(overrides) + return RomConvertoInfo(**defaults) + + +@pytest.fixture +def handler() -> FSRomsHandler: + return FSRomsHandler() + + +@pytest.fixture +def psx_rom() -> Rom: + return Rom( + id=1, + fs_name="PaRappa the Rapper (USA).chd", + fs_path="psx/roms", + fs_extension="chd", + platform=Platform(name="PlayStation", slug="psx", fs_slug="psx"), + ) + + +def _patch_service(mocker, read_infos=None, enabled: bool = True, scan_metadata=True): + mocker.patch.object( + rom_converto_service, "is_enabled", mocker.AsyncMock(return_value=enabled) + ) + if read_infos is not None: + mocker.patch.object(rom_converto_service, "read_infos", read_infos) + mocker.patch( + "handler.filesystem.roms_handler.cm.get_config", + lambda: SimpleNamespace(CONVERTO=SimpleNamespace(scan_metadata=scan_metadata)), + ) + + +class TestReadConvertoInfos: + @pytest.mark.asyncio + async def test_fills_title_id_and_version(self, handler, mocker): + rom_file = RomFile(file_name="game.chd", file_path="psx/roms") + path = Path("/romm/library/psx/roms/game.chd") + read_infos = mocker.AsyncMock(return_value={path: _info(title_version=65536)}) + _patch_service(mocker, read_infos) + + await handler._read_converto_infos([(path, rom_file)]) + + assert rom_file.title_id == "SCUS-94163" + assert rom_file.title_version == 65536 + + @pytest.mark.asyncio + async def test_no_sources_skip_the_subprocess(self, handler, mocker): + read_infos = mocker.AsyncMock() + _patch_service(mocker, read_infos) + + await handler._read_converto_infos([]) + + read_infos.assert_not_awaited() + + @pytest.mark.asyncio + async def test_idless_info_still_sets_version(self, handler, mocker): + unknown = RomFile( + file_name="a.chd", + file_path="psx/roms", + title_id="old-id", + title_version=2, + ) + idless = RomFile( + file_name="b.chd", + file_path="psx/roms", + title_id="old-id", + title_version=2, + ) + read_infos = mocker.AsyncMock( + return_value={Path("/lib/b.chd"): _info(title_id=None, title_version=3)} + ) + _patch_service(mocker, read_infos) + + await handler._read_converto_infos( + [(Path("/lib/a.chd"), unknown), (Path("/lib/b.chd"), idless)] + ) + + assert (unknown.title_id, unknown.title_version) == ("old-id", 2) + assert (idless.title_id, idless.title_version) == (None, 3) + + @pytest.mark.asyncio + async def test_marks_only_recognized_files(self, handler, mocker): + recognized = RomFile(file_name="game.chd", file_path="psx/roms") + unrecognized = RomFile(file_name="missing.chd", file_path="psx/roms") + recognized_path = Path("/lib/game.chd") + read_infos = mocker.AsyncMock(return_value={recognized_path: _info()}) + _patch_service(mocker, read_infos) + + await handler._read_converto_infos( + [(recognized_path, recognized), (Path("/lib/missing.chd"), unrecognized)] + ) + + assert recognized.converto_read_at is not None + assert unrecognized.converto_read_at is None + + +PS2_PLATFORM = Platform(name="PlayStation 2", slug="ps2", fs_slug="ps2") + + +class TestGetRomFilesWithConverto: + """rom-converto reads each rom's ids in one batch; sigil fills what it left unset.""" + + @pytest.fixture + def scan_env(self, tmp_path, mocker): + config = Config( + EXCLUDED_PLATFORMS=[], + EXCLUDED_SINGLE_EXT=[], + EXCLUDED_SINGLE_FILES=[], + EXCLUDED_MULTI_FILES=[], + EXCLUDED_MULTI_PARTS_EXT=[], + EXCLUDED_MULTI_PARTS_FILES=[], + PLATFORMS_BINDING={}, + PLATFORMS_VERSIONS={}, + STRUCTURE_TEMPLATES={ + "default": "{platform}/roms/{game}", + "firmware": "{platform}/bios", + }, + CONVERTO=ConvertoConfig(scan_metadata=True), + ) + mocker.patch( + "handler.filesystem.roms_handler.cm.get_config", return_value=config + ) + mocker.patch.object( + rom_converto_service, "is_enabled", mocker.AsyncMock(return_value=True) + ) + mocker.patch.object( + rom_converto_service, "_info_extensions", frozenset({".iso"}) + ) + handler = FSRomsHandler() + handler.base_path = tmp_path + rom_dir = tmp_path / "ps2" / "roms" / "Game" + rom_dir.mkdir(parents=True) + for name in ("Game (Disc 1).iso", "Game (Disc 2).iso", "readme.nfo"): + (rom_dir / name).write_bytes(b"rom-bytes") + rom = Rom( + id=1, + fs_name="Game", + fs_extension="", + fs_path="ps2/roms", + platform=PS2_PLATFORM, + ) + return SimpleNamespace(handler=handler, rom=rom, rom_dir=rom_dir) + + @pytest.fixture + def sigil_reads(self, mocker): + """Sigil identifies every file as SLUS-00001 with its own save target.""" + return mocker.patch( + "adapters.services.sigil.SigilService.extract_title_id", + mocker.AsyncMock( + return_value=SigilExtractionResult( + title_id="SLUS-00001", + save_target="SIGIL-TARGET", + usage="folder-prefix", + ) + ), + ) + + @pytest.mark.asyncio + async def test_converto_id_wins_and_sigil_keeps_the_save_target( + self, scan_env, sigil_reads, mocker + ): + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + disc2 = scan_env.rom_dir / "Game (Disc 2).iso" + read_infos = mocker.AsyncMock( + return_value={ + disc1: _info(title_id="SLUS-10001", title_version=2), + disc2: _info(title_id="SLUS-00002", title_version=1), + } + ) + mocker.patch.object(rom_converto_service, "read_infos", read_infos) + + parsed = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + + # The nfo has no inspectable extension, so it is never listed. + await_args = read_infos.await_args + assert await_args is not None + (listed,) = await_args.args + assert sorted(listed) == [disc1, disc2] + ids = {f.file_name: (f.title_id, f.title_version) for f in parsed.rom_files} + assert ids["Game (Disc 1).iso"] == ("SLUS-10001", 2) + assert ids["Game (Disc 2).iso"] == ("SLUS-00002", 1) + assert ids["readme.nfo"] == (None, None) + assert parsed.identity.title_id == "SLUS-10001" + assert parsed.identity.save_target == "SIGIL-TARGET" + + @pytest.mark.asyncio + async def test_sigil_fills_a_file_converto_does_not_recognize( + self, scan_env, sigil_reads, mocker + ): + mocker.patch.object( + rom_converto_service, "read_infos", mocker.AsyncMock(return_value={}) + ) + + parsed = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + + ids = {f.file_name: f.title_id for f in parsed.rom_files} + # Off Switch, sigil stops after the first disc it identifies. + assert ids["Game (Disc 1).iso"] == "SLUS-00001" + assert ids["Game (Disc 2).iso"] is None + assert parsed.identity.title_id == "SLUS-00001" + + @pytest.mark.asyncio + async def test_a_stored_sigil_id_yields_to_a_fresh_extraction( + self, scan_env, sigil_reads, mocker + ): + mocker.patch.object( + rom_converto_service, "read_infos", mocker.AsyncMock(return_value={}) + ) + first = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + by_name = {f.file_name: f for f in first.rom_files} + by_name["Game (Disc 1).iso"].title_id = "SLUS-99999" + + rescan = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False, existing_files=first.rom_files + ) + + ids = {f.file_name: f.title_id for f in rescan.rom_files} + assert ids["Game (Disc 1).iso"] == "SLUS-00001" + assert rescan.identity.title_id == "SLUS-00001" + + @pytest.mark.asyncio + async def test_converto_identity_when_sigil_reads_nothing(self, scan_env, mocker): + mocker.patch( + "adapters.services.sigil.SigilService.extract_title_id", + mocker.AsyncMock(return_value=None), + ) + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + mocker.patch.object( + rom_converto_service, + "read_infos", + mocker.AsyncMock(return_value={disc1: _info(title_id="SLUS-00001")}), + ) + + parsed = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + + assert parsed.identity.title_id == "SLUS-00001" + assert parsed.identity.save_target is None + + @pytest.mark.asyncio + async def test_stored_identity_survives_matching_converto_id( + self, scan_env, mocker + ): + stored = RomIdentity( + title_id="SLUS-00001", + save_target="stored-target", + save_target_layout=SaveTargetLayout.FILE_PREFIX, + ) + scan_env.rom.title_id = stored.title_id + scan_env.rom.save_target = stored.save_target + scan_env.rom.save_target_layout = stored.save_target_layout + mocker.patch( + "adapters.services.sigil.SigilService.extract_title_id", + mocker.AsyncMock(return_value=None), + ) + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + mocker.patch.object( + rom_converto_service, + "read_infos", + mocker.AsyncMock(return_value={disc1: _info(title_id=stored.title_id)}), + ) + + parsed = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + + assert parsed.identity == stored + + @pytest.mark.asyncio + async def test_title_ids_land_on_new_file_rows(self, scan_env, mocker): + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + mocker.patch.object( + rom_converto_service, + "read_infos", + mocker.AsyncMock( + return_value={disc1: _info(title_id="SLUS-10001", title_version=1)} + ), + ) + + parsed = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + + by_name = {f.file_name: f for f in parsed.rom_files} + disc1_row = by_name["Game (Disc 1).iso"] + assert (disc1_row.title_id, disc1_row.title_version) == ("SLUS-10001", 1) + # A file rom-converto returned nothing for stays unread. + assert by_name["Game (Disc 2).iso"].converto_read_at is None + + @pytest.mark.asyncio + async def test_converto_identity_is_the_first_disc_whatever_the_listing( + self, scan_env, mocker + ): + mocker.patch( + "adapters.services.sigil.SigilService.extract_title_id", + mocker.AsyncMock(return_value=None), + ) + list_rom_dir = FSRomsHandler._list_rom_dir + mocker.patch.object( + FSRomsHandler, + "_list_rom_dir", + lambda self, *args: sorted(list_rom_dir(self, *args), reverse=True), + ) + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + disc2 = scan_env.rom_dir / "Game (Disc 2).iso" + mocker.patch.object( + rom_converto_service, + "read_infos", + mocker.AsyncMock( + return_value={ + disc1: _info(title_id="SLUS-00001"), + disc2: _info(title_id="SLUS-00002"), + } + ), + ) + + parsed = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + + assert parsed.identity.title_id == "SLUS-00001" + + @pytest.mark.asyncio + async def test_stored_ids_give_no_identity_when_extraction_is_skipped( + self, scan_env, mocker + ): + mocker.patch( + "adapters.services.sigil.SigilService.extract_title_id", + mocker.AsyncMock(return_value=None), + ) + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + mocker.patch.object( + rom_converto_service, + "read_infos", + mocker.AsyncMock(return_value={disc1: _info(title_id="SLUS-00001")}), + ) + first = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + + rescan = await scan_env.handler.get_rom_files( + scan_env.rom, + calculate_hashes=False, + extract_title_ids=False, + existing_files=first.rom_files, + ) + + assert {f.title_id for f in rescan.rom_files} >= {"SLUS-00001"} + assert rescan.identity == RomIdentity() + + @pytest.mark.asyncio + async def test_title_ids_still_land_when_extraction_is_skipped( + self, scan_env, mocker + ): + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + mocker.patch.object( + rom_converto_service, + "read_infos", + mocker.AsyncMock(return_value={disc1: _info(title_id="SLUS-10001")}), + ) + + parsed = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False, extract_title_ids=False + ) + + by_name = {f.file_name: f for f in parsed.rom_files} + assert by_name["Game (Disc 1).iso"].title_id == "SLUS-10001" + # Skipping extraction also skips the rom-level id fallback, so the + # save target sigil wrote stays untouched. + assert parsed.identity == RomIdentity() + + @pytest.mark.asyncio + async def test_unchanged_rows_are_not_reread(self, scan_env, sigil_reads, mocker): + discs = [scan_env.rom_dir / f"Game (Disc {n}).iso" for n in (1, 2)] + read_infos = mocker.AsyncMock(return_value={disc: _info() for disc in discs}) + mocker.patch.object(rom_converto_service, "read_infos", read_infos) + first = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + read_infos.reset_mock() + + rescan = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False, existing_files=first.rom_files + ) + + read_infos.assert_not_awaited() + assert {f.file_name for f in rescan.rom_files} == { + "Game (Disc 1).iso", + "Game (Disc 2).iso", + "readme.nfo", + } + + @pytest.mark.asyncio + async def test_unchanged_rows_rom_converto_never_read_are_backfilled( + self, scan_env, sigil_reads, mocker + ): + disc1 = scan_env.rom_dir / "Game (Disc 1).iso" + read_infos = mocker.AsyncMock(return_value={}) + mocker.patch.object(rom_converto_service, "read_infos", read_infos) + first = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False + ) + read_infos.reset_mock() + read_infos.return_value = {disc1: _info(title_id="SLUS-10001")} + + rescan = await scan_env.handler.get_rom_files( + scan_env.rom, calculate_hashes=False, existing_files=first.rom_files + ) + + read_infos.assert_awaited_once() + await_args = read_infos.await_args + assert await_args is not None + assert set(await_args.args[0]) == { + disc1, + scan_env.rom_dir / "Game (Disc 2).iso", + } + by_name = {f.file_name: f for f in rescan.rom_files} + assert by_name["Game (Disc 1).iso"].title_id == "SLUS-10001" + assert by_name["Game (Disc 1).iso"].converto_read_at is not None + assert by_name["Game (Disc 2).iso"].converto_read_at is None + + +@pytest.mark.parametrize( + ("service_state", "expected"), + [ + pytest.param({}, True, id="enabled"), + pytest.param({"enabled": False}, False, id="service-disabled"), + pytest.param({"scan_metadata": False}, False, id="scan-metadata-off"), + ], +) +async def test_converto_active_for_supported_platform( + handler, psx_rom, mocker, service_state: dict[str, bool], expected: bool +): + _patch_service(mocker, **service_state) + + assert await handler.converto_active(psx_rom.platform_slug) is expected + + +async def test_converto_active_false_for_unsupported_platform(handler, mocker): + _patch_service(mocker) + + assert await handler.converto_active("n64") is False + + +def _fs_rom( + files: list[RomFile], sha1_hash: str, identity: RomIdentity | None = None +) -> FSRom: + fs_rom: FSRom = { + "fs_name": "PaRappa the Rapper (USA).chd", + "fs_path": "psx/roms", + "flat": True, + "files": files, + "crc_hash": "", + "md5_hash": "", + "sha1_hash": sha1_hash, + "ra_hash": "", + } + if identity is not None: + fs_rom["identity"] = identity + return fs_rom + + +def _rom_file(name: str, **kwargs) -> RomFile: + return RomFile(file_name=name, file_path="psx/roms", file_size_bytes=1, **kwargs) + + +@pytest.fixture +def patched_scan_env(mocker): + mocker.patch( + "handler.scan_handler.db_rom_handler.add_rom", side_effect=lambda rom: rom + ) + mocker.patch( + "handler.scan_handler.cm.get_config", + return_value=SimpleNamespace( + SCAN_METADATA_PRIORITY=[], + SCAN_ARTWORK_PRIORITY=[], + SCAN_ARTWORK_PRIORITY_OVERRIDES={}, + ), + ) + + +class TestScanRomTitleId: + @pytest.mark.asyncio + async def test_title_id_flows_from_identity(self, patched_scan_env, psx_rom): + # get_rom_files resolves the rom-level identity from converto/sigil; + # scan_rom trusts what fs_rom carries. + scanned = await scan_rom( + scan_type=ScanType.QUICK, + platform=psx_rom.platform, + rom=psx_rom, + fs_rom=_fs_rom( + [_rom_file("game.chd", sha1_hash="abc123")], + sha1_hash="abc123", + identity=RomIdentity(title_id="SCUS-94163"), + ), + metadata_sources=[], + newly_added=True, + ) + + assert scanned.title_id == "SCUS-94163" + + @pytest.mark.asyncio + async def test_title_id_carried_forward_without_files( + self, patched_scan_env, psx_rom + ): + psx_rom.title_id = "SCUS-94163" + scanned = await scan_rom( + scan_type=ScanType.QUICK, + platform=psx_rom.platform, + rom=psx_rom, + fs_rom=_fs_rom([], sha1_hash=""), + metadata_sources=[], + newly_added=True, + ) + + assert scanned.title_id == "SCUS-94163" + + @pytest.mark.asyncio + async def test_title_id_preserved_when_files_yield_none( + self, patched_scan_env, psx_rom + ): + # Files are present but extraction finds no title id on any of them; + # the re-scan must keep the value already stored on the rom. + psx_rom.title_id = "SCUS-94163" + scanned = await scan_rom( + scan_type=ScanType.QUICK, + platform=psx_rom.platform, + rom=psx_rom, + fs_rom=_fs_rom( + [_rom_file("game.chd", sha1_hash="abc123")], sha1_hash="abc123" + ), + metadata_sources=[], + newly_added=True, + ) + + assert scanned.title_id == "SCUS-94163" + + @pytest.mark.asyncio + async def test_title_id_updated_with_new_value(self, patched_scan_env, psx_rom): + # A new truthy title id from the re-scan must overwrite the stored one. + psx_rom.title_id = "OLD-ID" + scanned = await scan_rom( + scan_type=ScanType.QUICK, + platform=psx_rom.platform, + rom=psx_rom, + fs_rom=_fs_rom( + [_rom_file("game.chd", sha1_hash="abc123")], + sha1_hash="abc123", + identity=RomIdentity(title_id="SCUS-94163"), + ), + metadata_sources=[], + newly_added=True, + ) + + assert scanned.title_id == "SCUS-94163" + + +class TestRomLevelIdentity: + """A sigil extraction (already holding rom-converto's id) wins; else a file's bare id.""" + + def test_no_files_returns_empty_identity(self): + assert _rom_level_identity("psx", [], [], RomIdentity()) == RomIdentity() + + def test_converto_only_ids_use_first_found(self): + files = [ + _rom_file("disc1.chd", title_id=None), + _rom_file("disc2.chd", title_id="SCUS-94163"), + _rom_file("disc3.chd", title_id="OTHER-ID"), + ] + identity = _rom_level_identity("psx", [], files, RomIdentity()) + assert identity.title_id == "SCUS-94163" + assert identity.save_target is None + + def test_non_switch_first_file_id_wins_even_if_later_looks_like_base_id(self): + # These ids are shaped like Switch update/base ids, but the platform + # is not Switch, so the base-id preference must not kick in. + files = [ + _rom_file("disc1.bin", title_id="0100ABCD12340800"), + _rom_file("disc2.bin", title_id="0100ABCD12340000"), + ] + identity = _rom_level_identity("psx", [], files, RomIdentity()) + assert identity.title_id == "0100ABCD12340800" + + def test_switch_picks_base_id_when_present_among_converto_ids(self): + files = [ + _rom_file("update.nsp", title_id="0100ABCD12340800"), + _rom_file("base.nsp", title_id="0100ABCD12340000"), + ] + identity = _rom_level_identity("switch", [], files, RomIdentity()) + assert identity.title_id == "0100ABCD12340000" + assert switch.is_base_title_id(identity.title_id) + assert identity.save_target == "0100ABCD12340000" + assert identity.save_target_layout == SaveTargetLayout.FOLDER_EXACT + + def test_switch_base_id_keeps_the_stored_save_target(self): + stored = RomIdentity( + title_id="0100ABCD12340000", + save_target="stored-target", + save_target_layout=SaveTargetLayout.FOLDER_EXACT, + ) + files = [_rom_file("base.nsp", title_id="0100ABCD12340000")] + assert _rom_level_identity("switch", [], files, stored) == stored + + def test_switch_derives_base_id_when_only_update_id_present(self): + files = [_rom_file("update.nsp", title_id="0100ABCD12340800")] + identity = _rom_level_identity("switch", [], files, RomIdentity()) + + assert identity.title_id == "0100ABCD12340000" + assert identity.save_target == "0100ABCD12340000" + assert identity.save_target_layout == SaveTargetLayout.FOLDER_EXACT + + @pytest.mark.parametrize( + ("stored", "expected"), + [ + pytest.param( + RomIdentity( + title_id="SCUS-94163", + save_target="SCUS-94163", + save_target_layout=SaveTargetLayout.FILE_PREFIX, + ), + RomIdentity( + title_id="SCUS-94163", + save_target="SCUS-94163", + save_target_layout=SaveTargetLayout.FILE_PREFIX, + ), + id="same-id-keeps-the-stored-save-target", + ), + pytest.param( + RomIdentity( + title_id="SLUS-00001", + save_target="SLUS-00001", + save_target_layout=SaveTargetLayout.FILE_PREFIX, + ), + RomIdentity(title_id="SCUS-94163"), + id="another-id-drops-the-stored-save-target", + ), + ], + ) + def test_converto_fallback_against_the_stored_identity( + self, stored: RomIdentity, expected: RomIdentity + ): + # sigil read nothing this pass, e.g. a transient read error. + files = [_rom_file("game.chd", title_id="SCUS-94163")] + + assert _rom_level_identity("psx", [], files, stored) == expected + + def test_extraction_wins_over_file_ids(self): + files = [_rom_file("game.chd", title_id="CONVERTO-ID")] + extraction = SigilExtractionResult( + title_id="SIGIL-ID", save_target="SIGIL-TARGET", usage="file-prefix" + ) + + identity = _rom_level_identity("psx", [extraction], files, RomIdentity()) + + assert identity.title_id == "SIGIL-ID" + assert identity.save_target == "SIGIL-TARGET" + assert identity.save_target_layout == SaveTargetLayout.FILE_PREFIX diff --git a/backend/tests/handler/test_rom_files.py b/backend/tests/handler/test_rom_files.py index fd3742b74..f131e99a2 100644 --- a/backend/tests/handler/test_rom_files.py +++ b/backend/tests/handler/test_rom_files.py @@ -1,11 +1,16 @@ import hashlib import os +from datetime import datetime, timezone from pathlib import Path from unittest.mock import AsyncMock, patch import pytest from tests.factories import make_rom +from adapters.services.rom_converto import ( + RomConvertoTimeoutError, + rom_converto_service, +) from handler.database import db_rom_handler from handler.filesystem import fs_rom_handler from handler.filesystem.roms_handler import ParsedRomFiles, category_for_path_parts @@ -311,3 +316,113 @@ async def test_a_category_settled_on_a_reused_row_is_persisted( assert result.updated_files == 1 assert _files_by_name(rom.id)["game.bin"].category == RomFileCategory.DLC + + +async def test_a_title_id_read_off_a_reused_row_is_persisted( + platform, admin_user, library +): + """Extraction writes a file's title id onto the row it reused.""" + rom = _folder_rom(platform, admin_user, library, {"game.bin": b"game"}) + + async def read_title_id(*_args, **_kwargs) -> ParsedRomFiles: + parsed = _unchanged_parse(rom, RomIdentity()) + parsed.rom_files[0].title_id = "ULUS-10041" + parsed.rom_files[0].title_version = 1 + return parsed + + with patch.object(fs_rom_handler, "get_rom_files", read_title_id): + result = await refresh_rom_files(rom) + + stored = _files_by_name(rom.id)["game.bin"] + assert result.updated_files == 1 + assert (stored.title_id, stored.title_version) == ("ULUS-10041", 1) + + +async def test_a_read_that_confirms_a_reused_rows_title_id_is_persisted( + platform, admin_user, library +): + rom = _folder_rom(platform, admin_user, library, {"game.bin": b"game"}) + + async def confirm_title_id(*_args, **_kwargs) -> ParsedRomFiles: + parsed = _unchanged_parse(rom, RomIdentity()) + parsed.rom_files[0].converto_read_at = datetime.now(timezone.utc) + return parsed + + with patch.object(fs_rom_handler, "get_rom_files", confirm_title_id): + result = await refresh_rom_files(rom) + + assert result.updated_files == 1 + assert _files_by_name(rom.id)["game.bin"].converto_read_at is not None + + +async def test_refresh_writes_a_fresh_rom_converto_read( + platform, admin_user, library, mocker +): + rom = _folder_rom(platform, admin_user, library, {"game.bin": b"game"}) + stored_file = rom.files[0] + scanned_file = RomFile( + rom_id=rom.id, + file_name=stored_file.file_name, + file_path=stored_file.file_path, + file_size_bytes=stored_file.file_size_bytes, + last_modified=stored_file.last_modified, + crc_hash=stored_file.crc_hash, + md5_hash=stored_file.md5_hash, + sha1_hash=stored_file.sha1_hash, + category=stored_file.category, + title_id="ULUS-10041", + converto_read_at=datetime.now(timezone.utc), + ) + parsed = ParsedRomFiles( + rom_files=[scanned_file], + crc_hash=rom.crc_hash or "", + md5_hash=rom.md5_hash or "", + sha1_hash=rom.sha1_hash or "", + ra_hash="", + ) + mocker.patch.object(fs_rom_handler, "get_rom_files", AsyncMock(return_value=parsed)) + + await refresh_rom_files(rom) + + stored = _files_by_name(rom.id)["game.bin"] + assert stored.title_id == "ULUS-10041" + assert stored.converto_read_at is not None + + +async def test_failed_info_run_keeps_stored_title_ids(platform, library, mocker): + file_path = f"{platform.fs_slug}/roms/{FOLDER}" + st = _write(library, f"{file_path}/game.iso", b"game") + metadata = {"title_id": "ULUS-10041", "title_version": 1} + rom = make_rom( + platform, + FOLDER, + fs_extension="", + fs_path=f"{platform.fs_slug}/roms", + fs_size_bytes=st.st_size, + files=[ + RomFile( + file_name="game.iso", + file_path=file_path, + file_size_bytes=st.st_size, + last_modified=st.st_mtime, + **metadata, + ) + ], + ) + rom = _reload(rom.id) + file_id = rom.files[0].id + _write(library, f"{file_path}/new.iso", b"new-game") + mocker.patch.object(fs_rom_handler, "converto_active", AsyncMock(return_value=True)) + mocker.patch.object(rom_converto_service, "_info_extensions", frozenset({".iso"})) + mocker.patch( + "adapters.services.rom_converto._run", + side_effect=RomConvertoTimeoutError("slow"), + ) + + result = await refresh_rom_files(rom) + + stored = _files_by_name(rom.id)["game.iso"] + assert result.new_files == 1 + assert stored.id == file_id + assert stored.file_size_bytes == st.st_size + assert {column: getattr(stored, column) for column in metadata} == metadata diff --git a/backend/tests/test_migrations.py b/backend/tests/test_migrations.py index 37ba3b608..858661049 100644 --- a/backend/tests/test_migrations.py +++ b/backend/tests/test_migrations.py @@ -253,6 +253,7 @@ def _replay(connection: sa.Connection, filename: str) -> None: ("0144_user_oidc_sub.py", "users"), ("0145_drop_derivable_columns.py", "rom_file_doc_meta"), ("0146_roms_search_aliases.py", "roms"), + ("0147_rom_file_title_ids.py", "rom_files"), ], ) def test_a_revision_replayed_over_the_migrated_schema_is_a_no_op( @@ -414,6 +415,23 @@ def test_the_search_titles_revision_reverses_replays_and_fills(platform: Platfor assert expected[first.id] == "\x1ffinal fantasy vii\x1fff7\x1f" +def test_the_rom_file_title_ids_revision_reverses_and_replays(): + migration = _load_migration("0147_rom_file_title_ids.py") + + with sync_engine.begin() as connection: + before = _schema_of(connection, "rom_files") + with Operations.context(MigrationContext.configure(connection)): + migration.downgrade() + assert not has_column(connection, "rom_files", "converto_read_at") + assert not has_column(connection, "rom_files", "title_id") + + migration.downgrade() + migration.upgrade() + migration.upgrade() + + assert _schema_of(connection, "rom_files") == before + + def _slot_collations(connection: sa.Connection) -> dict[str, str | None]: inspector = sa.inspect(connection) collations = {} diff --git a/backend/tests/utils/test_switch.py b/backend/tests/utils/test_switch.py index b057ff6b7..ae46249ae 100644 --- a/backend/tests/utils/test_switch.py +++ b/backend/tests/utils/test_switch.py @@ -1,5 +1,6 @@ import pytest +from models.rom import RomIdentity, SaveTargetLayout from utils import switch @@ -44,6 +45,51 @@ class TestDeriveBaseTitleId: assert switch.derive_base_title_id(title_id) is None +_BASE_FOLDER = RomIdentity( + title_id="0100ABCD12340000", + save_target="0100ABCD12340000", + save_target_layout=SaveTargetLayout.FOLDER_EXACT, +) + + +class TestNormalizeIdentity: + @pytest.mark.parametrize( + "title_id", + [ + pytest.param("0100ABCD12340800", id="update"), + pytest.param("0100ABCD12340000", id="base-without-save-target"), + ], + ) + def test_switch_ids_key_saves_by_the_base_id(self, title_id: str): + identity = RomIdentity(title_id=title_id) + + assert switch.normalize_identity(True, identity) == _BASE_FOLDER + + def test_a_base_id_keeps_its_save_target(self): + identity = RomIdentity( + title_id="0100ABCD12340000", + save_target="sigil-target", + save_target_layout=SaveTargetLayout.FOLDER_EXACT, + ) + + assert switch.normalize_identity(True, identity) == identity + + @pytest.mark.parametrize( + ("is_switch", "title_id"), + [ + pytest.param(False, "0100ABCD12340800", id="not-switch"), + pytest.param(True, "ZZZ", id="not-derivable"), + pytest.param(True, None, id="no-id"), + ], + ) + def test_other_identities_are_left_alone( + self, is_switch: bool, title_id: str | None + ): + identity = RomIdentity(title_id=title_id) + + assert switch.normalize_identity(is_switch, identity) == identity + + class TestTitleIdRegexes: def test_a_bare_id_matches_the_plain_regex(self): assert switch.TITLE_ID_REGEX.fullmatch("0100ABCD12340000") diff --git a/backend/utils/switch.py b/backend/utils/switch.py index 673ca860b..ff7465ec1 100644 --- a/backend/utils/switch.py +++ b/backend/utils/switch.py @@ -49,7 +49,7 @@ def normalize_identity(is_switch: bool, identity: RomIdentity) -> RomIdentity: """ title_id = identity.title_id derived = derive_base_title_id(title_id) if is_switch and title_id else None - if derived is None or derived == title_id: + if derived is None or (derived == title_id and identity.save_target): return identity # Switch saves are keyed by the base title id itself. diff --git a/docs/BACKEND_ARCHITECTURE.md b/docs/BACKEND_ARCHITECTURE.md index d39e8bba8..c845d5ac2 100644 --- a/docs/BACKEND_ARCHITECTURE.md +++ b/docs/BACKEND_ARCHITECTURE.md @@ -598,15 +598,17 @@ Constants: `FILE_NAME_MAX_LENGTH=450`, `FILE_PATH_MAX_LENGTH=1000`, `FILE_EXTENS Tracks individual files within a ROM (archives can contain multiple files). -| Column | Type | Notes | -| ---------------------------------------------- | ----------- | ---------------------------------------------------------------------------------------------------------------------------------- | -| `id` | Integer | PK | -| `rom_id` | Integer | FK → roms | -| `file_name`, `file_path` | String | File identity | -| `file_size_bytes` | BigInteger | Size | -| `crc_hash`, `md5_hash`, `sha1_hash`, `ra_hash` | String(100) | Hashes | -| `category` | Enum | `GAME`, `DLC`, `HACK`, `MANUAL`, `PATCH`, `UPDATE`, `MOD`, `DEMO`, `TRANSLATION`, `PROTOTYPE`, `CHEAT`, `SOUNDTRACK`, `SCREENSHOT` | -| `missing_from_fs` | Boolean | Sync state | +| Column | Type | Notes | +| ---------------------------------------------- | ----------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | +| `id` | Integer | PK | +| `rom_id` | Integer | FK → roms | +| `file_name`, `file_path` | String | File identity | +| `file_size_bytes` | BigInteger | Size | +| `crc_hash`, `md5_hash`, `sha1_hash`, `ra_hash` | String(100) | Hashes | +| `category` | Enum | `GAME`, `DLC`, `HACK`, `MANUAL`, `PATCH`, `UPDATE`, `MOD`, `DEMO`, `TRANSLATION`, `PROTOTYPE`, `CHEAT`, `SOUNDTRACK`, `SCREENSHOT` | +| `title_id`, `title_version` | String(100), BigInteger | Platform-native id and numeric version read from the binary (rom-converto or sigil) | +| `converto_read_at` | Timestamp | Last time rom-converto read the file; NULL queues it on the next scan | +| `missing_from_fs` | Boolean | Sync state | **Relationships:** rom (M:1), track_meta (1:1, `SOUNDTRACK` files only) diff --git a/examples/config.example.yml b/examples/config.example.yml index cc07cd7c4..812d9f955 100644 --- a/examples/config.example.yml +++ b/examples/config.example.yml @@ -75,6 +75,7 @@ # # Skip file hash calculations on low power devices (eg. Raspberry PI) # skip_hash_calculation: false # # Skip reading platform-native title IDs out of ROM binaries during scans +# # (rom-converto's per-file title IDs follow `converto.scan_metadata` instead) # skip_title_id_extraction: false # # Rename Switch ROMs to embed ` [TITLEID][vVERSION]` in their filename # embed_switch_title_ids: false @@ -258,6 +259,8 @@ # # Convert a single-file download into a format the client lists with # # ?format=, e.g. ?format=zso,iso (needs ROM_CONVERTO_ENABLED) # download_conversion_enabled: false +# # Read per-file title IDs with rom-converto during scans +# scan_metadata: true # # Hours a converted download stays cached after it was last downloaded # cache_ttl_hours: 24 # # Cache size before the least recently downloaded copies are evicted (0: unbounded) diff --git a/frontend/src/__generated__/models/ConvertoConfig.ts b/frontend/src/__generated__/models/ConvertoConfig.ts index 0029b7e5f..b4d3189e0 100644 --- a/frontend/src/__generated__/models/ConvertoConfig.ts +++ b/frontend/src/__generated__/models/ConvertoConfig.ts @@ -4,6 +4,7 @@ /* eslint-disable */ export type ConvertoConfig = { download_conversion_enabled?: boolean; + scan_metadata?: boolean; cache_ttl_hours?: number; cache_max_size_gb?: number; platform_formats?: Record; diff --git a/frontend/src/__generated__/models/ConvertoSettingsPayload.ts b/frontend/src/__generated__/models/ConvertoSettingsPayload.ts index c6fc57ece..328295085 100644 --- a/frontend/src/__generated__/models/ConvertoSettingsPayload.ts +++ b/frontend/src/__generated__/models/ConvertoSettingsPayload.ts @@ -3,7 +3,7 @@ /* tslint:disable */ /* eslint-disable */ /** - * The converto.* settings editable at runtime; `cache_ttl_hours` stays config.yml-only. + * Runtime-editable converto.* settings; `cache_ttl_hours` and `scan_metadata` stay in config.yml. */ export type ConvertoSettingsPayload = { download_conversion_enabled: boolean; diff --git a/frontend/src/stores/config.ts b/frontend/src/stores/config.ts index 96c12c583..258fe490b 100644 --- a/frontend/src/stores/config.ts +++ b/frontend/src/stores/config.ts @@ -53,6 +53,7 @@ const defaultConfig = { PEGASUS_AUTO_EXPORT_ON_SCAN: false, CONVERTO: { download_conversion_enabled: false, + scan_metadata: true, cache_ttl_hours: 24, cache_max_size_gb: 20, platform_formats: {},