Fix custom library structure PR merge issues

backend/alembic/versions/0091_unique_platform_fs_name.py

@@ -1,7 +1,8 @@
-"""Enforce unique (platform_id, fs_name) on roms
+"""Enforce unique (platform_id, fs_path, fs_name) on roms
 
-This migration removes any pre-existing duplicates (keeping the lowest id) and
-upgrades the index to unique so the duplicate can never be created again.
+This migration removes any pre-existing full-path duplicates (keeping the
+lowest id) and upgrades the index to unique so the duplicate can never be
+created again.
 
 Revision ID: 0091_unique_platform_fs_name
 Revises: 0090_roms_sibling_cover_index
@@ -18,27 +19,29 @@
 branch_labels = None
 depends_on = None
 
-INDEX_NAME = "idx_roms_platform_id_fs_name"
-INDEX_COLUMNS = ["platform_id", "fs_name"]
+OLD_INDEX_NAME = "idx_roms_platform_id_fs_name"
+INDEX_NAME = "idx_roms_platform_id_fs_path_fs_name"
+INDEX_COLUMNS = ["platform_id", "fs_path", "fs_name"]
 
 
 def upgrade() -> None:
     connection = op.get_bind()
 
-    # Drop duplicate roms sharing (platform_id, fs_name), keeping the lowest id.
+    # Drop duplicate roms sharing (platform_id, fs_path, fs_name), keeping the
+    # lowest id.
     connection.execute(sa.text("""
             DELETE FROM roms
             WHERE id NOT IN (
                 SELECT keep_id FROM (
                     SELECT MIN(id) AS keep_id
                     FROM roms
-                    GROUP BY platform_id, fs_name
+                    GROUP BY platform_id, fs_path, fs_name
                 ) AS keepers
             )
             """))
 
     with op.batch_alter_table("roms", schema=None) as batch_op:
