Simon/romm
1
0
Fork 0
mirror of https://github.com/rommapp/romm.git synced 2026-06-27 14:25:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: split structure detection into has_structure_path_a, fail loudly on bad layouts

Browse Source 
Extract has_structure_path_a as its own cached property and have
has_structure_path_b delegate to it, removing duplicated isdir checks.
detect_library_structure and get_platforms_directory now read the named
properties instead of re-implementing the roms-path check inline.

Keep the inconclusive/bad-structure fallback defaulting to Structure A so
a malformed library raises FolderStructureNotMatchException rather than
listing the bare library root as a flat list of platforms.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Georges-Antoine Assi
2026-06-12 14:59:01 -04:00
parent 788d454d98
commit 49ef5097e4
3 changed files with 37 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
backend/config/config_manager.py
View File

@@ -139,12 +139,16 @@ class Config:
self.__dict__.update(entries)
@functools.cached_property
def has_structure_path_b(self) -> bool:
# Structure A (roms/{platform}) takes priority: if the top-level roms
# folder exists, never claim Structure B even if some platform dirs
# happen to contain a roms sub-folder.
def has_structure_path_a(self) -> bool:
# Structure A ({roms_folder}/{platform}) takes priority: if the top-level roms
# folder exists, claim Structure A even if some platform dirs happen to
# contain a {roms_folder} sub-folder.
roms_path = os.path.join(LIBRARY_BASE_PATH, self.ROMS_FOLDER_NAME)
if os.path.isdir(roms_path):
return os.path.isdir(roms_path)
@functools.cached_property
def has_structure_path_b(self) -> bool:
if self.has_structure_path_a:
return False
pattern = os.path.join(
@@ -153,6 +157,7 @@ class Config:
for match in glob.iglob(pattern):
if os.path.isdir(match):
return True
return False

14
backend/handler/filesystem/platforms_handler.py
View File

@@ -33,9 +33,9 @@ class FSPlatformsHandler(FSHandler):
def detect_library_structure(self) -> LibraryStructure | None:
"""Detects the library structure type.
Structure A (roms/{platform}) takes priority over Structure B
({platform}/roms) so that existing libraries are not broken when a
stray {platform}/roms directory happens to exist alongside them.
Structure A ({roms_folder}/{platform}) takes priority over Structure B
({platform}/{roms_folder}) so that existing libraries are not broken when a
stray {platform}/{roms_folder} directory happens to exist alongside them.
Returns:
"LibraryStructure.A" for Structure A (roms/{platform}) when the
@@ -47,8 +47,7 @@ class FSPlatformsHandler(FSHandler):
"""
cnfg = cm.get_config()
roms_path = os.path.join(LIBRARY_BASE_PATH, cnfg.ROMS_FOLDER_NAME)
if os.path.exists(roms_path):
if cnfg.has_structure_path_a:
return LibraryStructure.A
if cnfg.has_structure_path_b:
@@ -59,7 +58,10 @@ class FSPlatformsHandler(FSHandler):
def get_platforms_directory(self) -> str:
cnfg = cm.get_config()
# Fallback to config hint when detection is inconclusive
# Fallback to config hint when detection is inconclusive: default to
# Structure A (roms/{platform}) so a malformed library fails loudly
# (FolderStructureNotMatchException) rather than treating the bare
# library root as a flat list of platforms.
return "" if cnfg.has_structure_path_b else cnfg.ROMS_FOLDER_NAME
def get_platform_fs_structure(self, fs_slug: str) -> str:

39
backend/tests/handler/filesystem/test_platforms_handler.py
View File

@@ -101,6 +101,7 @@ class TestFSPlatformsHandler:
self, handler: FSPlatformsHandler, config
):
"""Test get_platforms_directory with Structure B ({platform}/roms)"""
config.has_structure_path_a = False
config.has_structure_path_b = True
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
@@ -203,7 +204,7 @@ class TestFSPlatformsHandler:
self, handler: FSPlatformsHandler, config
):
"""Test that get_platforms calls list_directories with correct path"""
config.has_structure_path_b = False
config.has_structure_path_a = True
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
):
@@ -217,6 +218,8 @@ class TestFSPlatformsHandler:
self, handler: FSPlatformsHandler, config
):
"""Test that get_platforms calls list_directories with empty path for normal structure"""
config.has_structure_path_a = False
config.has_structure_path_b = True
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
):
@@ -315,66 +318,62 @@ class TestFSPlatformsHandler:
self, handler: FSPlatformsHandler, config
):
"""Test detect_library_structure detects Structure A (roms/{platform})"""
roms_path = f"{LIBRARY_BASE_PATH}/{config.ROMS_FOLDER_NAME}"
config.has_structure_path_a = True
config.has_structure_path_b = False
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
):
with patch("os.path.exists") as mock_exists:
mock_exists.return_value = True
result = handler.detect_library_structure()
assert result == LibraryStructure.A
mock_exists.assert_called_once_with(roms_path)
result = handler.detect_library_structure()
assert result == LibraryStructure.A
def test_detect_library_structure_structure_b(
self, handler: FSPlatformsHandler, config
):
"""Test detect_library_structure detects Structure B ({platform}/roms)"""
config.has_structure_path_a = False
config.has_structure_path_b = True
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
):
with patch("os.path.exists", return_value=False):
result = handler.detect_library_structure()
assert result == LibraryStructure.B
result = handler.detect_library_structure()
assert result == LibraryStructure.B
def test_detect_library_structure_a_takes_priority_over_b(
self, handler: FSPlatformsHandler, config
):
"""Structure A is reported when the top-level roms folder exists, even
when Structure B directories are also present."""
config.has_structure_path_a = True
config.has_structure_path_b = True
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
):
with patch("os.path.exists", return_value=True):
result = handler.detect_library_structure()
assert result == LibraryStructure.A
result = handler.detect_library_structure()
assert result == LibraryStructure.A
def test_detect_library_structure_none(self, handler: FSPlatformsHandler, config):
"""Test detect_library_structure returns None when no structure detected"""
config.has_structure_path_a = False
config.has_structure_path_b = False
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
):
with patch("os.path.exists", return_value=False):
result = handler.detect_library_structure()
assert result is None
result = handler.detect_library_structure()
assert result is None
def test_detect_library_structure_empty_library(
self, handler: FSPlatformsHandler, config
):
"""Test detect_library_structure with empty library directory"""
config.has_structure_path_a = False
config.has_structure_path_b = False
with patch(
"handler.filesystem.platforms_handler.cm.get_config", return_value=config
):
with patch("os.path.exists", return_value=False):
result = handler.detect_library_structure()
assert result is None
result = handler.detect_library_structure()
assert result is None