perf(sync): resolve RetroArch core aliases in one query (#5192)

* fix(sync): resolve RetroArch paths across emulators sharing a core folder Several RomM emulator ids map to one RetroArch folder (mednafen_psx_hw, beetle_psx_hw and the raw "Beetle PSX HW" all sync through saves/Beetle PSX HW/). The manifest built paths from whichever id an asset carried, but GET parsed the folder back to the canonical id and matched it exactly, so an asset filed under another alias was advertised and then 404'd. Uploads also started a second autosave history beside the alias's. The manifest now lists the newest asset per folder, and GET, PUT and DELETE resolve a path across every alias. Rewrites of an existing unslotted save or state stay under that row's own emulator. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * fix(sync): prefer on-disk alias saves and share newest-per-key grouping pr-ready step 2 (code-review --fix). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * refactor(sync): one alias helper and a lineage computed once pr-ready step 3 (simplify). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * test(sync): type the alias fixture's asset kind pr-ready step 4 (review-polish). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * test(sync): isolate the asset dir per RetroArch sync test The alias tests write different bytes to the same asset paths, and user and ROM ids repeat across xdist worker databases, so parallel runs raced on those files. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * perf(sync): resolve RetroArch core aliases in one query The saves handler now filters on a collection of lineages and looks up a file name across several folders, so the RetroArch alias resolvers issue one query each instead of one per alias. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * perf(sync): filter a RetroArch delete to autosave and unslotted saves in SQL Addresses Greptile's review on #5192: saves_at_path loaded every slot of the matching lineages and dropped the named slots in Python. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
2026-10-12 02:21:58 +00:00 · 2026-10-08 18:54:27 -04:00
parent 79d1feba6c
commit 57ebac8efe
3 changed files with 184 additions and 57 deletions
@@ -2,7 +2,7 @@ import functools
 from collections.abc import Callable, Collection, Mapping, Sequence
 from typing import Any, Literal

-from sqlalchemy import Select, asc, delete, desc, func, or_, select, update
+from sqlalchemy import Select, and_, asc, delete, desc, false, func, or_, select, update
 from sqlalchemy.engine import Row
 from sqlalchemy.orm import Session
 from sqlalchemy.sql.elements import ColumnElement
@@ -22,10 +22,28 @@ _VERSION_COLUMNS = (Save.user_id, Save.rom_id, Save.slot, Save.content_hash)
 _SLOT_MOVE_ATTEMPTS = 3


-def _in_lineage(lineage: SaveLineage) -> tuple[ColumnElement[bool], ...]:
-    return (
-        Save.emulator.is_not_distinct_from(lineage.emulator),
-        func.lower(Save.file_extension) == lineage.file_extension,
+def _emulator_in(emulators: Collection[str | None]) -> ColumnElement[bool]:
+    named = sorted({emulator for emulator in emulators if emulator is not None})
+    clauses = [Save.emulator.in_(named)] if named else []
+    if None in emulators:
+        clauses.append(Save.emulator.is_(None))
+    return or_(false(), *clauses)
+
+
+def _in_lineages(lineages: Collection[SaveLineage]) -> ColumnElement[bool]:
+    """Saves in any of ``lineages``."""
+    emulators_by_extension: dict[str, set[str | None]] = {}
+    for emulator, file_extension in lineages:
+        emulators_by_extension.setdefault(file_extension, set()).add(emulator)
+    return or_(
+        false(),
+        *(
+            and_(
+                func.lower(Save.file_extension) == file_extension,
+                _emulator_in(emulators),
+            )
+            for file_extension, emulators in emulators_by_extension.items()
+        ),
    )


@@ -111,13 +129,23 @@ class DBSavesHandler(DBBaseHandler):
        session: Session = INJECTED_SESSION,
    ) -> Save | None:
        return session.scalars(
-            select(Save)
-            .filter_by(
-                rom_id=rom_id, user_id=user_id, file_path=file_path, file_name=file_name
-            )
-            .limit(1)
+            _at_paths_query(user_id, rom_id, [file_path], file_name).limit(1)
        ).first()

+    @begin_session
+    def get_saves_at_paths(
+        self,
+        user_id: int,
+        rom_id: int,
+        file_paths: Collection[str],
+        file_name: str,
+        session: Session = INJECTED_SESSION,
+    ) -> Sequence[Save]:
+        """Every save named ``file_name`` in any of ``file_paths``."""
+        return session.scalars(
+            _at_paths_query(user_id, rom_id, file_paths, file_name)
+        ).all()
+
    @begin_session
    def get_save_by_content_hash(
        self,
@@ -140,14 +168,14 @@ class DBSavesHandler(DBBaseHandler):
        user_id: int,
        rom_id: int,
        slot: str,
-        lineage: SaveLineage,
+        lineages: Collection[SaveLineage],
        session: Session = INJECTED_SESSION,
    ) -> Save | None:
-        """The newest version of ``lineage`` in ``slot`` whose file is on disk."""
+        """The newest version in ``slot`` across ``lineages`` whose file is on disk."""
        return session.scalar(
            select(Save)
            .filter_by(user_id=user_id, rom_id=rom_id, slot=slot, missing_from_fs=False)
-            .where(*_in_lineage(lineage))
+            .where(_in_lineages(lineages))
            .order_by(desc(Save.updated_at), desc(Save.id))
            .limit(1)
        )
@@ -191,8 +219,9 @@ class DBSavesHandler(DBBaseHandler):
        slot: str | None = None,
        slot_not_null: bool = False,
        slot_is_null: bool = False,
+        with_unslotted: bool = False,
        file_name_prefix: str | None = None,
-        lineage: SaveLineage | None = None,
+        lineages: Collection[SaveLineage] | None = None,
        order_by: Literal["updated_at", "created_at"] | None = None,
        order_dir: Literal["asc", "desc"] = "desc",
    ) -> Select[Save]:
