fix(metadata): identify archives by their largest member again

backend/handler/metadata/hasheous_handler.py

@@ -269,17 +269,12 @@ async def lookup_rom(
         # against any of them.
         data: list[dict] = []
         for file in filtered_files:
-            file_hashes: dict[str, str | None]
-            if file.chd_sha1_hash:
-                # CHD files are indexed by disc-data SHA1 only
-                # Raw file MD5/CRC are hashes of the container and won't match
-                file_hashes = {"shA1": file.chd_sha1_hash}
-            else:
-                file_hashes = {
-                    "mD5": file.md5_hash,
-                    "shA1": file.sha1_hash,
-                    "crc": file.crc_hash,
-                }
+            hashes = file.lookup_hashes
+            file_hashes: dict[str, str | None] = {
+                "mD5": hashes.md5,
+                "shA1": hashes.sha1,
+                "crc": hashes.crc,
+            }
 
             # Drop empty hashes and skip files that have none.
             file_hashes = {key: value for key, value in file_hashes.items() if value}

backend/handler/metadata/playmatch_handler.py

@@ -228,15 +228,17 @@ async def lookup_rom(self, files: list[RomFile]) -> PlaymatchRomMatch:
         if first_file is None:
             return fallback_rom
 
+        hashes = first_file.lookup_hashes
+
         try:
             response = await self._request(
                 self.identify_url,
                 {
                     "fileName": first_file.file_name,
                     "fileSize": first_file.file_size_bytes,
-                    "md5": first_file.md5_hash,
-                    "sha1": first_file.sha1_hash,
-                    "crc": first_file.crc_hash,
+                    "md5": hashes.md5,
+                    "sha1": hashes.sha1,
+                    "crc": hashes.crc,
                 },
             )
         except Exception as exc:
@@ -307,8 +309,8 @@ async def submit_manual_match_suggestion(self, rom: Rom) -> None:
                 None,
             )
             if first_file is not None:
-                md5 = first_file.md5_hash
-                sha1 = first_file.sha1_hash
+                md5 = first_file.lookup_hashes.md5
+                sha1 = first_file.lookup_hashes.sha1
                 file_name = first_file.file_name
                 file_size: int | None = first_file.file_size_bytes
             else:

backend/models/rom.py

@@ -5,7 +5,7 @@
 import re
 from datetime import datetime
 from functools import cached_property
