mirror of
https://github.com/rommapp/romm.git
synced 2026-06-27 22:35:57 +00:00
SaveSync: push null-slot exclusion into the SQL query
Three sync callsites (endpoints/sync.py, sync_watcher.py, and both branches of tasks/sync_push_pull_task.py) ran get_saves(...) and then discarded archival null-slot rows in a Python list comprehension. On libraries with many archival/web-UI uploads that's a strict waste: those rows are pulled from MariaDB, hydrated into Save model instances, and then immediately filtered out. Add a slot_not_null bool kwarg to DBSavesHandler.get_saves and apply the filter in the SQL query. Update all four callsites to use it and drop the Python-side comprehension. Default stays False so unrelated callers keep the current behavior.
This commit is contained in:
@@ -118,12 +118,11 @@ def negotiate_sync(
|
||||
# Build a set of server saves for this user, keyed by (rom_id, file_name).
|
||||
# Only slot-bound saves participate in sync; null-slot saves are web-UI or
|
||||
# archival uploads and act as backups (clients can opt-in to import them
|
||||
# outside the sync flow).
|
||||
server_saves = [
|
||||
s
|
||||
for s in db_save_handler.get_saves(user_id=request.user.id)
|
||||
if s.slot is not None
|
||||
]
|
||||
# outside the sync flow). Filtering in SQL avoids materializing archival
|
||||
# rows we'd immediately discard.
|
||||
server_saves = db_save_handler.get_saves(
|
||||
user_id=request.user.id, slot_not_null=True
|
||||
)
|
||||
server_save_map: dict[tuple[int, str], Save] = {}
|
||||
for save in server_saves:
|
||||
server_save_map[(save.rom_id, save.file_name)] = save
|
||||
|
||||
@@ -68,6 +68,7 @@ class DBSavesHandler(DBBaseHandler):
|
||||
rom_id: int | None = None,
|
||||
platform_id: int | None = None,
|
||||
slot: str | None = None,
|
||||
slot_not_null: bool = False,
|
||||
order_by: Literal["updated_at", "created_at"] | None = None,
|
||||
order_dir: Literal["asc", "desc"] = "desc",
|
||||
only_fields: Sequence[QueryableAttribute] | None = None,
|
||||
@@ -86,6 +87,9 @@ class DBSavesHandler(DBBaseHandler):
|
||||
if slot is not None:
|
||||
query = query.filter(Save.slot == slot)
|
||||
|
||||
if slot_not_null:
|
||||
query = query.filter(Save.slot.is_not(None))
|
||||
|
||||
if order_by:
|
||||
order_col = getattr(Save, order_by)
|
||||
order_fn = asc if order_dir == "asc" else desc
|
||||
|
||||
@@ -224,15 +224,13 @@ def _process_incoming_file(
|
||||
|
||||
# Try to find matching saves on this platform for this user. Only
|
||||
# slot-bound saves participate in sync; null-slot saves are web-UI /
|
||||
# archival uploads and must never be paired with a device push.
|
||||
saves_on_platform = [
|
||||
s
|
||||
for s in db_save_handler.get_saves(
|
||||
user_id=device.user_id,
|
||||
platform_id=platform.id,
|
||||
)
|
||||
if s.slot is not None
|
||||
]
|
||||
# archival uploads and must never be paired with a device push. Filter
|
||||
# in SQL so archival rows never load.
|
||||
saves_on_platform = db_save_handler.get_saves(
|
||||
user_id=device.user_id,
|
||||
platform_id=platform.id,
|
||||
slot_not_null=True,
|
||||
)
|
||||
|
||||
matched_save = None
|
||||
for save in saves_on_platform:
|
||||
|
||||
@@ -237,14 +237,10 @@ async def _process_remote_save(
|
||||
|
||||
# Find matching server save. Only slot-bound saves participate in sync;
|
||||
# null-slot saves are web-UI / archival uploads and must never be paired
|
||||
# with a device push.
|
||||
saves = [
|
||||
s
|
||||
for s in db_save_handler.get_saves(
|
||||
user_id=device.user_id, platform_id=platform.id
|
||||
)
|
||||
if s.slot is not None
|
||||
]
|
||||
# with a device push. Filter in SQL so archival rows never load.
|
||||
saves = db_save_handler.get_saves(
|
||||
user_id=device.user_id, platform_id=platform.id, slot_not_null=True
|
||||
)
|
||||
matched_save = None
|
||||
for save in saves:
|
||||
if save.file_name == remote_save.file_name:
|
||||
@@ -372,13 +368,10 @@ async def _push_missing_saves(
|
||||
|
||||
# Only slot-bound saves participate in sync; null-slot saves are
|
||||
# web-UI / archival uploads and must never be pushed to a device.
|
||||
server_saves = [
|
||||
s
|
||||
for s in db_save_handler.get_saves(
|
||||
user_id=device.user_id, platform_id=platform.id
|
||||
)
|
||||
if s.slot is not None
|
||||
]
|
||||
# Filter in SQL so archival rows never load.
|
||||
server_saves = db_save_handler.get_saves(
|
||||
user_id=device.user_id, platform_id=platform.id, slot_not_null=True
|
||||
)
|
||||
|
||||
remote_set = remote_files.get(platform_slug, set())
|
||||
remote_dir = platform_paths.get(platform_slug, "")
|
||||
|
||||
@@ -444,6 +444,127 @@ class TestDBSavesHandlerGetSaveByContentHash:
|
||||
assert result.id == created.id
|
||||
|
||||
|
||||
class TestDBSavesHandlerSlotNotNullFilter:
|
||||
"""Pin the slot_not_null kwarg: when true, archival/null-slot saves are
|
||||
excluded; when false (default), they are returned alongside slot-bound
|
||||
rows. Sync callsites use slot_not_null=True so the database does the
|
||||
filter instead of pulling the whole table into Python."""
|
||||
|
||||
def test_slot_not_null_false_returns_both(self, admin_user: User, rom: Rom):
|
||||
slot_save = Save(
|
||||
rom_id=rom.id,
|
||||
user_id=admin_user.id,
|
||||
file_name="slotted.sav",
|
||||
file_name_no_tags="slotted",
|
||||
file_name_no_ext="slotted",
|
||||
file_extension="sav",
|
||||
emulator="test_emu",
|
||||
file_path=f"{rom.platform_slug}/saves",
|
||||
file_size_bytes=100,
|
||||
slot="autosave",
|
||||
)
|
||||
archival_save = Save(
|
||||
rom_id=rom.id,
|
||||
user_id=admin_user.id,
|
||||
file_name="archival.sav",
|
||||
file_name_no_tags="archival",
|
||||
file_name_no_ext="archival",
|
||||
file_extension="sav",
|
||||
emulator="test_emu",
|
||||
file_path=f"{rom.platform_slug}/saves",
|
||||
file_size_bytes=100,
|
||||
slot=None,
|
||||
)
|
||||
db_save_handler.add_save(slot_save)
|
||||
db_save_handler.add_save(archival_save)
|
||||
|
||||
saves = db_save_handler.get_saves(user_id=admin_user.id, rom_id=rom.id)
|
||||
|
||||
names = {s.file_name for s in saves}
|
||||
assert "slotted.sav" in names
|
||||
assert "archival.sav" in names
|
||||
|
||||
def test_slot_not_null_true_excludes_null_slot(self, admin_user: User, rom: Rom):
|
||||
slot_save = Save(
|
||||
rom_id=rom.id,
|
||||
user_id=admin_user.id,
|
||||
file_name="slotted_only.sav",
|
||||
file_name_no_tags="slotted_only",
|
||||
file_name_no_ext="slotted_only",
|
||||
file_extension="sav",
|
||||
emulator="test_emu",
|
||||
file_path=f"{rom.platform_slug}/saves",
|
||||
file_size_bytes=100,
|
||||
slot="autosave",
|
||||
)
|
||||
archival_save = Save(
|
||||
rom_id=rom.id,
|
||||
user_id=admin_user.id,
|
||||
file_name="archival_only.sav",
|
||||
file_name_no_tags="archival_only",
|
||||
file_name_no_ext="archival_only",
|
||||
file_extension="sav",
|
||||
emulator="test_emu",
|
||||
file_path=f"{rom.platform_slug}/saves",
|
||||
file_size_bytes=100,
|
||||
slot=None,
|
||||
)
|
||||
db_save_handler.add_save(slot_save)
|
||||
db_save_handler.add_save(archival_save)
|
||||
|
||||
saves = db_save_handler.get_saves(
|
||||
user_id=admin_user.id, rom_id=rom.id, slot_not_null=True
|
||||
)
|
||||
|
||||
names = {s.file_name for s in saves}
|
||||
assert "slotted_only.sav" in names
|
||||
assert "archival_only.sav" not in names
|
||||
assert all(s.slot is not None for s in saves)
|
||||
|
||||
def test_slot_not_null_true_composes_with_slot_value(
|
||||
self, admin_user: User, rom: Rom
|
||||
):
|
||||
"""slot_not_null and slot can both be set; the explicit slot filter
|
||||
wins (and is implicitly not-null), so slot_not_null=True is a no-op
|
||||
in that case. Pin that behavior so callers don't have to reason
|
||||
about the interaction."""
|
||||
db_save_handler.add_save(
|
||||
Save(
|
||||
rom_id=rom.id,
|
||||
user_id=admin_user.id,
|
||||
file_name="compose_a.sav",
|
||||
file_name_no_tags="compose_a",
|
||||
file_name_no_ext="compose_a",
|
||||
file_extension="sav",
|
||||
emulator="test_emu",
|
||||
file_path=f"{rom.platform_slug}/saves",
|
||||
file_size_bytes=100,
|
||||
slot="A",
|
||||
)
|
||||
)
|
||||
db_save_handler.add_save(
|
||||
Save(
|
||||
rom_id=rom.id,
|
||||
user_id=admin_user.id,
|
||||
file_name="compose_b.sav",
|
||||
file_name_no_tags="compose_b",
|
||||
file_name_no_ext="compose_b",
|
||||
file_extension="sav",
|
||||
emulator="test_emu",
|
||||
file_path=f"{rom.platform_slug}/saves",
|
||||
file_size_bytes=100,
|
||||
slot="B",
|
||||
)
|
||||
)
|
||||
|
||||
saves = db_save_handler.get_saves(
|
||||
user_id=admin_user.id, rom_id=rom.id, slot="A", slot_not_null=True
|
||||
)
|
||||
|
||||
assert len(saves) == 1
|
||||
assert saves[0].slot == "A"
|
||||
|
||||
|
||||
class TestDBSavesHandlerGetSavesAfterId:
|
||||
"""Cover keyset pagination used by the recompute_save_content_hashes
|
||||
maintenance task. Per-page bounded reads avoid materializing the full
|
||||
|
||||
Reference in New Issue
Block a user