@@ -208,7 +237,12 @@ class DBSavesHandler(DBBaseHandler):
            )

        if slot is not None:
-            query = query.filter(Save.slot == slot)
+            # `with_unslotted` widens `slot` to also keep saves outside any slot.
+            query = query.filter(
+                or_(Save.slot == slot, Save.slot.is_(None))
+                if with_unslotted
+                else Save.slot == slot
+            )

        if slot_not_null:
            query = query.filter(Save.slot.is_not(None))
@@ -221,8 +255,8 @@ class DBSavesHandler(DBBaseHandler):
                Save.file_name.startswith(file_name_prefix, autoescape=True)
            )

-        if lineage is not None:
-            query = query.where(*_in_lineage(lineage))
+        if lineages is not None:
+            query = query.where(_in_lineages(lineages))

        if order_by:
            order_col = getattr(Save, order_by)
@@ -241,8 +275,9 @@ class DBSavesHandler(DBBaseHandler):
        slot: str | None = None,
        slot_not_null: bool = False,
        slot_is_null: bool = False,
+        with_unslotted: bool = False,
        file_name_prefix: str | None = None,
-        lineage: SaveLineage | None = None,
+        lineages: Collection[SaveLineage] | None = None,
        order_by: Literal["updated_at", "created_at"] | None = None,
        order_dir: Literal["asc", "desc"] = "desc",
        session: Session = INJECTED_SESSION,
@@ -254,8 +289,9 @@ class DBSavesHandler(DBBaseHandler):
            slot=slot,
            slot_not_null=slot_not_null,
            slot_is_null=slot_is_null,
+            with_unslotted=with_unslotted,
            file_name_prefix=file_name_prefix,
-            lineage=lineage,
+            lineages=lineages,
            order_by=order_by,
            order_dir=order_dir,
        )
@@ -473,7 +509,7 @@ class DBSavesHandler(DBBaseHandler):
            .offset(keep)
        )
        if lineage is not None:
-            past_keep = past_keep.where(*_in_lineage(lineage))
+            past_keep = past_keep.where(_in_lineages([lineage]))
        # Before this session holds a connection, since ensuring takes its own.
        if not self._any(past_keep):
            return []
@@ -599,6 +635,17 @@ class DBSavesHandler(DBBaseHandler):
        ).all()


