Merge pull request #3479 from tmgast/fix/save-device-attribution

Expose per-device save sync attribution and origin device

Author: gantoine

backend/alembic/versions/0082_save_origin_device.py

@@ -0,0 +1,43 @@
+"""Track the device that created a save
+
+Revision ID: 0082_save_origin_device
+Revises: 0081_add_archive_members
+Create Date: 2026-06-05 00:00:00.000000
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+revision = "0082_save_origin_device"
+down_revision = "0081_add_archive_members"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    # Order matters on MariaDB: the index must exist before the FK (it backs the
+    # constraint), so create column -> index -> FK.
+    with op.batch_alter_table("saves", schema=None) as batch_op:
+        batch_op.add_column(
+            sa.Column("origin_device_id", sa.String(length=255), nullable=True),
+            if_not_exists=True,
+        )
+        batch_op.create_index(
+            "ix_saves_origin_device_id", ["origin_device_id"], if_not_exists=True
+        )
+        batch_op.create_foreign_key(
+            "fk_saves_origin_device_id",
+            "devices",
+            ["origin_device_id"],
+            ["id"],
+            ondelete="SET NULL",
+        )
+
+
+def downgrade() -> None:
+    # Drop the FK before the index it backs, then the column.
+    with op.batch_alter_table("saves", schema=None) as batch_op:
+        batch_op.drop_constraint("fk_saves_origin_device_id", type_="foreignkey")
+        batch_op.drop_index("ix_saves_origin_device_id", if_exists=True)
+        batch_op.drop_column("origin_device_id", if_exists=True)

backend/endpoints/responses/assets.py

@@ -38,6 +38,7 @@ class SaveSchema(BaseAsset):
     slot: str | None = None
     content_hash: str | None = None
     screenshot: ScreenshotSchema | None
+    origin_device_id: str | None = None
     device_syncs: list[DeviceSyncSchema] = []
 
     @model_validator(mode="before")

backend/endpoints/saves.py

@@ -1,5 +1,6 @@
 import os
 import re
+from collections.abc import Sequence
 from datetime import datetime, timezone
 from typing import Annotated
 
@@ -34,34 +35,66 @@
 
 def _build_save_schema(
     save: Save,
+    syncs: Sequence[tuple[DeviceSaveSync, str | None]] = (),
     device: Device | None = None,
-    sync: DeviceSaveSync | None = None,
 ) -> SaveSchema:
+    """Attach one ``DeviceSyncSchema`` per device that has synced this save.
+
+    ``syncs`` is the full list of sync rows (paired with device name) for this
+    save across every device, so clients can attribute the save to its creator.
+    ``device`` is the caller's device, when supplied: its entry is emitted first
+    for stable ordering and old-client compatibility, and a placeholder entry is
+    synthesized when the caller has not yet synced this save.
+    """
     save_schema = SaveSchema.model_validate(save)
 
-    if device:
-        if sync:
-            is_current = to_utc(sync.last_synced_at) >= to_utc(save.updated_at)
-            last_synced = sync.last_synced_at
-            is_untracked = sync.is_untracked
-        else:
-            is_current = False
-            last_synced = save.updated_at
-            is_untracked = False
+    save_updated = to_utc(save.updated_at)
+    caller_present = False
+    entries: list[DeviceSyncSchema] = []
+    for sync, device_name in syncs:
+        if device and sync.device_id == device.id:
+            caller_present = True
+        entries.append(
+            DeviceSyncSchema(
+                device_id=sync.device_id,
+                device_name=device_name,
+                last_synced_at=sync.last_synced_at,
+                is_untracked=sync.is_untracked,
+                is_current=to_utc(sync.last_synced_at) >= save_updated,
+            )
+        )
 
-        save_schema.device_syncs = [
+    if device and not caller_present:
+        entries.append(
             DeviceSyncSchema(
                 device_id=device.id,
                 device_name=device.name,
-                last_synced_at=last_synced,
-                is_untracked=is_untracked,
-                is_current=is_current,
+                last_synced_at=save.updated_at,
+                is_untracked=False,
+                is_current=False,
             )
-        ]
+        )
 
+    if device:
+        entries.sort(key=lambda entry: entry.device_id != device.id)
+
+    save_schema.device_syncs = entries
     return save_schema
 
 
+def _syncs_for_save(
+    save_id: int, device: Device | None
+) -> list[tuple[DeviceSaveSync, str | None]]:
+    """Fetch every device sync for a single save when a device is in context.
+
+    Device attribution is only meaningful to a device-scoped caller, so callers
+    without a device get an empty list and no query is issued.
+    """
+    if not device:
+        return []
+    return db_device_save_sync_handler.get_syncs_for_saves([save_id]).get(save_id, [])
+
+
 DATETIME_TAG_PATTERN = re.compile(r" \[\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}\]")
 
 
@@ -229,12 +262,9 @@ async def add_save(
                 await fs_asset_handler.remove_file(f"{saves_path}/{actual_filename}")
             except FileNotFoundError:
                 pass
-            sync = None
-            if device:
-                sync = db_device_save_sync_handler.get_sync(
-                    device_id=device.id, save_id=existing_by_hash.id
-                )
-            return _build_save_schema(existing_by_hash, device, sync)
+            return _build_save_schema(
+                existing_by_hash, _syncs_for_save(existing_by_hash.id, device), device
+            )
 
     if db_save:
         update_data: dict = {
@@ -249,6 +279,7 @@ async def add_save(
         scanned_save.user_id = request.user.id
         scanned_save.emulator = emulator
         scanned_save.slot = slot
+        scanned_save.origin_device_id = device.id if device else None
         db_save = db_save_handler.add_save(save=scanned_save)
 
     if device:
@@ -322,12 +353,7 @@ async def add_save(
         rom_user.id, {"last_played": datetime.now(timezone.utc)}
     )
 
-    sync = None
-    if device:
-        sync = db_device_save_sync_handler.get_sync(
-            device_id=device.id, save_id=db_save.id
-        )
-    return _build_save_schema(db_save, device, sync)
+    return _build_save_schema(db_save, _syncs_for_save(db_save.id, device), device)
 
 
 @protected_route(router.get, "", [Scope.ASSETS_READ])
@@ -350,13 +376,13 @@ def get_saves(
     if not device:
         return [_build_save_schema(save) for save in saves]
 
-    syncs = db_device_save_sync_handler.get_syncs_for_device_and_saves(
-        device_id=device.id, save_ids=[s.id for s in saves]
+    syncs_by_save_id = db_device_save_sync_handler.get_syncs_for_saves(
+        [s.id for s in saves]
     )
-    sync_by_save_id = {s.save_id: s for s in syncs}
 
     return [
-        _build_save_schema(save, device, sync_by_save_id.get(save.id)) for save in saves
+        _build_save_schema(save, syncs_by_save_id.get(save.id, []), device)
+        for save in saves
     ]
 
 
@@ -404,12 +430,7 @@ def get_save(request: Request, id: int, device_id: str | None = None) -> SaveSch
             detail=f"Save with ID {id} not found",
         )
 
-    sync = None
-    if device:
-        sync = db_device_save_sync_handler.get_sync(
-            device_id=device.id, save_id=save.id
-        )
-    return _build_save_schema(save, device, sync)
+    return _build_save_schema(save, _syncs_for_save(save.id, device), device)
 
 
 @protected_route(router.get, "/{id}/content", [Scope.ASSETS_READ])
@@ -475,14 +496,14 @@ def confirm_download(
         )
 
     device = _resolve_device(device_id, request.user.id)
-    sync = db_device_save_sync_handler.upsert_sync(
+    db_device_save_sync_handler.upsert_sync(
         device_id=device_id,
         save_id=save.id,
         synced_at=save.updated_at,
     )
     db_device_handler.update_last_seen(device_id=device_id, user_id=request.user.id)
 
-    return _build_save_schema(save, device, sync)
+    return _build_save_schema(save, _syncs_for_save(save.id, device), device)
 
 
 @protected_route(router.put, "/{id}", [Scope.ASSETS_WRITE])
@@ -581,12 +602,7 @@ async def update_save(
         )
         db_device_handler.update_last_seen(device_id=device.id, user_id=request.user.id)
 
-    sync = None
-    if device:
-        sync = db_device_save_sync_handler.get_sync(
-            device_id=device.id, save_id=db_save.id
-        )
-    return _build_save_schema(db_save, device, sync)
+    return _build_save_schema(db_save, _syncs_for_save(db_save.id, device), device)
 
 
 @protected_route(
@@ -661,11 +677,11 @@ def track_save(
         )
 
     device = _resolve_device(device_id, request.user.id)
-    sync = db_device_save_sync_handler.set_untracked(
+    db_device_save_sync_handler.set_untracked(
         device_id=device_id, save_id=id, untracked=False
     )
 
-    return _build_save_schema(save, device, sync)
+    return _build_save_schema(save, _syncs_for_save(save.id, device), device)
 
 
 @protected_route(router.post, "/{id}/untrack", [Scope.DEVICES_WRITE])
@@ -683,8 +699,8 @@ def untrack_save(
         )
 
     device = _resolve_device(device_id, request.user.id)
-    sync = db_device_save_sync_handler.set_untracked(
+    db_device_save_sync_handler.set_untracked(
         device_id=device_id, save_id=id, untracked=True
     )
 
-    return _build_save_schema(save, device, sync)
+    return _build_save_schema(save, _syncs_for_save(save.id, device), device)

backend/handler/database/device_save_sync_handler.py

@@ -5,6 +5,7 @@
 from sqlalchemy.orm import Session
 
 from decorators.database import begin_session
+from models.device import Device
 from models.device_save_sync import DeviceSaveSync
 
 from .base_handler import DBBaseHandler
@@ -40,6 +41,31 @@ def get_syncs_for_device_and_saves(
             )
         ).all()
 
+    @begin_session
+    def get_syncs_for_saves(
+        self,
+        save_ids: list[int],
+        session: Session = None,  # type: ignore
+    ) -> dict[int, list[tuple[DeviceSaveSync, str | None]]]:
+        """Fetch every device sync row for the given saves, grouped by save id.
+
+        Each row is paired with its device name so callers can attribute a save
+        to the device that created it without triggering the lazy-raise
+        ``DeviceSaveSync.device`` relationship.
+        """
+        if not save_ids:
+            return {}
+        rows = session.execute(
+            select(DeviceSaveSync, Device.name)
+            .join(Device, DeviceSaveSync.device_id == Device.id)
+            .filter(DeviceSaveSync.save_id.in_(save_ids))
+            .order_by(DeviceSaveSync.last_synced_at.desc(), DeviceSaveSync.device_id)
+        ).all()
+        grouped: dict[int, list[tuple[DeviceSaveSync, str | None]]] = {}
+        for sync, device_name in rows:
+            grouped.setdefault(sync.save_id, []).append((sync, device_name))
+        return grouped
+
     @begin_session
     def upsert_sync(
         self,

backend/models/assets.py

@@ -65,6 +65,11 @@ class Save(RomAsset):
     emulator: Mapped[str | None] = mapped_column(String(length=50))
     slot: Mapped[str | None] = mapped_column(String(length=255))
     content_hash: Mapped[str | None] = mapped_column(String(length=32))
+    origin_device_id: Mapped[str | None] = mapped_column(
+        String(length=255),
+        ForeignKey("devices.id", ondelete="SET NULL"),
+        default=None,
+    )
 
     rom: Mapped[Rom] = relationship(lazy="joined", back_populates="saves")
     user: Mapped[User] = relationship(lazy="joined", back_populates="saves")