perf(scan): batch restored-ROM hydration reads (#5145)

* perf(scan): batch restored-ROM hydration reads

Replace one get_rom_simple read per restored ROM with a single get_roms_simple_by_ids call. Same load shape, emit order, and None-skip preserved.

* test(scan): assert emitted ROM ids in visit order

* fix(scan): return per-ROM ids from SimpleRomSchema mock

* refactor(scan): extract restored-ROM emit helper, tighten order test

pr-ready step 2 (code-review --fix): scope the hydrated batch to a helper so it
is freed before the scan, and make the test catch visit-order and skip bugs.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* test(scan): reuse make_fs_rom and drop dead get_rom mock

pr-ready step 3 (simplify).

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

---------

Co-authored-by: Georges-Antoine Assi <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
Shreeniwas
2026-10-07 12:18:13 -04:00
committed by GitHub
co-authored by Claude Opus 5.5 Georges-Antoine Assi
parent dbf5ae94c3
commit 3f57d11d0b
2 changed files with 60 additions and 16 deletions
+26 -12
View File
@@ -921,6 +921,30 @@ async def _scan_selected_roms(
return scan_stats
async def _emit_restored_roms(
socket_manager: socketio.AsyncRedisManager, restored_roms: list[Rom]
) -> None:
"""Emit each ROM whose file came back, from one batched hydration read."""
if not restored_roms:
return
# Reload since the scan-loop lookup only eager-loads the platform.
hydrated_by_id = {
hydrated.id: hydrated
for hydrated in db_rom_handler.get_roms_simple_by_ids(
[restored.id for restored in restored_roms]
)
}
for restored_rom in restored_roms:
log.info(
f"{hl(restored_rom.fs_name)} is back in the filesystem, "
f"no longer {hl('missing', color=LIGHTYELLOW)}"
)
hydrated_rom = hydrated_by_id.get(restored_rom.id)
if hydrated_rom is not None:
await emit_scanning_rom(socket_manager, hydrated_rom, is_new=False)
async def _identify_platform(
platform_slug: str,
scan_type: ScanType,
@@ -1086,18 +1110,8 @@ async def _identify_platform(
# A ROM whose file came back would otherwise keep its stale "missing"
# badge in an open gallery: a skipped one emits nothing, and a scanned
# one only emits when its files changed. Reload since the scan-loop
# lookup only eager-loads the platform.
for restored_rom in restored_roms:
log.info(
f"{hl(restored_rom.fs_name)} is back in the filesystem, "
f"no longer {hl('missing', color=LIGHTYELLOW)}"
)
hydrated_rom = db_rom_handler.get_rom_simple(restored_rom.id)
if hydrated_rom is None:
continue
await emit_scanning_rom(socket_manager, hydrated_rom, is_new=False)
# one only emits when its files changed.
await _emit_restored_roms(socket_manager, restored_roms)
# Process only ROMs that actually need scanning
scan_tasks = [
+34 -4
View File
@@ -1487,7 +1487,7 @@ class TestIdentifyPlatformEmitsRestoredRoms:
db_rom = mocker.patch.object(scan_module, "db_rom_handler")
db_rom.get_roms_by_fs_name.return_value = {"test/roms/Game.zip": rom}
db_rom.mark_missing_roms.return_value = []
db_rom.get_rom.return_value = rom
db_rom.get_roms_simple_by_ids.return_value = [rom]
db_firmware = mocker.patch.object(scan_module, "db_firmware_handler")
db_firmware.mark_missing_firmware.return_value = []
@@ -1495,7 +1495,9 @@ class TestIdentifyPlatformEmitsRestoredRoms:
mocker.patch.object(
SimpleRomSchema,
"from_orm_with_factory",
return_value=Mock(model_dump=Mock(return_value={"id": rom.id})),
side_effect=lambda r, *a, **k: Mock(
model_dump=Mock(return_value={"id": r.id})
),
)
mocker.patch.object(scan_module, "_identify_rom", AsyncMock())
@@ -1521,7 +1523,7 @@ class TestIdentifyPlatformEmitsRestoredRoms:
await self._run(socket_manager)
patched.bulk_mark_present.assert_called_once_with(1, [42])
patched.get_rom_simple.assert_called_once_with(42)
patched.get_roms_simple_by_ids.assert_called_once_with([42])
assert any(
call.args[0] == "scan:scanning_rom"
for call in socket_manager.emit.call_args_list
@@ -1545,12 +1547,40 @@ class TestIdentifyPlatformEmitsRestoredRoms:
await self._run(socket_manager)
patched.get_rom_simple.assert_not_called()
patched.get_roms_simple_by_ids.assert_not_called()
assert not any(
call.args[0] == "scan:scanning_rom"
for call in socket_manager.emit.call_args_list
)
async def test_restored_roms_hydrated_with_single_batched_call(
self, patched, mocker
):
"""Three restored ROMs issue one batched hydration read, in visit order."""
fs_roms = [make_fs_rom(f"Game{i}.zip") for i in range(3)]
mocker.patch.object(fs_rom_handler, "get_roms", AsyncMock(return_value=fs_roms))
roms = [
Rom(id=11 + i, fs_name=fs_rom["fs_name"], platform_id=1)
for i, fs_rom in enumerate(fs_roms)
]
patched.get_roms_by_fs_name.return_value = {
f"test/roms/{rom.fs_name}": rom for rom in roms
}
patched.get_missing_rom_ids.return_value = {rom.id for rom in roms}
# Rows come back out of visit order, and the middle ROM drops out.
patched.get_roms_simple_by_ids.return_value = [roms[2], roms[0]]
socket_manager = AsyncMock()
await self._run(socket_manager)
patched.get_roms_simple_by_ids.assert_called_once_with([11, 12, 13])
emits = [
call
for call in socket_manager.emit.call_args_list
if call.args[0] == "scan:scanning_rom"
]
assert [call.args[1]["id"] for call in emits] == [11, 13]
class TestIdentifyPlatformFirmwareReporting:
"""The platform emit reports firmware discovered by this scan only.