+def _at_paths_query(
+    user_id: int, rom_id: int, file_paths: Collection[str], file_name: str
+) -> Select[Save]:
+    return select(Save).where(
+        Save.user_id == user_id,
+        Save.rom_id == rom_id,
+        Save.file_path.in_(file_paths),
+        Save.file_name == file_name,
+    )
+
+
 def _version_query(id: int) -> Select[*tuple[Any, ...]]:
    return select(*_VERSION_COLUMNS).where(Save.id == id)

@@ -253,25 +253,20 @@ def advertised_save_name(save: Save) -> str:
    return f"{save.rom.fs_name_no_ext}{os.path.splitext(name)[1]}"


-def _distinct(saves: Iterable[Save]) -> list[Save]:
-    """``saves`` without repeats, which a case-insensitive collation returns per alias."""
-    return list({save.id: save for save in saves}.values())
-
-
 def _unslotted_saves_at(
    user: User, rom: Rom, emulator: str | None, file_name: str
 ) -> list[Save]:
    """The unslotted saves at a RetroArch path, under any of ``emulator``'s aliases."""
-    saves = [
-        db_save_handler.get_save_by_path(
-            user_id=user.id,
-            rom_id=rom.id,
-            file_path=build_asset_file_path(user, rom, "saves", alias),
-            file_name=file_name,
-        )
-        for alias in retroarch_aliases(emulator)
-    ]
-    return _distinct([save for save in saves if save and save.slot is None])
+    saves = db_save_handler.get_saves_at_paths(
+        user_id=user.id,
+        rom_id=rom.id,
+        file_paths=[
+            build_asset_file_path(user, rom, "saves", alias)
+            for alias in retroarch_aliases(emulator)
+        ],
+        file_name=file_name,
+    )
+    return [save for save in saves if save.slot is None]


 def _unslotted_save_at(
@@ -295,13 +290,12 @@ def _alias_lineages(lineage: SaveLineage) -> list[SaveLineage]:

 def _lineage_head(user: User, rom: Rom, lineage: SaveLineage) -> Save | None:
    """The newest `autosave` version across ``lineage``'s aliases."""
-    heads = [
-        db_save_handler.get_lineage_head(
-            user_id=user.id, rom_id=rom.id, slot=AUTOSAVE_SLOT, lineage=alias
-        )
-        for alias in _alias_lineages(lineage)
-    ]
-    return max((head for head in heads if head), key=recency_key, default=None)
+    return db_save_handler.get_lineage_head(
+        user_id=user.id,
+        rom_id=rom.id,
+        slot=AUTOSAVE_SLOT,
+        lineages=_alias_lineages(lineage),
+    )


 def resolve_save(
@@ -321,22 +315,20 @@ def saves_at_path(
    if not lineage:
        return _unslotted_saves_at(user, rom, emulator, requested_file_name)

-    saves: list[Save] = []
    folded = requested_file_name.casefold()
-    for alias in _alias_lineages(lineage):
-        saves += db_save_handler.get_saves(
-            user_id=user.id, rom_ids=[rom.id], slot=AUTOSAVE_SLOT, lineage=alias
+    return [
+        save
+        for save in db_save_handler.get_saves(
+            user_id=user.id,
+            rom_ids=[rom.id],
+            slot=AUTOSAVE_SLOT,
+            with_unslotted=True,
+            lineages=_alias_lineages(lineage),
        )
-        # The manifest hides these behind the head, so whatever their case they
-        # would resurface once the versions go.
-        saves += [
-            save
-            for save in db_save_handler.get_saves(
-                user_id=user.id, rom_ids=[rom.id], slot_is_null=True, lineage=alias
-            )
-            if save.file_name.casefold() == folded
-        ]
-    return _distinct(saves)
+        # The manifest hides unslotted ones behind the head, so whatever their
+        # case they would resurface once the versions go.
+        if save.slot is not None or save.file_name.casefold() == folded
+    ]


 SaveUpload = Literal["created", "updated", "unchanged"]
@@ -422,12 +422,84 @@ class TestDBSavesHandlerSlotFiltering:
            user_id=admin_user.id,
            rom_id=rom.id,
            slot="autosave",
-            lineage=SaveLineage("snes9x", "srm"),
+            lineages=[SaveLineage("snes9x", "srm")],
        )

        assert sorted(save.file_name for save in heads) == ["new.SRM", "other.srm"]
        assert head is not None and head.file_name == "new.SRM"

+    def test_a_lineage_head_is_the_newest_across_lineages(
+        self, admin_user: User, rom: Rom
+    ):
+        self._lineage_versions(admin_user, rom)
+        make_save(
+            rom,
+            admin_user,
+            "unfiled.srm",
+            emulator=None,
+            slot="autosave",
+            updated_at=datetime(2030, 1, 1, tzinfo=UTC),
+        )
+
+        def head(*lineages: SaveLineage) -> str | None:
+            save = db_save_handler.get_lineage_head(
+                user_id=admin_user.id,
+                rom_id=rom.id,
+                slot="autosave",
+                lineages=lineages,
+            )
+            return save.file_name if save else None
+
+        assert head(SaveLineage("snes9x", "srm"), SaveLineage(None, "srm")) == (
+            "unfiled.srm"
+        )
+        assert (
+            head(SaveLineage("snes9x", "srm"), SaveLineage("bsnes", "srm"))
+            == "other.srm"
+        )
+        assert head(SaveLineage(None, "rtc")) is None
+        assert head() is None
+
+    def test_get_saves_filters_on_a_set_of_lineages(self, admin_user: User, rom: Rom):
+        self._lineage_versions(admin_user, rom)
+        make_save(rom, admin_user, "unfiled.srm", emulator=None, slot="autosave")
+
+        def names(*lineages: SaveLineage) -> set[str]:
+            saves = db_save_handler.get_saves(
+                user_id=admin_user.id, rom_ids=[rom.id], lineages=lineages
+            )
+            return {save.file_name for save in saves}
+
+        assert names(SaveLineage("bsnes", "srm"), SaveLineage(None, "srm")) == {
+            "other.srm",
+            "unfiled.srm",
+        }
+        assert names(SaveLineage("snes9x", "rtc"), SaveLineage("bsnes", "srm")) == {
+            "clock.rtc",
+            "other.srm",
+        }
+        assert names(SaveLineage(None, "srm")) == {"unfiled.srm"}
+        assert names() == set()
+
+    def test_get_saves_at_paths_matches_any_listed_folder(
+        self, admin_user: User, rom: Rom
+    ):
+        for file_path in ("saves/snes9x", "saves/bsnes", "saves/other"):
+            make_save(rom, admin_user, "game.srm", file_path=file_path)
+        make_save(rom, admin_user, "else.srm", file_path="saves/snes9x")
+
+        saves = db_save_handler.get_saves_at_paths(
+            user_id=admin_user.id,
+            rom_id=rom.id,
+            file_paths=["saves/snes9x", "saves/bsnes"],
+            file_name="game.srm",
+        )
+
+        assert sorted(save.file_path for save in saves) == [
+            "saves/bsnes",
+            "saves/snes9x",
+        ]
+
    def test_get_saves_with_null_slot_filter(self, admin_user: User, rom: Rom):
        make_save(
            rom,
@@ -449,6 +521,22 @@ class TestDBSavesHandlerSlotFiltering:
        all_saves = db_save_handler.get_saves(user_id=admin_user.id, rom_ids=[rom.id])
        assert len(all_saves) >= 2

+    def test_get_saves_with_unslotted_widens_the_slot_filter(
+        self, admin_user: User, rom: Rom
+    ):
+        make_save(rom, admin_user, "autosave.srm", slot="autosave")
+        make_save(rom, admin_user, "named.srm", slot="Main")
+        make_save(rom, admin_user, "unslotted.srm", slot=None)
+
+        saves = db_save_handler.get_saves(
+            user_id=admin_user.id,
+            rom_ids=[rom.id],
+            slot="autosave",
+            with_unslotted=True,
+        )
+
+        assert {save.file_name for save in saves} == {"autosave.srm", "unslotted.srm"}
+
    def test_get_saves_order_by(self, admin_user: User, rom: Rom):
        from datetime import datetime, timedelta, timezone