-        batch_op.drop_index(INDEX_NAME, if_exists=True)
+        batch_op.drop_index(OLD_INDEX_NAME, if_exists=True)
         batch_op.create_index(
             INDEX_NAME,
             INDEX_COLUMNS,
@@ -51,8 +54,8 @@ def downgrade() -> None:
     with op.batch_alter_table("roms", schema=None) as batch_op:
         batch_op.drop_index(INDEX_NAME, if_exists=True)
         batch_op.create_index(
-            INDEX_NAME,
-            INDEX_COLUMNS,
+            OLD_INDEX_NAME,
+            ["platform_id", "fs_name"],
             unique=False,
             if_not_exists=True,
         )

backend/config/config_manager.py

@@ -4,6 +4,7 @@
 import json
 import os
 import sys
+from dataclasses import dataclass
 from pathlib import Path
 from typing import Final, NotRequired, TypedDict
 
@@ -28,6 +29,117 @@
 from logger.formatter import highlight as hl
 from logger.logger import log
 
+# Terminal macros for a custom library structure template. Exactly one must
+# appear, as the last path segment.
+STRUCTURE_GAME_FILE: Final = "gameFile"
+STRUCTURE_GAME_DIR: Final = "gameDir"
+# Macros that belong to Retrom's full-path templates but are resolved by RomM
+# itself (library base + roms folder + platform discovery). A per-platform
+# template is relative to the platform's ROM folder, so these are rejected with
+# a helpful message instead of being silently treated as wildcard levels.
+_STRUCTURE_RESERVED_NONTERMINAL: Final = frozenset({"platform", "library"})
+
+
+@dataclass(frozen=True)
+class StructureLevel:
+    """One intermediate directory level of a custom structure template.
+
+    ``literal`` is the exact folder name to match, or ``None`` for a wildcard
+    macro level (``{region}``, ``{category}``, …) that matches any folder.
+    """
+
+    literal: str | None
+
+
+@dataclass(frozen=True)
+class LibraryStructure:
+    """A parsed per-platform custom library structure.
+
+    Relative to the platform's ROM folder: ``levels`` are the intermediate
+    directory levels to descend (literal or wildcard), and ``each_file_is_game``
+    selects the terminal — ``{gameFile}`` (each file is a ROM) vs ``{gameDir}``
+    (each directory is a multi-file ROM).
+    """
+
+    levels: tuple[StructureLevel, ...]
+    each_file_is_game: bool
+
+
+def parse_library_structure(template: str) -> LibraryStructure:
+    """Parse a custom structure template into a ``LibraryStructure``.
+
+    Template syntax mirrors Retrom: ``/``-separated path sections where a
+    section wrapped in braces is a macro and a bare section is a literal folder
+    name. The last section must be the terminal ``{gameFile}`` or ``{gameDir}``;
+    every other braced section is a wildcard directory level. The template is
+    relative to the platform's ROM folder, so ``{platform}`` / ``{library}`` are
+    not used here.
+
+    Raises ``ValueError`` on an invalid template.
+    """
+    sections = [s for s in template.split("/") if s != ""]
+    if not sections:
+        raise ValueError("template is empty")
+
+    levels: list[StructureLevel] = []
+    each_file_is_game: bool | None = None
+
+    for index, section in enumerate(sections):
+        is_last = index == len(sections) - 1
+        is_macro = section.startswith("{") and section.endswith("}")
+
+        if not is_macro:
+            if "{" in section or "}" in section:
+                raise ValueError(f"malformed macro in section '{section}'")
+            levels.append(StructureLevel(literal=section))
+            continue
+
+        name = section[1:-1].strip()
+        if name in (STRUCTURE_GAME_FILE, STRUCTURE_GAME_DIR):
+            if not is_last:
+                raise ValueError(
+                    f"'{{{name}}}' must be the last path section of the template"
+                )
+            each_file_is_game = name == STRUCTURE_GAME_FILE
+            continue
+
+        if name in _STRUCTURE_RESERVED_NONTERMINAL:
+            raise ValueError(
+                f"'{{{name}}}' is not supported: a platform template is relative "
+                "to that platform's ROM folder (RomM resolves library root, roms "
+                "folder and platform on its own)"
+            )
+        if not name:
+            raise ValueError("empty macro '{}'")
+        # Any other braced section is an organizational wildcard directory level.
+        levels.append(StructureLevel(literal=None))
+
+    if each_file_is_game is None:
+        raise ValueError("template must end with '{gameFile}' or '{gameDir}'")
+
+    return LibraryStructure(levels=tuple(levels), each_file_is_game=each_file_is_game)
+
+
+def parse_platform_structures(
+    value: str | list[str],
+) -> tuple[LibraryStructure, ...]:
+    """Parse a platform's custom structure config into one or more structures.
+
+    A platform may declare a single template (string) or several (list) — the
+    list form lets one platform mix layouts, e.g. loose games at the root plus
+    games inside grouping subfolders::
+
+        nes:
+          - "{gameFile}"
+          - "{category}/{gameFile}"
+
+    Discovery is the union of all listed templates. Raises ``ValueError`` if any
+    template is invalid.
+    """
+    templates = [value] if isinstance(value, str) else list(value)
+    return tuple(parse_library_structure(template) for template in templates)
+
+
 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"
@@ -185,6 +297,7 @@ class Config:
     PLATFORMS_VERSIONS: dict[str, str]
     ROMS_FOLDER_NAME: str
     FIRMWARE_FOLDER_NAME: str
+    STRUCTURE_TEMPLATES: dict[str, str | list[str]]
     SKIP_HASH_CALCULATION: bool
     EJS_DEBUG: bool
     EJS_CACHE_LIMIT: int | None
@@ -231,6 +344,21 @@ def has_structure_path_b(self) -> bool:
 
         return False
 
+    def platform_structure(self, fs_slug: str) -> tuple[LibraryStructure, ...] | None:
+        """The custom library structure(s) for a platform, or ``None``.
+
+        Opt-in per platform via `filesystem.structure` in config.yml (a map of
+        ``fs_slug -> template`` or ``fs_slug -> [template, ...]``). When unset,
+        the platform uses RomM's default discovery (top-level files and
+        folders). Returns the parsed structures whose discovery is unioned;
+        templates are validated at load time, so parsing here is expected to
+        succeed.
+        """
+        value = getattr(self, "STRUCTURE_TEMPLATES", {}).get(fs_slug)
+        if not value:
+            return None
+        return parse_platform_structures(value)
+
 
 class ConfigManager:
     """
@@ -536,6 +664,9 @@ def _parse_config(self):
             PEGASUS_AUTO_EXPORT_ON_SCAN=pydash.get(
                 self._raw_config, "scan.pegasus.export", False
             ),
+            STRUCTURE_TEMPLATES=pydash.get(
+                self._raw_config, "filesystem.structure", {}
+            ),
             STREAMING_ENABLED=pydash.get(self._raw_config, "streaming.enabled", False),
             STREAMING_CONTAINERS=pydash.get(
                 self._raw_config, "streaming.containers", []
@@ -782,6 +913,33 @@ def _validate_config(self):
             log.critical("Invalid config.yml: scan.media must be a list")
             sys.exit(3)
 
+        if not isinstance(self.config.STRUCTURE_TEMPLATES, dict):
+            log.critical(
+                "Invalid config.yml: filesystem.structure must be a dictionary"
+            )
+            sys.exit(3)
+        for fs_slug, value in self.config.STRUCTURE_TEMPLATES.items():
+            if isinstance(value, str):
+                templates = [value]
+            elif isinstance(value, list) and all(isinstance(t, str) for t in value):
+                templates = value
+            else:
+                log.critical(
+                    f"Invalid config.yml: filesystem.structure.{fs_slug} must be a "
+                    "template string or a list of template strings"
+                )
+                sys.exit(3)
+
+            for template in templates:
+                try:
+                    parse_library_structure(template)
+                except ValueError as exc:
+                    log.critical(
+                        f"Invalid config.yml: filesystem.structure.{fs_slug} "
+                        f"('{template}'): {exc}"
+                    )
+                    sys.exit(3)
+
         # Drop unknown media types rather than exiting, since a newer release
         # may ship sample configs referencing media types this version doesn't know.
         unknown_media = [
@@ -873,6 +1031,7 @@ def _update_config_file(self) -> None:
             "filesystem": {
                 "roms_folder": self.config.ROMS_FOLDER_NAME,
                 "firmware_folder": self.config.FIRMWARE_FOLDER_NAME,
+                "structure": self.config.STRUCTURE_TEMPLATES,
                 "skip_hash_calculation": self.config.SKIP_HASH_CALCULATION,
             },
             "system": {

backend/endpoints/sockets/scan.py

@@ -327,7 +327,9 @@ async def _identify_rom(
 
     # Update properties that don't require metadata
     parsed_tags = fs_rom_handler.parse_tags(fs_rom["fs_name"])
-    roms_path = fs_rom_handler.get_roms_fs_structure(platform.fs_slug)
+    # The discovered path is the rom's actual directory, which may be nested
+    # under the platform roms folder when a custom structure is configured.
+    roms_path = fs_rom["fs_path"]
 
     rom_attrs = {
         "fs_name": fs_rom["fs_name"],
@@ -714,7 +716,9 @@ async def _identify_platform(
     # moved ROM (a new file with no fs_name match) can be reassociated by hash
     # with its now-missing entry instead of spawning a duplicate. The end-of-scan
     # call below re-syncs and logs, unmarking any entry that got reassociated.
-    db_rom_handler.mark_missing_roms(platform.id, [rom["fs_name"] for rom in fs_roms])
+    db_rom_handler.mark_missing_roms(
+        platform.id, [f"{rom['fs_path']}/{rom['fs_name']}" for rom in fs_roms]
+    )
 
     # Create semaphore to limit concurrent ROM scanning
     scan_semaphore = asyncio.Semaphore(SCAN_WORKERS)
@@ -736,7 +740,10 @@ async def scan_rom_with_semaphore(fs_rom: FSRom, rom: Rom | None) -> None:
             )
 
     for fs_roms_batch in batched(fs_roms, 200, strict=False):
-        roms_by_fs_name = db_rom_handler.get_roms_by_fs_name(
+        # Key matches on the rom's full path (fs_path/fs_name), not just the
+        # file name, so identically-named files in different folders don't
+        # collide under a custom library structure.
+        roms_by_full_path = db_rom_handler.get_roms_by_fs_name(
             platform_id=platform.id,
             fs_names={fs_rom["fs_name"] for fs_rom in fs_roms_batch},
         )
@@ -747,7 +754,7 @@ async def scan_rom_with_semaphore(fs_rom: FSRom, rom: Rom | None) -> None:
         roms_to_scan: list[tuple[FSRom, Rom | None]] = []
 
         for fs_rom in fs_roms_batch:
-            rom = roms_by_fs_name.get(fs_rom["fs_name"])
+            rom = roms_by_full_path.get(f"{fs_rom['fs_path']}/{fs_rom['fs_name']}")
             if should_scan_rom(
                 scan_type=scan_type,
                 rom=rom,
@@ -807,12 +814,27 @@ async def scan_rom_with_semaphore(fs_rom: FSRom, rom: Rom | None) -> None:
                     log.error(f"Error scanning ROM {fs_rom['fs_name']}: {result}")
 
     missing_roms = db_rom_handler.mark_missing_roms(
-        platform.id, [rom["fs_name"] for rom in fs_roms]
+        platform.id, [f"{rom['fs_path']}/{rom['fs_name']}" for rom in fs_roms]
     )
     if len(missing_roms) > 0:
         log.warning(f"{hl('Missing')} roms from filesystem:")
+        # A folder a custom structure now descends into used to be a single
+        # multi-file rom; that old entry shows up here as missing. Flag those so
+        # it's clear the "missing" is expected and the stale entry can be
+        # deleted. A superseded folder's path is a parent of a discovered rom.
+        descended_paths = {rom["fs_path"] for rom in fs_roms}
         for r in missing_roms:
-            log.warning(f" - {r.fs_name}")
+            superseded = any(
+                p == r.full_path or p.startswith(f"{r.full_path}/")
+                for p in descended_paths
+            )
+            if superseded:
+                log.warning(
+                    f" - {r.fs_name} (now scanned as a folder of roms — "
+                    "delete this stale entry to clean up)"
+                )
+            else:
+                log.warning(f" - {r.fs_name}")
 
     missing_firmware = db_firmware_handler.mark_missing_firmware(
         platform.id, [fw for fw in fs_firmware]