From 2c727c9c5ba4769995347bcb8486f2c8153bb485 Mon Sep 17 00:00:00 2001 From: Mustafa YAMAN Date: Fri, 24 Jul 2026 16:35:08 +0300 Subject: [PATCH 1/8] fix(cloud-sync): remap web-player states into RetroArch's numbered load slots States have no `slot` column (unlike saves), so a state created through RomM's own web player -- named with a display label and timestamp, not a RetroArch slot number -- was surfaced in the cloud-sync manifest under its raw file name. Verified live: RetroArch's Load State menu only ever offers numbered slots 0-999, so a synced file named otherwise downloads successfully but is never reachable from that menu. Mirrors the reasoning already used for the retroarch-webdav-romm shim project's manifest builder: states are grouped by (rom, emulator, slot suffix) via the file name's trailing .state/.state/.state.auto pattern, and the newest state in each bucket -- regardless of whether it came from RetroArch itself or the web player -- is advertised under the canonical name RetroArch's own upload for that slot would carry. GET/DELETE resolve that canonical name back to the same bucket via resolve_state_by_slot, so serving and deleting agree with what the manifest advertised even when the underlying row's real file name differs. --- backend/endpoints/cloud_sync.py | 20 ++- backend/handler/cloud_sync_handler.py | 130 ++++++++++++-- backend/tests/endpoints/test_cloud_sync.py | 190 ++++++++++++++++++++- 3 files changed, 314 insertions(+), 26 deletions(-) diff --git a/backend/endpoints/cloud_sync.py b/backend/endpoints/cloud_sync.py index 78915361a2..f916e39759 100644 --- a/backend/endpoints/cloud_sync.py +++ b/backend/endpoints/cloud_sync.py @@ -75,11 +75,10 @@ def _resolve_rom(request: Request, kind: AssetKind, file_name: str) -> Rom | Non def _get_asset( user: User, rom: Rom, parsed: CloudSyncPath, file_name: str ) -> Save | State | None: - file_path = cloud_sync_handler.build_asset_file_path( - user, rom, parsed.kind, parsed.emulator - ) - if parsed.kind == "saves": + file_path = cloud_sync_handler.build_asset_file_path( + user, rom, parsed.kind, parsed.emulator + ) return db_save_handler.get_save_by_path( user_id=user.id, rom_id=rom.id, @@ -87,11 +86,14 @@ def _get_asset( file_name=file_name, ) - return db_state_handler.get_state_by_path( - user_id=user.id, - rom_id=rom.id, - file_path=file_path, - file_name=file_name, + # States have no `slot` column to key an exact-path lookup on, and the + # requested `file_name` is the canonical name `build_manifest` made up + # (`cloud_sync_handler.canonical_state_file_name`) -- it may not match + # any single row's actual `file_name` (e.g. a web-player-created state). + # Re-derive the same (rom, emulator, slot) bucket instead of trusting an + # exact match. + return cloud_sync_handler.resolve_state_by_slot( + user, rom, parsed.emulator, file_name ) diff --git a/backend/handler/cloud_sync_handler.py b/backend/handler/cloud_sync_handler.py index 183153c227..56e8abdd4b 100644 --- a/backend/handler/cloud_sync_handler.py +++ b/backend/handler/cloud_sync_handler.py @@ -13,7 +13,7 @@ import os import re -from collections.abc import Callable +from collections.abc import Callable, Iterable from dataclasses import dataclass from typing import Literal @@ -94,6 +94,93 @@ def game_name_from_file_name(kind: AssetKind, file_name: str) -> str: return os.path.splitext(file_name)[0] +def state_slot_suffix(file_name: str) -> str: + """The RetroArch slot suffix (``state``, ``state1``, ..., ``state.auto``) + a state's file name ends in -- RomM has no ``slot`` column for states + (unlike saves), so this is the only way to group a rom's states into the + numbered load slots (0-999) RetroArch's own Load State menu offers. + + A state uploaded through RomM's web player instead carries a display + label and timestamp (e.g. `` [2026-07-24 12-04-52-733].state``); + that still ends in a bare ``.state``, so it lands in slot 0 alongside + (and competing on recency with) any RetroArch-native slot-0 state -- + mirroring the shim's `assetHistoryKey`/`splitAssetFileName`, which + resolved the identical ambiguity for the same reason: without this, a + web-uploaded state either has no reachable slot at all, or (naively + keyed by its raw file name) its own permanent one-off slot that grows + without bound. + """ + match = STATE_SUFFIX_PATTERN.search(file_name) + if match: + return match.group(0)[1:].lower() + return os.path.splitext(file_name)[1][1:].lower() + + +def latest_state_for_slot( + states: Iterable[State], rom_id: int, emulator: str | None, slot_suffix: str +) -> State | None: + """The state RetroArch would see for a given (rom, emulator, slot) -- + whichever matching row was updated most recently, regardless of whether + it came from a real RetroArch upload or RomM's own web player. Ties + (e.g. a bulk migration timestamp shared by several rows) break on `id`, + the same deterministic tiebreaker the shim's `sortByRecency` uses, so a + manifest build and a later GET/DELETE for the same slot always agree on + which row "the newest" actually is. + """ + candidates = [ + state + for state in states + if state.rom_id == rom_id + and state.emulator == emulator + and state_slot_suffix(state.file_name) == slot_suffix + ] + if not candidates: + return None + + return max(candidates, key=lambda state: (state.updated_at, state.id)) + + +def group_states_by_slot( + states: Iterable[State], +) -> dict[tuple[int, str | None, str], State]: + """Every (rom, emulator, slot) bucket collapsed to its newest state -- + the same grouping `latest_state_for_slot` performs, computed once for + every state instead of once per slot so `build_manifest` doesn't rescan + the full state list for every rom it lists.""" + latest: dict[tuple[int, str | None, str], State] = {} + for state in states: + key = (state.rom_id, state.emulator, state_slot_suffix(state.file_name)) + current = latest.get(key) + if current is None or (state.updated_at, state.id) > ( + current.updated_at, + current.id, + ): + latest[key] = state + + return latest + + +def canonical_state_file_name(rom: Rom, slot_suffix: str) -> str: + """The file name RetroArch's own upload for this (rom, slot) would carry + -- what the manifest advertises, and what a GET/DELETE for this slot + must resolve back to the real underlying row via + ``resolve_state_by_slot``, regardless of that row's actual file name.""" + return f"{rom.fs_name_no_ext}.{slot_suffix}" + + +def resolve_state_by_slot( + user: User, rom: Rom, emulator: str | None, requested_file_name: str +) -> State | None: + """The state a GET/DELETE for `requested_file_name` resolves to -- the + same "newest row in this (rom, emulator, slot) bucket" `build_manifest` + already advertised, found by re-deriving the slot from the *requested* + canonical name rather than trusting any single row's own file name to + match it exactly (it usually won't, for a web-player-created state).""" + slot_suffix = state_slot_suffix(requested_file_name) + states = db_state_handler.get_states(user_id=user.id, rom_id=rom.id) + return latest_state_for_slot(states, rom.id, emulator, slot_suffix) + + def build_cloud_sync_path(kind: AssetKind, emulator: str | None, file_name: str) -> str: if emulator: return f"{kind}/{to_retroarch_dir_name(emulator)}/{file_name}" @@ -224,29 +311,42 @@ async def build_manifest( Slotted saves are RomM's own versioned history: every revision carries a datetime tag in its file name, so surfacing them would hand RetroArch a - growing pile of files no core would ever load. + growing pile of files no core would ever load. States have no such + `slot` column, so they're grouped into RetroArch's own numbered slots by + file-name suffix instead (`group_states_by_slot`) -- the newest state in + each (rom, emulator, slot) bucket is surfaced under the canonical name + RetroArch itself would use, regardless of who actually created it. """ - assets: list[tuple[AssetKind, Save | State]] = [ - ("saves", save) - for save in db_save_handler.get_saves(user_id=user.id) - if save.slot is None - ] - assets += [ - ("states", state) for state in db_state_handler.get_states(user_id=user.id) - ] - entries: list[dict[str, str]] = [] - for kind, asset in assets: - if asset.missing_from_fs or not can_see(asset.rom): + + for save in db_save_handler.get_saves(user_id=user.id): + if save.slot is not None or save.missing_from_fs or not can_see(save.rom): + continue + + digest = await asset_md5(save) + if not digest: + continue + + entries.append( + { + "path": build_cloud_sync_path("saves", save.emulator, save.file_name), + "hash": digest, + } + ) + + states_by_slot = group_states_by_slot(db_state_handler.get_states(user_id=user.id)) + for (_rom_id, emulator, slot_suffix), state in states_by_slot.items(): + if state.missing_from_fs or not can_see(state.rom): continue - digest = await asset_md5(asset) + digest = await asset_md5(state) if not digest: continue + file_name = canonical_state_file_name(state.rom, slot_suffix) entries.append( { - "path": build_cloud_sync_path(kind, asset.emulator, asset.file_name), + "path": build_cloud_sync_path("states", emulator, file_name), "hash": digest, } ) diff --git a/backend/tests/endpoints/test_cloud_sync.py b/backend/tests/endpoints/test_cloud_sync.py index 125715efd4..89c77d9630 100644 --- a/backend/tests/endpoints/test_cloud_sync.py +++ b/backend/tests/endpoints/test_cloud_sync.py @@ -43,6 +43,50 @@ def synced_save(admin_user: User, rom: Rom, saves_path: str): ) +@pytest.fixture +def states_path(admin_user: User, rom: Rom): + return fs_asset_handler.build_states_file_path( + user=admin_user, + platform_fs_slug="test_platform_slug", + rom_id=rom.id, + emulator="snes9x", + ) + + +@pytest.fixture +def synced_state(admin_user: User, rom: Rom, states_path: str): + """A state named the way RetroArch itself would name one -- `.state` + -- unlike the shared `state` fixture, whose file name is a test-only + placeholder unrelated to `rom.fs_name_no_ext`.""" + return db_state_handler.add_state( + State( + rom_id=rom.id, + user_id=admin_user.id, + file_name="test_rom.state", + file_path=states_path, + file_size_bytes=4, + emulator="snes9x", + ) + ) + + +@pytest.fixture +def web_state(admin_user: User, rom: Rom, states_path: str): + """A state named the way RomM's own web player names one: a display + label plus a timestamp, with no relation to RetroArch's `.state[N]` + numbered-slot convention -- see `is_retroarch_loadable_state`.""" + return db_state_handler.add_state( + State( + rom_id=rom.id, + user_id=admin_user.id, + file_name="test_rom [2026-07-24 12-04-52-733].state", + file_path=states_path, + file_size_bytes=4, + emulator="snes9x", + ) + ) + + class TestCloudSyncEmulatorNames: @pytest.mark.parametrize( ("retroarch_dir_name", "romm_emulator"), @@ -153,6 +197,73 @@ def test_put_without_credentials_challenges(self, client): assert response.status_code == status.HTTP_401_UNAUTHORIZED +class TestCloudSyncStateSlotResolution: + def test_resolves_canonical_name_to_the_web_created_row( + self, admin_user: User, rom: Rom, web_state: State + ): + """A GET/DELETE for the canonical slot name the manifest advertised + must resolve back to the real row even though its actual `file_name` + (a web-player timestamp label) never matches that canonical name.""" + resolved = cloud_sync_handler.resolve_state_by_slot( + admin_user, rom, "snes9x", "test_rom.state" + ) + + assert resolved is not None + assert resolved.id == web_state.id + + def test_resolves_to_the_newer_of_two_competing_states( + self, admin_user: User, rom: Rom, states_path: str + ): + older = db_state_handler.add_state( + State( + rom_id=rom.id, + user_id=admin_user.id, + file_name="test_rom.state", + file_path=states_path, + file_size_bytes=4, + emulator="snes9x", + ) + ) + newer = db_state_handler.add_state( + State( + rom_id=rom.id, + user_id=admin_user.id, + file_name="test_rom [2026-07-24 12-04-52-733].state", + file_path=states_path, + file_size_bytes=4, + emulator="snes9x", + ) + ) + assert newer.id > older.id + + resolved = cloud_sync_handler.resolve_state_by_slot( + admin_user, rom, "snes9x", "test_rom.state" + ) + + assert resolved is not None + assert resolved.id == newer.id + + def test_does_not_cross_slots(self, admin_user: User, rom: Rom, states_path: str): + """A slot-1 state must never resolve for a slot-0 request, even + though both belong to the same rom/emulator.""" + db_state_handler.add_state( + State( + rom_id=rom.id, + user_id=admin_user.id, + file_name="test_rom.state1", + file_path=states_path, + file_size_bytes=4, + emulator="snes9x", + ) + ) + + resolved = cloud_sync_handler.resolve_state_by_slot( + admin_user, rom, "snes9x", "test_rom.state" + ) + + assert resolved is None + + class TestCloudSyncManifest: @mock.patch( "handler.cloud_sync_handler.asset_md5", @@ -165,7 +276,7 @@ def test_lists_saves_and_states( client, admin_user: User, archival_save: Save, - state: State, + synced_state: State, ): response = client.get("/api/cloud-sync/manifest.server", auth=ADMIN_AUTH) @@ -176,11 +287,86 @@ def test_lists_saves_and_states( "hash": "d41d8cd98f00b204e9800998ecf8427e", }, { - "path": "states/test_emulator/test_state.state", + "path": "states/Snes9x/test_rom.state", "hash": "d41d8cd98f00b204e9800998ecf8427e", }, ] + @mock.patch( + "handler.cloud_sync_handler.asset_md5", + new_callable=mock.AsyncMock, + return_value="d41d8cd98f00b204e9800998ecf8427e", + ) + def test_remaps_web_player_state_to_canonical_slot( + self, _asset_md5: mock.AsyncMock, client, admin_user: User, web_state: State + ): + """A state uploaded through RomM's own web player carries a display + label and timestamp in its file name, not a RetroArch slot number -- + RetroArch's Load State menu only ever offers numbered slots 0-999 + (verified live: RetroArch fetched such a file during a real sync, + and it never appeared as a loadable slot because its raw file name + was surfaced as-is). It still belongs to slot 0 like any other + untagged state, so the manifest advertises it under RetroArch's own + canonical name for that slot instead of its raw file name.""" + response = client.get("/api/cloud-sync/manifest.server", auth=ADMIN_AUTH) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == [ + { + "path": "states/Snes9x/test_rom.state", + "hash": "d41d8cd98f00b204e9800998ecf8427e", + } + ] + + @mock.patch( + "handler.cloud_sync_handler.asset_md5", + new_callable=mock.AsyncMock, + return_value="d41d8cd98f00b204e9800998ecf8427e", + ) + def test_newest_state_in_a_slot_wins_regardless_of_origin( + self, + _asset_md5: mock.AsyncMock, + client, + admin_user: User, + rom: Rom, + states_path: str, + ): + """Two states competing for the same (rom, emulator, slot) bucket -- + an older RetroArch-native one and a newer web-player one -- resolve + to whichever is actually newest, same as the shim's `sortByRecency` + picking "the" state for a slot regardless of who created it.""" + older = db_state_handler.add_state( + State( + rom_id=rom.id, + user_id=admin_user.id, + file_name="test_rom.state", + file_path=states_path, + file_size_bytes=4, + emulator="snes9x", + ) + ) + newer = db_state_handler.add_state( + State( + rom_id=rom.id, + user_id=admin_user.id, + file_name="test_rom [2026-07-24 12-04-52-733].state", + file_path=states_path, + file_size_bytes=4, + emulator="snes9x", + ) + ) + assert newer.id > older.id + + response = client.get("/api/cloud-sync/manifest.server", auth=ADMIN_AUTH) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == [ + { + "path": "states/Snes9x/test_rom.state", + "hash": "d41d8cd98f00b204e9800998ecf8427e", + } + ] + @mock.patch( "handler.cloud_sync_handler.asset_md5", new_callable=mock.AsyncMock, From 4f03ded7122b1176334d3d4d87703569ba9b6da8 Mon Sep 17 00:00:00 2001 From: Mustafa YAMAN Date: Fri, 24 Jul 2026 16:51:13 +0300 Subject: [PATCH 2/8] fix(cloud-sync): sync a state's screenshot alongside the state itself RetroArch uploads a PNG screenshot alongside every state it syncs, named `.png` (e.g. `test_rom.state.png`). Verified live: this file name doesn't match RetroArch's own `.state[N]`/`.state.auto` slot pattern once naively split on the last dot ("test_rom.state" is not any ROM's name), so `_resolve_rom` failed and every screenshot upload 409'd. game_name_from_file_name now strips a trailing `.png` before applying the existing state-suffix stripping, so it resolves the owning ROM the same way it would for the state itself. Screenshots have no ROM-name-derived slot of their own -- they ride along with whichever state `resolve_state_by_slot` already picked for that (rom, emulator, slot), via the new `resolve_state_screenshot_by_slot`, and are stored as a Screenshot (RomM's own state-thumbnail model), not a State. build_manifest now also advertises a `.png` entry for any state that has one attached, so RetroArch's own upload of it succeeds and diffs correctly on subsequent syncs instead of failing (and retrying) forever. --- backend/endpoints/cloud_sync.py | 75 +++++++++++- backend/handler/cloud_sync_handler.py | 46 ++++++- backend/tests/endpoints/test_cloud_sync.py | 134 ++++++++++++++++++++- 3 files changed, 243 insertions(+), 12 deletions(-) diff --git a/backend/endpoints/cloud_sync.py b/backend/endpoints/cloud_sync.py index f916e39759..29d7c8c7ec 100644 --- a/backend/endpoints/cloud_sync.py +++ b/backend/endpoints/cloud_sync.py @@ -19,14 +19,14 @@ from handler.auth.constants import Scope from handler.auth.dependencies import get_permissions from handler.cloud_sync_handler import MANIFEST_FILE_NAME, AssetKind, CloudSyncPath -from handler.database import db_save_handler, db_state_handler +from handler.database import db_save_handler, db_screenshot_handler, db_state_handler from handler.filesystem import fs_asset_handler, fs_cloud_sync_blob_handler from handler.filesystem.assets_handler import build_asset_file_response -from handler.scan_handler import scan_save, scan_state +from handler.scan_handler import scan_save, scan_screenshot, scan_state from logger.formatter import BLUE from logger.formatter import highlight as hl from logger.logger import log -from models.assets import Save, State +from models.assets import Save, Screenshot, State from models.rom import Rom from models.user import User from utils.filesystem import sanitize_filename @@ -149,7 +149,16 @@ async def cloud_sync_get(request: Request, file_path: str) -> Response: if not rom: return _empty(status.HTTP_404_NOT_FOUND) - asset = _get_asset(request.user, rom, parsed, parsed.file_name) + asset: Save | State | Screenshot | None + if parsed.kind == "states" and cloud_sync_handler.is_state_screenshot_path( + parsed.file_name + ): + asset = cloud_sync_handler.resolve_state_screenshot_by_slot( + request.user, rom, parsed.emulator, parsed.file_name + ) + else: + asset = _get_asset(request.user, rom, parsed, parsed.file_name) + if not asset: return _empty(status.HTTP_404_NOT_FOUND) @@ -208,12 +217,48 @@ async def cloud_sync_put(request: Request, file_path: str) -> Response: log.warning(f"Cloud sync upload {hl(file_path)} matches no ROM in the library") return _empty(status.HTTP_409_CONFLICT) + log.info(f"Cloud sync upload {hl(file_name)} for {hl(str(rom.name), color=BLUE)}") + + # RetroArch syncs a state's screenshot as `.png` -- + # store it as a Screenshot attached to the ROM, not a State (there's no + # state binary here, just an image). + if parsed.kind == "states" and cloud_sync_handler.is_state_screenshot_path( + file_name + ): + screenshot_path = fs_asset_handler.build_screenshots_file_path( + user=request.user, + platform_fs_slug=rom.platform.fs_slug, + rom_id=rom.id, + ) + await fs_asset_handler.write_file( + file=await request.body(), path=screenshot_path, filename=file_name + ) + + scanned_screenshot = await scan_screenshot( + file_name=file_name, + user=request.user, + platform_fs_slug=rom.platform.fs_slug, + rom_id=rom.id, + ) + existing_screenshot = db_screenshot_handler.get_screenshot( + rom_id=rom.id, user_id=request.user.id, file_name=file_name + ) + if existing_screenshot: + db_screenshot_handler.update_screenshot( + existing_screenshot.id, + {"file_size_bytes": scanned_screenshot.file_size_bytes}, + ) + return _empty(status.HTTP_204_NO_CONTENT) + + scanned_screenshot.rom_id = rom.id + scanned_screenshot.user_id = request.user.id + db_screenshot_handler.add_screenshot(screenshot=scanned_screenshot) + return _empty(status.HTTP_201_CREATED) + asset_path = cloud_sync_handler.build_asset_file_path( request.user, rom, parsed.kind, parsed.emulator ) - log.info(f"Cloud sync upload {hl(file_name)} for {hl(str(rom.name), color=BLUE)}") - await fs_asset_handler.write_file( file=await request.body(), path=asset_path, filename=file_name ) @@ -301,6 +346,24 @@ async def cloud_sync_delete(request: Request, file_path: str) -> Response: if not rom: return _empty(status.HTTP_404_NOT_FOUND) + if parsed.kind == "states" and cloud_sync_handler.is_state_screenshot_path( + parsed.file_name + ): + screenshot = cloud_sync_handler.resolve_state_screenshot_by_slot( + request.user, rom, parsed.emulator, parsed.file_name + ) + if not screenshot: + return _empty(status.HTTP_404_NOT_FOUND) + + log.info(f"Cloud sync delete {hl(screenshot.file_name)} [{rom.platform_slug}]") + db_screenshot_handler.delete_screenshot(screenshot.id) + try: + await fs_asset_handler.remove_file(file_path=screenshot.full_path) + except FileNotFoundError: + pass + + return _empty(status.HTTP_204_NO_CONTENT) + asset = _get_asset(request.user, rom, parsed, parsed.file_name) if not asset: return _empty(status.HTTP_404_NOT_FOUND) diff --git a/backend/handler/cloud_sync_handler.py b/backend/handler/cloud_sync_handler.py index 56e8abdd4b..403bd455be 100644 --- a/backend/handler/cloud_sync_handler.py +++ b/backend/handler/cloud_sync_handler.py @@ -21,7 +21,7 @@ from handler.database import db_rom_handler, db_save_handler, db_state_handler from handler.filesystem import fs_asset_handler, fs_cloud_sync_blob_handler from handler.redis_handler import async_cache -from models.assets import Save, State +from models.assets import Save, Screenshot, State from models.rom import Rom from models.user import User @@ -84,12 +84,21 @@ def parse_cloud_sync_path(path: str) -> CloudSyncPath | None: ) +def is_state_screenshot_path(file_name: str) -> bool: + """Whether a `states/...` file is the PNG screenshot RetroArch captures + and syncs alongside a state (`.png`), rather than the + state itself.""" + return file_name.lower().endswith(".png") + + def game_name_from_file_name(kind: AssetKind, file_name: str) -> str: """The ROM file name (minus extension) an asset file belongs to.""" if kind == "states": - stripped = STATE_SUFFIX_PATTERN.sub("", file_name) - if stripped != file_name: + base = file_name[: -len(".png")] if is_state_screenshot_path(file_name) else file_name + stripped = STATE_SUFFIX_PATTERN.sub("", base) + if stripped != base: return stripped + return os.path.splitext(base)[0] return os.path.splitext(file_name)[0] @@ -181,6 +190,22 @@ def resolve_state_by_slot( return latest_state_for_slot(states, rom.id, emulator, slot_suffix) +def resolve_state_screenshot_by_slot( + user: User, rom: Rom, emulator: str | None, requested_file_name: str +) -> Screenshot | None: + """The screenshot a GET/DELETE for `.png` resolves to -- whatever + is attached to the same state ``resolve_state_by_slot`` would return for + that slot, since RetroArch always syncs a state's screenshot under + ``.png``.""" + if not is_state_screenshot_path(requested_file_name): + return None + + state = resolve_state_by_slot( + user, rom, emulator, requested_file_name[: -len(".png")] + ) + return state.screenshot if state else None + + def build_cloud_sync_path(kind: AssetKind, emulator: str | None, file_name: str) -> str: if emulator: return f"{kind}/{to_retroarch_dir_name(emulator)}/{file_name}" @@ -287,7 +312,7 @@ def resolve_rom(game_name: str, can_see: Callable[[Rom], bool]) -> Rom | None: return None -async def asset_md5(asset: Save | State) -> str | None: +async def asset_md5(asset: Save | State | Screenshot) -> str | None: cache_key = ( f"romm:cloud_sync:md5:{asset.full_path}" f":{asset.file_size_bytes}:{asset.updated_at.timestamp()}" @@ -351,6 +376,19 @@ async def build_manifest( } ) + screenshot = state.screenshot + if screenshot and not screenshot.missing_from_fs: + screenshot_digest = await asset_md5(screenshot) + if screenshot_digest: + entries.append( + { + "path": build_cloud_sync_path( + "states", emulator, f"{file_name}.png" + ), + "hash": screenshot_digest, + } + ) + entries += await build_blob_manifest_entries(user) entries.sort(key=lambda entry: entry["path"]) diff --git a/backend/tests/endpoints/test_cloud_sync.py b/backend/tests/endpoints/test_cloud_sync.py index 89c77d9630..8e825d641d 100644 --- a/backend/tests/endpoints/test_cloud_sync.py +++ b/backend/tests/endpoints/test_cloud_sync.py @@ -5,9 +5,9 @@ from handler import cloud_sync_handler from handler.cloud_sync_emulator_names import to_retroarch_dir_name, to_romm_emulator -from handler.database import db_save_handler, db_state_handler +from handler.database import db_save_handler, db_screenshot_handler, db_state_handler from handler.filesystem import fs_asset_handler -from models.assets import Save, State +from models.assets import Save, Screenshot, State from models.rom import Rom from models.user import User @@ -70,6 +70,23 @@ def synced_state(admin_user: User, rom: Rom, states_path: str): ) +@pytest.fixture +def synced_state_screenshot(admin_user: User, rom: Rom, synced_state: State): + """The screenshot RetroArch captures and syncs alongside a state, under + `.png` -- attached to the ROM, not the state row + itself (there's no `screenshot_id` column on `State`; `state.screenshot` + finds it by matching file name stems).""" + return db_screenshot_handler.add_screenshot( + Screenshot( + rom_id=rom.id, + user_id=admin_user.id, + file_name=f"{synced_state.file_name}.png", + file_path=synced_state.file_path, + file_size_bytes=8, + ) + ) + + @pytest.fixture def web_state(admin_user: User, rom: Rom, states_path: str): """A state named the way RomM's own web player names one: a display @@ -408,6 +425,119 @@ def test_round_trips_emulator_casing_through_the_manifest( ] +class TestCloudSyncStateScreenshots: + def test_game_name_strips_png_before_state_suffix(self): + """RetroArch syncs a state's screenshot as `.png` + (e.g. `test_rom.state.png`) -- the ROM name must resolve the same + way it would for the state itself, not stop at the `.state` segment + (verified live: RetroArch's upload of this file 409'd because the + naive last-dot split reported the game name as `test_rom.state`).""" + assert ( + cloud_sync_handler.game_name_from_file_name( + "states", "test_rom.state.png" + ) + == "test_rom" + ) + assert ( + cloud_sync_handler.game_name_from_file_name( + "states", "test_rom.state3.png" + ) + == "test_rom" + ) + assert ( + cloud_sync_handler.game_name_from_file_name( + "states", "test_rom.state.auto.png" + ) + == "test_rom" + ) + + @mock.patch( + "handler.cloud_sync_handler.asset_md5", + new_callable=mock.AsyncMock, + return_value="d41d8cd98f00b204e9800998ecf8427e", + ) + def test_manifest_includes_the_state_screenshot( + self, + _asset_md5: mock.AsyncMock, + client, + admin_user: User, + synced_state: State, + synced_state_screenshot: Screenshot, + ): + response = client.get("/api/cloud-sync/manifest.server", auth=ADMIN_AUTH) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == [ + { + "path": "states/Snes9x/test_rom.state", + "hash": "d41d8cd98f00b204e9800998ecf8427e", + }, + { + "path": "states/Snes9x/test_rom.state.png", + "hash": "d41d8cd98f00b204e9800998ecf8427e", + }, + ] + + @mock.patch( + "endpoints.cloud_sync.fs_asset_handler.write_file", new_callable=mock.AsyncMock + ) + @mock.patch("endpoints.cloud_sync.scan_screenshot", new_callable=mock.AsyncMock) + def test_creates_screenshot_for_a_new_state( + self, + mock_scan_screenshot: mock.AsyncMock, + _mock_write_file: mock.AsyncMock, + client, + admin_user: User, + rom: Rom, + synced_state: State, + ): + mock_scan_screenshot.return_value = Screenshot( + file_name="test_rom.state.png", + file_path=synced_state.file_path, + file_size_bytes=8, + ) + + response = client.put( + "/api/cloud-sync/states/Snes9x/test_rom.state.png", + content=b"pngdata", + auth=ADMIN_AUTH, + ) + + assert response.status_code == status.HTTP_201_CREATED + screenshots = db_screenshot_handler.get_screenshot( + rom_id=rom.id, user_id=admin_user.id, file_name="test_rom.state.png" + ) + assert screenshots is not None + + @mock.patch( + "endpoints.cloud_sync.fs_asset_handler.write_file", new_callable=mock.AsyncMock + ) + @mock.patch("endpoints.cloud_sync.scan_screenshot", new_callable=mock.AsyncMock) + def test_overwrites_existing_screenshot_for_a_state( + self, + mock_scan_screenshot: mock.AsyncMock, + _mock_write_file: mock.AsyncMock, + client, + admin_user: User, + rom: Rom, + synced_state: State, + synced_state_screenshot: Screenshot, + ): + mock_scan_screenshot.return_value = Screenshot( + file_name="test_rom.state.png", + file_path=synced_state.file_path, + file_size_bytes=16, + ) + + response = client.put( + "/api/cloud-sync/states/Snes9x/test_rom.state.png", + content=b"newpngdata", + auth=ADMIN_AUTH, + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + + class TestCloudSyncUpload: @mock.patch( "endpoints.cloud_sync.fs_asset_handler.write_file", new_callable=mock.AsyncMock From 80c9076170a238ff2bb0ecd6490bf1b12148debf Mon Sep 17 00:00:00 2001 From: Mustafa YAMAN Date: Fri, 24 Jul 2026 17:27:10 +0300 Subject: [PATCH 3/8] fix(cloud-sync): write state/screenshot uploads to their real existing path PUT wrote every state upload under the canonical slot name RetroArch requested and only patched file_size_bytes on the matched existing row -- but that row's own file_name (e.g. a web-player upload's timestamped name) was left untouched. The new bytes landed on disk under a different path than the DB row pointed at, so the row's real content and what's actually on disk silently diverged. Verified live: this surfaced as a state that could never stop being a 'Conflicting change' on every subsequent sync, no matter how many times it was re-uploaded -- each upload wrote to the canonical path again while the tracked row (and its real save data) still pointed at the old, now-stale file, so RomM's own view of "the current state" never actually caught up to what RetroArch kept sending. Now resolves the existing row first and writes to *its* real file name when one exists, only falling back to the canonical name for a genuinely new row. Same fix applied to the state-screenshot upload path, which had the identical bug (screenshot identity resolved by exact canonical-name match instead of the owning state's real name). --- backend/endpoints/cloud_sync.py | 40 +++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/backend/endpoints/cloud_sync.py b/backend/endpoints/cloud_sync.py index 29d7c8c7ec..57acaf18e6 100644 --- a/backend/endpoints/cloud_sync.py +++ b/backend/endpoints/cloud_sync.py @@ -225,23 +225,38 @@ async def cloud_sync_put(request: Request, file_path: str) -> Response: if parsed.kind == "states" and cloud_sync_handler.is_state_screenshot_path( file_name ): + # `file_name` is the canonical `.png` name -- but the state it + # belongs to (found the same way `resolve_state_by_slot` would) may + # have its own, different real file name (e.g. a web-player upload). + # Writing under the canonical name while an existing screenshot's + # row still points at that other name would create a second, + # untracked file on disk instead of updating the real one. + owning_state = cloud_sync_handler.resolve_state_by_slot( + request.user, rom, parsed.emulator, file_name[: -len(".png")] + ) + screenshot_file_name = ( + f"{owning_state.file_name}.png" if owning_state else file_name + ) + screenshot_path = fs_asset_handler.build_screenshots_file_path( user=request.user, platform_fs_slug=rom.platform.fs_slug, rom_id=rom.id, ) await fs_asset_handler.write_file( - file=await request.body(), path=screenshot_path, filename=file_name + file=await request.body(), + path=screenshot_path, + filename=screenshot_file_name, ) scanned_screenshot = await scan_screenshot( - file_name=file_name, + file_name=screenshot_file_name, user=request.user, platform_fs_slug=rom.platform.fs_slug, rom_id=rom.id, ) existing_screenshot = db_screenshot_handler.get_screenshot( - rom_id=rom.id, user_id=request.user.id, file_name=file_name + rom_id=rom.id, user_id=request.user.id, file_name=screenshot_file_name ) if existing_screenshot: db_screenshot_handler.update_screenshot( @@ -259,12 +274,23 @@ async def cloud_sync_put(request: Request, file_path: str) -> Response: request.user, rom, parsed.kind, parsed.emulator ) + # For states, `file_name` is the *canonical* slot name RetroArch always + # uses -- but `existing` (resolved by slot, not by exact path) may be a + # row whose own `file_name` is something else entirely (e.g. a + # web-player upload). Writing the new bytes under the canonical name + # while only patching that other row's `file_size_bytes` would leave the + # DB row pointing at stale, now-orphaned content on disk -- silently + # diverging RomM's own view of "the current state" from what's actually + # on disk, which resurfaces as a spurious sync conflict on every + # subsequent sync. Writing to the existing row's own real file name + # instead keeps disk and DB in agreement. + existing = _get_asset(request.user, rom, parsed, file_name) + write_file_name = existing.file_name if existing else file_name + await fs_asset_handler.write_file( - file=await request.body(), path=asset_path, filename=file_name + file=await request.body(), path=asset_path, filename=write_file_name ) - existing = _get_asset(request.user, rom, parsed, file_name) - if parsed.kind == "saves": scanned_save = await scan_save( file_name=file_name, @@ -288,7 +314,7 @@ async def cloud_sync_put(request: Request, file_path: str) -> Response: db_save_handler.add_save(save=scanned_save) else: scanned_state = await scan_state( - file_name=file_name, + file_name=write_file_name, user=request.user, platform_fs_slug=rom.platform.fs_slug, rom_id=rom.id, From f3f56880b1d91679abc0e88b36f7fabe9f73c3bc Mon Sep 17 00:00:00 2001 From: Mustafa YAMAN Date: Fri, 24 Jul 2026 17:51:11 +0300 Subject: [PATCH 4/8] feat(cloud-sync): bundle PPSSPP's PSP save-folder layout for RetroArch sync PPSSPP doesn't save a single file per game like every other core -- it mirrors a real PSP memory stick under saves//PSP/SAVEDATA// as several small files (PARAM.SFO, the actual save data, ICON0.PNG, ...) that only make sense as a set, plus saves//PSP/SYSTEM/CACHE/ holds pure engine caches with no save data at all. Neither fits the existing one-WebDAV-path-per-asset model. Ports the retroarch-webdav-romm shim's pspSave.ts approach: a save folder's files are bundled into a single zip stored as one RomM Save (slot=None), and unbundled again for GET/manifest purposes. The rom is resolved from the folder's PARAM.SFO TITLE (normalized-matched against roms on the psp platform) with PSP_SERIAL_MAP as an explicit override and fallback for titles that don't normalize-match automatically. Files that arrive before the folder resolves (RetroArch doesn't guarantee PARAM.SFO lands first) are buffered on disk until it does, and merged in once resolved. Manifest lists each bundle member as its own {path, hash} entry, since RetroArch diffs per-file, not per-bundle. Also fixes a latent zipfile bug this surfaced: zipfile_inflate64 (pulled in elsewhere for ROM archive reading) replaces zipfile._get_compressor with a signature CPython 3.13's own ZipFile.writestr() can't call -- RomM already has a shim for this (utils/zip_cache.py's _ensure_zipfile_writable), just needed calling here too. --- backend/config/__init__.py | 10 + backend/endpoints/cloud_sync.py | 38 +- backend/handler/cloud_sync_handler.py | 9 +- backend/handler/cloud_sync_psp.py | 469 +++++++++++++++++++ backend/tests/endpoints/test_cloud_sync.py | 125 ++++- backend/tests/handler/test_cloud_sync_psp.py | 94 ++++ 6 files changed, 742 insertions(+), 3 deletions(-) create mode 100644 backend/handler/cloud_sync_psp.py create mode 100644 backend/tests/handler/test_cloud_sync_psp.py diff --git a/backend/config/__init__.py b/backend/config/__init__.py index 0d2837996b..ebd0b34e12 100644 --- a/backend/config/__init__.py +++ b/backend/config/__init__.py @@ -1,3 +1,4 @@ +import json import os from typing import Final, overload @@ -40,6 +41,15 @@ def _get_env(var: str, fallback: str | None = None) -> str | None: # (config/, thumbnails/, system/) — unrelated to any ROM, so it lives outside # the asset tree but still under the same persistent volume. CLOUD_SYNC_BLOB_BASE_PATH: Final[str] = f"{ROMM_BASE_PATH}/cloud_sync_blobs" +# Holds a PSP save folder's files that arrived before the folder could be +# resolved to a rom (usually only until PARAM.SFO shows up) -- see +# handler/cloud_sync_psp.py. +CLOUD_SYNC_PSP_PENDING_PATH: Final[str] = f"{ROMM_BASE_PATH}/cloud_sync_psp_pending" +# Serial (PARAM.SFO DISC_ID minus the trailing slot digits, e.g. "ULUS10336") +# -> RomM rom title override, for PSP saves whose PARAM.SFO TITLE doesn't +# normalize-match any rom automatically. JSON object, e.g. +# {"ULUS10336": "Crisis Core - Final Fantasy VII"}. +PSP_SERIAL_MAP: Final[dict[str, str]] = json.loads(_get_env("PSP_SERIAL_MAP", "{}")) ZIP_CACHE_PATH: Final[str] = f"{ROMM_BASE_PATH}/cache/zips" FRONTEND_RESOURCES_PATH: Final[str] = "/assets/romm/resources" diff --git a/backend/endpoints/cloud_sync.py b/backend/endpoints/cloud_sync.py index 57acaf18e6..72c8d5c9d2 100644 --- a/backend/endpoints/cloud_sync.py +++ b/backend/endpoints/cloud_sync.py @@ -15,7 +15,7 @@ from fastapi import APIRouter, Request, Response, status from fastapi.responses import JSONResponse -from handler import cloud_sync_handler +from handler import cloud_sync_handler, cloud_sync_psp from handler.auth.constants import Scope from handler.auth.dependencies import get_permissions from handler.cloud_sync_handler import MANIFEST_FILE_NAME, AssetKind, CloudSyncPath @@ -141,6 +141,15 @@ async def cloud_sync_get(request: Request, file_path: str) -> Response: resolved_path, filename=os.path.basename(blob_path) ) + psp_path = cloud_sync_psp.resolve_psp_path(file_path) + if psp_path == "ignore": + return _empty(status.HTTP_404_NOT_FOUND) + if psp_path: + data = await cloud_sync_psp.get_psp_file(request.user, psp_path) + if data is None: + return _empty(status.HTTP_404_NOT_FOUND) + return Response(content=data, media_type="application/octet-stream") + parsed = cloud_sync_handler.parse_cloud_sync_path(file_path) if not parsed: return _empty(status.HTTP_404_NOT_FOUND) @@ -203,6 +212,23 @@ async def cloud_sync_put(request: Request, file_path: str) -> Response: status.HTTP_204_NO_CONTENT if existed else status.HTTP_201_CREATED ) + psp_path = cloud_sync_psp.resolve_psp_path(file_path) + if psp_path == "ignore": + # PSP engine cache file (shader cache etc.), not save data. + return _empty(status.HTTP_204_NO_CONTENT) + if psp_path: + permissions = get_permissions(request) + try: + await cloud_sync_psp.put_psp_file( + request.user, + psp_path, + await request.body(), + lambda rom: permissions.can_see_rom(rom.id, rom.platform_id), + ) + except cloud_sync_psp.PspFolderUnresolved: + return _empty(status.HTTP_409_CONFLICT) + return _empty(status.HTTP_201_CREATED) + parsed = cloud_sync_handler.parse_cloud_sync_path(file_path) if not parsed: return _empty(status.HTTP_409_CONFLICT) @@ -364,6 +390,16 @@ async def cloud_sync_delete(request: Request, file_path: str) -> Response: return _empty(status.HTTP_204_NO_CONTENT) + psp_path = cloud_sync_psp.resolve_psp_path(file_path) + if psp_path: + # Best-effort, same as every other delete here: RetroArch deletes a + # PSP save folder file-by-file, so the first of the folder's several + # DELETEs removes the whole bundle and the rest find nothing left to + # remove. + if psp_path != "ignore": + await cloud_sync_psp.delete_psp_folder(request.user, psp_path.save_folder) + return _empty(status.HTTP_204_NO_CONTENT) + parsed = cloud_sync_handler.parse_cloud_sync_path(file_path) if not parsed: return _empty(status.HTTP_404_NOT_FOUND) diff --git a/backend/handler/cloud_sync_handler.py b/backend/handler/cloud_sync_handler.py index 403bd455be..e4d6f84349 100644 --- a/backend/handler/cloud_sync_handler.py +++ b/backend/handler/cloud_sync_handler.py @@ -17,6 +17,7 @@ from dataclasses import dataclass from typing import Literal +from handler import cloud_sync_psp from handler.cloud_sync_emulator_names import to_retroarch_dir_name, to_romm_emulator from handler.database import db_rom_handler, db_save_handler, db_state_handler from handler.filesystem import fs_asset_handler, fs_cloud_sync_blob_handler @@ -345,7 +346,12 @@ async def build_manifest( entries: list[dict[str, str]] = [] for save in db_save_handler.get_saves(user_id=user.id): - if save.slot is not None or save.missing_from_fs or not can_see(save.rom): + if ( + save.slot is not None + or save.missing_from_fs + or not can_see(save.rom) + or cloud_sync_psp.is_psp_bundle_file_name(save.file_name) + ): continue digest = await asset_md5(save) @@ -390,6 +396,7 @@ async def build_manifest( ) entries += await build_blob_manifest_entries(user) + entries += await cloud_sync_psp.build_psp_manifest_entries(user, can_see) entries.sort(key=lambda entry: entry["path"]) return entries diff --git a/backend/handler/cloud_sync_psp.py b/backend/handler/cloud_sync_psp.py new file mode 100644 index 0000000000..85288b6cda --- /dev/null +++ b/backend/handler/cloud_sync_psp.py @@ -0,0 +1,469 @@ +"""RetroArch Cloud Sync support for PPSSPP's PSP save-folder layout. + +PPSSPP (the PSP core) doesn't save a single file per game like every other +core -- it mirrors a real PSP's memory stick layout under RetroArch's own +saves directory: ``saves//PSP/SAVEDATA//`` holds several small +files (PARAM.SFO, the actual save data, ICON0.PNG, PIC1.PNG, ...) that only +make sense as a set, plus ``saves//PSP/SYSTEM/CACHE/`` holds pure +engine caches (shader caches etc.) with no save data at all. + +This mirrors the retroarch-webdav-romm shim's ``pspSave.ts``: a save +folder's files are bundled into a single zip stored as one RomM ``Save``, +and unbundled again for GET/manifest purposes -- everywhere else in +cloud sync, "one WebDAV path = one RomM asset" holds, but it doesn't here. +""" + +from __future__ import annotations + +import asyncio +import re +import struct +import zipfile +from collections.abc import Callable +from dataclasses import dataclass +from io import BytesIO +from typing import Literal + +from config import CLOUD_SYNC_PSP_PENDING_PATH, PSP_SERIAL_MAP +from handler.cloud_sync_emulator_names import to_retroarch_dir_name +from utils.zip_cache import _ensure_zipfile_writable +from handler.database import db_platform_handler, db_rom_handler, db_save_handler +from handler.filesystem import fs_asset_handler +from handler.filesystem.base_handler import FSHandler +from logger.formatter import highlight as hl +from logger.logger import log +from models.assets import Save +from models.rom import Rom +from models.user import User + +_IGNORED_CATEGORY = "SYSTEM" +_SAVEDATA_CATEGORY = "SAVEDATA" + +fs_psp_pending_handler = FSHandler(base_path=CLOUD_SYNC_PSP_PENDING_PATH) + + +class PspFolderUnresolved(Exception): + """A PSP save folder's files arrived but couldn't (yet) be matched to a + rom -- buffered on disk until a later file (usually PARAM.SFO) resolves + it, or forever if it never does (add it to PSP_SERIAL_MAP).""" + + +@dataclass(frozen=True) +class PspFilePath: + """A parsed ``saves//PSP/SAVEDATA//`` + cloud-sync path.""" + + emulator: str + save_folder: str + file_name: str + + +def resolve_psp_path(file_path: str) -> PspFilePath | Literal["ignore"] | None: + """Classifies a ``saves/...`` cloud-sync path as a PSP save-folder file, + PSP engine-cache noise to ignore, or neither (a normal single-file save + -- None, let the generic save/state path handle it).""" + segments = [s for s in file_path.strip("/").split("/") if s] + if len(segments) < 4 or segments[0] != "saves": + return None + if segments[2].upper() != "PSP": + return None + + category = segments[3].upper() + if category == _IGNORED_CATEGORY: + return "ignore" + if category != _SAVEDATA_CATEGORY: + return None + if len(segments) < 6: + return None + + return PspFilePath( + emulator=segments[1], + save_folder=segments[4], + file_name="/".join(segments[5:]), + ) + + +def _bundle_base_name(save_folder: str) -> str: + return f"PSP-{save_folder}.zip" + + +def is_psp_bundle_file_name(file_name: str) -> bool: + """Whether a stored save's file name is a PSP bundle -- used by + `build_manifest` to exclude these from normal single-file save + handling.""" + return re.match(r"^PSP-.+\.zip$", file_name) is not None + + +def _bundle_pattern(save_folder: str) -> re.Pattern[str]: + """Matches a stored bundle filename by prefix/suffix only, tolerating + whatever else ends up between them -- there is nothing else to key on, + since bundles are update-in-place (one row per save folder), not + history-preserving.""" + return re.compile(rf"^PSP-{re.escape(save_folder)}\b.*\.zip$") + + +def _find_bundle_by_folder(user: User, save_folder: str) -> Save | None: + """Finds the current bundle for a save folder by its name alone -- the + folder name (e.g. "ULUS10336DATA0") is already a globally unique + identifier for a given game+slot, so this works without ever needing + PSP_SERIAL_MAP or PARAM.SFO except on the very first upload of a new + folder.""" + pattern = _bundle_pattern(save_folder) + saves = db_save_handler.get_saves(user_id=user.id) + candidates = [s for s in saves if pattern.match(s.file_name)] + if not candidates: + return None + return max(candidates, key=lambda s: (s.updated_at, s.id)) + + +def _derive_serial(save_folder: str) -> str: + return re.sub(r"DATA\d+$", "", save_folder, flags=re.IGNORECASE) + + +def _normalize_title(s: str) -> str: + """Loose title comparison for matching a PARAM.SFO TITLE (e.g. "CRISIS + CORE -FINAL FANTASY VII-") against RomM's filename-derived titles (e.g. + "Crisis Core - Final Fantasy VII (USA)") -- collapses both down to bare + alphanumerics so punctuation/casing/spacing differences (the norm + between PSF titles and filename-derived ones) don't block an otherwise + obvious match.""" + return re.sub(r"[^a-z0-9]+", " ", s.lower()).strip() + + +def parse_sfo(data: bytes) -> dict[str, str | int]: + """Minimal parser for the PSP's PARAM.SFO ("PSF") format -- the file + PPSSPP writes into every save folder describing that save. Only two of + its fields matter here: DISC_ID (the game's serial, e.g. "ULUS10336") + and TITLE (the game's display name), to resolve a save folder to a RomM + rom. + + Format: a 20-byte header, a fixed-size index table (one 16-byte entry + per key), a key table (NUL-terminated ASCII strings), and a data table + (UTF-8 strings or little-endian integers, per entry). + """ + if len(data) < 20 or data[0:4] != b"\x00PSF": + raise ValueError("Not a PARAM.SFO file (bad magic)") + + key_table_offset, data_table_offset, entry_count = struct.unpack_from( + "= 4 else 0 + else: + nul = raw_value.find(b"\x00") + result[key] = raw_value[: nul if nul != -1 else None].decode( + "utf-8", errors="replace" + ) + + return result + + +def _match_by_normalized_title( + title: str, can_see: Callable[[Rom], bool] +) -> Rom | None: + platform = db_platform_handler.get_platform_by_fs_slug("psp") + platform_ids = [platform.id] if platform else None + candidates = [ + rom + for rom in db_rom_handler.get_roms_scalar( + search_term=title, platform_ids=platform_ids + ) + if can_see(rom) + ] + + target = _normalize_title(title) + for attr in ("fs_name_no_tags", "name", "fs_name_no_ext"): + for rom in candidates: + value = getattr(rom, attr, None) + if value and _normalize_title(value) == target: + return rom + + return None + + +def _resolve_rom( + save_folder: str, sfo_title: str | None, can_see: Callable[[Rom], bool] +) -> Rom | None: + """Resolves a PSP save folder to a RomM rom. Tries the PARAM.SFO title + first -- it's already sent as part of every save, so this is what makes + PSP saves sync automatically with zero manual setup for the common + case. PSP_SERIAL_MAP is an explicit override checked first when + present, and the fallback when SFO title matching doesn't find + anything -- some games' PSF titles are abbreviated/stylized enough that + normalization alone won't bridge the gap to RomM's filename-derived + title.""" + serial = _derive_serial(save_folder) + mapped_title = PSP_SERIAL_MAP.get(serial) + if mapped_title: + candidates = [ + rom + for rom in db_rom_handler.get_roms_by_fs_name_no_ext(mapped_title) + if can_see(rom) + ] + if candidates: + return candidates[0] + log.warning( + f"PSP_SERIAL_MAP entry for {hl(serial)} ({hl(mapped_title)}) " + "didn't match any rom in the library" + ) + + if sfo_title: + rom = _match_by_normalized_title(sfo_title, can_see) + if rom: + log.info( + f"Resolved PSP save folder {hl(save_folder)} via PARAM.SFO " + f"title {hl(sfo_title)} to {hl(str(rom.name))}" + ) + return rom + log.warning( + f"Couldn't auto-match PARAM.SFO title {hl(sfo_title)} for PSP save " + f"folder {hl(save_folder)} -- add serial {hl(serial)} to " + "PSP_SERIAL_MAP if this keeps happening" + ) + + return None + + +def _load_bundle_entries(zip_bytes: bytes) -> dict[str, bytes]: + with zipfile.ZipFile(BytesIO(zip_bytes)) as zf: + return {name: zf.read(name) for name in zf.namelist()} + + +def _write_bundle(entries: dict[str, bytes]) -> bytes: + # `zipfile_inflate64` (pulled in elsewhere for ROM archive reading) + # replaces `zipfile._get_compressor` with a signature CPython 3.13's + # own `ZipFile.writestr()` can't call -- see `_ensure_zipfile_writable`. + _ensure_zipfile_writable() + buffer = BytesIO() + with zipfile.ZipFile(buffer, "w", zipfile.ZIP_DEFLATED) as zf: + for name, data in entries.items(): + zf.writestr(name, data) + return buffer.getvalue() + + +def _pending_dir(user: User, save_folder: str) -> str: + return f"{user.id}/{save_folder}" + + + + +# Per-(user, save folder) locks. FastAPI/uvicorn typically runs this as a +# single worker process for a RomM instance, so an in-process asyncio.Lock +# is enough to serialize the read-modify-write over a folder's bundle the +# same way the shim's single-threaded Node process naturally did -- +# concurrent multi-worker deployments would need a distributed lock +# instead, which nothing else in cloud sync uses either. +_folder_locks: dict[str, asyncio.Lock] = {} + + +def _get_folder_lock(key: str) -> asyncio.Lock: + lock = _folder_locks.get(key) + if lock is None: + lock = asyncio.Lock() + _folder_locks[key] = lock + return lock + + +async def put_psp_file( + user: User, info: PspFilePath, content: bytes, can_see: Callable[[Rom], bool] +) -> None: + """Merges one uploaded file into its save folder's bundle and + re-uploads it. Deliberately does not preserve per-file history the way + normal saves/states do: PPSSPP writes a save folder as a burst of + several individual file PUTs a fraction of a second apart, so keeping + every intermediate partially-merged bundle as its own history entry + would just be noise -- only the final, fully-merged state after a save + event is a meaningful checkpoint. The previous bundle row is deleted + once the merged one is up, so RomM holds exactly one row per (rom, save + folder) at a time. + + Raises `PspFolderUnresolved` if the folder can't yet be matched to a + rom -- the file is buffered on disk and will be folded in once it is. + """ + lock = _get_folder_lock(f"{user.id}:{info.save_folder}") + async with lock: + existing = _find_bundle_by_folder(user, info.save_folder) + + prior_entries: dict[str, bytes] = {} + if existing: + rom_id = existing.rom_id + zip_bytes = await fs_asset_handler.read_file(existing.full_path) + prior_entries = _load_bundle_entries(zip_bytes) + else: + sfo_title = None + if info.file_name.upper() == "PARAM.SFO": + try: + parsed = parse_sfo(content) + if isinstance(parsed.get("TITLE"), str): + sfo_title = str(parsed["TITLE"]) + except ValueError as exc: + log.warning(f"Failed to parse PARAM.SFO: {exc}") + + rom = _resolve_rom(info.save_folder, sfo_title, can_see) + if rom is None: + pending_dir = _pending_dir(user, info.save_folder) + await fs_psp_pending_handler.write_file( + file=content, + path=pending_dir, + filename=info.file_name, + ) + serial = _derive_serial(info.save_folder) + log.warning( + f"No rom found yet for PSP save folder {hl(info.save_folder)} " + f"(serial {hl(serial)}) -- buffered {hl(info.file_name)}, will " + "merge it in once resolved (e.g. PARAM.SFO arrives)" + ) + raise PspFolderUnresolved(info.save_folder) + rom_id = rom.id + + # Now resolved (either an existing bundle, or fresh via this call) + # -- fold in anything buffered earlier while this folder was + # unresolved. + pending_dir = _pending_dir(user, info.save_folder) + pending: dict[str, bytes] = {} + try: + pending_names = await fs_psp_pending_handler.list_files(pending_dir) + except FileNotFoundError: + pending_names = [] + for name in pending_names: + pending[name] = await fs_psp_pending_handler.read_file( + f"{pending_dir}/{name}" + ) + + merged = {**prior_entries, **pending} + merged[info.file_name] = content + zip_bytes = _write_bundle(merged) + + rom = db_rom_handler.get_rom(rom_id) + assert rom is not None + saves_path = fs_asset_handler.build_saves_file_path( + user=user, + platform_fs_slug=rom.platform.fs_slug, + rom_id=rom.id, + emulator=info.emulator, + ) + bundle_name = _bundle_base_name(info.save_folder) + await fs_asset_handler.write_file( + file=zip_bytes, path=saves_path, filename=bundle_name + ) + + if existing: + db_save_handler.update_save( + existing.id, {"file_size_bytes": len(zip_bytes)} + ) + else: + db_save_handler.add_save( + Save( + rom_id=rom_id, + user_id=user.id, + file_name=bundle_name, + file_path=saves_path, + file_size_bytes=len(zip_bytes), + emulator=info.emulator, + slot=None, + ) + ) + + if pending_names: + for name in pending_names: + try: + await fs_psp_pending_handler.remove_file( + f"{pending_dir}/{name}" + ) + except FileNotFoundError: + pass + + +async def get_psp_file(user: User, info: PspFilePath) -> bytes | None: + bundle = _find_bundle_by_folder(user, info.save_folder) + if not bundle: + return None + zip_bytes = await fs_asset_handler.read_file(bundle.full_path) + entries = _load_bundle_entries(zip_bytes) + return entries.get(info.file_name) + + +async def delete_psp_folder(user: User, save_folder: str) -> bool: + """Drops an entire PSP save folder's bundle -- RetroArch deletes a save + folder file-by-file, but since the bundle is one row, the first delete + for a folder removes it and the rest are silent no-ops (matching + `find_bundle_by_folder` returning nothing for them).""" + bundle = _find_bundle_by_folder(user, save_folder) + if not bundle: + return False + + db_save_handler.delete_save(bundle.id) + try: + await fs_asset_handler.remove_file(file_path=bundle.full_path) + except FileNotFoundError: + pass + return True + + +_BUNDLE_FOLDER_PATTERN = re.compile(r"^PSP-(.+?)(?: \[.*])?\.zip$") + + +async def build_psp_manifest_entries( + user: User, can_see: Callable[[Rom], bool] +) -> list[dict[str, str]]: + """Lists every member of every PSP bundle as its own manifest entry -- + RetroArch diffs per-file, so each PARAM.SFO/ICON0.PNG/save-data file + within a folder needs its own {path, hash}, not one entry for the whole + bundle.""" + import hashlib + + saves = db_save_handler.get_saves(user_id=user.id) + + latest_by_folder: dict[str, Save] = {} + for save in saves: + match = _BUNDLE_FOLDER_PATTERN.match(save.file_name) + if not match: + continue + if save.missing_from_fs or not can_see(save.rom): + continue + save_folder = match.group(1) + current = latest_by_folder.get(save_folder) + if current is None or (save.updated_at, save.id) > ( + current.updated_at, + current.id, + ): + latest_by_folder[save_folder] = save + + entries: list[dict[str, str]] = [] + for save_folder, save in latest_by_folder.items(): + try: + zip_bytes = await fs_asset_handler.read_file(save.full_path) + members = _load_bundle_entries(zip_bytes) + except (FileNotFoundError, zipfile.BadZipFile) as exc: + log.warning( + f"Failed to read PSP bundle for {hl(save_folder)}, skipping " + f"from manifest: {exc}" + ) + continue + + dir_name = to_retroarch_dir_name(save.emulator) if save.emulator else "PPSSPP" + for member_name, data in members.items(): + entries.append( + { + "path": f"saves/{dir_name}/PSP/SAVEDATA/{save_folder}/{member_name}", + "hash": hashlib.md5(data, usedforsecurity=False).hexdigest(), + } + ) + + return entries diff --git a/backend/tests/endpoints/test_cloud_sync.py b/backend/tests/endpoints/test_cloud_sync.py index 8e825d641d..e0fc15bd64 100644 --- a/backend/tests/endpoints/test_cloud_sync.py +++ b/backend/tests/endpoints/test_cloud_sync.py @@ -3,7 +3,7 @@ import pytest from fastapi import status -from handler import cloud_sync_handler +from handler import cloud_sync_handler, cloud_sync_psp from handler.cloud_sync_emulator_names import to_retroarch_dir_name, to_romm_emulator from handler.database import db_save_handler, db_screenshot_handler, db_state_handler from handler.filesystem import fs_asset_handler @@ -743,6 +743,129 @@ def test_delete_of_unknown_file_is_not_found(self, client, admin_user: User): assert response.status_code == status.HTTP_404_NOT_FOUND +class TestCloudSyncPsp: + """End-to-end coverage of the PPSSPP save-folder bundling wired into the + GET/PUT/DELETE endpoints and the manifest -- unit coverage for the pure + parsing/matching logic lives in tests/handler/test_cloud_sync_psp.py. + + Uses PSP_SERIAL_MAP to resolve the rom deterministically instead of a + real PARAM.SFO capture + fulltext title search, which would make this + test depend on the DB driver's fulltext support. + """ + + @pytest.fixture(autouse=True) + def _serial_map(self, monkeypatch: pytest.MonkeyPatch, rom: Rom): + monkeypatch.setattr( + cloud_sync_psp, "PSP_SERIAL_MAP", {"TEST12345": rom.fs_name_no_ext} + ) + + def test_ignores_system_cache_files(self, client, admin_user: User): + response = client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SYSTEM/CACHE/shader.bin", + content=b"cache data", + auth=ADMIN_AUTH, + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + + def test_bundles_multiple_files_into_one_save( + self, client, admin_user: User, rom: Rom + ): + put_sfo = client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/PARAM.SFO", + content=b"not real sfo bytes, resolved via PSP_SERIAL_MAP instead", + auth=ADMIN_AUTH, + ) + assert put_sfo.status_code == status.HTTP_201_CREATED + + put_data = client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/SAVE.BIN", + content=b"the actual save data", + auth=ADMIN_AUTH, + ) + assert put_data.status_code == status.HTTP_201_CREATED + + saves = db_save_handler.get_saves(user_id=admin_user.id, rom_id=rom.id) + assert len(saves) == 1 + assert saves[0].file_name == "PSP-TEST12345DATA0.zip" + + get_sfo = client.get( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/PARAM.SFO", + auth=ADMIN_AUTH, + ) + assert get_sfo.status_code == status.HTTP_200_OK + assert get_sfo.content == b"not real sfo bytes, resolved via PSP_SERIAL_MAP instead" + + get_data = client.get( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/SAVE.BIN", + auth=ADMIN_AUTH, + ) + assert get_data.status_code == status.HTTP_200_OK + assert get_data.content == b"the actual save data" + + def test_manifest_lists_each_bundle_member_separately( + self, client, admin_user: User + ): + client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/PARAM.SFO", + content=b"sfo", + auth=ADMIN_AUTH, + ) + client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/SAVE.BIN", + content=b"data", + auth=ADMIN_AUTH, + ) + + response = client.get("/api/cloud-sync/manifest.server", auth=ADMIN_AUTH) + + assert response.status_code == status.HTTP_200_OK + paths = {entry["path"] for entry in response.json()} + assert paths == { + "saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/PARAM.SFO", + "saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/SAVE.BIN", + } + + def test_unresolved_folder_is_buffered_and_conflicts( + self, client, admin_user: User, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.setattr(cloud_sync_psp, "PSP_SERIAL_MAP", {}) + + response = client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/UNKNOWN99999DATA0/SAVE.BIN", + content=b"orphaned save data", + auth=ADMIN_AUTH, + ) + + assert response.status_code == status.HTTP_409_CONFLICT + + def test_delete_removes_the_whole_bundle(self, client, admin_user: User, rom: Rom): + client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/PARAM.SFO", + content=b"sfo", + auth=ADMIN_AUTH, + ) + client.put( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/SAVE.BIN", + content=b"data", + auth=ADMIN_AUTH, + ) + + response = client.request( + "DELETE", + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/SAVE.BIN", + auth=ADMIN_AUTH, + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert db_save_handler.get_saves(user_id=admin_user.id, rom_id=rom.id) == [] + + get_response = client.get( + "/api/cloud-sync/saves/PPSSPP/PSP/SAVEDATA/TEST12345DATA0/PARAM.SFO", + auth=ADMIN_AUTH, + ) + assert get_response.status_code == status.HTTP_404_NOT_FOUND + + class TestCloudSyncMkcol: def test_mkcol_succeeds_without_creating_anything(self, client, admin_user: User): response = client.request( diff --git a/backend/tests/handler/test_cloud_sync_psp.py b/backend/tests/handler/test_cloud_sync_psp.py new file mode 100644 index 0000000000..150546549f --- /dev/null +++ b/backend/tests/handler/test_cloud_sync_psp.py @@ -0,0 +1,94 @@ +import struct + +import pytest + +from handler.cloud_sync_psp import ( + PspFilePath, + is_psp_bundle_file_name, + parse_sfo, + resolve_psp_path, +) + + +def _build_fake_sfo(fields: dict[str, str | int]) -> bytes: + """Builds a minimal valid PARAM.SFO buffer for the given fields, to + round-trip against `parse_sfo` without needing a real PPSSPP capture.""" + key_table = bytearray() + data_table = bytearray() + entries = [] + + for key, value in fields.items(): + key_offset = len(key_table) + key_table += key.encode("ascii") + b"\x00" + + data_offset = len(data_table) + if isinstance(value, int): + data_fmt = 0x0404 + data_bytes = struct.pack(" Date: Fri, 24 Jul 2026 18:38:59 +0300 Subject: [PATCH 5/8] feat(cloud-sync): read-only WebDAV browsing (PROPFIND) for the rom library RetroArch's own Cloud Sync client never issues PROPFIND (verified against its source, already noted in this router's docstring), so this isn't on RetroArch's actual sync path -- it's for a real WebDAV client (iOS Files' "Connect to Server", Cyberduck, ...) to mount the same /api/cloud-sync URL and browse/download the library as plain files, same as the retroarch-webdav-romm shim's romBrowser.ts + webdavXml.ts. Adds PROPFIND for roms// (RomM's own library, read-only -- no PUT/DELETE) and saves/states/ (the current cloud-sync manifest; unlike the shim, only current entries are browsable here, not full history -- RomM's own web UI covers that). LOCK/UNLOCK are a fake always-succeeds handshake some WebDAV clients require before they'll mount a server at all, matching the shim. GET/HEAD for a rom file redirects (307) to RomM's existing /api/roms/{id}/content/{file_name} endpoint rather than reimplementing Range support, multi-file zip caching and (in production) nginx X-Accel-Redirect -- Basic Auth carries over on the redirect since that endpoint already accepts it alongside OAuth. Found and fixed along the way: Rom.has_multiple_files / .files depend on columns/relationships get_roms_scalar doesn't eager-load, so accessing them outside the query's own session raised DetachedInstanceError -- re-fetch the visibility-filtered ids via get_roms_by_ids (which does eager-load them) instead of using the raw scalar results directly. --- backend/endpoints/cloud_sync.py | 262 ++++++++++++++++++++- backend/handler/webdav_browser.py | 165 +++++++++++++ backend/tests/endpoints/test_cloud_sync.py | 114 ++++++++- 3 files changed, 536 insertions(+), 5 deletions(-) create mode 100644 backend/handler/webdav_browser.py diff --git a/backend/endpoints/cloud_sync.py b/backend/endpoints/cloud_sync.py index 78915361a2..2fa779909a 100644 --- a/backend/endpoints/cloud_sync.py +++ b/backend/endpoints/cloud_sync.py @@ -11,11 +11,13 @@ """ import os +import uuid +from urllib.parse import quote from fastapi import APIRouter, Request, Response, status -from fastapi.responses import JSONResponse +from fastapi.responses import JSONResponse, RedirectResponse -from handler import cloud_sync_handler +from handler import cloud_sync_handler, webdav_browser from handler.auth.constants import Scope from handler.auth.dependencies import get_permissions from handler.cloud_sync_handler import MANIFEST_FILE_NAME, AssetKind, CloudSyncPath @@ -33,7 +35,7 @@ router = APIRouter(prefix="/cloud-sync", tags=["cloud-sync"]) -ALLOWED_METHODS = "OPTIONS, GET, HEAD, PUT, DELETE, MKCOL, MOVE" +ALLOWED_METHODS = "OPTIONS, PROPFIND, GET, HEAD, PUT, DELETE, MKCOL, MOVE, LOCK, UNLOCK" def _empty(status_code: int, headers: dict[str, str] | None = None) -> Response: @@ -104,7 +106,237 @@ def cloud_sync_options(request: Request, file_path: str) -> Response: return _empty( status.HTTP_200_OK, - {"DAV": "1", "Allow": ALLOWED_METHODS, "MS-Author-Via": "DAV"}, + # Class 2 (locking) is advertised alongside the fake LOCK/UNLOCK + # below -- some WebDAV clients (iOS Files among them, by report) + # refuse to treat a server as mountable at all without it, even for + # read-only browsing. + {"DAV": "1, 2", "Allow": ALLOWED_METHODS, "MS-Author-Via": "DAV"}, + ) + + +@router.api_route("/{file_path:path}", methods=["LOCK"], include_in_schema=False) +def cloud_sync_lock(request: Request, file_path: str) -> Response: + """Fake, always-succeeds locking. Nothing here is actually lockable -- + RetroArch's own Cloud Sync client never sends LOCK, and this WebDAV + surface has no concept of concurrent writers to guard against -- but + some WebDAV clients (iOS Files among them, by report) won't complete + "Connect to Server" without a server that at least answers LOCK/UNLOCK, + so this exists purely for that compatibility handshake.""" + denied = _authorize(request, Scope.ASSETS_READ) + if denied: + return denied + + token = f"opaquelocktoken:{uuid.uuid4()}" + body = ( + '' + '' + "" + "" + "0" + "Second-3600" + f"{token}" + "" + ) + return Response( + content=body, + media_type="text/xml; charset=utf-8", + headers={"Lock-Token": f"<{token}>"}, + ) + + +@router.api_route("/{file_path:path}", methods=["UNLOCK"], include_in_schema=False) +def cloud_sync_unlock(request: Request, file_path: str) -> Response: + denied = _authorize(request, Scope.ASSETS_READ) + if denied: + return denied + + return _empty(status.HTTP_204_NO_CONTENT) + + +@router.api_route("/{file_path:path}", methods=["PROPFIND"], include_in_schema=False) +async def cloud_sync_propfind(request: Request, file_path: str) -> Response: + """Read-only directory browsing for `roms/` (RomM's own library) and + `saves/`/`states/` (the current cloud-sync manifest), so a real WebDAV + client (iOS Files' "Connect to Server", Cyberduck, ...) can mount this + same URL and browse it like a normal file share. + + RetroArch's own Cloud Sync client never issues PROPFIND -- verified + against its source -- so none of this is on RetroArch's actual sync + path; it exists solely for read-only human browsing. Unlike the + retroarch-webdav-romm shim this mirrors, saves/states browsing here + only shows the manifest's *current* entries, not every historical + revision -- RomM's own web UI is the place to browse save history. + """ + denied = _authorize(request, Scope.ASSETS_READ) + if denied: + return denied + + depth = 0 if request.headers.get("depth") == "0" else 1 + permissions = get_permissions(request) + parts = [p for p in file_path.strip("/").split("/") if p] + + entries: list[webdav_browser.PropfindEntry] | None + if not parts: + entries = [_root_entry()] + if depth != 0: + entries += [ + _roms_root_entry(), + _virtual_root_entry("saves"), + _virtual_root_entry("states"), + ] + elif parts == ["roms"]: + entries = [_roms_root_entry()] + if depth != 0: + platforms = webdav_browser.list_platforms(permissions.can_see_platform) + entries += [ + webdav_browser.PropfindEntry( + href=f"roms/{p.fs_slug}/", is_collection=True, display_name=p.name + ) + for p in platforms + ] + elif len(parts) == 2 and parts[0] == "roms": + entries = _platform_listing(parts[1], depth, permissions) + elif len(parts) == 3 and parts[0] == "roms": + entries = _rom_file_entry(parts[1], parts[2], permissions) + elif parts[0] in ("saves", "states"): + entries = await _save_state_listing(parts, depth, request.user, permissions) + else: + entries = None + + if entries is None: + return _empty(status.HTTP_404_NOT_FOUND) + + body = webdav_browser.build_multistatus(entries) + return Response( + content=body, + status_code=207, + # iOS Files' WebDAV client is known to be picky about this -- + # "text/xml" (the traditional WebDAV content type) is the safer bet + # over "application/xml", which some Apple WebDAV client versions + # have reportedly failed to parse. + media_type="text/xml; charset=utf-8", + ) + + +def _root_entry() -> "webdav_browser.PropfindEntry": + return webdav_browser.PropfindEntry(href="", is_collection=True, display_name="") + + +def _roms_root_entry() -> "webdav_browser.PropfindEntry": + return webdav_browser.PropfindEntry( + href="roms/", is_collection=True, display_name="roms" + ) + + +def _virtual_root_entry(name: str) -> "webdav_browser.PropfindEntry": + return webdav_browser.PropfindEntry( + href=f"{name}/", is_collection=True, display_name=name + ) + + +def _platform_listing( + slug: str, depth: int, permissions +) -> list["webdav_browser.PropfindEntry"] | None: + platforms = webdav_browser.list_platforms(permissions.can_see_platform) + platform = next((p for p in platforms if p.fs_slug == slug), None) + if not platform: + return None + + self_entry = webdav_browser.PropfindEntry( + href=f"roms/{slug}/", is_collection=True, display_name=platform.name + ) + if depth == 0: + return [self_entry] + + files = ( + webdav_browser.list_rom_files( + slug, lambda rom: permissions.can_see_rom(rom.id, rom.platform_id) + ) + or [] + ) + return [self_entry] + [ + webdav_browser.PropfindEntry( + href=f"roms/{slug}/{f.display_name}", + is_collection=False, + display_name=f.display_name, + content_length=f.size_bytes, + last_modified=f.updated_at, + ) + for f in files + ] + + +def _rom_file_entry( + slug: str, file_name: str, permissions +) -> list["webdav_browser.PropfindEntry"] | None: + file = webdav_browser.find_rom_file( + slug, file_name, lambda rom: permissions.can_see_rom(rom.id, rom.platform_id) + ) + if not file: + return None + + return [ + webdav_browser.PropfindEntry( + href=f"roms/{slug}/{file_name}", + is_collection=False, + display_name=file_name, + content_length=file.size_bytes, + last_modified=file.updated_at, + ) + ] + + +async def _save_state_listing( + parts: list[str], depth: int, user: User, permissions +) -> list["webdav_browser.PropfindEntry"] | None: + manifest = await cloud_sync_handler.build_manifest( + user, lambda rom: permissions.can_see_rom(rom.id, rom.platform_id) + ) + clean = "/".join(parts) + + exact = next((e for e in manifest if e["path"] == clean), None) if len(parts) > 1 else None + if exact: + return [_manifest_file_entry(exact)] + + prefix = f"{clean}/" + has_children = any(e["path"].startswith(prefix) for e in manifest) + if len(parts) > 1 and not has_children: + return None + + self_entry = webdav_browser.PropfindEntry( + href=prefix, is_collection=True, display_name=parts[-1] + ) + if depth == 0: + return [self_entry] + + child_folders: set[str] = set() + child_files = [] + for entry in manifest: + if not entry["path"].startswith(prefix): + continue + rest = entry["path"][len(prefix) :] + if "/" in rest: + child_folders.add(rest.split("/", 1)[0]) + else: + child_files.append(entry) + + return ( + [self_entry] + + [ + webdav_browser.PropfindEntry( + href=f"{prefix}{folder}/", is_collection=True, display_name=folder + ) + for folder in sorted(child_folders) + ] + + [_manifest_file_entry(entry) for entry in child_files] + ) + + +def _manifest_file_entry(entry: dict[str, str]) -> "webdav_browser.PropfindEntry": + return webdav_browser.PropfindEntry( + href=entry["path"], + is_collection=False, + display_name=entry["path"].rsplit("/", 1)[-1], ) @@ -139,6 +371,28 @@ async def cloud_sync_get(request: Request, file_path: str) -> Response: resolved_path, filename=os.path.basename(blob_path) ) + rom_parts = [p for p in file_path.strip("/").split("/") if p] + if len(rom_parts) == 3 and rom_parts[0] == "roms": + permissions = get_permissions(request) + file = webdav_browser.find_rom_file( + rom_parts[1], + rom_parts[2], + lambda rom: permissions.can_see_rom(rom.id, rom.platform_id), + ) + if not file: + return _empty(status.HTTP_404_NOT_FOUND) + + # RomM's own content endpoint already handles Range requests, the + # multi-file zip cache and (in production) nginx X-Accel-Redirect -- + # duplicating that here would either miss the X-Accel-Redirect step + # (nothing would actually stream in production) or reimplement it + # badly. Basic Auth carries over on the redirect, so this stays a + # single unauthenticated-looking hop from the client's perspective. + return RedirectResponse( + url=f"/api/roms/{file.rom_id}/content/{quote(file.display_name)}", + status_code=status.HTTP_307_TEMPORARY_REDIRECT, + ) + parsed = cloud_sync_handler.parse_cloud_sync_path(file_path) if not parsed: return _empty(status.HTTP_404_NOT_FOUND) diff --git a/backend/handler/webdav_browser.py b/backend/handler/webdav_browser.py new file mode 100644 index 0000000000..d198f50c35 --- /dev/null +++ b/backend/handler/webdav_browser.py @@ -0,0 +1,165 @@ +"""Read-only WebDAV browsing (PROPFIND) for RomM's rom library, layered onto +the same `/api/cloud-sync` WebDAV surface RetroArch's Cloud Sync uses. + +RetroArch's own Cloud Sync client never issues PROPFIND -- verified against +its source, and already noted in `cloud_sync.py` -- so none of this is on +RetroArch's actual sync path. It exists purely so a real WebDAV client (e.g. +iOS Files app's "Connect to Server", Cyberduck, ...) can mount the same URL +and browse/download the library as plain files, mirroring the +retroarch-webdav-romm shim's `romBrowser.ts` + `webdavXml.ts`. + +Read-only by design: there is no PUT/DELETE support for `roms/`, only +GET/HEAD/PROPFIND. +""" + +from __future__ import annotations + +from collections.abc import Callable +from dataclasses import dataclass, field +from datetime import datetime +from xml.sax.saxutils import escape as xml_escape + +from handler.database import db_platform_handler, db_rom_handler +from models.platform import Platform +from models.rom import Rom + + +@dataclass(frozen=True) +class PropfindEntry: + """One `` entry. `href` is relative to the WebDAV root, e.g. + `roms/` or `roms/psx/Game.zip` -- never URL-encoded here, that's + `_href_escape`'s job at render time.""" + + href: str + is_collection: bool + display_name: str + content_length: int | None = None + last_modified: datetime | None = None + + +@dataclass(frozen=True) +class RomFile: + """A rom as it appears over WebDAV -- possibly a synthesized zip name + for a multi-file rom, never the raw per-part file names.""" + + rom_id: int + display_name: str + size_bytes: int + updated_at: datetime + file_ids: list[int] = field(default_factory=list) + + +def _href_escape(path: str) -> str: + return "/" + "/".join(segment for segment in path.split("/")) + + +def _response_xml(entry: PropfindEntry) -> str: + resource_type = "" if entry.is_collection else "" + extra = ( + "" + if entry.is_collection + else ( + f"{entry.content_length or 0}" + "application/octet-stream" + ) + ) + last_modified = ( + f"{entry.last_modified.strftime('%a, %d %b %Y %H:%M:%S GMT')}" + if entry.last_modified + else "" + ) + + return ( + "" + f"{xml_escape(_href_escape(entry.href))}" + "" + f"{resource_type}" + f"{xml_escape(entry.display_name)}" + f"{extra}{last_modified}" + "HTTP/1.1 200 OK" + "" + ) + + +def build_multistatus(entries: list[PropfindEntry]) -> str: + body = "".join(_response_xml(entry) for entry in entries) + return ( + '' + '' + body + "" + ) + + +def _display_name(rom: Rom) -> str: + """RomM zips up genuinely multi-file roms (multi-disc/multi-track games) + on download and includes an .m3u -- the WebDAV listing should show that + reality (a .zip) rather than the original fs_name. `has_nested_single_file` + (one real file sitting a folder deep) still downloads as the raw file, not + a zip -- only `has_multiple_files` actually triggers zipping server-side. + + For the nested-single-file case, `fs_name` is the *folder* name with no + extension; the real filename (with extension) is on `files[0].file_name`. + """ + if rom.has_multiple_files: + return f"{rom.fs_name_no_ext}.zip" + files = sorted(rom.files, key=lambda f: f.file_name) + return files[0].file_name if files else rom.fs_name + + +def list_platforms( + can_see_platform: Callable[[int], bool], +) -> list[Platform]: + platforms = db_platform_handler.get_platforms() + return [p for p in platforms if p.rom_count > 0 and can_see_platform(p.id)] + + +def _visible_roms_for_platform( + platform_fs_slug: str, can_see_rom: Callable[[Rom], bool] +) -> tuple[Platform, list[Rom]] | None: + platform = db_platform_handler.get_platform_by_fs_slug(platform_fs_slug) + if not platform: + return None + + # `get_roms_scalar` doesn't eager-load `files`/`multi_file`/ + # `top_level_file_count` -- filtering visibility only needs `id` and + # `platform_id`, cheap on the plain query, but `_display_name` below + # needs those eager-loaded columns, so the visible ids are re-fetched + # via `get_roms_by_ids` (which does eager-load them) rather than risking + # a `DetachedInstanceError` on first access outside this session. + candidate_ids = [ + rom.id + for rom in db_rom_handler.get_roms_scalar(platform_ids=[platform.id]) + if can_see_rom(rom) + ] + roms = db_rom_handler.get_roms_by_ids(candidate_ids) + return platform, list(roms) + + +def list_rom_files( + platform_fs_slug: str, can_see_rom: Callable[[Rom], bool] +) -> list[RomFile] | None: + """None means the platform itself doesn't exist/isn't visible; an empty + list means it exists but has nothing the caller can see.""" + resolved = _visible_roms_for_platform(platform_fs_slug, can_see_rom) + if resolved is None: + return None + + _platform, roms = resolved + return [ + RomFile( + rom_id=rom.id, + display_name=_display_name(rom), + size_bytes=rom.fs_size_bytes, + updated_at=rom.updated_at, + file_ids=[f.id for f in rom.files], + ) + for rom in roms + ] + + +def find_rom_file( + platform_fs_slug: str, file_name: str, can_see_rom: Callable[[Rom], bool] +) -> RomFile | None: + files = list_rom_files(platform_fs_slug, can_see_rom) + if not files: + return None + return next((f for f in files if f.display_name == file_name), None) diff --git a/backend/tests/endpoints/test_cloud_sync.py b/backend/tests/endpoints/test_cloud_sync.py index 125715efd4..fc3cb591b4 100644 --- a/backend/tests/endpoints/test_cloud_sync.py +++ b/backend/tests/endpoints/test_cloud_sync.py @@ -139,8 +139,9 @@ def test_options_with_basic_auth_advertises_dav(self, client, admin_user: User): response = client.options("/api/cloud-sync/", auth=ADMIN_AUTH) assert response.status_code == status.HTTP_200_OK - assert response.headers["dav"] == "1" + assert response.headers["dav"] == "1, 2" assert "MKCOL" in response.headers["allow"] + assert "PROPFIND" in response.headers["allow"] def test_get_without_credentials_challenges(self, client): response = client.get("/api/cloud-sync/manifest.server") @@ -543,3 +544,114 @@ def test_manifest_includes_blobs_alongside_assets(self, client, admin_user: User "hash": "8d777f385d3dfec8815d20f7496026dc", } ] + + +class TestCloudSyncWebdavBrowsing: + """PROPFIND/LOCK/UNLOCK + the `roms/` GET redirect -- read-only WebDAV + browsing layered onto the same surface, for real WebDAV clients (iOS + Files, Cyberduck, ...) rather than RetroArch itself (which never issues + PROPFIND).""" + + def test_lock_succeeds(self, client, admin_user: User): + response = client.request("LOCK", "/api/cloud-sync/roms/", auth=ADMIN_AUTH) + + assert response.status_code == status.HTTP_200_OK + assert response.headers["lock-token"].startswith("/roms/" in body + assert "/saves/" in body + assert "/states/" in body + + def test_propfind_roms_lists_platforms_with_roms( + self, client, admin_user: User, rom: Rom + ): + response = client.request( + "PROPFIND", "/api/cloud-sync/roms/", auth=ADMIN_AUTH + ) + + assert response.status_code == 207 + assert f"/roms/{rom.platform.fs_slug}/" in response.text + + def test_propfind_platform_lists_rom_files( + self, client, admin_user: User, rom: Rom + ): + response = client.request( + "PROPFIND", + f"/api/cloud-sync/roms/{rom.platform.fs_slug}/", + auth=ADMIN_AUTH, + ) + + assert response.status_code == 207 + assert f"/roms/{rom.platform.fs_slug}/{rom.fs_name}" in response.text + + def test_propfind_unknown_platform_is_not_found(self, client, admin_user: User): + response = client.request( + "PROPFIND", "/api/cloud-sync/roms/nope/", auth=ADMIN_AUTH + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_get_rom_file_redirects_to_rest_content_endpoint( + self, client, admin_user: User, rom: Rom + ): + response = client.get( + f"/api/cloud-sync/roms/{rom.platform.fs_slug}/{rom.fs_name}", + auth=ADMIN_AUTH, + follow_redirects=False, + ) + + assert response.status_code == status.HTTP_307_TEMPORARY_REDIRECT + assert response.headers["location"] == f"/api/roms/{rom.id}/content/{rom.fs_name}" + + def test_get_unknown_rom_file_is_not_found( + self, client, admin_user: User, rom: Rom + ): + response = client.get( + f"/api/cloud-sync/roms/{rom.platform.fs_slug}/nope.zip", + auth=ADMIN_AUTH, + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + @mock.patch( + "handler.cloud_sync_handler.asset_md5", + new_callable=mock.AsyncMock, + return_value="d41d8cd98f00b204e9800998ecf8427e", + ) + def test_propfind_saves_lists_the_emulator_subfolder( + self, _asset_md5: mock.AsyncMock, client, admin_user: User, synced_save: Save + ): + response = client.request("PROPFIND", "/api/cloud-sync/saves/", auth=ADMIN_AUTH) + + assert response.status_code == 207 + assert "/saves/Snes9x/" in response.text + + @mock.patch( + "handler.cloud_sync_handler.asset_md5", + new_callable=mock.AsyncMock, + return_value="d41d8cd98f00b204e9800998ecf8427e", + ) + def test_propfind_saves_subfolder_lists_the_file( + self, _asset_md5: mock.AsyncMock, client, admin_user: User, synced_save: Save + ): + response = client.request( + "PROPFIND", "/api/cloud-sync/saves/Snes9x/", auth=ADMIN_AUTH + ) + + assert response.status_code == 207 + assert "/saves/Snes9x/test_rom.srm" in response.text From 209a07b04b19d65913623068df0dd7539f959f2a Mon Sep 17 00:00:00 2001 From: Mustafa YAMAN Date: Fri, 24 Jul 2026 18:57:10 +0300 Subject: [PATCH 6/8] fix(cloud-sync): use absolute WebDAV hrefs so PROPFIND doesn't loop forever Verified live with a real WebDAV client (Cyberduck): every PropfindEntry href was relative to this router's own mount point (e.g. "roms/", "saves/Snes9x/"), not absolute from the server root the way WebDAV clients expect a to be. The client couldn't match a "self" entry's href back to the path it had just requested, so it rendered that entry as an extra nested subfolder instead of recognizing it as the current directory -- and since every subfolder has the same mismatch, browsing into it produced another apparent copy of the whole tree, forever. Prefixing every href with this router's actual mount path (/api/cloud-sync) fixes the self-entry match and ends the loop. Also fixed along the way: href path segments were never percent-encoded, unlike the retroarch-webdav-romm shim's own hrefEscape (which used encodeURIComponent) that this was supposed to mirror -- a save/state/rom filename containing a space or other reserved character would have produced invalid hrefs. --- backend/handler/webdav_browser.py | 19 ++++++++++++++++++- backend/tests/endpoints/test_cloud_sync.py | 14 +++++++------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/backend/handler/webdav_browser.py b/backend/handler/webdav_browser.py index d198f50c35..bed069caed 100644 --- a/backend/handler/webdav_browser.py +++ b/backend/handler/webdav_browser.py @@ -17,6 +17,7 @@ from collections.abc import Callable from dataclasses import dataclass, field from datetime import datetime +from urllib.parse import quote from xml.sax.saxutils import escape as xml_escape from handler.database import db_platform_handler, db_rom_handler @@ -49,8 +50,24 @@ class RomFile: file_ids: list[int] = field(default_factory=list) +# This module's PropfindEntry.href values are relative to the router's own +# mount point (e.g. "roms/", "saves/Snes9x/"), matching the shim's +# `romBrowser.ts`/`webdavXml.ts` -- but WebDAV clients expect every +# in a multistatus response to be an absolute path from the *server* root, +# not relative to the collection being PROPFIND'd. Verified live: without +# this prefix, a client (Cyberduck) couldn't match the "self" entry's href +# back to the path it had just requested, and rendered it as an extra, +# never-ending nested subfolder instead of recognizing it as the current +# directory -- the same mismatch made every subfolder look like it +# contained a copy of the whole tree again. +WEBDAV_MOUNT_PREFIX = "/api/cloud-sync" + + def _href_escape(path: str) -> str: - return "/" + "/".join(segment for segment in path.split("/")) + full_path = f"{WEBDAV_MOUNT_PREFIX}/{path}" if path else f"{WEBDAV_MOUNT_PREFIX}/" + segments = full_path.strip("/").split("/") + escaped = "/" + "/".join(quote(segment, safe="") for segment in segments) + return escaped + "/" if full_path.endswith("/") else escaped def _response_xml(entry: PropfindEntry) -> str: diff --git a/backend/tests/endpoints/test_cloud_sync.py b/backend/tests/endpoints/test_cloud_sync.py index 269b0d3fac..fae35270d2 100644 --- a/backend/tests/endpoints/test_cloud_sync.py +++ b/backend/tests/endpoints/test_cloud_sync.py @@ -1012,9 +1012,9 @@ def test_propfind_root_lists_virtual_roots(self, client, admin_user: User): assert response.status_code == 207 body = response.text - assert "/roms/" in body - assert "/saves/" in body - assert "/states/" in body + assert "/api/cloud-sync/roms/" in body + assert "/api/cloud-sync/saves/" in body + assert "/api/cloud-sync/states/" in body def test_propfind_roms_lists_platforms_with_roms( self, client, admin_user: User, rom: Rom @@ -1024,7 +1024,7 @@ def test_propfind_roms_lists_platforms_with_roms( ) assert response.status_code == 207 - assert f"/roms/{rom.platform.fs_slug}/" in response.text + assert f"/api/cloud-sync/roms/{rom.platform.fs_slug}/" in response.text def test_propfind_platform_lists_rom_files( self, client, admin_user: User, rom: Rom @@ -1036,7 +1036,7 @@ def test_propfind_platform_lists_rom_files( ) assert response.status_code == 207 - assert f"/roms/{rom.platform.fs_slug}/{rom.fs_name}" in response.text + assert f"/api/cloud-sync/roms/{rom.platform.fs_slug}/{rom.fs_name}" in response.text def test_propfind_unknown_platform_is_not_found(self, client, admin_user: User): response = client.request( @@ -1078,7 +1078,7 @@ def test_propfind_saves_lists_the_emulator_subfolder( response = client.request("PROPFIND", "/api/cloud-sync/saves/", auth=ADMIN_AUTH) assert response.status_code == 207 - assert "/saves/Snes9x/" in response.text + assert "/api/cloud-sync/saves/Snes9x/" in response.text @mock.patch( "handler.cloud_sync_handler.asset_md5", @@ -1093,4 +1093,4 @@ def test_propfind_saves_subfolder_lists_the_file( ) assert response.status_code == 207 - assert "/saves/Snes9x/test_rom.srm" in response.text + assert "/api/cloud-sync/saves/Snes9x/test_rom.srm" in response.text From a9989854533769a270393ed08621795ea7ad860b Mon Sep 17 00:00:00 2001 From: Mustafa YAMAN Date: Fri, 24 Jul 2026 20:57:40 +0300 Subject: [PATCH 7/8] Fix: don't strip "+" from filenames, it's valid on every real filesystem sanitize_filename treated "+" as an invalid character alongside the actual Windows-reserved set (< > : " / \ | ? *), which broke cloud-sync's filename-based ROM matching for any title containing one -- RomM itself stores such ROMs (e.g. combo carts like "Super Mario All-Stars + Super Mario World") on disk with the "+" intact, so stripping it before resolving the ROM caused every save/state upload for that game to 409. Confirmed live against production (romm.vmyaman.com) via a real RetroArch client. --- backend/tests/endpoints/test_cloud_sync.py | 62 +++++++++++++++++++++- backend/tests/utils/test_filesystem.py | 2 +- backend/utils/filesystem.py | 7 ++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/backend/tests/endpoints/test_cloud_sync.py b/backend/tests/endpoints/test_cloud_sync.py index fae35270d2..b5811b61ed 100644 --- a/backend/tests/endpoints/test_cloud_sync.py +++ b/backend/tests/endpoints/test_cloud_sync.py @@ -5,9 +5,15 @@ from handler import cloud_sync_handler, cloud_sync_psp from handler.cloud_sync_emulator_names import to_retroarch_dir_name, to_romm_emulator -from handler.database import db_save_handler, db_screenshot_handler, db_state_handler +from handler.database import ( + db_rom_handler, + db_save_handler, + db_screenshot_handler, + db_state_handler, +) from handler.filesystem import fs_asset_handler from models.assets import Save, Screenshot, State +from models.platform import Platform from models.rom import Rom from models.user import User @@ -666,6 +672,60 @@ def test_rejects_upload_with_no_matching_rom(self, client, admin_user: User): assert response.status_code == status.HTTP_409_CONFLICT assert response.content == b"" + @mock.patch( + "endpoints.cloud_sync.fs_asset_handler.write_file", new_callable=mock.AsyncMock + ) + @mock.patch("endpoints.cloud_sync.scan_save", new_callable=mock.AsyncMock) + def test_matches_rom_with_plus_in_name( + self, + mock_scan_save: mock.AsyncMock, + _mock_write_file: mock.AsyncMock, + client, + admin_user: User, + platform: Platform, + ): + """"+" is not invalid on any real filesystem -- RomM itself stores + combo-cart ROMs with it in `fs_name` untouched, so stripping it + before resolving the ROM (as `sanitize_filename` used to) broke + matching for exactly those titles.""" + combo_rom = Rom( + platform_id=platform.id, + name="Super Mario All-Stars + Super Mario World", + slug="combo-rom-slug", + fs_name="Super Mario All-Stars + Super Mario World (USA).sfc", + fs_name_no_tags="Super Mario All-Stars + Super Mario World", + fs_name_no_ext="Super Mario All-Stars + Super Mario World (USA)", + fs_extension="sfc", + fs_path=f"{platform.slug}/roms", + ) + combo_rom = db_rom_handler.add_rom(combo_rom) + db_rom_handler.add_rom_user(rom_id=combo_rom.id, user_id=admin_user.id) + + saves_path = fs_asset_handler.build_saves_file_path( + user=admin_user, + platform_fs_slug="test_platform_slug", + rom_id=combo_rom.id, + emulator="snes9x", + ) + mock_scan_save.return_value = Save( + file_name="Super Mario All-Stars + Super Mario World (USA).srm", + file_path=saves_path, + file_size_bytes=4, + content_hash="8d777f385d3dfec8815d20f7496026dc", + ) + + response = client.put( + "/api/cloud-sync/saves/Snes9x/Super Mario All-Stars + Super Mario World (USA).srm", + content=b"data", + auth=ADMIN_AUTH, + ) + + assert response.status_code == status.HTTP_201_CREATED + + saves = db_save_handler.get_saves(user_id=admin_user.id, rom_id=combo_rom.id) + assert len(saves) == 1 + assert saves[0].file_name == "Super Mario All-Stars + Super Mario World (USA).srm" + def test_rejects_unsupported_sync_root(self, client, admin_user: User, rom: Rom): response = client.put( "/api/cloud-sync/deleted/saves/test_rom.srm", diff --git a/backend/tests/utils/test_filesystem.py b/backend/tests/utils/test_filesystem.py index 038b4de9da..ab757f2d85 100644 --- a/backend/tests/utils/test_filesystem.py +++ b/backend/tests/utils/test_filesystem.py @@ -10,7 +10,7 @@ from utils.filesystem import link_or_copy_file, sanitize_filename -INVALID_AFTER_SANITIZE = set('\\/:|*?"<>+\0') +INVALID_AFTER_SANITIZE = set('\\/:|*?"<>\0') class TestLinkOrCopyFile: diff --git a/backend/utils/filesystem.py b/backend/utils/filesystem.py index 0bbfa85f06..e5bcb8c483 100644 --- a/backend/utils/filesystem.py +++ b/backend/utils/filesystem.py @@ -88,7 +88,12 @@ def link_or_copy_file(source: Path, dest: Path) -> None: INVALID_CHARS_HYPHENS = re.compile(r"[\\/:|]") -INVALID_CHARS_EMPTY = re.compile(r'[*?"<>+]') +# "+" is not invalid on any major filesystem (Windows' actual reserved set is +# `< > : " / \ | ? *`) -- stripping it here broke cloud-sync's filename-based +# ROM matching for any game whose real fs_name contains one (e.g. "Super +# Mario All-Stars + Super Mario World (USA)"), which RomM itself stores +# on disk with the "+" intact. +INVALID_CHARS_EMPTY = re.compile(r'[*?"<>]') def sanitize_filename(filename: str) -> str: From 31efc6f26fe2f49ed0bb5b59d06a8603098610f0 Mon Sep 17 00:00:00 2001 From: Mustafa YAMAN Date: Fri, 24 Jul 2026 21:53:56 +0300 Subject: [PATCH 8/8] Remove the "+" filename regression test Keeping the test-file footprint down; the production fix (dropping "+" from sanitize_filename's invalid-char set) stands on its own and is covered live. --- backend/tests/endpoints/test_cloud_sync.py | 62 +--------------------- 1 file changed, 1 insertion(+), 61 deletions(-) diff --git a/backend/tests/endpoints/test_cloud_sync.py b/backend/tests/endpoints/test_cloud_sync.py index b5811b61ed..fae35270d2 100644 --- a/backend/tests/endpoints/test_cloud_sync.py +++ b/backend/tests/endpoints/test_cloud_sync.py @@ -5,15 +5,9 @@ from handler import cloud_sync_handler, cloud_sync_psp from handler.cloud_sync_emulator_names import to_retroarch_dir_name, to_romm_emulator -from handler.database import ( - db_rom_handler, - db_save_handler, - db_screenshot_handler, - db_state_handler, -) +from handler.database import db_save_handler, db_screenshot_handler, db_state_handler from handler.filesystem import fs_asset_handler from models.assets import Save, Screenshot, State -from models.platform import Platform from models.rom import Rom from models.user import User @@ -672,60 +666,6 @@ def test_rejects_upload_with_no_matching_rom(self, client, admin_user: User): assert response.status_code == status.HTTP_409_CONFLICT assert response.content == b"" - @mock.patch( - "endpoints.cloud_sync.fs_asset_handler.write_file", new_callable=mock.AsyncMock - ) - @mock.patch("endpoints.cloud_sync.scan_save", new_callable=mock.AsyncMock) - def test_matches_rom_with_plus_in_name( - self, - mock_scan_save: mock.AsyncMock, - _mock_write_file: mock.AsyncMock, - client, - admin_user: User, - platform: Platform, - ): - """"+" is not invalid on any real filesystem -- RomM itself stores - combo-cart ROMs with it in `fs_name` untouched, so stripping it - before resolving the ROM (as `sanitize_filename` used to) broke - matching for exactly those titles.""" - combo_rom = Rom( - platform_id=platform.id, - name="Super Mario All-Stars + Super Mario World", - slug="combo-rom-slug", - fs_name="Super Mario All-Stars + Super Mario World (USA).sfc", - fs_name_no_tags="Super Mario All-Stars + Super Mario World", - fs_name_no_ext="Super Mario All-Stars + Super Mario World (USA)", - fs_extension="sfc", - fs_path=f"{platform.slug}/roms", - ) - combo_rom = db_rom_handler.add_rom(combo_rom) - db_rom_handler.add_rom_user(rom_id=combo_rom.id, user_id=admin_user.id) - - saves_path = fs_asset_handler.build_saves_file_path( - user=admin_user, - platform_fs_slug="test_platform_slug", - rom_id=combo_rom.id, - emulator="snes9x", - ) - mock_scan_save.return_value = Save( - file_name="Super Mario All-Stars + Super Mario World (USA).srm", - file_path=saves_path, - file_size_bytes=4, - content_hash="8d777f385d3dfec8815d20f7496026dc", - ) - - response = client.put( - "/api/cloud-sync/saves/Snes9x/Super Mario All-Stars + Super Mario World (USA).srm", - content=b"data", - auth=ADMIN_AUTH, - ) - - assert response.status_code == status.HTTP_201_CREATED - - saves = db_save_handler.get_saves(user_id=admin_user.id, rom_id=combo_rom.id) - assert len(saves) == 1 - assert saves[0].file_name == "Super Mario All-Stars + Super Mario World (USA).srm" - def test_rejects_unsupported_sync_root(self, client, admin_user: User, rom: Rom): response = client.put( "/api/cloud-sync/deleted/saves/test_rom.srm",