Merge pull request #3861 from rommapp/feat/reassociate-missing-roms-by-hash

feat(scan): reassociate renamed/moved ROMs with missing entries by file hash

Author: gantoine

backend/endpoints/roms/__init__.py

@@ -1210,6 +1210,11 @@ async def head_rom_content(
         if len(files) == 1:
             file = files[0]
             rom_path = f"{LIBRARY_BASE_PATH}/{file.full_path}"
+            if not await Path(rom_path).is_file():
+                raise HTTPException(
+                    status_code=status.HTTP_404_NOT_FOUND,
+                    detail=f"File {file.file_name} not found on disk for ROM {id}",
+                )
             return FileResponse(
                 path=rom_path,
                 filename=file.file_name,
@@ -1319,6 +1324,11 @@ async def get_rom_content(
         if len(files) == 1:
             file = files[0]
             rom_path = f"{LIBRARY_BASE_PATH}/{file.full_path}"
+            if not await Path(rom_path).is_file():
+                raise HTTPException(
+                    status_code=status.HTTP_404_NOT_FOUND,
+                    detail=f"File {file.file_name} not found on disk for ROM {id}",
+                )
             return FileResponse(
                 path=rom_path,
                 filename=file.file_name,

backend/endpoints/sockets/scan.py

@@ -272,43 +272,97 @@ async def _identify_rom(
     parsed_tags = fs_rom_handler.parse_tags(fs_rom["fs_name"])
     roms_path = fs_rom_handler.get_roms_fs_structure(platform.fs_slug)
 
-    # Create the entry early so we have the ID
+    rom_attrs = {
+        "fs_name": fs_rom["fs_name"],
+        "fs_path": roms_path,
+        "regions": parsed_tags.regions,
+        "revision": parsed_tags.revision,
+        "version": parsed_tags.version,
+        "languages": parsed_tags.languages,
+        "tags": parsed_tags.other_tags,
+        "platform_id": platform.id,
+        "name": fs_rom_handler.get_file_name_with_no_tags(fs_rom["fs_name"]),
+        "url_cover": "",
+        "url_manual": "",
+        "url_screenshots": [],
+    }
+
+    calculate_hashes = not cm.get_config().SKIP_HASH_CALCULATION
+
     newly_added: bool = rom is None
-    if not rom:
-        try:
-            rom = db_rom_handler.add_rom(
-                Rom(
-                    fs_name=fs_rom["fs_name"],
-                    fs_path=roms_path,
-                    regions=parsed_tags.regions,
-                    revision=parsed_tags.revision,
-                    version=parsed_tags.version,
-                    languages=parsed_tags.languages,
-                    tags=parsed_tags.other_tags,
-                    platform_id=platform.id,
-                    name=fs_rom_handler.get_file_name_with_no_tags(fs_rom["fs_name"]),
-                    url_cover="",
-                    url_manual="",
-                    url_screenshots=[],
-                )
+    reassociated: bool = False
+    files_built: bool = False
+
+    if rom is None:
+        # No entry matches this filename. Before treating it as new, hash
+        # the files and check whether they belong to an existing entry that went
+        # missing (a renamed or moved ROM), so its collections, notes, and
+        # uploaded assets carry over instead of being orphaned on a duplicate.
+        parsed_rom_files = await fs_rom_handler.get_rom_files(
+            Rom(
+                **rom_attrs,
+                platform=platform,
+            ),
+            calculate_hashes=calculate_hashes,
+        )
+        fs_rom.update(
+            {
+                "files": parsed_rom_files.rom_files,
+                "crc_hash": parsed_rom_files.crc_hash,
+                "md5_hash": parsed_rom_files.md5_hash,
+                "sha1_hash": parsed_rom_files.sha1_hash,
+                "ra_hash": parsed_rom_files.ra_hash,
+            }
+        )
+        files_built = True
+
+        missing_match = db_rom_handler.get_matching_missing_rom(
+            platform_id=platform.id,
+            crc_hash=parsed_rom_files.crc_hash,
+            md5_hash=parsed_rom_files.md5_hash,
+            sha1_hash=parsed_rom_files.sha1_hash,
+        )
+        if missing_match is not None:
+            # Move the existing entry onto the new file, clearing its missing state.
+            rom = db_rom_handler.update_rom(
+                missing_match.id,
+                {
+                    "fs_name": fs_rom["fs_name"],
+                    "fs_path": roms_path,
+                    "regions": parsed_tags.regions,
+                    "revision": parsed_tags.revision,
+                    "version": parsed_tags.version,
+                    "languages": parsed_tags.languages,
+                    "tags": parsed_tags.other_tags,
+                    "missing_from_fs": False,
+                },
             )
-        except IntegrityError:
-            # A concurrent scan already created this ROM, so skip it here.
-            log.debug(
-                f"Skipping {hl(fs_rom['fs_name'])}: already created by a concurrent scan"
+            reassociated = True
+            newly_added = False
+            log.info(
+                f"Reassociated {hl(fs_rom['fs_name'])} with existing entry "
+                f"{hl(rom.name or rom.fs_name, color=BLUE)} by file hash"
             )
-            return
+        else:
+            try:
+                rom = db_rom_handler.add_rom(Rom(**rom_attrs))
+            except IntegrityError:
+                # A concurrent scan already created this ROM, so skip it here.
+                log.debug(
+                    f"Skipping {hl(fs_rom['fs_name'])}: already created by a concurrent scan"
+                )
+                return
 
-    # Build rom files object before scanning
-    should_update_files = _should_get_rom_files(
+    # Build rom files object before scanning. A reassociated ROM always rebuilds
+    # its files so the stale paths from the old filename are replaced.
+    should_update_files = reassociated or _should_get_rom_files(
         scan_type=scan_type,
         rom=rom,
         newly_added=newly_added,
         roms_ids=roms_ids,
     )
-    if should_update_files:
+    if should_update_files and not files_built:
         # Get hash calculation setting from config
-        calculate_hashes = not cm.get_config().SKIP_HASH_CALCULATION
         if calculate_hashes:
             log.debug(f"Calculating file hashes for {rom.fs_name}...")
 
@@ -612,6 +666,12 @@ async def _identify_platform(
     else:
         log.info(f"{hl(str(len(fs_roms)))} roms found in the file system")
 
+    # Flag entries whose file is gone before identifying files, so a renamed or
+    # 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])
+
     # Create semaphore to limit concurrent ROM scanning
     scan_semaphore = asyncio.Semaphore(SCAN_WORKERS)
 

backend/handler/database/roms_handler.py

@@ -2221,6 +2221,43 @@ def get_rom_by_hash(
         # Return the first ROM matching any of the provided hash values
         return session.scalar(query.outerjoin(Rom.files).filter(or_(*filters)).limit(1))
 
+    @begin_session
+    def get_matching_missing_rom(
+        self,
+        platform_id: int,
+        crc_hash: str | None = None,
+        md5_hash: str | None = None,
+        sha1_hash: str | None = None,
+        session: Session = None,  # type: ignore
+    ) -> Rom | None:
+        """Find a ROM marked missing on a platform whose hashes match the file.
+
+        Used during scanning to reassociate a renamed or moved file with its
+        existing entry (preserving collections, notes, and assets) instead of
+        creating a duplicate. Requires the CRC, MD5, and SHA1 hashes to all
+        match. Any missing hash yields no match, so non-hashable platforms and
+        pre-hash entries safely fall back to creating a new entry.
+        """
+        if not (crc_hash and md5_hash and sha1_hash):
+            return None
+
+        matches = session.scalars(
+            select(Rom)
+            .where(
+                and_(
+                    Rom.platform_id == platform_id,
+                    Rom.missing_from_fs.is_(True),
+                    Rom.crc_hash == crc_hash,
+                    Rom.md5_hash == md5_hash,
+                    Rom.sha1_hash == sha1_hash,
+                )
+            )
+            .limit(2)
+        ).all()
+
+        # Return None when more than one match to avoid ambiguity.
+        return matches[0] if len(matches) == 1 else None
+
     def _collect_filter_values(
         self,
         session: Session,

backend/tests/endpoints/roms/test_rom.py

@@ -333,6 +333,22 @@ def test_get_rom_content_single_file(
     assert "X-Accel-Redirect" in response.headers
 
 
+def test_get_rom_content_single_file_missing_on_disk_returns_404(
+    client: TestClient, access_token: str, rom: Rom, rom_file, mocker
+):
+    # In DEV_MODE the endpoint serves the file directly. If the file is gone
+    # from disk (e.g. a renamed/moved ROM whose old entry is now missing), it
+    # must return a clean 404 instead of raising a RuntimeError from
+    # FileResponse when starlette fails to stat the path.
+    mocker.patch("endpoints.roms.DEV_MODE", True)
+    response = client.get(
+        f"/api/roms/{rom.id}/content/test_rom.zip",
+        headers={"Authorization": f"Bearer {access_token}"},
+        follow_redirects=False,
+    )
+    assert response.status_code == status.HTTP_404_NOT_FOUND
+
+
 def test_get_rom_content_valid_file_id(
     client: TestClient, access_token: str, rom: Rom, rom_file
 ):