mirror of
https://github.com/rommapp/romm.git
synced 2026-10-12 02:21:58 +00:00
fix(sync): serve the newest on-disk RetroArch state in a slot (#5193)
* fix(sync): resolve RetroArch paths across emulators sharing a core folder Several RomM emulator ids map to one RetroArch folder (mednafen_psx_hw, beetle_psx_hw and the raw "Beetle PSX HW" all sync through saves/Beetle PSX HW/). The manifest built paths from whichever id an asset carried, but GET parsed the folder back to the canonical id and matched it exactly, so an asset filed under another alias was advertised and then 404'd. Uploads also started a second autosave history beside the alias's. The manifest now lists the newest asset per folder, and GET, PUT and DELETE resolve a path across every alias. Rewrites of an existing unslotted save or state stay under that row's own emulator. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * fix(sync): prefer on-disk alias saves and share newest-per-key grouping pr-ready step 2 (code-review --fix). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * refactor(sync): one alias helper and a lineage computed once pr-ready step 3 (simplify). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * test(sync): type the alias fixture's asset kind pr-ready step 4 (review-polish). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * test(sync): isolate the asset dir per RetroArch sync test The alias tests write different bytes to the same asset paths, and user and ROM ids repeat across xdist worker databases, so parallel runs raced on those files. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * fix(sync): serve the newest on-disk RetroArch state in a slot Missing states are dropped before slot grouping for the manifest and reads, so a newer missing state (or alias) no longer hides an older one still on disk. A PUT still matches the missing row and revives it in place. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * fix(sync): fall back past a RetroArch file that vanished unflagged Addresses Greptile's review on #5193: a state whose file was deleted before `missing_from_fs` was set still won its slot. The manifest now rebuilds when hashing flags an asset, and GET flags a missing file and resolves again, so an older asset at the path is listed and served. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
57ebac8efe
commit
be6ad905b1
@@ -444,6 +444,13 @@ async def retroarch_sync_get(request: Request, file_path: str) -> Response:
|
||||
return _empty(status.HTTP_404_NOT_FOUND)
|
||||
|
||||
asset = await _get_asset(request.user, rom, parsed)
|
||||
# A file deleted outside RomM hides any older asset at this path until flagged.
|
||||
while (
|
||||
asset
|
||||
and not asset.missing_from_fs
|
||||
and await asyncio.to_thread(sync_handler.flag_if_missing, asset)
|
||||
):
|
||||
asset = await _get_asset(request.user, rom, parsed)
|
||||
if not asset:
|
||||
return _empty(status.HTTP_404_NOT_FOUND)
|
||||
|
||||
@@ -581,6 +588,7 @@ async def retroarch_sync_put(request: Request, file_path: str) -> Response:
|
||||
rom,
|
||||
parsed.emulator,
|
||||
file_name,
|
||||
prefer_on_disk=False,
|
||||
)
|
||||
write_file_name = existing.file_name if existing else file_name
|
||||
emulator = existing.emulator if existing else parsed.emulator
|
||||
|
||||
@@ -221,12 +221,21 @@ def states_in_slot(
|
||||
|
||||
|
||||
def resolve_state_by_slot(
|
||||
user: User, rom: Rom, emulator: str | None, requested_file_name: str
|
||||
user: User,
|
||||
rom: Rom,
|
||||
emulator: str | None,
|
||||
requested_file_name: str,
|
||||
prefer_on_disk: bool = True,
|
||||
) -> State | None:
|
||||
"""The state the slot a canonical name points at serves, whatever its name."""
|
||||
"""The state the slot a canonical name points at serves, whatever its name.
|
||||
|
||||
Args:
|
||||
prefer_on_disk: False ranks a `missing_from_fs` row by recency alone, so
|
||||
an upload revives the newest state in place.
|
||||
"""
|
||||
return max(
|
||||
states_in_slot(user, rom, emulator, requested_file_name),
|
||||
key=on_disk_recency_key,
|
||||
key=on_disk_recency_key if prefer_on_disk else recency_key,
|
||||
default=None,
|
||||
)
|
||||
|
||||
@@ -631,6 +640,22 @@ def _mark_missing_from_fs(asset: Save | State | Screenshot) -> None:
|
||||
db_state_handler.update_state(asset.id, update, touch=False)
|
||||
else:
|
||||
db_screenshot_handler.update_screenshot(asset.id, update)
|
||||
asset.missing_from_fs = True
|
||||
|
||||
|
||||
def flag_if_missing(asset: Save | State | Screenshot) -> bool:
|
||||
"""Flag ``asset`` `missing_from_fs` when its file is gone.
|
||||
|
||||
Returns:
|
||||
Whether it was gone.
|
||||
"""
|
||||
try:
|
||||
if fs_asset_handler.validate_path(asset.full_path).is_file():
|
||||
return False
|
||||
except ValueError:
|
||||
return False
|
||||
_mark_missing_from_fs(asset)
|
||||
return True
|
||||
|
||||
|
||||
async def _asset_md5(asset: Save | State | Screenshot) -> str | None:
|
||||
@@ -760,8 +785,12 @@ async def build_manifest(
|
||||
"""The server manifest RetroArch diffs against, sorted by path."""
|
||||
# Off the event loop: on a large library the queries run for seconds and
|
||||
# would stall every other request the worker is serving.
|
||||
assets, saves = await asyncio.to_thread(_manifest_assets, user, can_see)
|
||||
digests = await asset_md5s([entry.asset for entry in assets])
|
||||
while True:
|
||||
assets, saves = await asyncio.to_thread(_manifest_assets, user, can_see)
|
||||
digests = await asset_md5s([entry.asset for entry in assets])
|
||||
# Hashing flags a vanished file, which can unhide an older asset at its path.
|
||||
if not any(entry.asset.missing_from_fs for entry in assets):
|
||||
break
|
||||
hashed = [
|
||||
(entry, digest) for entry, digest in zip(assets, digests, strict=True) if digest
|
||||
]
|
||||
|
||||
@@ -1214,15 +1214,18 @@ class TestRetroArchSyncCoreAliases:
|
||||
)
|
||||
|
||||
def _state(
|
||||
self, admin_user: User, rom: Rom, emulator: str, content: bytes
|
||||
self,
|
||||
admin_user: User,
|
||||
rom: Rom,
|
||||
emulator: str,
|
||||
content: bytes,
|
||||
file_name: str = "test_rom.state",
|
||||
) -> State:
|
||||
path = self._write(
|
||||
admin_user, rom, "states", emulator, "test_rom.state", content
|
||||
)
|
||||
path = self._write(admin_user, rom, "states", emulator, file_name, content)
|
||||
return factories.make_state(
|
||||
rom,
|
||||
admin_user,
|
||||
"test_rom.state",
|
||||
file_name,
|
||||
file_path=path,
|
||||
file_size_bytes=len(content),
|
||||
emulator=emulator,
|
||||
@@ -1348,16 +1351,87 @@ class TestRetroArchSyncCoreAliases:
|
||||
assert self._manifest_paths(client) == ["states/Beetle PSX HW/test_rom.state"]
|
||||
assert client.get(self.STATE_URL, auth=ADMIN_AUTH).content == b"newer"
|
||||
|
||||
def _missing_state(
|
||||
self, admin_user: User, rom: Rom, emulator: str, file_name: str
|
||||
) -> State:
|
||||
state = self._state(admin_user, rom, emulator, b"gone", file_name)
|
||||
return db_state_handler.update_state(
|
||||
state.id, {"missing_from_fs": True}, touch=False
|
||||
)
|
||||
|
||||
@_mock_asset_md5()
|
||||
def test_a_state_on_disk_wins_over_a_newer_missing_one_in_its_slot(
|
||||
self, _asset_md5: mock.AsyncMock, client, admin_user: User, rom: Rom
|
||||
):
|
||||
self._state(admin_user, rom, "beetle_psx_hw", b"older")
|
||||
self._missing_state(
|
||||
admin_user, rom, "beetle_psx_hw", "test_rom [2026-01-01 00-00-00].state"
|
||||
)
|
||||
|
||||
assert self._manifest_paths(client) == ["states/Beetle PSX HW/test_rom.state"]
|
||||
assert client.get(self.STATE_URL, auth=ADMIN_AUTH).content == b"older"
|
||||
|
||||
@_mock_asset_md5()
|
||||
def test_a_state_on_disk_wins_over_a_newer_missing_alias(
|
||||
self, _asset_md5: mock.AsyncMock, client, admin_user: User, rom: Rom
|
||||
):
|
||||
self._state(admin_user, rom, "beetle_psx_hw", b"argosy")
|
||||
missing = self._state(admin_user, rom, "mednafen_psx_hw", b"gone")
|
||||
db_state_handler.update_state(missing.id, {"missing_from_fs": True})
|
||||
self._state(admin_user, rom, "beetle_psx_hw", b"older")
|
||||
self._missing_state(admin_user, rom, "mednafen_psx_hw", "test_rom.state")
|
||||
|
||||
assert self._manifest_paths(client) == ["states/Beetle PSX HW/test_rom.state"]
|
||||
assert client.get(self.STATE_URL, auth=ADMIN_AUTH).content == b"argosy"
|
||||
assert client.get(self.STATE_URL, auth=ADMIN_AUTH).content == b"older"
|
||||
|
||||
def _vanished_state(self, admin_user: User, rom: Rom, emulator: str) -> State:
|
||||
state = self._state(admin_user, rom, emulator, b"gone")
|
||||
fs_asset_handler.validate_path(state.full_path).unlink()
|
||||
return state
|
||||
|
||||
def test_the_manifest_falls_back_when_the_newest_state_file_vanished(
|
||||
self, client, admin_user: User, rom: Rom
|
||||
):
|
||||
self._state(admin_user, rom, "beetle_psx_hw", b"older")
|
||||
vanished = self._vanished_state(admin_user, rom, "mednafen_psx_hw")
|
||||
|
||||
assert self._manifest_paths(client) == ["states/Beetle PSX HW/test_rom.state"]
|
||||
state = db_state_handler.get_state(user_id=admin_user.id, id=vanished.id)
|
||||
assert state is not None and state.missing_from_fs
|
||||
|
||||
def test_get_falls_back_when_the_newest_state_file_vanished(
|
||||
self, client, admin_user: User, rom: Rom
|
||||
):
|
||||
self._state(admin_user, rom, "beetle_psx_hw", b"older")
|
||||
vanished = self._vanished_state(admin_user, rom, "mednafen_psx_hw")
|
||||
|
||||
assert client.get(self.STATE_URL, auth=ADMIN_AUTH).content == b"older"
|
||||
state = db_state_handler.get_state(user_id=admin_user.id, id=vanished.id)
|
||||
assert state is not None and state.missing_from_fs
|
||||
|
||||
def test_get_404s_when_every_state_file_in_the_slot_vanished(
|
||||
self, client, admin_user: User, rom: Rom
|
||||
):
|
||||
self._vanished_state(admin_user, rom, "beetle_psx_hw")
|
||||
self._vanished_state(admin_user, rom, "mednafen_psx_hw")
|
||||
|
||||
response = client.get(self.STATE_URL, auth=ADMIN_AUTH)
|
||||
|
||||
assert response.status_code == status.HTTP_404_NOT_FOUND
|
||||
|
||||
def test_uploading_to_a_slot_revives_its_newest_missing_state(
|
||||
self, client, admin_user: User, rom: Rom
|
||||
):
|
||||
older = self._state(admin_user, rom, "beetle_psx_hw", b"older")
|
||||
missing = self._missing_state(
|
||||
admin_user, rom, "mednafen_psx_hw", "test_rom.state"
|
||||
)
|
||||
|
||||
response = client.put(self.STATE_URL, content=b"fresh", auth=ADMIN_AUTH)
|
||||
|
||||
assert response.status_code == status.HTTP_204_NO_CONTENT
|
||||
states = {s.id: s for s in db_state_handler.get_states(user_id=admin_user.id)}
|
||||
assert states.keys() == {older.id, missing.id}
|
||||
assert not states[missing.id].missing_from_fs
|
||||
assert states[missing.id].file_size_bytes == len(b"fresh")
|
||||
assert client.get(self.STATE_URL, auth=ADMIN_AUTH).content == b"fresh"
|
||||
|
||||
@_mock_asset_md5()
|
||||
def test_delete_removes_states_under_every_alias_in_the_slot(
|
||||
|
||||
@@ -1033,7 +1033,10 @@ World`), so a name shared across platforms resolves to the lowest visible ROM id
|
||||
claims it in Redis, so concurrent or same-second uploads from any endpoint
|
||||
never share a file. Every slot prune is scoped to the uploaded save's lineage.
|
||||
- **Manifest:** assets whose file is gone are flagged `missing_from_fs`. Hashes
|
||||
are MD5s of the bytes on disk, Redis-cached by path, size and mtime.
|
||||
are MD5s of the bytes on disk, Redis-cached by path, size and mtime. A state
|
||||
slot lists and serves its newest state on disk, and a PUT revives a newer
|
||||
missing one in place. A manifest or GET that finds a file gone flags it and
|
||||
resolves again, so an older asset at that path takes its place.
|
||||
- **PSP:** PPSSPP's `PSP/SAVEDATA/<folder>/` files are stored as one zipped
|
||||
`Save` per folder. A folder whose title matches no ROM is buffered in the
|
||||
user's `retroarch/psp_pending/` folder until it resolves, or mapped through
|
||||
|
||||
Reference in New Issue
Block a user