feat(adapters): SaveApiV47 device sync methods (#182) (#187)

danielcopper · web-flow · commit ef1340eb987d · 2026-03-23T15:14:08.000+01:00
* feat(adapters): add supports_device_sync() capability check to ApiRouter (#182) * feat(adapters): add register_device() to RommApiV47 (#182) * feat(adapters): extend list_saves with device_id and slot params on V47 (#182) * feat(adapters): extend upload_save with device_id, slot, overwrite on V47 (#182) * feat(adapters): add download_save_content() to RommApiV47 (#182) * feat(adapters): add confirm_download() to RommApiV47 (#182) POST /api/saves/{id}/downloaded with device_id for manual sync confirmation when download_save_content() is called with optimistic=false. Also adds the method signature to RommApiProtocol. * feat(adapters): add get_save_summary() to RommApiV47 (#182) * test(adapters): add router delegation and v46 fallback tests for device sync (#182) * chore: update FakeSaveApi stubs and fix import-linter config (#182) * fix: update flaky_list closure signature to match extended list_saves protocol (#182)
diff --git a/.importlinter b/.importlinter
@@ -15,6 +15,7 @@ source_modules =
 forbidden_modules =
     adapters.romm.http
     adapters.romm.api_base
+    adapters.romm.api_v46
     adapters.romm.api_v47
     adapters.romm.api_router
     adapters.steam_config
diff --git a/py_modules/adapters/romm/api_router.py b/py_modules/adapters/romm/api_router.py
@@ -47,5 +47,9 @@ def set_version(self, version: str) -> None:
         else:
             self._active = self._v46
 
+    def supports_device_sync(self) -> bool:
+        """Check if the active RomM version supports device sync features."""
+        return isinstance(self._active, RommApiV47)
+
     def __getattr__(self, name: str):
         return getattr(self._active, name)
diff --git a/py_modules/adapters/romm/api_v47.py b/py_modules/adapters/romm/api_v47.py
@@ -37,3 +37,96 @@ def list_roms_by_collection(self, collection_id: int, limit: int = 50, offset: i
     def list_roms_by_virtual_collection(self, virtual_id: str, limit: int = 50, offset: int = 0) -> dict:
         encoded_id = urllib.parse.quote(str(virtual_id), safe="")
         return self._client.request(f"/api/roms?virtual_collection_id={encoded_id}&limit={limit}&offset={offset}")
+
+    def list_saves(
+        self,
+        rom_id: int,
+        *,
+        device_id: str | None = None,
+        slot: str | None = None,
+    ) -> list[dict]:
+        """List saves with optional device sync info and slot filtering."""
+        query = f"/api/saves?rom_id={rom_id}"
+        if device_id is not None:
+            query += f"&device_id={device_id}"
+        if slot is not None:
+            query += f"&slot={slot}"
+        result = self._client.request(query)
+        return result if isinstance(result, list) else []
+
+    def upload_save(
+        self,
+        rom_id: int,
+        file_path: str,
+        emulator: str,
+        save_id: int | None = None,
+        *,
+        device_id: str | None = None,
+        slot: str | None = None,
+        overwrite: bool = False,
+    ) -> dict:
+        """Upload a save with optional device tracking and slot assignment.
+
+        Raises RommConflictError on 409 (another device uploaded since last sync).
+        """
+        params = f"rom_id={rom_id}&emulator={urllib.parse.quote(emulator)}"
+        if device_id is not None:
+            params += f"&device_id={device_id}"
+        if slot is not None:
+            params += f"&slot={slot}"
+        if overwrite:
+            params += "&overwrite=true"
+        if save_id is not None:
+            return self._client.upload_multipart(f"/api/saves/{save_id}?{params}", file_path, method="PUT")
+        return self._client.upload_multipart(f"/api/saves?{params}", file_path, method="POST")
+
+    def download_save_content(
+        self,
+        save_id: int,
+        dest_path: str,
+        *,
+        device_id: str | None = None,
+        optimistic: bool = True,
+    ) -> None:
+        """Download save content with optional device sync tracking.
+
+        When device_id is provided, the server records the download.
+        optimistic=True (default) auto-marks device as synced.
+        optimistic=False requires a manual confirm_download() call after.
+        """
+        path = f"/api/saves/{save_id}/content"
+        if device_id is not None:
+            opt = "true" if optimistic else "false"
+            path += f"?device_id={device_id}&optimistic={opt}"
+        self._client.download(path, dest_path)
+
+    def confirm_download(self, save_id: int, device_id: str) -> dict:
+        """Confirm a save download for manual sync (when optimistic=false)."""
+        return self._client.post_json(
+            f"/api/saves/{save_id}/downloaded",
+            {"device_id": device_id},
+        )
+
+    def get_save_summary(self, rom_id: int, device_id: str | None = None) -> dict:
+        """Fetch grouped save summary for a ROM with slot breakdown.
+
+        Uses the dedicated /api/saves/summary endpoint which returns
+        a structured response grouped by slot, unlike the flat list
+        from list_saves.
+        """
+        query = f"/api/saves/summary?rom_id={rom_id}"
+        if device_id is not None:
+            query += f"&device_id={device_id}"
+        return self._client.request(query)
+
+    def register_device(self, name: str, platform: str, client: str, version: str) -> dict:
+        """Register this client as a device via POST /api/devices."""
+        return self._client.post_json(
+            "/api/devices",
+            {
+                "name": name,
+                "platform": platform,
+                "client": client,
+                "version": version,
+            },
+        )
diff --git a/py_modules/services/protocols.py b/py_modules/services/protocols.py
@@ -139,10 +139,17 @@ def download_firmware(self, firmware_id: int, filename: str, dest: str) -> None:
         """
         ...
 
-    def list_saves(self, rom_id: int) -> list[dict]:
-        """List all saves for a ROM.
-
-        Returns a list of save dicts from /api/saves?rom_id={rom_id}.
+    def list_saves(
+        self,
+        rom_id: int,
+        *,
+        device_id: str | None = None,
+        slot: str | None = None,
+    ) -> list[dict]:
+        """List saves for a ROM.
+
+        On v4.7+, pass device_id to populate device_syncs in response,
+        and slot to filter by save slot.
         """
         ...
 
@@ -152,11 +159,49 @@ def upload_save(
         file_path: str,
         emulator: str,
         save_id: int | None = None,
+        *,
+        device_id: str | None = None,
+        slot: str | None = None,
+        overwrite: bool = False,
     ) -> dict:
         """Upload or update a save file.
 
-        Creates via POST /api/saves or updates via PUT /api/saves/{save_id}.
-        Upserts by filename. Returns the save dict.
+        On v4.7+, pass device_id for sync tracking, slot for slot assignment,
+        and overwrite=True to force-upload over conflicts.
+        Raises RommConflictError on 409 when overwrite=False and conflict detected.
+        """
+        ...
+
+    def download_save_content(
+        self,
+        save_id: int,
+        dest_path: str,
+        *,
+        device_id: str | None = None,
+        optimistic: bool = True,
+    ) -> None:
+        """Download save content with optional device sync tracking.
+
+        Only available on RomM >= 4.7.0.
+        When device_id is set, optimistic=True auto-marks device as synced;
+        optimistic=False requires a manual confirm_download() call.
+        """
+        ...
+
+    def confirm_download(self, save_id: int, device_id: str) -> dict:
+        """Manually confirm a save download for device sync tracking.
+
+        Only needed when download_save_content() was called with optimistic=False.
+        Only available on RomM >= 4.7.0.
+        """
+        ...
+
+    def get_save_summary(self, rom_id: int, device_id: str | None = None) -> dict:
+        """Fetch grouped save summary for a ROM with slot breakdown.
+
+        Only available on RomM >= 4.7.0.
+        Uses /api/saves/summary — returns structured response grouped by slot.
+        Pass device_id to include device sync status per save.
         """
         ...
 
@@ -215,6 +260,22 @@ def list_roms_by_virtual_collection(self, virtual_id: str, limit: int = 50, offs
         """
         ...
 
+    def supports_device_sync(self) -> bool:
+        """Check if the connected RomM server supports device sync (v4.7+).
+
+        Returns True if device registration, slot-based saves, and
+        server-side conflict detection are available.
+        """
+        ...
+
+    def register_device(self, name: str, platform: str, client: str, version: str) -> dict:
+        """Register this client as a sync device on the RomM server.
+
+        Only available on RomM >= 4.7.0 (check supports_device_sync() first).
+        Returns device dict with id, name, created_at.
+        """
+        ...
+
 
 # ---------------------------------------------------------------------------
 # Infrastructure callback Protocols
diff --git a/tests/fakes/fake_save_api.py b/tests/fakes/fake_save_api.py
@@ -102,12 +102,40 @@ def list_roms_by_collection(self, collection_id: int, limit: int = 50, offset: i
     def list_roms_by_virtual_collection(self, virtual_id: str, limit: int = 50, offset: int = 0) -> dict:
         raise NotImplementedError
 
+    def supports_device_sync(self) -> bool:
+        return False
+
+    def register_device(self, name: str, platform: str, client: str, version: str) -> dict:
+        raise NotImplementedError
+
+    def download_save_content(
+        self,
+        save_id: int,
+        dest_path: str,
+        *,
+        device_id: str | None = None,
+        optimistic: bool = True,
+    ) -> None:
+        raise NotImplementedError
+
+    def confirm_download(self, save_id: int, device_id: str) -> dict:
+        raise NotImplementedError
+
+    def get_save_summary(self, rom_id: int, device_id: str | None = None) -> dict:
+        raise NotImplementedError
+
     # ------------------------------------------------------------------
     # Implemented save/note methods
     # ------------------------------------------------------------------
 
-    def list_saves(self, rom_id: int) -> list[dict]:
-        self.call_log.append(("list_saves", (rom_id,), {}))
+    def list_saves(
+        self,
+        rom_id: int,
+        *,
+        device_id: str | None = None,
+        slot: str | None = None,
+    ) -> list[dict]:
+        self.call_log.append(("list_saves", (rom_id,), {"device_id": device_id, "slot": slot}))
         self._check_fail()
         return [s for s in self.saves.values() if s.get("rom_id") == rom_id]
 
@@ -117,8 +145,23 @@ def upload_save(
         file_path: str,
         emulator: str,
         save_id: int | None = None,
+        *,
+        device_id: str | None = None,
+        slot: str | None = None,
+        overwrite: bool = False,
     ) -> dict:
-        self.call_log.append(("upload_save", (rom_id, file_path, emulator), {"save_id": save_id}))
+        self.call_log.append(
+            (
+                "upload_save",
+                (rom_id, file_path, emulator),
+                {
+                    "save_id": save_id,
+                    "device_id": device_id,
+                    "slot": slot,
+                    "overwrite": overwrite,
+                },
+            )
+        )
         self._check_fail()
 
         import os
diff --git a/tests/test_romm_api_router.py b/tests/test_romm_api_router.py
@@ -186,6 +186,46 @@ def test_update_note(self, router, mock_client):
         assert result == {"id": 1}
         mock_client.put_json.assert_called_with("/api/roms/42/notes/1", {"body": "updated"})
 
+    def test_register_device_v47(self, router, mock_client):
+        router.set_version("4.7.0")
+        mock_client.post_json.return_value = {"id": "abc"}
+        result = router.register_device("deck", "linux", "decky", "1.0")
+        assert result == {"id": "abc"}
+
+    def test_list_saves_with_device_id_v47(self, router, mock_client):
+        router.set_version("4.7.0")
+        mock_client.request.return_value = [{"id": 1}]
+        result = router.list_saves(42, device_id="abc")
+        assert result == [{"id": 1}]
+        assert "device_id=abc" in mock_client.request.call_args[0][0]
+
+    def test_upload_save_with_device_params_v47(self, router, mock_client):
+        router.set_version("4.7.0")
+        mock_client.upload_multipart.return_value = {"id": 1}
+        router.upload_save(42, "/tmp/s.srm", "retroarch-mgba", device_id="abc", slot="default")
+        path = mock_client.upload_multipart.call_args[0][0]
+        assert "device_id=abc" in path
+        assert "slot=default" in path
+
+    def test_download_save_content_v47(self, router, mock_client):
+        router.set_version("4.7.0")
+        router.download_save_content(99, "/tmp/s.srm", device_id="abc")
+        path = mock_client.download.call_args[0][0]
+        assert "/api/saves/99/content" in path
+        assert "device_id=abc" in path
+
+    def test_confirm_download_v47(self, router, mock_client):
+        router.set_version("4.7.0")
+        mock_client.post_json.return_value = {"status": "ok"}
+        result = router.confirm_download(99, "abc")
+        assert result == {"status": "ok"}
+
+    def test_get_save_summary_v47(self, router, mock_client):
+        router.set_version("4.7.0")
+        mock_client.request.return_value = {"slots": []}
+        result = router.get_save_summary(42, "abc")
+        assert result == {"slots": []}
+
 
 # -- __getattr__ safety net --
 
@@ -205,6 +245,48 @@ def test_unknown_attr_is_romm_api_error(self, router):
             _ = router.nonexistent
         assert exc_info.value.min_version == "unknown"
 
+    def test_register_device_raises_on_v46(self, router):
+        with pytest.raises(RommUnsupportedError):
+            router.register_device("deck", "linux", "decky", "1.0")
+
+    def test_download_save_content_raises_on_v46(self, router):
+        with pytest.raises(RommUnsupportedError):
+            router.download_save_content(99, "/tmp/s.srm")
+
+    def test_confirm_download_raises_on_v46(self, router):
+        with pytest.raises(RommUnsupportedError):
+            router.confirm_download(99, "abc")
+
+    def test_get_save_summary_raises_on_v46(self, router):
+        with pytest.raises(RommUnsupportedError):
+            router.get_save_summary(42)
+
+
+# -- supports_device_sync --
+
+
+class TestSupportsDeviceSync:
+    def test_false_by_default(self, router):
+        assert router.supports_device_sync() is False
+
+    def test_false_on_v46(self, router):
+        router.set_version("4.6.1")
+        assert router.supports_device_sync() is False
+
+    def test_true_on_v47(self, router):
+        router.set_version("4.7.0")
+        assert router.supports_device_sync() is True
+
+    def test_true_on_development(self, router):
+        router.set_version("development")
+        assert router.supports_device_sync() is True
+
+    def test_switches_back_to_false(self, router):
+        router.set_version("4.7.0")
+        assert router.supports_device_sync() is True
+        router.set_version("4.6.1")
+        assert router.supports_device_sync() is False
+
 
 # -- Delegation works on both versions --
 
diff --git a/tests/test_romm_api_v47.py b/tests/test_romm_api_v47.py
diff --git a/tests/test_save_service.py b/tests/test_save_service.py
