mirror of
https://github.com/rommapp/romm.git
synced 2026-10-12 02:21:58 +00:00
refactor(scan): resolve platform folder names in both directions
Fold the folder name resolution into one home and make it case-insensitive end to end. Config folder names are lowercased as the config is parsed, so bindings, versions and the built-in alias table are all matched the same way. A SNES/ folder now resolves, and a binding written GameCube: matches a gamecube/ folder. Add resolve_fs_slug(), the inverse used to recover a renamed folder, which replaces the two swapped dicts in scan_platform. Only slugs reached by exactly one folder name are reversible, so recovery never guesses between amiga500, amiga600 and amiga1200. Validate system.platforms and system.versions through one helper that rejects empty and non-string values, and treat a null block as empty rather than exiting. Restore the atari800 override in the example configs: its folder name is itself a RomM slug, so it can never be an alias. A new test derives the needed overrides from the alias table to keep the examples honest. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
7a54b1c6b0
commit
80185b225c
@@ -5,7 +5,7 @@ import json
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Final, NotRequired, TypedDict
|
||||
from typing import Any, Final, NotRequired, TypedDict
|
||||
|
||||
import pydash
|
||||
import yaml
|
||||
@@ -435,8 +435,10 @@ class ConfigManager:
|
||||
),
|
||||
}
|
||||
),
|
||||
PLATFORMS_BINDING=pydash.get(self._raw_config, "system.platforms", {}),
|
||||
PLATFORMS_VERSIONS=pydash.get(self._raw_config, "system.versions", {}),
|
||||
PLATFORMS_BINDING=pydash.get(self._raw_config, "system.platforms", {})
|
||||
or {},
|
||||
PLATFORMS_VERSIONS=pydash.get(self._raw_config, "system.versions", {})
|
||||
or {},
|
||||
ROMS_FOLDER_NAME=pydash.get(
|
||||
self._raw_config, "filesystem.roms_folder", "roms"
|
||||
),
|
||||
@@ -596,6 +598,30 @@ class ConfigManager:
|
||||
|
||||
return yaml_controls
|
||||
|
||||
def _validated_platform_map(self, raw: Any, config_key: str) -> dict[str, str]:
|
||||
"""Check a folder name to slug mapping, lowercasing the folder names.
|
||||
|
||||
Args:
|
||||
raw: Value read from the config file.
|
||||
config_key: Dotted config path, used in error messages.
|
||||
Returns:
|
||||
The mapping with lowercased keys, so lookups can be case-insensitive.
|
||||
"""
|
||||
if not isinstance(raw, dict):
|
||||
log.critical(f"Invalid config.yml: {config_key} must be a dictionary")
|
||||
sys.exit(3)
|
||||
|
||||
normalized: dict[str, str] = {}
|
||||
for fs_slug, slug in raw.items():
|
||||
if not isinstance(slug, str) or not slug:
|
||||
log.critical(
|
||||
f"Invalid config.yml: {config_key}.{fs_slug} must be a non-empty string"
|
||||
)
|
||||
sys.exit(3)
|
||||
normalized[str(fs_slug).lower()] = slug
|
||||
|
||||
return normalized
|
||||
|
||||
def _validate_config(self):
|
||||
"""Validates the config.yml file"""
|
||||
if not isinstance(self.config.EXCLUDED_PLATFORMS, list):
|
||||
@@ -640,27 +666,12 @@ class ConfigManager:
|
||||
log.critical("Invalid config.yml: scan.pegasus.export must be a boolean")
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.PLATFORMS_BINDING, dict):
|
||||
log.critical("Invalid config.yml: system.platforms must be a dictionary")
|
||||
sys.exit(3)
|
||||
else:
|
||||
for fs_slug, slug in self.config.PLATFORMS_BINDING.items():
|
||||
if slug is None:
|
||||
log.critical(
|
||||
f"Invalid config.yml: system.platforms.{fs_slug} must be a string"
|
||||
)
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.PLATFORMS_VERSIONS, dict):
|
||||
log.critical("Invalid config.yml: system.versions must be a dictionary")
|
||||
sys.exit(3)
|
||||
else:
|
||||
for fs_slug, slug in self.config.PLATFORMS_VERSIONS.items():
|
||||
if slug is None:
|
||||
log.critical(
|
||||
f"Invalid config.yml: system.versions.{fs_slug} must be a string"
|
||||
)
|
||||
sys.exit(3)
|
||||
self.config.PLATFORMS_BINDING = self._validated_platform_map(
|
||||
self.config.PLATFORMS_BINDING, "system.platforms"
|
||||
)
|
||||
self.config.PLATFORMS_VERSIONS = self._validated_platform_map(
|
||||
self.config.PLATFORMS_VERSIONS, "system.versions"
|
||||
)
|
||||
|
||||
if not isinstance(self.config.ROMS_FOLDER_NAME, str):
|
||||
log.critical("Invalid config.yml: filesystem.roms_folder must be a string")
|
||||
@@ -962,6 +973,7 @@ class ConfigManager:
|
||||
raise ConfigNotWritableException from exc
|
||||
|
||||
def add_platform_binding(self, fs_slug: str, slug: str) -> None:
|
||||
fs_slug = fs_slug.lower()
|
||||
platform_bindings = self.config.PLATFORMS_BINDING
|
||||
if fs_slug in platform_bindings:
|
||||
log.warning(f"Binding for {hl(fs_slug)} already exists")
|
||||
@@ -975,7 +987,7 @@ class ConfigManager:
|
||||
platform_bindings = self.config.PLATFORMS_BINDING
|
||||
|
||||
try:
|
||||
del platform_bindings[fs_slug]
|
||||
del platform_bindings[fs_slug.lower()]
|
||||
except KeyError:
|
||||
pass
|
||||
|
||||
@@ -983,6 +995,7 @@ class ConfigManager:
|
||||
self._update_config_file()
|
||||
|
||||
def add_platform_version(self, fs_slug: str, slug: str) -> None:
|
||||
fs_slug = fs_slug.lower()
|
||||
platform_versions = self.config.PLATFORMS_VERSIONS
|
||||
if fs_slug in platform_versions:
|
||||
log.warning(f"Version for {hl(fs_slug)} already exists")
|
||||
@@ -996,7 +1009,7 @@ class ConfigManager:
|
||||
platform_versions = self.config.PLATFORMS_VERSIONS
|
||||
|
||||
try:
|
||||
del platform_versions[fs_slug]
|
||||
del platform_versions[fs_slug.lower()]
|
||||
except KeyError:
|
||||
pass
|
||||
|
||||
|
||||
@@ -79,7 +79,7 @@ from models.user import User
|
||||
from utils import emoji
|
||||
from utils.audio_tags import persist_embedded_cover, remove_persisted_cover
|
||||
from utils.filesystem import sanitize_filename
|
||||
from utils.platform_aliases import resolve_platform_slug
|
||||
from utils.platform_aliases import resolve_fs_slug, resolve_platform_slug
|
||||
|
||||
LOGGER_MODULE_NAME = {"module_name": "scan"}
|
||||
|
||||
@@ -181,8 +181,8 @@ def build_hashless_fs_rom(fs_name: str, *, flat: bool) -> FSRom:
|
||||
def get_main_platform_igdb_id(platform: Platform):
|
||||
cnfg = cm.get_config()
|
||||
|
||||
if platform.fs_slug in cnfg.PLATFORMS_VERSIONS.keys():
|
||||
main_platform_slug = cnfg.PLATFORMS_VERSIONS[platform.fs_slug]
|
||||
main_platform_slug = cnfg.PLATFORMS_VERSIONS.get(platform.fs_slug.lower())
|
||||
if main_platform_slug:
|
||||
main_platform = db_platform_handler.get_platform_by_fs_slug(main_platform_slug)
|
||||
if main_platform:
|
||||
main_platform_igdb_id = main_platform.igdb_id
|
||||
@@ -285,8 +285,6 @@ async def scan_platform(
|
||||
platform_attrs["fs_slug"] = fs_slug
|
||||
|
||||
cnfg = cm.get_config()
|
||||
swapped_platform_bindings = {v: k for k, v in cnfg.PLATFORMS_BINDING.items()}
|
||||
swapped_platform_versions = {v: k for k, v in cnfg.PLATFORMS_VERSIONS.items()}
|
||||
|
||||
# Sometimes users change the name of the folder, so we try to match it with the config
|
||||
if fs_slug not in fs_platforms:
|
||||
@@ -294,14 +292,11 @@ async def scan_platform(
|
||||
f"{hl(fs_slug)} not found in file system, trying to match via config",
|
||||
extra=LOGGER_MODULE_NAME,
|
||||
)
|
||||
if fs_slug in swapped_platform_bindings.keys():
|
||||
platform = db_platform_handler.get_platform_by_fs_slug(fs_slug)
|
||||
if platform:
|
||||
platform_attrs["fs_slug"] = swapped_platform_bindings[platform.slug]
|
||||
elif fs_slug in swapped_platform_versions.keys():
|
||||
platform = db_platform_handler.get_platform_by_fs_slug(fs_slug)
|
||||
if platform:
|
||||
platform_attrs["fs_slug"] = swapped_platform_versions[platform.slug]
|
||||
platform = db_platform_handler.get_platform_by_fs_slug(fs_slug)
|
||||
if platform:
|
||||
known_fs_slug = resolve_fs_slug(platform.slug, cnfg)
|
||||
if known_fs_slug:
|
||||
platform_attrs["fs_slug"] = known_fs_slug
|
||||
|
||||
platform_attrs["slug"] = resolve_platform_slug(fs_slug, cnfg)
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from config.config_manager import (
|
||||
DEFAULT_EXCLUDED_DIRS,
|
||||
DEFAULT_EXCLUDED_EXTENSIONS,
|
||||
@@ -350,3 +352,41 @@ def test_config_update_preserves_streaming_section(tmp_path):
|
||||
"label": "PCSX2",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _write_config(tmp_path: Path, system_block: str) -> ConfigManager:
|
||||
config_file = tmp_path / "config.yml"
|
||||
config_file.write_text(f"system:\n{system_block}")
|
||||
return ConfigManager(str(config_file))
|
||||
|
||||
|
||||
def test_platform_folder_names_are_lowercased(tmp_path):
|
||||
loader = _write_config(
|
||||
tmp_path,
|
||||
' platforms:\n GameCube: "ngc"\n versions:\n NAOMI: "arcade"\n',
|
||||
)
|
||||
|
||||
assert loader.config.PLATFORMS_BINDING == {"gamecube": "ngc"}
|
||||
assert loader.config.PLATFORMS_VERSIONS == {"naomi": "arcade"}
|
||||
|
||||
|
||||
def test_null_platforms_block_means_empty(tmp_path):
|
||||
loader = _write_config(tmp_path, " platforms:\n versions:\n")
|
||||
|
||||
assert loader.config.PLATFORMS_BINDING == {}
|
||||
assert loader.config.PLATFORMS_VERSIONS == {}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", ['""', "5", "~"])
|
||||
def test_platform_binding_must_be_a_non_empty_string(tmp_path, value):
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
_write_config(tmp_path, f" platforms:\n gamecube: {value}\n")
|
||||
|
||||
assert excinfo.value.code == 3
|
||||
|
||||
|
||||
def test_platform_binding_lookup_ignores_case(tmp_path):
|
||||
loader = _write_config(tmp_path, ' platforms:\n GameCube: "ngc"\n')
|
||||
|
||||
loader.remove_platform_binding("GAMECUBE")
|
||||
assert loader.config.PLATFORMS_BINDING == {}
|
||||
|
||||
@@ -1,25 +1,33 @@
|
||||
import copy
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
from config.config_manager import Config
|
||||
from config.config_manager import config_manager as cm
|
||||
from utils.platform_aliases import PLATFORM_FS_ALIASES, resolve_platform_slug
|
||||
from utils.platform_aliases import (
|
||||
PLATFORM_FS_ALIASES,
|
||||
PLATFORM_SLUG_FOLDERS,
|
||||
resolve_fs_slug,
|
||||
resolve_platform_slug,
|
||||
)
|
||||
from utils.platform_slugs import UniversalPlatformSlug as UPS
|
||||
|
||||
EXAMPLE_CONFIGS = sorted((Path(__file__).parents[3] / "examples").glob("config.*.yml"))
|
||||
|
||||
|
||||
def _config(
|
||||
bindings: dict[str, str] | None = None,
|
||||
versions: dict[str, str] | None = None,
|
||||
) -> Config:
|
||||
config = copy.copy(cm.get_config())
|
||||
config.PLATFORMS_BINDING = bindings or {}
|
||||
config.PLATFORMS_VERSIONS = versions or {}
|
||||
return config
|
||||
return Config(
|
||||
PLATFORMS_BINDING=bindings or {},
|
||||
PLATFORMS_VERSIONS=versions or {},
|
||||
)
|
||||
|
||||
|
||||
def test_alias_keys_are_not_slugs():
|
||||
"""An alias for a folder name that already is a slug could never be reached."""
|
||||
slugs = {slug.value for slug in UPS}
|
||||
assert not {k for k in PLATFORM_FS_ALIASES if k in slugs}
|
||||
assert not [k for k in PLATFORM_FS_ALIASES if k in UPS]
|
||||
|
||||
|
||||
def test_alias_keys_are_lowercase():
|
||||
@@ -34,13 +42,12 @@ def test_resolves_frontend_folder_names():
|
||||
assert resolve_platform_slug("GameCube", config) == "ngc"
|
||||
|
||||
|
||||
def test_valid_slug_wins_over_alias():
|
||||
"""atari800 is both a Batocera folder and a RomM slug; the slug wins."""
|
||||
assert resolve_platform_slug("atari800", _config()) == "atari800"
|
||||
|
||||
|
||||
def test_unknown_folder_passes_through():
|
||||
assert resolve_platform_slug("my-custom-folder", _config()) == "my-custom-folder"
|
||||
def test_folder_without_alias_passes_through():
|
||||
config = _config()
|
||||
# atari800 is a Batocera folder name that is also a RomM slug, so it can
|
||||
# only be remapped through a config binding.
|
||||
assert resolve_platform_slug("atari800", config) == "atari800"
|
||||
assert resolve_platform_slug("my-custom-folder", config) == "my-custom-folder"
|
||||
|
||||
|
||||
def test_binding_and_version_win_over_alias():
|
||||
@@ -52,3 +59,41 @@ def test_binding_and_version_win_over_alias():
|
||||
|
||||
both = _config(bindings={"gamecube": "arcade"}, versions={"gamecube": "wii"})
|
||||
assert resolve_platform_slug("gamecube", both) == "arcade"
|
||||
|
||||
|
||||
def test_resolution_is_case_insensitive():
|
||||
config = _config(bindings={"gamecube": "arcade"})
|
||||
assert resolve_platform_slug("GameCube", config) == "arcade"
|
||||
assert resolve_platform_slug("SNES", _config()) == "snes"
|
||||
|
||||
|
||||
def test_resolves_slug_back_to_folder_name():
|
||||
assert resolve_fs_slug("dc", _config()) == "dreamcast"
|
||||
assert resolve_fs_slug("wii", _config(bindings={"wiiware": "wii"})) == "wiiware"
|
||||
|
||||
|
||||
def test_slug_reached_by_several_folders_has_no_reverse():
|
||||
"""amiga500, amiga600 and amiga1200 all resolve to amiga, so none is canonical."""
|
||||
assert resolve_fs_slug("amiga", _config()) is None
|
||||
assert resolve_fs_slug("ngc", _config()) is None
|
||||
assert resolve_fs_slug("my-custom-slug", _config()) is None
|
||||
|
||||
|
||||
def test_reverse_map_is_unambiguous():
|
||||
for slug, fs_slug in PLATFORM_SLUG_FOLDERS.items():
|
||||
assert PLATFORM_FS_ALIASES[fs_slug].value == slug
|
||||
|
||||
|
||||
@pytest.mark.parametrize("config_file", EXAMPLE_CONFIGS, ids=lambda p: p.name)
|
||||
def test_example_configs_list_only_needed_overrides(config_file: Path):
|
||||
"""Every example entry must be one the alias table cannot resolve on its own."""
|
||||
raw = yaml.safe_load(config_file.read_text()) or {}
|
||||
platforms = (raw.get("system") or {}).get("platforms") or {}
|
||||
empty = _config()
|
||||
|
||||
redundant = {
|
||||
fs_slug: slug
|
||||
for fs_slug, slug in platforms.items()
|
||||
if resolve_platform_slug(fs_slug, empty) == slug
|
||||
}
|
||||
assert not redundant
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
from typing import Final
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING, Final
|
||||
|
||||
from config.config_manager import Config
|
||||
from utils.platform_slugs import UniversalPlatformSlug as UPS
|
||||
|
||||
_SLUGS: Final = frozenset(slug.value for slug in UPS)
|
||||
if TYPE_CHECKING:
|
||||
from config.config_manager import Config
|
||||
|
||||
# Folder names used by Batocera, RetroBat and ES-DE that differ from RomM slugs.
|
||||
PLATFORM_FS_ALIASES: Final[dict[str, UPS]] = {
|
||||
@@ -147,22 +149,50 @@ PLATFORM_FS_ALIASES: Final[dict[str, UPS]] = {
|
||||
}
|
||||
|
||||
|
||||
def _unambiguous_folders() -> dict[str, str]:
|
||||
folders_by_slug: dict[UPS, list[str]] = {}
|
||||
for fs_slug, slug in PLATFORM_FS_ALIASES.items():
|
||||
folders_by_slug.setdefault(slug, []).append(fs_slug)
|
||||
|
||||
return {
|
||||
slug.value: folders[0]
|
||||
for slug, folders in folders_by_slug.items()
|
||||
if len(folders) == 1
|
||||
}
|
||||
|
||||
|
||||
# Only a slug reached by exactly one folder name can be resolved back to it.
|
||||
PLATFORM_SLUG_FOLDERS: Final[dict[str, str]] = _unambiguous_folders()
|
||||
|
||||
|
||||
def resolve_platform_slug(fs_slug: str, config: Config) -> str:
|
||||
"""Resolve a platform folder name to a RomM platform slug.
|
||||
|
||||
Args:
|
||||
fs_slug: Platform folder name as found on disk.
|
||||
config: Loaded config, whose bindings and versions take precedence.
|
||||
Returns:
|
||||
The bound slug, the folder name if it already is a slug, the built-in
|
||||
alias, or the folder name unchanged.
|
||||
Config bindings and versions win, then the folder name if it already is a
|
||||
slug, then the built-in alias.
|
||||
"""
|
||||
bound = config.PLATFORMS_BINDING.get(fs_slug) or config.PLATFORMS_VERSIONS.get(
|
||||
fs_slug
|
||||
)
|
||||
key = fs_slug.lower()
|
||||
bound = config.PLATFORMS_BINDING.get(key) or config.PLATFORMS_VERSIONS.get(key)
|
||||
if bound:
|
||||
return bound
|
||||
if fs_slug in _SLUGS:
|
||||
return fs_slug
|
||||
alias = PLATFORM_FS_ALIASES.get(fs_slug.lower())
|
||||
if key in UPS:
|
||||
return key
|
||||
alias = PLATFORM_FS_ALIASES.get(key)
|
||||
return alias.value if alias else fs_slug
|
||||
|
||||
|
||||
def resolve_fs_slug(slug: str, config: Config) -> str | None:
|
||||
"""Resolve a RomM platform slug back to the folder name it came from.
|
||||
|
||||
Args:
|
||||
slug: RomM platform slug.
|
||||
config: Loaded config, whose bindings and versions take precedence.
|
||||
Returns:
|
||||
The folder name, or None when none is known or several alias folders
|
||||
collapse onto the slug.
|
||||
"""
|
||||
for mapping in (config.PLATFORMS_BINDING, config.PLATFORMS_VERSIONS):
|
||||
for fs_slug, bound in mapping.items():
|
||||
if bound == slug:
|
||||
return fs_slug
|
||||
return PLATFORM_SLUG_FOLDERS.get(slug)
|
||||
|
||||
@@ -19,6 +19,7 @@ system:
|
||||
# Batocera and RetroBat folder names are recognised automatically. The entries
|
||||
# below override folders RomM would otherwise match to their own platform.
|
||||
platforms:
|
||||
atari800: atari8bit
|
||||
model2: arcade
|
||||
model3: arcade
|
||||
pico: sega-pico
|
||||
|
||||
@@ -22,8 +22,10 @@ exclude:
|
||||
- ".Trash*"
|
||||
|
||||
system:
|
||||
# ES-DE folder names are recognised automatically. Add entries only to override one.
|
||||
platforms: {}
|
||||
# ES-DE folder names are recognised automatically. The entry below overrides a
|
||||
# folder RomM would otherwise match to its own platform.
|
||||
platforms:
|
||||
atari800: "atari8bit" # Atari 8-bit
|
||||
|
||||
scan:
|
||||
# Metadata priority during scans
|
||||
|
||||
Reference in New Issue
Block a user