diff --git a/backend/config/config_manager.py b/backend/config/config_manager.py index f29ae8292..d763ad279 100644 --- a/backend/config/config_manager.py +++ b/backend/config/config_manager.py @@ -4,8 +4,8 @@ import functools import glob import json import os +import posixpath import sys -from collections.abc import Callable from dataclasses import dataclass from pathlib import Path from typing import Any, Final, NotRequired, Self, TextIO, TypedDict @@ -47,6 +47,9 @@ RESERVED_STRUCTURE_KEYS: Final = frozenset( {STRUCTURE_DEFAULT_KEY, STRUCTURE_FIRMWARE_KEY} ) +# Folder-name keys 5.2 wrote to config.yml, with the default each one had. +RETIRED_FOLDER_KEYS: Final = {"roms_folder": "roms", "firmware_folder": "bios"} + STRUCTURE_DOCS_URL: Final = ( "https://docs.romm.app/latest/getting-started/folder-structure/" ) @@ -244,6 +247,20 @@ def parse_platform_templates( ) +def _platform_first_layout(folder: str) -> bool | None: + """Whether the library keeps `folder` inside each platform folder. + + `` at the library root wins when both layouts exist. Returns ``None`` + when neither does, as in an empty or not yet mounted library. + """ + if os.path.isdir(os.path.join(LIBRARY_BASE_PATH, folder)): + return False + pattern = os.path.join(LIBRARY_BASE_PATH, "*", glob.escape(folder)) + if any(os.path.isdir(match) for match in glob.iglob(pattern)): + return True + return None + + ROMM_USER_CONFIG_PATH: Final = f"{ROMM_BASE_PATH}/config" ROMM_USER_CONFIG_FILE: Final = f"{ROMM_USER_CONFIG_PATH}/config.yml" SQLITE_DB_BASE_PATH: Final = f"{ROMM_BASE_PATH}/database" @@ -596,6 +613,8 @@ class ConfigManager: # Tests require custom config path def __init__(self, config_file: str = ROMM_USER_CONFIG_FILE) -> None: self.config_file = config_file + self._retired_layout_detected: bool | None = None + self._platform_first_layouts: dict[str, bool] = {} try: # Check if the config file is mounted @@ -718,6 +737,7 @@ class ConfigManager: def _parse_config(self) -> None: """Parses each entry in the config.yml""" + self._retired_layout_detected = self._translate_retired_filesystem_keys() self.config = Config( CONFIG_FILE_MOUNTED=self._config_file_mounted, @@ -972,35 +992,116 @@ class ConfigManager: return normalized - def _check_retired_filesystem_keys(self) -> None: - """Exit if config.yml still sets a folder name that a template replaced. + def _detect_platform_first(self, folder: str) -> bool | None: + # get_config() reparses on every call, and a found layout doesn't flip. + if folder not in self._platform_first_layouts: + detected = _platform_first_layout(folder) + if detected is None: + return None + self._platform_first_layouts[folder] = detected + return self._platform_first_layouts[folder] - Ignoring one would relocate the library under the user. + def _translate_retired_filesystem_keys(self) -> bool | None: + """Replace, in memory, the folder-name keys 5.2 saved on every settings + change with the templates that reproduce the layout 5.2 resolved them to. + + Returns: + ``None`` when there was nothing to translate, otherwise whether the + library showed which layout it uses. """ - retired: dict[str, tuple[str, Callable[[str], str]]] = { - "filesystem.roms_folder": ( - STRUCTURE_DEFAULT_KEY, - lambda folder: f"{folder}/{{platform}}/{{game}}", - ), - "filesystem.firmware_folder": ( - STRUCTURE_FIRMWARE_KEY, - lambda folder: f"{folder}/{{platform}}", - ), + filesystem = get_nested(self._raw_config, "filesystem") + if not isinstance(filesystem, dict) or not ( + RETIRED_FOLDER_KEYS & filesystem.keys() + ): + return None + structure = filesystem.setdefault("structure", {}) + if not isinstance(structure, dict): + return None + + roms_folder, firmware_folder = ( + filesystem.pop(key, default) for key, default in RETIRED_FOLDER_KEYS.items() + ) + for key, folder in zip( + RETIRED_FOLDER_KEYS, (roms_folder, firmware_folder), strict=False + ): + if not isinstance(folder, str) or not folder: + log.critical( + f"Invalid config.yml: filesystem.{key} must be a non-empty string" + ) + sys.exit(3) + + # Validation folds structure keys to lowercase, so `Default` is declared too. + undeclared = RESERVED_STRUCTURE_KEYS - {str(key).lower() for key in structure} + if not undeclared: + return True + platform_first = self._detect_platform_first(roms_folder) + + def place(folder: str) -> str: + # A `.` folder is the library root, which a template spells as no section. + return posixpath.normpath( + f"{{platform}}/{folder}" if platform_first else f"{folder}/{{platform}}" + ) + + templates = { + STRUCTURE_DEFAULT_KEY: f"{place(roms_folder)}/{{game}}", + STRUCTURE_FIRMWARE_KEY: place(firmware_folder), } - for key, (structure_key, to_template) in retired.items(): - folder = get_nested(self._raw_config, key) - if folder is None: - continue + # Templates have no escape for braces, so some folder names can't migrate. + try: + parse_structure_template(templates[STRUCTURE_DEFAULT_KEY]) + parse_firmware_template(templates[STRUCTURE_FIRMWARE_KEY]) + except ValueError as exc: log.critical( - f"Invalid config.yml: {key} is no longer supported. Replace it " - f"with the equivalent layout:\n\n" - f" filesystem:\n" - f" structure:\n" - f' {structure_key}: "{to_template(folder)}"\n\n' + "Invalid config.yml: filesystem.roms_folder and " + "filesystem.firmware_folder can't be migrated to a layout " + f"template ({exc}). Rename the folder or set filesystem.structure. " f"See {STRUCTURE_DOCS_URL}." ) sys.exit(3) + structure.update({key: templates[key] for key in undeclared}) + return platform_first is not None + + def migrate_retired_filesystem_keys(self) -> None: + """Write the layout that replaces the retired folder keys to config.yml.""" + config = self.get_config() + if self._retired_layout_detected is None: + return + + layout = "\n".join( + f' {key}: "{config.STRUCTURE_TEMPLATES[key]}"' + for key in (STRUCTURE_DEFAULT_KEY, STRUCTURE_FIRMWARE_KEY) + ) + if not self._retired_layout_detected: + log.warning( + "The library is empty or not mounted, so filesystem.roms_folder and " + "filesystem.firmware_folder are migrated in memory only, as:\n\n" + f" filesystem:\n structure:\n{layout}\n\n" + "config.yml is updated on the first startup that finds the library." + ) + return + + try: + if not config.CONFIG_FILE_WRITABLE: + raise ConfigNotWritableException + self._write_raw_config() + except ConfigNotWritableException: + self._config_file_writable = False + log.warning( + "config.yml is read-only, so filesystem.roms_folder and " + "filesystem.firmware_folder are migrated in memory only. Replace " + f"them with the equivalent layout:\n\n" + f" filesystem:\n structure:\n{layout}\n\n" + f"See {STRUCTURE_DOCS_URL}." + ) + return + + log.info( + "Migrated filesystem.roms_folder and filesystem.firmware_folder " + f"in config.yml to the equivalent layout:\n\n" + f" filesystem:\n structure:\n{layout}\n" + ) + def check_library_layout(self) -> None: """Exit if the library is laid out as `{platform}/roms` with no template. @@ -1009,11 +1110,7 @@ class ConfigManager: """ if STRUCTURE_DEFAULT_KEY in self.config.STRUCTURE_TEMPLATES: return - if os.path.isdir(os.path.join(LIBRARY_BASE_PATH, "roms")): - return - - pattern = os.path.join(LIBRARY_BASE_PATH, "*", "roms") - if not any(os.path.isdir(match) for match in glob.iglob(pattern)): + if not _platform_first_layout("roms"): return log.critical( @@ -1029,8 +1126,6 @@ class ConfigManager: def _validate_config(self) -> None: """Validates the config.yml file""" - self._check_retired_filesystem_keys() - if not isinstance(self.config.GAMELIST_AUTO_EXPORT_ON_SCAN, bool): log.critical("Invalid config.yml: scan.gamelist.export must be a boolean") sys.exit(3) @@ -1433,6 +1528,9 @@ class ConfigManager: "containers": self.config.STREAMING_CONTAINERS, } + self._write_raw_config() + + def _write_raw_config(self) -> None: try: # Ensure the config directory exists os.makedirs(os.path.dirname(self.config_file), exist_ok=True) diff --git a/backend/startup.py b/backend/startup.py index de16393d8..0668d35e7 100644 --- a/backend/startup.py +++ b/backend/startup.py @@ -175,6 +175,7 @@ async def main() -> None: async with initialize_context(): log.info("Running startup tasks") + cm.migrate_retired_filesystem_keys() cm.check_library_layout() if SMTP_HOST and SMTP_SECURITY not in SMTP_SECURITY_MODES: diff --git a/backend/tests/config/test_config_loader.py b/backend/tests/config/test_config_loader.py index 96f70f802..445be827e 100644 --- a/backend/tests/config/test_config_loader.py +++ b/backend/tests/config/test_config_loader.py @@ -4,6 +4,7 @@ from contextlib import contextmanager from pathlib import Path import pytest +import yaml from config.config_manager import ( DEFAULT_EXCLUDED_EXTENSIONS, @@ -16,6 +17,7 @@ from config.config_manager import ( parse_platform_templates, parse_structure_template, ) +from exceptions.config_exceptions import ConfigNotWritableException @contextmanager @@ -830,23 +832,164 @@ def _write_filesystem_config(tmp_path: Path, block: str) -> ConfigManager: return ConfigManager(str(config_file)) +@pytest.fixture +def library(tmp_path, mocker): + library = tmp_path / "library" + library.mkdir() + mocker.patch("config.config_manager.LIBRARY_BASE_PATH", str(library)) + return library + + @pytest.mark.parametrize( - ("key", "folder", "expected"), + ("block", "folders", "expected"), [ - ("roms_folder", "retro_games", 'default: "retro_games/{platform}/{game}"'), - ("firmware_folder", "fw", 'firmware: "fw/{platform}"'), + ( + " roms_folder: roms\n firmware_folder: bios\n", + ["roms/n64"], + {"default": "roms/{platform}/{game}", "firmware": "bios/{platform}"}, + ), + ( + " roms_folder: roms\n firmware_folder: bios\n", + ["n64/roms"], + {"default": "{platform}/roms/{game}", "firmware": "{platform}/bios"}, + ), + ( + " roms_folder: retro_games\n", + ["retro_games/n64"], + {"default": "retro_games/{platform}/{game}", "firmware": "bios/{platform}"}, + ), + ( + " firmware_folder: fw\n", + ["n64/roms"], + {"default": "{platform}/roms/{game}", "firmware": "{platform}/fw"}, + ), + ( + " roms_folder: .\n", + ["n64"], + {"default": "{platform}/{game}", "firmware": "bios/{platform}"}, + ), ], ) -def test_retired_folder_keys_exit_with_the_replacement( - tmp_path, key, folder, expected, critical +def test_retired_folder_keys_migrate_to_the_layout_5_2_resolved( + tmp_path, library, block, folders, expected ): - """Ignoring them would silently relocate the library, so refuse to start and - name the template that reproduces the layout.""" + for folder in folders: + (library / folder).mkdir(parents=True) + + loader = _write_filesystem_config(tmp_path, block) + loader.migrate_retired_filesystem_keys() + + assert loader.get_config().STRUCTURE_TEMPLATES == expected + saved = yaml.safe_load((tmp_path / "config.yml").read_text()) + assert saved["filesystem"] == {"structure": expected} + loader.check_library_layout() + + +def test_loading_a_config_with_retired_folder_keys_never_rewrites_it(tmp_path, library): + """Every process loads the config, so only startup writes the migration.""" + (library / "roms").mkdir() + + loader = _write_filesystem_config(tmp_path, " roms_folder: roms\n") + + assert loader.get_config().STRUCTURE_TEMPLATES["default"] == ( + "roms/{platform}/{game}" + ) + assert (tmp_path / "config.yml").read_text() == "filesystem:\n roms_folder: roms\n" + + +@pytest.mark.parametrize("default_key", ["default", "Default"]) +def test_a_declared_structure_wins_over_a_retired_folder_key( + tmp_path, library, default_key +): + (library / "n64" / "roms").mkdir(parents=True) + + loader = _write_filesystem_config( + tmp_path, + " roms_folder: roms\n" + " structure:\n" + f' {default_key}: "roms/{{platform}}/{{game}}"\n', + ) + + assert loader.get_config().STRUCTURE_TEMPLATES == { + "default": "roms/{platform}/{game}", + "firmware": "{platform}/bios", + } + + +def test_an_undetectable_layout_migrates_in_memory_until_detected(tmp_path, library): + """An empty or unmounted library can't tell the layouts apart, so the guess + isn't written until the library shows which one it uses.""" + loader = _write_filesystem_config(tmp_path, " roms_folder: roms\n") + loader.migrate_retired_filesystem_keys() + + assert loader.get_config().STRUCTURE_TEMPLATES == { + "default": "roms/{platform}/{game}", + "firmware": "bios/{platform}", + } + assert "roms_folder" in (tmp_path / "config.yml").read_text() + + (library / "n64" / "roms").mkdir(parents=True) + loader.migrate_retired_filesystem_keys() + + expected = {"default": "{platform}/roms/{game}", "firmware": "{platform}/bios"} + assert loader.get_config().STRUCTURE_TEMPLATES == expected + saved = yaml.safe_load((tmp_path / "config.yml").read_text()) + assert saved["filesystem"] == {"structure": expected} + + +def test_retired_folder_keys_migrate_in_memory_when_read_only( + tmp_path, library, mocker +): + (library / "games").mkdir() + warning = mocker.patch("config.config_manager.log.warning") + mocker.patch("config.config_manager.os.access", return_value=False) + config_file = tmp_path / "config.yml" + config_file.write_text("filesystem:\n roms_folder: games\n") + + loader = ConfigManager(str(config_file)) + loader.migrate_retired_filesystem_keys() + loader.get_config() + + assert loader.config.STRUCTURE_TEMPLATES["default"] == "games/{platform}/{game}" + assert "roms_folder" in config_file.read_text() + assert warning.call_count == 1 + assert 'default: "games/{platform}/{game}"' in warning.call_args[0][0] + + +def test_a_failed_migration_write_marks_the_config_read_only(tmp_path, library, mocker): + (library / "roms").mkdir() + write = mocker.patch.object( + ConfigManager, "_write_raw_config", side_effect=ConfigNotWritableException + ) + + loader = _write_filesystem_config(tmp_path, " roms_folder: roms\n") + loader.migrate_retired_filesystem_keys() + config = loader.get_config() + + assert write.call_count == 1 + assert config.CONFIG_FILE_WRITABLE is False + assert config.STRUCTURE_TEMPLATES["default"] == "roms/{platform}/{game}" + + +def test_an_unmigratable_folder_exits_without_rewriting_the_config( + tmp_path, library, critical +): + block = " roms_folder: games{old}\n" with pytest.raises(SystemExit) as excinfo: - _write_filesystem_config(tmp_path, f" {key}: {folder}\n") + _write_filesystem_config(tmp_path, block) assert excinfo.value.code == 3 - assert expected in critical() + assert "games{old}" in critical() + assert (tmp_path / "config.yml").read_text() == f"filesystem:\n{block}" + + +@pytest.mark.parametrize("value", ['""', "42"]) +def test_an_invalid_retired_folder_key_exits(tmp_path, library, value, critical): + with pytest.raises(SystemExit) as excinfo: + _write_filesystem_config(tmp_path, f" roms_folder: {value}\n") + + assert excinfo.value.code == 3 + assert "filesystem.roms_folder" in critical() def test_an_override_may_not_move_the_platform_folder(tmp_path, critical): @@ -886,13 +1029,11 @@ def test_an_invalid_firmware_template_is_rejected(tmp_path): def test_the_retired_library_layout_check_names_the_template( - tmp_path, mocker, critical + tmp_path, library, critical ): """A `{platform}/roms` library used to be auto-detected; now it has to say so rather than scan as empty and mark every rom missing.""" - library = tmp_path / "library" (library / "n64" / "roms").mkdir(parents=True) - mocker.patch("config.config_manager.LIBRARY_BASE_PATH", str(library)) loader = _write_filesystem_config(tmp_path, " skip_hash_calculation: false\n") with pytest.raises(SystemExit) as excinfo: @@ -902,10 +1043,8 @@ def test_the_retired_library_layout_check_names_the_template( assert 'default: "{platform}/roms/{game}"' in critical() -def test_the_retired_library_layout_check_passes_once_declared(tmp_path, mocker): - library = tmp_path / "library" +def test_the_retired_library_layout_check_passes_once_declared(tmp_path, library): (library / "n64" / "roms").mkdir(parents=True) - mocker.patch("config.config_manager.LIBRARY_BASE_PATH", str(library)) loader = _write_filesystem_config( tmp_path, ' structure:\n default: "{platform}/roms/{game}"\n'