Skip to content

Commit 509e4d1

Browse files
gantoineclaude
andcommitted
fix(hasheous): keep unverified ROMs that predate a match key
`hasheous_metadata` gains keys as RomM maps more of Hasheous' signature sources, so rows written before `mame_redump_match` existed don't carry it. On PostgreSQL `hasheous_metadata->>'mame_redump_match'` is then NULL, the OR chain over the match flags evaluates to NULL instead of false, and `NOT (...)` stays NULL: the unverified filter returned nothing for those rows until they were rescanned. Each extraction is now coalesced to false. The JSON path is untouched, since SQLAlchemy compiles `as_boolean()` to a CASE whose ELSE branch already folds a missing key into false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 56f8a51 commit 509e4d1

2 files changed

Lines changed: 149 additions & 1 deletion

File tree

backend/handler/database/roms_handler.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -856,9 +856,15 @@ def _filter_by_verified(self, query: Query, value: bool) -> Query:
856856
"puredos_match",
857857
]
858858

859+
# A key absent from `hasheous_metadata` (rows stored before it existed, or
860+
# rows with no Hasheous match at all) extracts as NULL, and NULL poisons
861+
# both the OR and its negation, so the unverified side would drop those
862+
# rows. The JSON path below folds a missing key into false on its own;
863+
# `->>` does not, hence the coalesce.
859864
if ROMM_DB_DRIVER == "postgresql":
860865
conditions = " OR ".join(
861-
f"(hasheous_metadata->>'{key}')::boolean" for key in keys_to_check
866+
f"COALESCE((hasheous_metadata->>'{key}')::boolean, false)"
867+
for key in keys_to_check
862868
)
863869
predicate = text(f"({conditions})")
864870
if not value:
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
"""The `verified` filter over the Hasheous signature-match flags.
2+
3+
`hasheous_metadata` is a JSON blob whose keys grow as RomM maps more of
4+
Hasheous' signature sources (`mame_redump_match` was the latest addition), so
5+
rows written before a key existed simply don't carry it. Extracting a missing
6+
key yields NULL, and an OR chain containing a NULL is NULL rather than false,
7+
which makes `NOT (...)` NULL too: the unverified side would drop every row it
8+
should have returned.
9+
10+
The JSON path already collapses a missing key into false (SQLAlchemy compiles
11+
`as_boolean()` to a CASE whose ELSE branch catches it), so only the PostgreSQL
12+
`->>` extraction needs the coalesce. The suite runs against one driver at a
13+
time, hence the compiled-SQL check below.
14+
"""
15+
16+
import pytest
17+
18+
from handler.database import db_rom_handler
19+
from handler.database.roms_handler import DBRomsHandler
20+
from models.platform import Platform
21+
from models.rom import Rom
22+
from models.user import User
23+
24+
# The keys as they were written before `mame_redump_match` joined them.
25+
LEGACY_KEYS = [
26+
"tosec_match",
27+
"mame_arcade_match",
28+
"mame_mess_match",
29+
"nointro_match",
30+
"redump_match",
31+
"whdload_match",
32+
"ra_match",
33+
"fbneo_match",
34+
"puredos_match",
35+
]
36+
37+
38+
def _add_rom(platform: Platform, user: User, name: str, metadata: dict) -> Rom:
39+
rom = db_rom_handler.add_rom(
40+
Rom(
41+
platform_id=platform.id,
42+
name=name,
43+
slug=name,
44+
fs_name=f"{name}.zip",
45+
fs_name_no_tags=name,
46+
fs_name_no_ext=name,
47+
fs_extension="zip",
48+
fs_path=f"{platform.slug}/roms",
49+
hasheous_metadata=metadata,
50+
)
51+
)
52+
db_rom_handler.add_rom_user(rom_id=rom.id, user_id=user.id)
53+
return rom
54+
55+
56+
@pytest.fixture
57+
def legacy_unverified_rom(platform: Platform, admin_user: User) -> Rom:
58+
"""Scanned before `mame_redump_match` existed, and matched nothing."""
59+
return _add_rom(
60+
platform,
61+
admin_user,
62+
"legacy_unverified",
63+
{key: False for key in LEGACY_KEYS},
64+
)
65+
66+
67+
@pytest.fixture
68+
def legacy_verified_rom(platform: Platform, admin_user: User) -> Rom:
69+
return _add_rom(
70+
platform,
71+
admin_user,
72+
"legacy_verified",
73+
{key: key == "nointro_match" for key in LEGACY_KEYS},
74+
)
75+
76+
77+
@pytest.fixture
78+
def chd_verified_rom(platform: Platform, admin_user: User) -> Rom:
79+
"""Only the newest key is set, as a CHD rescan writes it."""
80+
return _add_rom(
81+
platform,
82+
admin_user,
83+
"chd_verified",
84+
{key: False for key in LEGACY_KEYS} | {"mame_redump_match": True},
85+
)
86+
87+
88+
class TestVerifiedFilter:
89+
def test_unverified_keeps_roms_missing_the_newest_key(
90+
self,
91+
admin_user: User,
92+
legacy_unverified_rom: Rom,
93+
legacy_verified_rom: Rom,
94+
chd_verified_rom: Rom,
95+
):
96+
roms = db_rom_handler.get_roms_scalar(user_id=admin_user.id, verified=False)
97+
98+
assert [r.id for r in roms] == [legacy_unverified_rom.id]
99+
100+
def test_verified_matches_both_legacy_and_newest_keys(
101+
self,
102+
admin_user: User,
103+
legacy_unverified_rom: Rom,
104+
legacy_verified_rom: Rom,
105+
chd_verified_rom: Rom,
106+
):
107+
roms = db_rom_handler.get_roms_scalar(user_id=admin_user.id, verified=True)
108+
109+
assert sorted(r.id for r in roms) == sorted(
110+
[legacy_verified_rom.id, chd_verified_rom.id]
111+
)
112+
113+
def test_unverified_keeps_roms_without_any_hasheous_metadata(
114+
self, admin_user: User, rom: Rom, legacy_verified_rom: Rom
115+
):
116+
roms = db_rom_handler.get_roms_scalar(user_id=admin_user.id, verified=False)
117+
118+
assert [r.id for r in roms] == [rom.id]
119+
120+
121+
class TestVerifiedPostgresPredicate:
122+
"""The PostgreSQL branch builds raw SQL, so it can only be checked by
123+
compiling it (the suite runs on a single driver at a time)."""
124+
125+
@pytest.fixture
126+
def postgres_handler(self, monkeypatch: pytest.MonkeyPatch) -> DBRomsHandler:
127+
monkeypatch.setattr(
128+
"handler.database.roms_handler.ROMM_DB_DRIVER", "postgresql"
129+
)
130+
return db_rom_handler
131+
132+
@pytest.mark.parametrize("verified", [True, False])
133+
def test_every_key_is_coalesced_to_false(
134+
self, postgres_handler: DBRomsHandler, verified: bool
135+
):
136+
query, _ = postgres_handler.get_roms_query()
137+
filtered = postgres_handler.filter_roms(query=query, verified=verified)
138+
139+
sql = str(filtered.compile(compile_kwargs={"literal_binds": True}))
140+
141+
for key in [*LEGACY_KEYS, "mame_redump_match"]:
142+
assert f"COALESCE((hasheous_metadata->>'{key}')::boolean, false)" in sql

0 commit comments

Comments
 (0)