Merge pull request #4021 from rommapp/fix/fpkgi-unique-entry-names

Give each FPKGi package a unique name within its rom

Author: gantoine

backend/endpoints/feeds.py

@@ -1,6 +1,7 @@
 import csv
 import io
 import re
+from collections import Counter
 from collections.abc import Sequence
 from datetime import datetime
 from typing import Annotated
@@ -582,14 +583,40 @@ def format_release_date(timestamp: int | None) -> str | None:
 FPKGI_TITLE_ID_REGEX = re.compile(r"(?:CUSA|PPSA)\d{5}", re.IGNORECASE)
 
 
-def fpkgi_item_name(rom: Rom, file: RomFile, *, is_single_file: bool) -> str:
-    """Name shown in FPKGi, disambiguated when a rom holds several packages."""
+def fpkgi_name_candidates(rom_name: str, file: RomFile) -> list[str]:
+    """Names for a package, from the most readable to the most specific."""
+    label = FPKGI_CATEGORY_LABELS.get(file.category) if file.category else None
+    stem = file.file_name_no_ext
+
+    candidates = []
+    if label:
+        candidates.append(f"{rom_name} - {label}")
+    candidates.append(f"{rom_name} - {stem}")
+    if label:
+        candidates.append(f"{rom_name} - {label} - {stem}")
+    # Last resort: two categories can hold packages with the same file name
+    candidates.append(f"{rom_name} - {stem} ({file.id})")
+    return candidates
+
+
+def fpkgi_item_names(rom: Rom, files: list[RomFile]) -> dict[int, str]:
+    """Name shown in FPKGi per package, keyed by rom file id.
+
+    FPKGi downloads to `[<title_id>] <name>.pkg`, and packages of a rom share a
+    title id, so two of them sharing a name overwrite each other on the console.
+    Each package therefore takes the first of its candidate names that no other
+    package in the rom lays claim to.
+    """
     rom_name = rom.name or rom.fs_name
-    if is_single_file:
-        return rom_name
+    if len(files) == 1:
+        return {files[0].id: rom_name}
 
-    label = FPKGI_CATEGORY_LABELS.get(file.category) if file.category else None
-    return f"{rom_name} - {label or file.file_name_no_ext}"
+    candidates = {f.id: fpkgi_name_candidates(rom_name, f) for f in files}
+    claimed = Counter(name for names in candidates.values() for name in names)
+    return {
+        file_id: next(name for name in names if claimed[name] == 1)
+        for file_id, names in candidates.items()
+    }
 
 
 def fpkgi_title_id(rom: Rom, files: list[RomFile]) -> str:
@@ -656,6 +683,9 @@ def fpkgi_feed(
             if f.file_extension.lower() == "pkg" and not f.missing_from_fs
         ]
         title_id = fpkgi_title_id(rom, pkg_files)
+        # Named over every package of the rom, not just the filtered ones, so a
+        # name means the same thing whichever CONTENT_URLS slot serves it
+        item_names = fpkgi_item_names(rom, pkg_files)
         cover_url = (
             str(URLPath(rom.path_cover_large).make_absolute_url(request.base_url))
             if rom.path_cover_large
@@ -670,7 +700,7 @@ def fpkgi_feed(
 
             download_url = generate_romfile_download_url(request, file)
             response_data[download_url] = FPKGiFeedItemSchema(
-                name=fpkgi_item_name(rom, file, is_single_file=len(pkg_files) == 1),
+                name=item_names[file.id],
                 size=file.file_size_bytes,
                 title_id=title_id,
                 region=rom.regions[0] if rom.regions else None,

backend/tests/endpoints/feeds.py

@@ -327,6 +327,79 @@ def test_fpkgi_feed_multi_file_rom(
     assert response.status_code == status.HTTP_400_BAD_REQUEST
 
 
+def test_fpkgi_feed_names_are_unique_within_a_rom(
+    client: TestClient, access_token: str, platform: Platform, rom: Rom
+):
+    platform = db_platform_handler.update_platform(
+        platform.id, {"name": "PlayStation 4", "slug": UPS.PS4, "fs_slug": UPS.PS4}
+    )
+    rom = db_rom_handler.update_rom(
+        rom.id,
+        {
+            "platform_id": platform.id,
+            "name": "Test PS4",
+            "fs_name": "Test PS4",
+            "fs_name_no_tags": "Test PS4",
+            "fs_name_no_ext": "Test PS4",
+            "fs_extension": "",
+            "fs_path": f"{platform.slug}/roms",
+            "fs_size_bytes": 369,
+            "regions": ["US"],
+        },
+    )
+    for sub_path, file_name, category in (
+        ("", "Test PS4 base.pkg", None),
+        ("update", "Test PS4 patch.pkg", RomFileCategory.UPDATE),
+        ("dlc", "Test PS4 brawler.pkg", RomFileCategory.DLC),
+        ("dlc", "Test PS4 loadout.pkg", RomFileCategory.DLC),
+        # Same file name in two categories, so the file name alone is ambiguous
+        ("dlc", "Test PS4 extra.pkg", RomFileCategory.DLC),
+        ("demo", "Test PS4 extra.pkg", RomFileCategory.DEMO),
+        ("demo", "Test PS4 trial.pkg", RomFileCategory.DEMO),
+    ):
+        db_rom_handler.add_rom_file(
+            RomFile(
+                rom_id=rom.id,
+                file_name=file_name,
+                file_path=f"{rom.fs_path}/{rom.fs_name}/{sub_path}".rstrip("/"),
+                file_size_bytes=123,
+                category=category,
+            )
+        )
+
+    response = client.get(
+        "/api/feeds/fpkgi/ps4",
+        headers={"Authorization": f"Bearer {access_token}"},
+    )
+    assert response.status_code == status.HTTP_200_OK
+
+    data = response.json()["DATA"]
+    assert len(data) == 7
+    assert sorted(entry["name"] for entry in data.values()) == [
+        "Test PS4 - DLC - Test PS4 extra",
+        "Test PS4 - Demo - Test PS4 extra",
+        "Test PS4 - Test PS4 base",
+        "Test PS4 - Test PS4 brawler",
+        "Test PS4 - Test PS4 loadout",
+        "Test PS4 - Test PS4 trial",
+        "Test PS4 - Update",
+    ]
+
+    # Filtering must not change the name a package is served under
+    response = client.get(
+        "/api/feeds/fpkgi/ps4?content_type=dlc",
+        headers={"Authorization": f"Bearer {access_token}"},
+    )
+    assert response.status_code == status.HTTP_200_OK
+
+    dlc_data = response.json()["DATA"]
+    assert sorted(entry["name"] for entry in dlc_data.values()) == [
+        "Test PS4 - DLC - Test PS4 extra",
+        "Test PS4 - Test PS4 brawler",
+        "Test PS4 - Test PS4 loadout",
+    ]
+
+
 def test_kekatsu_feed(
     client: TestClient, access_token: str, platform: Platform, rom: Rom
 ):