Merge pull request #2486 from rommapp/rom-multi-file-rework

Split `rom.multi` into more specific fields

Author: gantoine

backend/endpoints/responses/rom.py

@@ -2,10 +2,10 @@
 
 import re
 from datetime import datetime, timezone
-from typing import NotRequired, TypedDict, get_type_hints
+from typing import Annotated, NotRequired, TypedDict, get_type_hints
 
 from fastapi import Request
-from pydantic import computed_field, field_validator
+from pydantic import Field, computed_field, field_validator
 
 from endpoints.responses.assets import SaveSchema, ScreenshotSchema, StateSchema
 from handler.metadata.flashpoint_handler import FlashpointMetadata
@@ -255,7 +255,11 @@ class RomSchema(BaseModel):
     md5_hash: str | None
     sha1_hash: str | None
 
-    multi: bool
+    # TODO: Remove this after 4.3 release
+    multi: Annotated[bool, Field(deprecated="Replaced by has_multiple_files")]
+    has_simple_single_file: bool
+    has_nested_single_file: bool
+    has_multiple_files: bool
     files: list[RomFileSchema]
     full_path: str
     created_at: datetime

backend/endpoints/sockets/scan.py

@@ -200,7 +200,6 @@ async def _identify_rom(
                 tags=fs_other_tags,
                 platform_id=platform.id,
                 name=fs_rom["fs_name"],
-                multi=fs_rom["multi"],
                 url_cover="",
                 url_manual="",
                 url_screenshots=[],

backend/handler/filesystem/roms_handler.py

@@ -93,8 +93,9 @@
 
 
 class FSRom(TypedDict):
-    multi: bool
     fs_name: str
+    flat: bool
+    nested: bool
     files: list[RomFile]
     crc_hash: str
     md5_hash: str
@@ -247,7 +248,7 @@ def parse_tags(self, fs_name: str) -> tuple:
             other_tags.append(tag)
         return regs, rev, langs, other_tags
 
-    def _exclude_multi_roms(self, roms: list[str]) -> list[str]:
+    def exclude_multi_roms(self, roms: list[str]) -> list[str]:
         excluded_names = cm.get_config().EXCLUDED_MULTI_FILES
         filtered_files: list = []
 
@@ -511,18 +512,19 @@ async def get_roms(self, platform: Platform) -> list[FSRom]:
             raise RomsNotFoundException(platform=platform.fs_slug) from e
 
         fs_roms: list[dict] = [
-            {"multi": False, "fs_name": rom}
+            {"fs_name": rom, "flat": True, "nested": False}
             for rom in self.exclude_single_files(fs_single_roms)
         ] + [
-            {"multi": True, "fs_name": rom}
-            for rom in self._exclude_multi_roms(fs_multi_roms)
+            {"fs_name": rom, "flat": False, "nested": True}
+            for rom in self.exclude_multi_roms(fs_multi_roms)
         ]
 
         return sorted(
             [
                 FSRom(
-                    multi=rom["multi"],
                     fs_name=rom["fs_name"],
+                    flat=rom["flat"],
+                    nested=rom["nested"],
                     files=[],
                     crc_hash="",
                     md5_hash="",

backend/handler/scan_handler.py

@@ -295,7 +295,6 @@ async def scan_rom(
         "platform_id": platform.id,
         "name": fs_rom["fs_name"],
         "fs_name": fs_rom["fs_name"],
-        "multi": fs_rom["multi"],
         "crc_hash": fs_rom["crc_hash"],
         "md5_hash": fs_rom["md5_hash"],
         "sha1_hash": fs_rom["sha1_hash"],
@@ -741,7 +740,8 @@ async def fetch_sgdb_details() -> SGDBRom:
         f"{hl(rom_attrs['fs_name'])} identified as {hl(rom_attrs['name'], color=BLUE)} {emoji.EMOJI_ALIEN_MONSTER}",
         extra=LOGGER_MODULE_NAME,
     )
-    if rom.multi:
+
+    if rom.has_nested_single_file or rom.has_multiple_files:
         for file in fs_rom["files"]:
             log.info(
                 f"\t · {hl(file.file_name, color=LIGHTYELLOW)}",

backend/models/rom.py

@@ -100,6 +100,10 @@ def file_extension(self) -> str:
 
         return fs_rom_handler.parse_file_extension(self.file_name)
 
+    @cached_property
+    def is_nested(self) -> bool:
+        return self.file_path.count("/") > 1
+
     def file_name_for_download(self, rom: Rom, hidden_folder: bool = False) -> str:
         # This needs a trailing slash in the path to work!
         return self.full_path.replace(
@@ -286,11 +290,22 @@ def merged_screenshots(self) -> list[str]:
 
         return []
 
+    # TODO: Remove this after 4.3 release
     @cached_property
     def multi(self) -> bool:
-        return len(self.files) > 1 or (
-            len(self.files) > 0 and len(self.files[0].full_path.split("/")) > 3
-        )
+        return self.has_nested_single_file or self.has_multiple_files
+
+    @cached_property
+    def has_simple_single_file(self) -> bool:
+        return len(self.files) == 1 and not self.files[0].is_nested
+
+    @cached_property
+    def has_nested_single_file(self) -> bool:
+        return len(self.files) == 1 and self.files[0].is_nested
+
+    @cached_property
+    def has_multiple_files(self) -> bool:
+        return len(self.files) > 1
 
     @property
     def fs_resources_path(self) -> str:

backend/tests/handler/filesystem/test_roms_handler.py

@@ -48,14 +48,24 @@ def rom_single(self, platform: Platform):
 
     @pytest.fixture
     def rom_multi(self, platform: Platform):
-        rom = Rom(
+        return Rom(
             id=2,
             fs_name="Super Mario 64 (J) (Rev A)",
             fs_path="n64/roms",
             platform=platform,
+            files=[
+                RomFile(
+                    id=1,
+                    file_name="Super Mario 64 (J) (Rev A) [Part 1].z64",
+                    file_path="n64/roms",
+                ),
+                RomFile(
+                    id=2,
+                    file_name="Super Mario 64 (J) (Rev A) [Part 2].z64",
+                    file_path="n64/roms",
+                ),
+            ],
         )
-        rom.multi = True
-        return rom
 
     def test_init_uses_library_base_path(self, handler: FSRomsHandler):
         """Test that FSRomsHandler initializes with LIBRARY_BASE_PATH"""
@@ -155,19 +165,19 @@ def test_parse_tags_no_tags(self, handler: FSRomsHandler):
         assert other_tags == []
 
     def test_exclude_multi_roms_filters_excluded(self, handler: FSRomsHandler, config):
-        """Test _exclude_multi_roms filters out excluded multi-file ROMs"""
+        """Test exclude_multi_roms filters out excluded multi-file ROMs"""
         roms = ["Game1", "excluded_multi", "Game2", "Game3"]
 
         with pytest.MonkeyPatch.context() as m:
             m.setattr("handler.filesystem.roms_handler.cm.get_config", lambda: config)
 
-            result = handler._exclude_multi_roms(roms)
+            result = handler.exclude_multi_roms(roms)
             expected = ["Game1", "Game2", "Game3"]
 
             assert result == expected
 
     def test_exclude_multi_roms_no_exclusions(self, handler: FSRomsHandler):
-        """Test _exclude_multi_roms with no exclusions"""
+        """Test exclude_multi_roms with no exclusions"""
         roms = ["Game1", "Game2", "Game3"]
         config = Config(
             EXCLUDED_PLATFORMS=[],
@@ -185,7 +195,7 @@ def test_exclude_multi_roms_no_exclusions(self, handler: FSRomsHandler):
         with pytest.MonkeyPatch.context() as m:
             m.setattr("handler.filesystem.roms_handler.cm.get_config", lambda: config)
 
-            result = handler._exclude_multi_roms(roms)
+            result = handler.exclude_multi_roms(roms)
             assert result == roms
 
     def test_build_rom_file_single_file(self, handler: FSRomsHandler):
@@ -254,8 +264,8 @@ async def test_get_roms(self, handler: FSRomsHandler, platform, config):
             assert len(result) > 0
 
             # Check that we have both single and multi ROMs
-            single_roms = [r for r in result if not r["multi"]]
-            multi_roms = [r for r in result if r["multi"]]
+            single_roms = [r for r in result if not r["flat"]]
+            multi_roms = [r for r in result if r["nested"]]
 
             assert len(single_roms) > 0
             assert len(multi_roms) > 0
@@ -477,7 +487,7 @@ async def test_multi_rom_directory_handling(self, handler: FSRomsHandler, config
             assert "Test Multi Rom [USA]" in directories
 
             # After exclusion, normal directories should remain
-            filtered_dirs = handler._exclude_multi_roms(directories)
+            filtered_dirs = handler.exclude_multi_roms(directories)
             assert "Super Mario 64 (J) (Rev A)" in filtered_dirs
             assert "Test Multi Rom [USA]" in filtered_dirs
 

backend/tests/handler/test_fastapi.py

@@ -42,7 +42,6 @@ async def test_scan_rom():
         igdb_id=3340,
         fs_size_bytes=1024,
         tags=[],
-        multi=False,
     )
 
     async with initialize_context():
@@ -52,7 +51,8 @@ async def test_scan_rom():
             rom=rom,
             fs_rom={
                 "fs_name": "Paper Mario (USA).z64",
-                "multi": False,
+                "flat": True,
+                "nested": False,
                 "files": [
                     RomFile(
                         file_name="Paper Mario (USA).z64",
@@ -73,7 +73,7 @@ async def test_scan_rom():
     assert type(rom) is Rom
     assert rom.fs_name == "Paper Mario (USA).z64"
     assert rom.name == "Paper Mario"
+    assert rom.fs_path == "n64/Paper Mario (USA)"
     assert rom.igdb_id == 3340
     assert rom.fs_size_bytes == 1024
     assert rom.tags == []
-    assert not rom.multi

frontend/src/__generated__/models/DetailedRomSchema.ts

@@ -46,7 +46,11 @@ watch(
 <template>
   <v-row no-gutters>
     <v-col>
-      <v-row v-if="!rom.multi" class="align-center my-3" no-gutters>
+      <v-row
+        v-if="rom.has_simple_single_file"
+        class="align-center my-3"
+        no-gutters
+      >
         <v-col cols="3" xl="2" class="mr-2">
           <span>{{ t("rom.file") }}</span>
         </v-col>
@@ -58,7 +62,7 @@ watch(
           /><span class="text-body-1">{{ rom.fs_name }}</span>
         </v-col>
       </v-row>
-      <v-row v-if="rom.multi" class="align-center my-3" no-gutters>
+      <v-row v-else class="align-center my-3" no-gutters>
         <v-col cols="3" xl="2" class="mr-2">
           <span>{{ t("rom.files") }}</span>
         </v-col>