mirror of
https://github.com/rommapp/romm.git
synced 2026-06-28 06:46:00 +00:00
fix: fetch metadata for manually-set IDs on UNMATCHED scan
When a user manually sets metadata IDs (e.g., ra_id, launchbox_id, hasheous_id) via the UI and then runs "Refresh Metadata" (which defaults to UNMATCHED scan type), no metadata was fetched because UNMATCHED conditions checked `not rom.xxx_id` — so if the ID was already set, the handler was skipped entirely. Fix: Change each handler's UNMATCHED condition to also trigger when the ID is set but the corresponding metadata dict is empty (i.e., `not rom.xxx_id or not rom.xxx_metadata`). For handlers that support ID-based lookup (RA, Launchbox, IGDB, MobyGames, SS, Flashpoint), also add the `get_rom_by_id` path inside the function. For Hasheous: when hash lookup fails but `hasheous_id` is set on an existing ROM (not newly added), return a partial HasheousRom built from the existing sub-IDs (igdb_id, ra_id, tgdb_id) so the downstream get_igdb_game / get_ra_game proxy calls can still enrich the ROM. Add three targeted tests to validate: - UNMATCHED scan fetches RA metadata when ra_id is set but ra_metadata is empty - UNMATCHED scan skips RA when both ra_id and ra_metadata are already populated - UNMATCHED scan passes existing sub-IDs to Hasheous proxies when hash lookup fails Agent-Logs-Url: https://github.com/rommapp/romm/sessions/098b482f-9f73-4f35-819a-b55004a79b13 Co-authored-by: gantoine <3247106+gantoine@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
0eff589250
commit
90bbf6c9d1
@@ -412,14 +412,28 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.hasheous_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.hasheous_id
|
||||
and (not rom.hasheous_id or not rom.hasheous_metadata)
|
||||
and rom.platform_slug in HASHEOUS_PLATFORM_LIST
|
||||
)
|
||||
)
|
||||
):
|
||||
return await meta_hasheous_handler.lookup_rom(
|
||||
result = await meta_hasheous_handler.lookup_rom(
|
||||
platform.slug, fs_rom["files"] or rom.files
|
||||
)
|
||||
if result.get("hasheous_id"):
|
||||
return result
|
||||
|
||||
# Hash lookup failed - if rom already has a hasheous_id set (e.g. manually),
|
||||
# return a partial HasheousRom so downstream handlers can still use the
|
||||
# existing IDs to fetch metadata. Only do this for existing roms (not newly
|
||||
# added) to avoid missing required fields like `name` in rom_attrs.
|
||||
if not newly_added and rom.hasheous_id:
|
||||
return HasheousRom(
|
||||
hasheous_id=rom.hasheous_id,
|
||||
igdb_id=rom.igdb_id,
|
||||
ra_id=rom.ra_id,
|
||||
tgdb_id=rom.tgdb_id,
|
||||
)
|
||||
|
||||
return HasheousRom(hasheous_id=None, igdb_id=None, tgdb_id=None, ra_id=None)
|
||||
|
||||
@@ -463,11 +477,15 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.igdb_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.igdb_id
|
||||
and (not rom.igdb_id or not rom.igdb_metadata)
|
||||
and rom.platform_slug in IGDB_PLATFORM_LIST
|
||||
)
|
||||
)
|
||||
):
|
||||
# If the ID is already set (e.g. manually) but metadata is missing, fetch directly
|
||||
if scan_type == ScanType.UNMATCHED and rom.igdb_id and not rom.igdb_metadata:
|
||||
return await meta_igdb_handler.get_rom_by_id(rom, rom.igdb_id)
|
||||
|
||||
# Use Hasheous match to get the IGDB ID
|
||||
h_igdb_id = hasheous_rom.get("igdb_id")
|
||||
if h_igdb_id:
|
||||
@@ -509,7 +527,10 @@ async def scan_rom(
|
||||
newly_added
|
||||
or scan_type == ScanType.COMPLETE
|
||||
or (scan_type == ScanType.UPDATE and rom.gamelist_id)
|
||||
or (scan_type == ScanType.UNMATCHED and not rom.gamelist_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and (not rom.gamelist_id or not rom.gamelist_metadata)
|
||||
)
|
||||
):
|
||||
return await meta_gamelist_handler.get_rom(
|
||||
rom_attrs["fs_name"], platform, rom
|
||||
@@ -527,12 +548,16 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.flashpoint_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.flashpoint_id
|
||||
and (not rom.flashpoint_id or not rom.flashpoint_metadata)
|
||||
and platform.slug in FLASHPOINT_PLATFORM_LIST
|
||||
)
|
||||
)
|
||||
):
|
||||
if scan_type == ScanType.UPDATE and rom.flashpoint_id:
|
||||
if (scan_type == ScanType.UPDATE and rom.flashpoint_id) or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and rom.flashpoint_id
|
||||
and not rom.flashpoint_metadata
|
||||
):
|
||||
return await meta_flashpoint_handler.get_rom_by_id(rom.flashpoint_id)
|
||||
else:
|
||||
return await meta_flashpoint_handler.get_rom(
|
||||
@@ -549,7 +574,10 @@ async def scan_rom(
|
||||
newly_added
|
||||
or scan_type == ScanType.COMPLETE
|
||||
or (scan_type == ScanType.UPDATE and rom.libretro_id)
|
||||
or (scan_type == ScanType.UNMATCHED and not rom.libretro_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.libretro_id
|
||||
)
|
||||
)
|
||||
):
|
||||
return await meta_libretro_handler.get_rom(
|
||||
@@ -566,7 +594,10 @@ async def scan_rom(
|
||||
newly_added
|
||||
or scan_type == ScanType.COMPLETE
|
||||
or (scan_type == ScanType.UPDATE and rom.hltb_id)
|
||||
or (scan_type == ScanType.UNMATCHED and not rom.hltb_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and (not rom.hltb_id or not rom.hltb_metadata)
|
||||
)
|
||||
)
|
||||
):
|
||||
return await meta_hltb_handler.get_rom(rom_attrs["fs_name"], platform.slug)
|
||||
@@ -583,7 +614,7 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.moby_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.moby_id
|
||||
and (not rom.moby_id or not rom.moby_metadata)
|
||||
and rom.platform_slug in MOBYGAMES_PLATFORM_LIST
|
||||
)
|
||||
)
|
||||
@@ -591,6 +622,10 @@ async def scan_rom(
|
||||
if scan_type == ScanType.UPDATE and rom.moby_id:
|
||||
return await meta_moby_handler.get_rom_by_id(rom.moby_id)
|
||||
|
||||
# If the ID is already set (e.g. manually) but metadata is missing, fetch directly
|
||||
if scan_type == ScanType.UNMATCHED and rom.moby_id and not rom.moby_metadata:
|
||||
return await meta_moby_handler.get_rom_by_id(rom.moby_id)
|
||||
|
||||
if playmatch_rom["moby_id"] is not None:
|
||||
log.debug(
|
||||
f"{hl(rom_attrs['fs_name'])} identified by Playmatch as MobyGames "
|
||||
@@ -615,7 +650,7 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.ss_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.ss_id
|
||||
and (not rom.ss_id or not rom.ss_metadata)
|
||||
and rom.platform_slug in SCREENSAVER_PLATFORM_LIST
|
||||
)
|
||||
)
|
||||
@@ -624,6 +659,10 @@ async def scan_rom(
|
||||
if scan_type == ScanType.UPDATE and rom.ss_id:
|
||||
return await meta_ss_handler.get_rom_by_id(rom, rom.ss_id)
|
||||
|
||||
# If the ID is already set (e.g. manually) but metadata is missing, fetch directly
|
||||
if scan_type == ScanType.UNMATCHED and rom.ss_id and not rom.ss_metadata:
|
||||
return await meta_ss_handler.get_rom_by_id(rom, rom.ss_id)
|
||||
|
||||
# Use Playmatch's hash-based id when available
|
||||
if playmatch_rom["ss_id"] is not None:
|
||||
log.debug(
|
||||
@@ -656,7 +695,7 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.launchbox_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.launchbox_id
|
||||
and (not rom.launchbox_id or not rom.launchbox_metadata)
|
||||
and rom.platform_slug in LAUNCHBOX_PLATFORM_LIST
|
||||
)
|
||||
):
|
||||
@@ -671,6 +710,19 @@ async def scan_rom(
|
||||
fs_name=rom_attrs["fs_name"],
|
||||
platform_slug=platform_slug,
|
||||
)
|
||||
elif (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and rom.launchbox_id
|
||||
and not rom.launchbox_metadata
|
||||
and launchbox_remote_enabled
|
||||
):
|
||||
# ID was set manually but metadata was never fetched
|
||||
launchbox_rom = await meta_launchbox_handler.get_rom_by_id(
|
||||
rom.launchbox_id,
|
||||
remote_enabled=True,
|
||||
fs_name=rom_attrs["fs_name"],
|
||||
platform_slug=platform_slug,
|
||||
)
|
||||
elif playmatch_rom["launchbox_id"] is not None and launchbox_remote_enabled:
|
||||
log.debug(
|
||||
f"{hl(rom_attrs['fs_name'])} identified by Playmatch as LaunchBox "
|
||||
@@ -709,7 +761,7 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.ra_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.ra_id
|
||||
and (not rom.ra_id or not rom.ra_metadata)
|
||||
and rom.platform_slug in RA_PLATFORM_LIST
|
||||
)
|
||||
)
|
||||
@@ -724,7 +776,9 @@ async def scan_rom(
|
||||
)
|
||||
return await meta_ra_handler.get_rom_by_id(rom=rom, ra_id=h_ra_id)
|
||||
|
||||
if scan_type == ScanType.UPDATE and rom.ra_id:
|
||||
if (scan_type == ScanType.UPDATE and rom.ra_id) or (
|
||||
scan_type == ScanType.UNMATCHED and rom.ra_id and not rom.ra_metadata
|
||||
):
|
||||
return await meta_ra_handler.get_rom_by_id(rom=rom, ra_id=rom.ra_id)
|
||||
else:
|
||||
return await meta_ra_handler.get_rom(
|
||||
@@ -743,7 +797,7 @@ async def scan_rom(
|
||||
or (scan_type == ScanType.UPDATE and rom.hasheous_id)
|
||||
or (
|
||||
scan_type == ScanType.UNMATCHED
|
||||
and not rom.hasheous_id
|
||||
and (not rom.hasheous_id or not rom.hasheous_metadata)
|
||||
and rom.platform_slug in HASHEOUS_PLATFORM_LIST
|
||||
)
|
||||
)
|
||||
|
||||
@@ -3,8 +3,10 @@ from unittest.mock import AsyncMock, patch
|
||||
import pytest
|
||||
|
||||
from handler.database import db_platform_handler, db_rom_handler
|
||||
from handler.metadata import meta_hasheous_handler, meta_playmatch_handler
|
||||
from handler.metadata import meta_hasheous_handler, meta_launchbox_handler, meta_playmatch_handler, meta_ra_handler
|
||||
from handler.metadata.hasheous_handler import HasheousRom
|
||||
from handler.metadata.launchbox_handler.types import LaunchboxRom
|
||||
from handler.metadata.ra_handler import RAGameRom
|
||||
from handler.scan_handler import MetadataSource, ScanType, scan_platform, scan_rom
|
||||
from models.platform import Platform
|
||||
from models.rom import Rom, RomFile
|
||||
@@ -174,3 +176,219 @@ async def test_scan_rom_complete_clears_unselected_metadata(
|
||||
assert result.ra_metadata == {}
|
||||
# Hasheous is still selected and should remain populated.
|
||||
assert result.hasheous_id == 999
|
||||
|
||||
|
||||
@patch.object(meta_playmatch_handler, "is_enabled", return_value=False)
|
||||
@patch.object(meta_ra_handler, "get_rom_by_id", new_callable=AsyncMock)
|
||||
@patch.object(meta_ra_handler, "get_rom", new_callable=AsyncMock)
|
||||
async def test_scan_rom_unmatched_fetches_ra_when_id_set_but_no_metadata(
|
||||
mock_get_rom, mock_get_rom_by_id, mock_playmatch_enabled
|
||||
):
|
||||
"""UNMATCHED scan must fetch RA metadata when ra_id is set manually but
|
||||
ra_metadata is empty (the user manually set the ID)."""
|
||||
ra_result = RAGameRom(
|
||||
ra_id=2774,
|
||||
name="Jak and Daxter: The Precursor's Legacy",
|
||||
url_cover="https://media.retroachievements.org/Images/jpg",
|
||||
ra_metadata={"achievements_count": 60},
|
||||
)
|
||||
mock_get_rom_by_id.return_value = ra_result
|
||||
mock_get_rom.return_value = RAGameRom(ra_id=None)
|
||||
|
||||
platform = Platform(
|
||||
id=1,
|
||||
slug="ps2",
|
||||
fs_slug="ps2",
|
||||
name="PlayStation 2",
|
||||
igdb_id=8,
|
||||
ra_id=21,
|
||||
)
|
||||
platform = db_platform_handler.add_platform(platform)
|
||||
|
||||
# ROM has ra_id set manually but no ra_metadata (never fetched before)
|
||||
rom = Rom(
|
||||
platform_id=platform.id,
|
||||
fs_name="Jak and Daxter.chd",
|
||||
fs_name_no_tags="Jak and Daxter",
|
||||
fs_name_no_ext="Jak and Daxter",
|
||||
fs_extension="chd",
|
||||
fs_path="ps2",
|
||||
name="Jak and Daxter",
|
||||
ra_id=2774,
|
||||
ra_metadata={}, # empty - never fetched
|
||||
fs_size_bytes=1024,
|
||||
tags=[],
|
||||
)
|
||||
rom = db_rom_handler.add_rom(rom)
|
||||
|
||||
async with initialize_context():
|
||||
result = await scan_rom(
|
||||
platform=platform,
|
||||
scan_type=ScanType.UNMATCHED,
|
||||
rom=rom,
|
||||
fs_rom={
|
||||
"fs_name": "Jak and Daxter.chd",
|
||||
"flat": True,
|
||||
"nested": False,
|
||||
"files": [],
|
||||
"crc_hash": "",
|
||||
"md5_hash": "",
|
||||
"sha1_hash": "",
|
||||
"ra_hash": "",
|
||||
},
|
||||
metadata_sources=[MetadataSource.RA],
|
||||
newly_added=False,
|
||||
)
|
||||
|
||||
# ra_id was set manually - get_rom_by_id should be called, not get_rom
|
||||
mock_get_rom_by_id.assert_called_once()
|
||||
mock_get_rom.assert_not_called()
|
||||
assert result.ra_id == 2774
|
||||
|
||||
|
||||
@patch.object(meta_playmatch_handler, "is_enabled", return_value=False)
|
||||
@patch.object(meta_ra_handler, "get_rom_by_id", new_callable=AsyncMock)
|
||||
@patch.object(meta_ra_handler, "get_rom", new_callable=AsyncMock)
|
||||
async def test_scan_rom_unmatched_skips_ra_when_id_and_metadata_exist(
|
||||
mock_get_rom, mock_get_rom_by_id, mock_playmatch_enabled
|
||||
):
|
||||
"""UNMATCHED scan must NOT re-fetch RA metadata when both ra_id and
|
||||
ra_metadata are already populated."""
|
||||
mock_get_rom_by_id.return_value = RAGameRom(ra_id=None)
|
||||
mock_get_rom.return_value = RAGameRom(ra_id=None)
|
||||
|
||||
platform = Platform(
|
||||
id=1,
|
||||
slug="ps2",
|
||||
fs_slug="ps2",
|
||||
name="PlayStation 2",
|
||||
igdb_id=8,
|
||||
ra_id=21,
|
||||
)
|
||||
platform = db_platform_handler.add_platform(platform)
|
||||
|
||||
# ROM has both ra_id and ra_metadata populated
|
||||
rom = Rom(
|
||||
platform_id=platform.id,
|
||||
fs_name="Jak and Daxter.chd",
|
||||
fs_name_no_tags="Jak and Daxter",
|
||||
fs_name_no_ext="Jak and Daxter",
|
||||
fs_extension="chd",
|
||||
fs_path="ps2",
|
||||
name="Jak and Daxter",
|
||||
ra_id=2774,
|
||||
ra_metadata={"achievements_count": 60}, # already populated
|
||||
fs_size_bytes=1024,
|
||||
tags=[],
|
||||
)
|
||||
rom = db_rom_handler.add_rom(rom)
|
||||
|
||||
async with initialize_context():
|
||||
result = await scan_rom(
|
||||
platform=platform,
|
||||
scan_type=ScanType.UNMATCHED,
|
||||
rom=rom,
|
||||
fs_rom={
|
||||
"fs_name": "Jak and Daxter.chd",
|
||||
"flat": True,
|
||||
"nested": False,
|
||||
"files": [],
|
||||
"crc_hash": "",
|
||||
"md5_hash": "",
|
||||
"sha1_hash": "",
|
||||
"ra_hash": "",
|
||||
},
|
||||
metadata_sources=[MetadataSource.RA],
|
||||
newly_added=False,
|
||||
)
|
||||
|
||||
# Both ID and metadata exist - should not re-fetch
|
||||
mock_get_rom_by_id.assert_not_called()
|
||||
mock_get_rom.assert_not_called()
|
||||
# Existing ra_id should be preserved
|
||||
assert result.ra_id == 2774
|
||||
|
||||
|
||||
@patch.object(meta_playmatch_handler, "is_enabled", return_value=False)
|
||||
@patch.object(meta_hasheous_handler, "get_ra_game", new_callable=AsyncMock)
|
||||
@patch.object(meta_hasheous_handler, "get_igdb_game", new_callable=AsyncMock)
|
||||
@patch.object(meta_hasheous_handler, "lookup_rom", new_callable=AsyncMock)
|
||||
async def test_scan_rom_unmatched_hasheous_uses_existing_ids_on_hash_fail(
|
||||
mock_lookup, mock_get_igdb, mock_get_ra, mock_playmatch_enabled
|
||||
):
|
||||
"""UNMATCHED scan: when Hasheous hash lookup fails but hasheous_id is set
|
||||
manually, the existing sub-IDs (igdb_id, ra_id) are passed to the metadata
|
||||
proxy calls so enrichment can still happen."""
|
||||
# Hash lookup returns no match
|
||||
mock_lookup.return_value = HasheousRom(
|
||||
hasheous_id=None, igdb_id=None, tgdb_id=None, ra_id=None
|
||||
)
|
||||
# get_igdb_game returns an enriched rom with name/cover
|
||||
enriched = HasheousRom(
|
||||
hasheous_id=262940,
|
||||
igdb_id=4614,
|
||||
tgdb_id=None,
|
||||
ra_id=2774,
|
||||
name="Jak and Daxter: The Precursor's Legacy",
|
||||
url_cover="https://example.com/cover.jpg",
|
||||
)
|
||||
mock_get_igdb.return_value = enriched
|
||||
mock_get_ra.return_value = enriched
|
||||
|
||||
platform = Platform(
|
||||
id=1,
|
||||
slug="ps2",
|
||||
fs_slug="ps2",
|
||||
name="PlayStation 2",
|
||||
igdb_id=8,
|
||||
hasheous_id=87,
|
||||
ra_id=21,
|
||||
)
|
||||
platform = db_platform_handler.add_platform(platform)
|
||||
|
||||
# ROM has hasheous_id set manually, with sub-IDs also set, but no metadata
|
||||
rom = Rom(
|
||||
platform_id=platform.id,
|
||||
fs_name="Jak and Daxter.chd",
|
||||
fs_name_no_tags="Jak and Daxter",
|
||||
fs_name_no_ext="Jak and Daxter",
|
||||
fs_extension="chd",
|
||||
fs_path="ps2",
|
||||
name="Jak and Daxter",
|
||||
hasheous_id=262940,
|
||||
hasheous_metadata={}, # empty
|
||||
igdb_id=4614,
|
||||
ra_id=2774,
|
||||
fs_size_bytes=1024,
|
||||
tags=[],
|
||||
)
|
||||
rom = db_rom_handler.add_rom(rom)
|
||||
|
||||
async with initialize_context():
|
||||
result = await scan_rom(
|
||||
platform=platform,
|
||||
scan_type=ScanType.UNMATCHED,
|
||||
rom=rom,
|
||||
fs_rom={
|
||||
"fs_name": "Jak and Daxter.chd",
|
||||
"flat": True,
|
||||
"nested": False,
|
||||
"files": [],
|
||||
"crc_hash": "",
|
||||
"md5_hash": "",
|
||||
"sha1_hash": "",
|
||||
"ra_hash": "",
|
||||
},
|
||||
metadata_sources=[MetadataSource.HASHEOUS],
|
||||
newly_added=False,
|
||||
)
|
||||
|
||||
# Hash lookup was attempted
|
||||
mock_lookup.assert_called_once()
|
||||
# get_igdb_game was called with the partial HasheousRom built from existing IDs
|
||||
mock_get_igdb.assert_called_once()
|
||||
called_arg = mock_get_igdb.call_args[0][0]
|
||||
assert called_arg.get("hasheous_id") == 262940
|
||||
assert called_arg.get("igdb_id") == 4614
|
||||
# hasheous_id should be set in the result
|
||||
assert result.hasheous_id == 262940
|
||||
|
||||
Reference in New Issue
Block a user