-from typing import TYPE_CHECKING, Any, TypedDict
+from typing import TYPE_CHECKING, Any, NamedTuple, TypedDict
 
 from sqlalchemy import (
     TIMESTAMP,
@@ -103,6 +103,14 @@ class RomArchiveMember(TypedDict):
     sha1_hash: str
 
 
+class LookupHashes(NamedTuple):
+    """The hashes a ROM-database provider should be queried with."""
+
+    crc: str | None
+    md5: str | None
+    sha1: str | None
+
+
 class RomFile(BaseModel):
     __tablename__ = "rom_files"
 
@@ -164,6 +172,28 @@ def file_extension(self) -> str:
 
         return fs_rom_handler.parse_file_extension(self.file_name)
 
+    @cached_property
+    def lookup_hashes(self) -> LookupHashes:
+        """The hashes to identify this file by against a ROM database.
+
+        Neither is the file's own digest: a CHD is indexed by the disc data
+        embedded in its header, and a multi-file archive by its largest member
+        (the ROM itself, next to readmes and the like). The file's own hashes
+        cover the container, which no database holds.
+        """
+        if self.chd_sha1_hash:
+            return LookupHashes(crc=None, md5=None, sha1=self.chd_sha1_hash)
+
+        if self.archive_members:
+            largest = max(self.archive_members, key=lambda m: m.get("size") or 0)
+            return LookupHashes(
+                crc=largest.get("crc_hash"),
+                md5=largest.get("md5_hash"),
+                sha1=largest.get("sha1_hash"),
+            )
+
+        return LookupHashes(crc=self.crc_hash, md5=self.md5_hash, sha1=self.sha1_hash)
+
     @cached_property
     def is_nested(self) -> bool:
         return self.file_path.count("/") > 1

backend/tests/handler/metadata/test_playmatch_handler.py

@@ -4,6 +4,7 @@
 import httpx
 
 from handler.metadata.playmatch_handler import PlaymatchHandler
+from models.rom import RomFile
 from utils import get_version
 
 
@@ -62,3 +63,52 @@ async def test_heartbeat_returns_false_when_disabled():
     handler = PlaymatchHandler()
     with patch.object(handler, "is_enabled", return_value=False):
         assert await handler.heartbeat() is False
+
+
+async def test_lookup_rom_identifies_an_archive_by_its_largest_member():
+    """Playmatch indexes a multi-file archive by the ROM inside it, so the
+    archive's composite hash must not be what we ask about. The file name and
+    size stay those of the archive on disk."""
+    handler = PlaymatchHandler()
+    archive = RomFile(
+        file_name="set.zip",
+        file_path="arcade",
+        file_size_bytes=300,
+        crc_hash="compositecrc",
+        md5_hash="compositemd5",
+        sha1_hash="compositesha1",
+        archive_members=[
+            {
+                "name": "readme.txt",
+                "size": 10,
+                "crc_hash": "readmecrc",
+                "md5_hash": "readmemd5",
+                "sha1_hash": "readmesha1",
+            },
+            {
+                "name": "game.rom",
+                "size": 2048,
+                "crc_hash": "gamecrc",
+                "md5_hash": "gamemd5",
+                "sha1_hash": "gamesha1",
+            },
+        ],
+    )
+
+    with (
+        patch.object(handler, "is_enabled", return_value=True),
+        patch.object(handler, "_request", new_callable=AsyncMock) as mock_request,
+    ):
+        mock_request.return_value = {}
+        await handler.lookup_rom([archive])
+
+    mock_request.assert_awaited_once_with(
+        handler.identify_url,
+        {
+            "fileName": "set.zip",
+            "fileSize": 300,
+            "md5": "gamemd5",
+            "sha1": "gamesha1",
+            "crc": "gamecrc",
+        },
+    )

backend/tests/handler/test_fastapi.py

@@ -770,6 +770,47 @@ async def test_lookup_rom_sends_all_top_level_file_hashes(
     ]
 
 
+@patch.object(meta_hasheous_handler, "_request", new_callable=AsyncMock)
+@patch.object(meta_hasheous_handler, "is_enabled", return_value=True)
+async def test_lookup_rom_sends_the_largest_archive_member_hashes(
+    mock_is_enabled, mock_request
+):
+    """Hasheous indexes a multi-file archive by the ROM inside it, so the
+    archive's composite hash must not be what we ask about."""
+    mock_request.return_value = {}
+
+    files = [
+        _top_level_rom_file(
+            file_name="set.zip",
+            file_size_bytes=300,
+            crc_hash="compositecrc",
+            md5_hash="compositemd5",
+            sha1_hash="compositesha1",
+            archive_members=[
+                {
+                    "name": "readme.txt",
+                    "size": 10,
+                    "crc_hash": "readmecrc",
+                    "md5_hash": "readmemd5",
+                    "sha1_hash": "readmesha1",
+                },
+                {
+                    "name": "game.n64",
+                    "size": 2048,
+                    "crc_hash": "gamecrc",
+                    "md5_hash": "gamemd5",
+                    "sha1_hash": "gamesha1",
+                },
+            ],
+        ),
+    ]
+
+    await meta_hasheous_handler.lookup_rom("n64", files)
+
+    sent_data = mock_request.call_args.kwargs["data"]
+    assert sent_data == [{"mD5": "gamemd5", "shA1": "gamesha1", "crc": "gamecrc"}]
+
+
 @patch.object(meta_hasheous_handler, "_request", new_callable=AsyncMock)
 @patch.object(meta_hasheous_handler, "is_enabled", return_value=True)
 async def test_lookup_rom_skips_request_when_no_hashes(mock_is_enabled, mock_request):

backend/tests/models/test_rom.py

@@ -1,4 +1,4 @@
-from models.rom import Rom
+from models.rom import LookupHashes, Rom, RomFile
 
 
 def test_rom(rom: Rom):
@@ -11,3 +11,65 @@ def test_rom_with_libretro_match_is_identified(rom: Rom):
 
     assert rom.is_unidentified is False
     assert rom.is_identified is True
+
+
+def test_lookup_hashes_uses_the_files_own_digests_by_default():
+    file = RomFile(
+        file_name="game.nes",
+        file_path="nes",
+        file_size_bytes=100,
+        crc_hash="crc",
+        md5_hash="md5",
+        sha1_hash="sha1",
+    )
+
+    assert file.lookup_hashes == LookupHashes(crc="crc", md5="md5", sha1="sha1")
+
+
+def test_lookup_hashes_prefers_the_chd_disc_data_sha1():
+    """A CHD's own digests cover the container, so only the embedded SHA1 is
+    worth sending."""
+    file = RomFile(
+        file_name="game.chd",
+        file_path="dc",
+        file_size_bytes=100,
+        crc_hash="containercrc",
+        md5_hash="containermd5",
+        sha1_hash="containersha1",
+        chd_sha1_hash="discsha1",
+    )
+
+    assert file.lookup_hashes == LookupHashes(crc=None, md5=None, sha1="discsha1")
+
+
+def test_lookup_hashes_picks_the_largest_archive_member():
+    """ROM databases index a multi-file archive by the ROM inside it, not by
+    the composite hash RomM stores for the archive as a whole."""
+    file = RomFile(
+        file_name="sf2.zip",
+        file_path="arcade",
+        file_size_bytes=100,
+        crc_hash="compositecrc",
+        md5_hash="compositemd5",
+        sha1_hash="compositesha1",
+        archive_members=[
+            {
+                "name": "readme.txt",
+                "size": 10,
+                "crc_hash": "readmecrc",
+                "md5_hash": "readmemd5",
+                "sha1_hash": "readmesha1",
+            },
+            {
+                "name": "sf2.rom",
+                "size": 2048,
+                "crc_hash": "romcrc",
+                "md5_hash": "rommd5",
+                "sha1_hash": "romsha1",
+            },
+        ],
+    )
+
+    assert file.lookup_hashes == LookupHashes(
+        crc="romcrc", md5="rommd5", sha1="romsha1"
+    )