fix(scan): stop rescans inflating live platform game counts (#4867)

This commit is contained in:
Sam Dornan
2026-09-28 08:54:50 -04:00
committed by GitHub
parent d033921efe
commit b4901551c8
7 changed files with 276 additions and 75 deletions
+5 -27
View File
@@ -20,7 +20,6 @@ from config import DEV_MODE, SCAN_TIMEOUT, SCAN_WORKERS, TASK_RESULT_TTL
from config.config_manager import MetadataMediaType
from config.config_manager import config_manager as cm
from endpoints.responses.platform import PlatformSchema
from endpoints.responses.rom import SimpleRomSchema
from endpoints.sockets.activity import get_authenticated_user
from exceptions.fs_exceptions import (
FOLDER_STRUCT_MSG,
@@ -68,6 +67,7 @@ from handler.scan_handler import (
ScanType,
build_hashless_fs_rom,
download_rom_resources,
emit_scanning_rom,
persist_soundtrack_cover,
scan_firmware,
scan_platform,
@@ -362,28 +362,6 @@ async def _identify_firmware(
return 1 if not firmware else 0
# `files` is left out so a scan does not ship every file row of every rom.
SCANNING_ROM_EXCLUDE: Final = {
"created_at",
"updated_at",
"rom_user",
"last_modified",
"files",
"sibling_roms",
}
async def _emit_scanning_rom(
socket_manager: socketio.AsyncRedisManager, rom: Rom
) -> None:
await socket_manager.emit(
"scan:scanning_rom",
SimpleRomSchema.from_orm_with_factory(rom).model_dump(
exclude=SCANNING_ROM_EXCLUDE
),
)
def should_scan_rom(
scan_type: ScanType,
rom: Rom | None,
@@ -628,7 +606,7 @@ async def _identify_rom(
)
hydrated_rom = db_rom_handler.get_rom_simple(rom.id)
if hydrated_rom is not None:
await _emit_scanning_rom(socket_manager, hydrated_rom)
await emit_scanning_rom(socket_manager, hydrated_rom, is_new=False)
return
# Update properties that don't require metadata
@@ -811,7 +789,7 @@ async def _identify_rom(
scanned_rom_ids.add(_added_rom.id)
if _added_rom.is_identified:
await _emit_scanning_rom(socket_manager, _added_rom)
await emit_scanning_rom(socket_manager, _added_rom, is_new=newly_added)
if should_update_files:
# Reconcile against the existing rows instead of replacing them, so file
@@ -835,7 +813,7 @@ async def _identify_rom(
metadata_sources=metadata_sources,
)
await _emit_scanning_rom(socket_manager, _added_rom)
await emit_scanning_rom(socket_manager, _added_rom, is_new=newly_added)
async def _scan_selected_roms(
@@ -1099,7 +1077,7 @@ async def _identify_platform(
if hydrated_rom is None:
continue
await _emit_scanning_rom(socket_manager, hydrated_rom)
await emit_scanning_rom(socket_manager, hydrated_rom, is_new=False)
# Process only ROMs that actually need scanning
scan_tasks = [
+24 -16
View File
@@ -2,7 +2,7 @@ import asyncio
import enum
import functools
from collections.abc import Callable
from typing import Any, NotRequired, TypedDict
from typing import Any, Final, NotRequired, TypedDict
import pydash
import socketio
@@ -537,6 +537,28 @@ async def resolve_steam_rom(
return await meta_steam_handler.get_rom(fs_name, platform_slug)
# `files` is left out so a scan does not ship every file row of every rom.
_SCANNING_ROM_EXCLUDE: Final = {
"created_at",
"updated_at",
"rom_user",
"last_modified",
"files",
"sibling_roms",
}
async def emit_scanning_rom(
socket_manager: socketio.AsyncRedisManager, rom: Rom, *, is_new: bool
) -> None:
"""Report a scanned rom; `is_new` lets clients count only roms the scan added."""
payload = SimpleRomSchema.from_orm_with_factory(rom).model_dump(
exclude=_SCANNING_ROM_EXCLUDE
)
payload["is_new"] = is_new
await socket_manager.emit("scan:scanning_rom", payload)
async def scan_rom(
scan_type: ScanType,
platform: Platform,
@@ -737,21 +759,7 @@ async def scan_rom(
_added_rom.is_identifying = True
if socket_manager:
await socket_manager.emit(
"scan:scanning_rom",
{
**SimpleRomSchema.from_orm_with_factory(_added_rom).model_dump(
exclude={
"created_at",
"updated_at",
"rom_user",
"last_modified",
"files",
"sibling_roms",
}
),
},
)
await emit_scanning_rom(socket_manager, _added_rom, is_new=newly_added)
# Run hash fetches concurrently
(
+41 -2
View File
@@ -40,6 +40,7 @@ from endpoints.sockets.scan import (
from exceptions.fs_exceptions import FolderStructureNotMatchException
from exceptions.socket_exceptions import ScanStoppedException
from handler import notification_handler
from handler import scan_handler as scan_handler_module
from handler.audit_handler import SYSTEM_ACTOR
from handler.auth.constants import Scope
from handler.database import db_collection_handler, db_platform_handler, db_rom_handler
@@ -833,7 +834,7 @@ class TestIdentifyRomTagReparse:
# A COMPLETE scan runs past the point a HASHES scan returns at, into the
# resource downloads and the closing emit, none of which is under test.
mocker.patch.object(scan_module, "download_rom_resources", new=AsyncMock())
mocker.patch.object(scan_module, "SimpleRomSchema", MagicMock())
mocker.patch.object(scan_handler_module, "SimpleRomSchema", MagicMock())
db = mocker.patch.object(scan_module, "db_rom_handler")
db.add_rom.return_value = MagicMock(
@@ -2717,7 +2718,7 @@ def identify_harness(mocker):
mocker.patch.object(scan_module, "fs_resource_handler", new=AsyncMock())
mocker.patch.object(scan_module, "download_rom_resources", new=AsyncMock())
mocker.patch.object(scan_module, "SimpleRomSchema", MagicMock())
mocker.patch.object(scan_handler_module, "SimpleRomSchema", MagicMock())
db = mocker.patch.object(scan_module, "db_rom_handler")
db.add_rom.return_value = MagicMock(
@@ -2841,6 +2842,44 @@ class TestIdentifyRomFiles:
identify_harness.db.add_rom.assert_called()
class TestIdentifyRomEmitsNewFlag:
"""Clients bump live game counts off `is_new`, so a rescan of an existing
rom must not report it as new."""
@pytest.mark.parametrize(
"case, is_new",
[("new", True), ("existing", False), ("reassociated", False)],
)
async def test_scanning_rom_payload_flags_new_roms(
self, mocker, identify_harness, case, is_new
):
dumped = Mock(model_dump=Mock(side_effect=lambda **_: {"id": 1}))
mocker.patch.object(
scan_handler_module,
"SimpleRomSchema",
Mock(from_orm_with_factory=Mock(return_value=dumped)),
)
# Identified, so both the post-scan and the post-download emits fire.
identify_harness.db.add_rom.return_value.is_identified = True
if case == "reassociated":
missing = identify_harness.existing_rom()
identify_harness.db.get_matching_missing_rom.return_value = missing
identify_harness.db.update_rom.return_value = missing
socket_manager = AsyncMock()
rom = identify_harness.existing_rom() if case == "existing" else None
await identify_harness.run(
rom, ScanType.COMPLETE, [], socket_manager=socket_manager
)
payloads = [
call.args[1]
for call in socket_manager.emit.await_args_list
if call.args[0] == "scan:scanning_rom"
]
assert [payload["is_new"] for payload in payloads] == [is_new, is_new]
class TestIdentifyRomIncrementalHashing:
"""Selected-rom metadata scans hand the stored rows to the file rebuild so
unchanged files keep their hashes; full rescans read every file."""
+31
View File
@@ -32,6 +32,7 @@ from handler.metadata.ss_handler import (
from handler.scan_handler import (
MetadataSource,
ScanType,
build_hashless_fs_rom,
scan_platform,
scan_rom,
)
@@ -1711,3 +1712,33 @@ async def test_scan_rom_games_still_use_fuzzy_catalog_covers(
assert result.igdb_id == 3340
assert result.name == "Paper Mario"
assert result.url_cover == "https://images.igdb.com/paper-mario.jpg"
@pytest.mark.parametrize("newly_added", [True, False])
async def test_scan_rom_emit_flags_new_roms(newly_added: bool):
"""Clients count a ROM toward its platform's games off `is_new`."""
platform = db_platform_handler.add_platform(
Platform(id=1, slug="n64", fs_slug="n64", name="Nintendo 64")
)
rom = db_rom_handler.add_rom(
Rom(platform_id=platform.id, fs_name="Game.z64", fs_path="n64", tags=[])
)
socket_manager = AsyncMock()
async with initialize_context():
await scan_rom(
platform=platform,
scan_type=ScanType.QUICK,
rom=rom,
fs_rom=build_hashless_fs_rom(rom.fs_name, rom.fs_path, flat=True),
metadata_sources=[],
newly_added=newly_added,
socket_manager=socket_manager,
)
payloads = [
call.args[1]
for call in socket_manager.emit.await_args_list
if call.args[0] == "scan:scanning_rom"
]
assert [payload["is_new"] for payload in payloads] == [newly_added]
+1 -3
View File
@@ -189,9 +189,7 @@ class TestStalledExtractorIsKilled:
@pytest.mark.usefixtures("stalling_7zz")
def test_largest_member_hashing_fails(self):
start = time.monotonic()
assert not archives._process_largest_7z_member(
Path("/fake/game.7z"), MagicMock()
)
assert not archives.hash_largest_7z_member(Path("/fake/game.7z"), MagicMock())
assert time.monotonic() - start < 10
@pytest.mark.usefixtures("stalling_7zz")
@@ -4,8 +4,11 @@ import { createPinia, setActivePinia } from "pinia";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { defineComponent, reactive } from "vue";
import type { ScanStats } from "@/__generated__";
import platformApi from "@/services/api/platform";
import taskApi from "@/services/api/task";
import storeCollections from "@/stores/collections";
import storePlatforms, { type Platform } from "@/stores/platforms";
import storeRoms, { type SimpleRom } from "@/stores/roms";
import storeScanning from "@/stores/scanning";
import type { Events } from "@/types/emitter";
import { installScanLifecycle } from "./index";
@@ -55,6 +58,51 @@ const getTaskStatus = vi.mocked(taskApi.getTaskStatus);
/** Drain pending microtasks so the reconcile's promise chain has settled. */
const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0));
/** Outlast the handler's 100ms batching debounce for `scan:scanning_rom`. */
const drainRomBatch = () => new Promise((resolve) => setTimeout(resolve, 150));
function platform(overrides: Partial<Platform> = {}): Platform {
return {
id: 1,
slug: "n64",
fs_slug: "n64",
rom_count: 2,
name: "Nintendo 64",
igdb_slug: null,
moby_slug: null,
hltb_slug: null,
libretro_slug: null,
created_at: "",
updated_at: "",
fs_size_bytes: 0,
is_unidentified: false,
is_identified: true,
missing_from_fs: false,
display_name: "Nintendo 64",
firmware_count: 0,
...overrides,
};
}
function simpleRom(overrides: Partial<SimpleRom> = {}): SimpleRom {
return { id: 1, name: "Game", platform_id: 1, ...overrides } as SimpleRom;
}
function scanningRom(
id: number,
isNew: boolean,
extra: Partial<SimpleRom> = {},
) {
return {
id,
platform_id: 1,
platform_fs_slug: "n64",
platform_display_name: "Nintendo 64",
is_new: isNew,
...extra,
};
}
function makeStats(overrides: Partial<ScanStats> = {}): ScanStats {
return {
total_platforms: 0,
@@ -170,6 +218,86 @@ describe("installScanLifecycle", () => {
expect(refresh).toHaveBeenCalledTimes(1);
});
it("counts only the ROMs a scan added toward the platform's games", async () => {
install();
const platforms = storePlatforms();
platforms.set([platform()]);
fire("scan:scanning_rom", scanningRom(1, false));
fire("scan:scanning_rom", scanningRom(2, false));
fire("scan:scanning_rom", scanningRom(3, true));
fire("scan:scanning_rom", scanningRom(3, true));
await drainRomBatch();
expect(platforms.get(1)?.rom_count).toBe(3);
});
it("counts a new ROM once when a platform event rebuilds the live log", async () => {
install();
const platforms = storePlatforms();
platforms.set([platform()]);
const newRom = scanningRom(3, true);
fire("scan:scanning_rom", newRom);
await drainRomBatch();
fire("scan:scanning_platform", {
id: 1,
name: "Nintendo 64",
display_name: "Nintendo 64",
slug: "n64",
fs_slug: "n64",
is_identified: true,
new_firmware_count: 0,
});
fire("scan:scanning_rom", newRom);
await drainRomBatch();
expect(platforms.get(1)?.rom_count).toBe(3);
});
it("applies the last ROM batch before reconciling counts on scan:done", async () => {
vi.mocked(platformApi.getPlatforms).mockResolvedValueOnce({
data: [platform({ rom_count: 3 })],
} as never);
install();
const platforms = storePlatforms();
platforms.set([platform()]);
fire("scan:scanning_rom", scanningRom(3, true));
fire("scan:done", makeStats());
await drainRomBatch();
expect(platforms.get(1)?.rom_count).toBe(3);
});
it("counts a new ROM once when a scan fails with its next emit queued", async () => {
install();
const platforms = storePlatforms();
platforms.set([platform()]);
fire("scan:scanning_rom", scanningRom(3, true));
await drainRomBatch();
fire("scan:scanning_rom", scanningRom(3, true));
fire("scan:done_ko", "disk gone");
await drainRomBatch();
expect(platforms.get(1)?.rom_count).toBe(3);
});
it("puts only the ROMs a scan added at the top of the recent list", async () => {
install();
const roms = storeRoms();
roms.setRecentRoms([simpleRom({ name: "Old" })]);
fire("scan:scanning_rom", scanningRom(1, false, { name: "Rescanned" }));
fire("scan:scanning_rom", scanningRom(2, false));
fire("scan:scanning_rom", scanningRom(3, true));
await drainRomBatch();
expect(roms.recentRoms.map((r) => r.id)).toEqual([3, 1]);
expect(roms.recentRoms[1].name).toBe("Rescanned");
});
it("reconciles with a running scan job on install", async () => {
getTaskStatus.mockResolvedValue({
data: [runningScanTask(makeStats({ scanned_roms: 40, total_roms: 100 }))],
@@ -1,27 +1,27 @@
// useScanLifecycle — global wire-up between the scan socket events and the
// useScanLifecycle: global wire-up between the scan socket events and the
// `scanning` Pinia store. Mounted once at the top of the v2 tree (AppLayout)
// so the navbar's ScanningIndicator, the dedicated /scan view, and anyone
// else reading `scanning` / `scanningPlatforms` / `scanStats` always see
// the same truth — even when the user navigates between routes mid-scan.
// the same truth, even when the user navigates between routes mid-scan.
//
// Events handled:
// * `scan:scanning_platform` — backend announces the platform it's about
// * `scan:scanning_platform`: backend announces the platform it's about
// to process; push it onto the live log so
// the /scan view can render a panel for it.
// * `scan:scanning_rom` — per-ROM update during a scan. Batched on
// * `scan:scanning_rom`: per-ROM update during a scan. Batched on
// a 100ms debounce window so a thousand
// rapid-fire updates don't tank rendering.
// * `scan:update_stats` — periodic progress; keep `scanStats` fresh.
// * `scan:done` — scan finished; persist the final stats,
// * `scan:update_stats`: periodic progress; keep `scanStats` fresh.
// * `scan:done`: scan finished; persist the final stats,
// flip `scanning` off so the indicator hides,
// then refetch platforms to reconcile counts.
// * `scan:done_ko` — scan errored; flip `scanning` off.
// * `scan:done_ko`: scan errored; flip `scanning` off.
//
// Both reach every connected user, so only the tab that started the scan
// toasts its end. Whoever else it concerns gets a notification.
//
// Events alone can't tell a tab that loads mid-scan what's going on, so
// install also reconciles against the running RQ job — see
// install also reconciles against the running RQ job; see
// `reconcileWithRunningScan` below.
//
// `useSocketEvent` is the typed subscription wrapper that auto-cleans up
@@ -42,6 +42,9 @@ import type { Events } from "@/types/emitter";
import { useSocketEvent } from "@/v2/composables/useSocketEvent";
import storeGalleryRoms from "@/v2/stores/galleryRoms";
/** `scan:scanning_rom` payload; `is_new` marks a ROM this scan added. */
type ScanningRom = SimpleRom & { is_new: boolean };
export function installScanLifecycle() {
const scanningStore = storeScanning();
const romsStore = storeRoms();
@@ -69,7 +72,7 @@ export function installScanLifecycle() {
(p) => p.display_name !== display_name,
);
// Prepend so the platform being scanned right now stays at the top of
// the live log — no scrolling to follow progress.
// the live log, where progress can be followed without scrolling.
scanningStore.scanningPlatforms.unshift({
name,
display_name,
@@ -81,10 +84,8 @@ export function installScanLifecycle() {
is_identified,
});
// Surface brand-new platforms in the canonical platforms store the
// moment the scan reaches them — previously they only appeared after
// a manual page refresh. The socket payload is a partial (8 fields),
// so fetch the full PlatformSchema before adding it to the store.
// Surface brand-new platforms the moment the scan reaches them. The
// socket payload is a partial, so fetch the full PlatformSchema.
if (!platformsStore.has(id)) {
platformApi
.getPlatform(id)
@@ -99,7 +100,10 @@ export function installScanLifecycle() {
// Batch per-ROM updates so a fast scan doesn't trigger one render per
// ROM. Queue drains every 100ms; matches the v1 behavior. Stored
// outside the handler so multiple events share the same queue + flush.
const romUpdateQueue: SimpleRom[] = [];
const romUpdateQueue: ScanningRom[] = [];
// A new ROM emits several times, and a platform event can rebuild the live
// log between those emits, so each new ROM is counted once by id.
const countedNewRomIds = new Set<number>();
const refreshGallery = debounce(
() => {
galleryRomsStore.invalidateWindows();
@@ -111,11 +115,24 @@ export function installScanLifecycle() {
const processRomUpdates = debounce(() => {
if (romUpdateQueue.length === 0) return;
const updates = romUpdateQueue.splice(0, romUpdateQueue.length);
updates.forEach((rom) => {
// Keep the global "recent" list fresh so any view watching it
// (Home) reflects the new ROM at the top.
romsStore.removeFromRecent(rom);
romsStore.addToRecent(rom);
updates.forEach(({ is_new: isNew, ...rom }) => {
// Home's "recently added" row takes new ROMs at the top; a rescanned
// one is refreshed where it already sits.
if (isNew) {
romsStore.removeFromRecent(rom);
romsStore.addToRecent(rom);
if (!countedNewRomIds.has(rom.id)) {
countedNewRomIds.add(rom.id);
// The gallery/nav getters gate on `rom_count > 0`, so this is what
// makes a freshly-scanned platform render mid-scan.
const storePlatform = platformsStore.get(rom.platform_id);
if (storePlatform) storePlatform.rom_count += 1;
}
} else if (romsStore.recentRoms.some((r) => r.id === rom.id)) {
romsStore.recentRoms = romsStore.recentRoms.map((r) =>
r.id === rom.id ? rom : r,
);
}
// If the user is currently looking at the gallery of the platform
// being scanned, refresh from the server to preserve sorting/filtering.
@@ -132,7 +149,7 @@ export function installScanLifecycle() {
(p) => p.fs_slug === rom.platform_fs_slug,
);
// Socket may have dropped the `scan:scanning_platform` event — add
// The socket may have dropped the `scan:scanning_platform` event. Add
// the platform synthetically so the user still sees something.
if (!scannedPlatform) {
scanningStore.scanningPlatforms.unshift({
@@ -156,18 +173,13 @@ export function installScanLifecycle() {
r.id === rom.id ? rom : r,
);
} else {
// Newest ROM first, same as platforms — most recent stays on top.
// Newest ROM first, same as platforms, so the most recent stays on top.
scannedPlatform.roms.unshift(rom);
// Keep the canonical platforms store's count live for genuinely new
// ROMs: the gallery/nav getters gate on `rom_count > 0`, so this is
// what makes a freshly-scanned platform actually render mid-scan.
const storePlatform = platformsStore.get(rom.platform_id);
if (storePlatform) storePlatform.rom_count += 1;
}
});
}, 100);
useSocketEvent<SimpleRom>("scan:scanning_rom", (rom) => {
useSocketEvent<ScanningRom>("scan:scanning_rom", (rom) => {
scanningStore.setScanning(true);
romUpdateQueue.push(rom);
processRomUpdates();
@@ -185,6 +197,10 @@ export function installScanLifecycle() {
useSocketEvent<ScanStats>("scan:done", (stats) => {
markScanEnded();
// Apply the last ROM batch now, or its count bumps would land on the
// platforms refetched below.
processRomUpdates.flush();
countedNewRomIds.clear();
const startedHere = scanningStore.startedInThisTab;
scanningStore.setScanStats(stats);
scanningStore.setScanning(false);
@@ -203,6 +219,9 @@ export function installScanLifecycle() {
useSocketEvent<string>("scan:done_ko", (msg) => {
markScanEnded();
// Apply the last ROM batch while its counted ids are still known.
processRomUpdates.flush();
countedNewRomIds.clear();
const startedHere = scanningStore.startedInThisTab;
scanningStore.setScanning(false);
if (!startedHere) return;