From 9593c302926ea7973da13e52a2810bb1cfb6cde7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 3 May 2026 23:34:30 +0000 Subject: [PATCH] Address PR review: normalize exclusion sets, avoid duplicates, add multi-dot test for get_rom_files Agent-Logs-Url: https://github.com/rommapp/romm/sessions/8cbbc2ca-a3e3-4c61-9e47-f8544d59231a Co-authored-by: gantoine <3247106+gantoine@users.noreply.github.com> --- backend/handler/filesystem/base_handler.py | 22 ++++--- backend/handler/filesystem/roms_handler.py | 6 +- .../handler/filesystem/test_roms_handler.py | 61 ++++++++++++++++++- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/backend/handler/filesystem/base_handler.py b/backend/handler/filesystem/base_handler.py index bfcb48057..689d3ef42 100644 --- a/backend/handler/filesystem/base_handler.py +++ b/backend/handler/filesystem/base_handler.py @@ -210,24 +210,30 @@ class FSHandler: return match.group(0) if match else "" def exclude_single_files(self, files: list[str]) -> list[str]: - excluded_extensions = cm.get_config().EXCLUDED_SINGLE_EXT - excluded_names = cm.get_config().EXCLUDED_SINGLE_FILES + config = cm.get_config() + # Config already stores extensions lowercased; use a set for O(1) lookups. + excluded_extensions = set(config.EXCLUDED_SINGLE_EXT) + excluded_names = config.EXCLUDED_SINGLE_FILES excluded_files: list[str] = [] for file_name in files: + file_name_lower = file_name.lower() + # Check whether the filename ends with any excluded extension entry. # Using ends-with handles both simple rules ("txt") and compound rules # ("hash.txt") against multi-dot filenames like "game.nds.enc.hash.txt". if any( - file_name.lower().endswith("." + ext.lower()) - for ext in excluded_extensions + file_name_lower.endswith("." + ext) for ext in excluded_extensions ): excluded_files.append(file_name) + continue - # Additionally, check if the file name matches a pattern in the excluded list. - for name in excluded_names: - if file_name == name or fnmatch.fnmatch(file_name, name): - excluded_files.append(file_name) + # Check if the file name matches a pattern in the excluded list. + if any( + file_name == name or fnmatch.fnmatch(file_name, name) + for name in excluded_names + ): + excluded_files.append(file_name) # Return files that are not in the filtered list. return [f for f in files if f not in excluded_files] diff --git a/backend/handler/filesystem/roms_handler.py b/backend/handler/filesystem/roms_handler.py index 29c021c33..8c7fb256e 100644 --- a/backend/handler/filesystem/roms_handler.py +++ b/backend/handler/filesystem/roms_handler.py @@ -434,7 +434,8 @@ class FSRomsHandler(FSHandler): ) excluded_file_names = cm.get_config().EXCLUDED_MULTI_PARTS_FILES - excluded_file_exts = cm.get_config().EXCLUDED_MULTI_PARTS_EXT + # Config already stores extensions lowercased; use a set for O(1) lookups. + excluded_file_exts = set(cm.get_config().EXCLUDED_MULTI_PARTS_EXT) rom_crc_c = 0 rom_md5_h = hashlib.md5(usedforsecurity=False) if calculate_hashes else None @@ -458,8 +459,9 @@ class FSRomsHandler(FSHandler): # Check if file is excluded by extension. # Using ends-with handles both simple rules ("txt") and compound # rules ("hash.txt") for multi-dot filenames like "game.nds.enc.hash.txt". + file_name_lower = file_name.lower() if any( - file_name.lower().endswith("." + ext.lower()) + file_name_lower.endswith("." + ext) for ext in excluded_file_exts ): continue diff --git a/backend/tests/handler/filesystem/test_roms_handler.py b/backend/tests/handler/filesystem/test_roms_handler.py index e6c5bdc74..9e1dbce39 100644 --- a/backend/tests/handler/filesystem/test_roms_handler.py +++ b/backend/tests/handler/filesystem/test_roms_handler.py @@ -378,7 +378,66 @@ class TestFSRomsHandler: assert rom_file.file_size_bytes > 0 assert rom_file.last_modified is not None - async def test_rename_fs_rom_same_name(self, handler: FSRomsHandler): + @pytest.mark.asyncio + async def test_get_rom_files_multi_rom_multi_dot_exclusion( + self, handler: FSRomsHandler, rom_multi + ): + """Multi-dot filenames in a multi-part dir are excluded by simple or compound ext rules.""" + multi_dot_file = ( + handler.base_path + / "n64/roms/Super Mario 64 (J) (Rev A)/game.nds.hash.txt" + ) + multi_dot_file.write_text("hash data") + + try: + # Exclude by the last single extension "txt" + config_txt = Config( + EXCLUDED_PLATFORMS=[], + EXCLUDED_SINGLE_EXT=[], + EXCLUDED_SINGLE_FILES=[], + EXCLUDED_MULTI_FILES=[], + EXCLUDED_MULTI_PARTS_EXT=["txt"], + EXCLUDED_MULTI_PARTS_FILES=[], + PLATFORMS_BINDING={}, + PLATFORMS_VERSIONS={}, + ROMS_FOLDER_NAME="roms", + FIRMWARE_FOLDER_NAME="bios", + ) + with pytest.MonkeyPatch.context() as m: + m.setattr( + "handler.filesystem.roms_handler.cm.get_config", lambda: config_txt + ) + parsed = await handler.get_rom_files(rom_multi) + file_names = [rf.file_name for rf in parsed.rom_files] + assert "game.nds.hash.txt" not in file_names + assert "Super Mario 64 (J) (Rev A) [Part 1].z64" in file_names + + # Exclude by the compound extension "hash.txt" + config_compound = Config( + EXCLUDED_PLATFORMS=[], + EXCLUDED_SINGLE_EXT=[], + EXCLUDED_SINGLE_FILES=[], + EXCLUDED_MULTI_FILES=[], + EXCLUDED_MULTI_PARTS_EXT=["hash.txt"], + EXCLUDED_MULTI_PARTS_FILES=[], + PLATFORMS_BINDING={}, + PLATFORMS_VERSIONS={}, + ROMS_FOLDER_NAME="roms", + FIRMWARE_FOLDER_NAME="bios", + ) + with pytest.MonkeyPatch.context() as m: + m.setattr( + "handler.filesystem.roms_handler.cm.get_config", + lambda: config_compound, + ) + parsed = await handler.get_rom_files(rom_multi) + file_names = [rf.file_name for rf in parsed.rom_files] + assert "game.nds.hash.txt" not in file_names + assert "Super Mario 64 (J) (Rev A) [Part 1].z64" in file_names + finally: + multi_dot_file.unlink(missing_ok=True) + + """Test rename_fs_rom when old and new names are the same""" old_name = "test_rom.n64" new_name = "test_rom.n64"