From 10a6df585f77ef40a1ceff5b14afc3e2465793d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 12:31:46 +0000 Subject: [PATCH] perf(scan): stop eager-loading files in get_roms_by_fs_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore the "platform only" contract of `get_roms_by_fs_name` (per its docstring) by dropping the `selectinload(Rom.files)` + `joinedload`. That load only existed for `scan_rom`'s rare `fs_rom["files"] or rom.files` fallback, but it forced files (and a per-file join back to roms) for every ROM in a scan batch — expensive on large platforms, and only used when the filesystem scan yielded no files. Instead, fetch the persisted files on demand: `scan_rom` now resolves match files via a small helper that returns the filesystem-scanned files, falling back to `db_rom_handler.get_rom_files_by_rom_id(rom.id)` only when there are none. The new getter eager-loads the `RomFile.rom` backref so `is_top_level` keeps working on the detached results (the rare path was already latently broken on master, which loaded files without the backref). https://claude.ai/code/session_01PSXKmejPRzdxLFMN6P2QQ4 --- backend/handler/database/roms_handler.py | 26 +++++++++++++++++++++--- backend/handler/scan_handler.py | 22 ++++++++++++++++---- backend/tests/handler/test_db_handler.py | 16 +++++++++++++++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/backend/handler/database/roms_handler.py b/backend/handler/database/roms_handler.py index ecbe7168d..f76961af7 100644 --- a/backend/handler/database/roms_handler.py +++ b/backend/handler/database/roms_handler.py @@ -986,9 +986,6 @@ class DBRomsHandler(DBBaseHandler): select(Rom) .options( selectinload(Rom.platform), - selectinload(Rom.files).options( - joinedload(RomFile.rom).load_only(Rom.fs_path, Rom.fs_name) - ), ) .where( and_( @@ -1189,6 +1186,29 @@ class DBRomsHandler(DBBaseHandler): ) -> RomFile | None: return session.scalar(select(RomFile).filter_by(id=id).limit(1)) + @begin_session + def get_rom_files_by_rom_id( + self, + rom_id: int, + session: Session = None, # type: ignore + ) -> list[RomFile]: + """Return a ROM's files with the parent `rom` backref eager-loaded. + + Used by the scan fallback when the filesystem yielded no files: the + backref keeps `is_top_level` / `file_name_for_download` usable on the + detached results without `get_roms_by_fs_name` eager-loading files for + every ROM in a scan batch. + """ + return list( + session.scalars( + select(RomFile) + .options(joinedload(RomFile.rom).load_only(Rom.fs_path, Rom.fs_name)) + .filter_by(rom_id=rom_id) + ) + .unique() + .all() + ) + @begin_session def update_rom_file( self, diff --git a/backend/handler/scan_handler.py b/backend/handler/scan_handler.py index c7e3d3fbf..fe9ebd61e 100644 --- a/backend/handler/scan_handler.py +++ b/backend/handler/scan_handler.py @@ -47,7 +47,7 @@ from logger.logger import log from models.assets import Save, Screenshot, State from models.firmware import Firmware from models.platform import Platform -from models.rom import Rom +from models.rom import Rom, RomFile from models.user import User from utils import emoji @@ -373,6 +373,20 @@ async def scan_rom( } ) + # Files fed to metadata matchers: the freshly-scanned filesystem files, or + # the persisted files as a fallback. `rom` is detached and no longer carries + # eager-loaded files (get_roms_by_fs_name only loads `platform`), so fetch + # persisted files on demand — and only when the filesystem yielded none. + _persisted_files: list[RomFile] | None = None + + def rom_files_for_matching() -> list[RomFile]: + nonlocal _persisted_files + if fs_rom["files"]: + return fs_rom["files"] + if _persisted_files is None: + _persisted_files = db_rom_handler.get_rom_files_by_rom_id(rom.id) + return _persisted_files + async def fetch_playmatch_hash_match() -> PlaymatchRomMatch: if ( meta_playmatch_handler.is_enabled() @@ -385,7 +399,7 @@ async def scan_rom( or scan_type == ScanType.UNMATCHED ) ): - return await meta_playmatch_handler.lookup_rom(fs_rom["files"] or rom.files) + return await meta_playmatch_handler.lookup_rom(rom_files_for_matching()) return PlaymatchRomMatch( igdb_id=None, @@ -418,7 +432,7 @@ async def scan_rom( ) ): return await meta_hasheous_handler.lookup_rom( - platform.slug, fs_rom["files"] or rom.files + platform.slug, rom_files_for_matching() ) return HasheousRom(hasheous_id=None, igdb_id=None, tgdb_id=None, ra_id=None) @@ -645,7 +659,7 @@ async def scan_rom( # Use the file hashes for lookup game_by_hash, is_not_game = await meta_ss_handler.lookup_rom( - rom, platform.ss_id, fs_rom["files"] or rom.files + rom, platform.ss_id, rom_files_for_matching() ) if game_by_hash.get("ss_id") or is_not_game: return game_by_hash diff --git a/backend/tests/handler/test_db_handler.py b/backend/tests/handler/test_db_handler.py index 0fe45b3f1..c69caaa0e 100644 --- a/backend/tests/handler/test_db_handler.py +++ b/backend/tests/handler/test_db_handler.py @@ -96,6 +96,22 @@ def test_multi_file_rom_backref_survives_session_close(multi_file_rom: Rom): assert file.rom.full_path == folder_path +def test_get_rom_files_by_rom_id_eager_loads_parent(multi_file_rom: Rom): + """The scan fallback fetches a ROM's files on demand (since + `get_roms_by_fs_name` no longer eager-loads them). The returned files must + carry the `rom` backref so `is_top_level` works on the detached results. + """ + folder_path = f"{multi_file_rom.fs_path}/{multi_file_rom.fs_name}" + + files = db_rom_handler.get_rom_files_by_rom_id(multi_file_rom.id) + + assert {f.file_name for f in files} == {"disc1.bin", "disc2.bin"} + for file in files: + # Detached access — would raise DetachedInstanceError without the backref. + assert file.rom.full_path == folder_path + assert file.is_top_level + + def test_filter_last_played(rom: Rom, platform: Platform, admin_user: User): second_rom = db_rom_handler.add_rom( Rom(