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

Expose per-device save sync attribution and origin device

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 @@ from utils.router import APIRouter
 
 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 import delete, select, update
 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 @@ class DBDeviceSaveSyncHandler(DBBaseHandler):
             )
         ).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")

backend/tests/endpoints/test_saves.py

@@ -89,6 +89,109 @@ class TestSaveSyncEndpoints:
         assert data[0]["device_syncs"][0]["is_untracked"] is False
         assert data[0]["device_syncs"][0]["is_current"] is True
 
+    def test_get_saves_lists_all_device_syncs(
+        self, client, access_token: str, admin_user: User, save: Save, device: Device
+    ):
+        creator = db_device_handler.add_device(
+            Device(id="creator-device", user_id=admin_user.id, name="Creator Device")
+        )
+        # Creator synced at save creation: stays current. Caller is stale.
+        db_device_save_sync_handler.upsert_sync(
+            device_id=creator.id, save_id=save.id, synced_at=save.updated_at
+        )
+        db_device_save_sync_handler.upsert_sync(
+            device_id=device.id,
+            save_id=save.id,
+            synced_at=save.updated_at - timedelta(days=1),
+        )
+
+        response = client.get(
+            f"/api/saves?device_id={device.id}",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        syncs = data[0]["device_syncs"]
+        assert len(syncs) == 2
+
+        # Caller's own entry is emitted first for old-client compatibility.
+        assert syncs[0]["device_id"] == device.id
+        assert syncs[0]["is_current"] is False
+
+        by_id = {s["device_id"]: s for s in syncs}
+        assert by_id[creator.id]["device_name"] == "Creator Device"
+        assert by_id[creator.id]["is_current"] is True
+
+    def test_get_saves_without_device_id_omits_device_syncs(
+        self, client, access_token: str, admin_user: User, save: Save, device: Device
+    ):
+        creator = db_device_handler.add_device(
+            Device(id="creator-device", user_id=admin_user.id, name="Creator Device")
+        )
+        db_device_save_sync_handler.upsert_sync(
+            device_id=creator.id, save_id=save.id, synced_at=save.updated_at
+        )
+        db_device_save_sync_handler.upsert_sync(device_id=device.id, save_id=save.id)
+
+        response = client.get(
+            "/api/saves",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        # Device attribution is only returned to a device-scoped caller, so a
+        # request without device_id omits device_syncs even when syncs exist.
+        assert data[0]["device_syncs"] == []
+
+    def test_get_single_save_lists_all_device_syncs(
+        self, client, access_token: str, admin_user: User, save: Save, device: Device
+    ):
+        creator = db_device_handler.add_device(
+            Device(id="creator-device", user_id=admin_user.id, name="Creator Device")
+        )
+        db_device_save_sync_handler.upsert_sync(
+            device_id=creator.id, save_id=save.id, synced_at=save.updated_at
+        )
+
+        response = client.get(
+            f"/api/saves/{save.id}?device_id={device.id}",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        syncs = data["device_syncs"]
+        # Caller (no sync row yet) plus the creator device.
+        assert {s["device_id"] for s in syncs} == {device.id, creator.id}
+        assert syncs[0]["device_id"] == device.id
+        by_id = {s["device_id"]: s for s in syncs}
+        assert by_id[creator.id]["is_current"] is True
+        assert by_id[device.id]["is_current"] is False
+
+    def test_device_syncs_surface_per_device_is_untracked(
+        self, client, access_token: str, admin_user: User, save: Save, device: Device
+    ):
+        other = db_device_handler.add_device(
+            Device(id="untracked-other", user_id=admin_user.id, name="Other")
+        )
+        db_device_save_sync_handler.upsert_sync(device_id=device.id, save_id=save.id)
+        db_device_save_sync_handler.set_untracked(
+            device_id=other.id, save_id=save.id, untracked=True
+        )
+
+        response = client.get(
+            f"/api/saves/{save.id}?device_id={device.id}",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        by_id = {s["device_id"]: s for s in response.json()["device_syncs"]}
+        # Each device carries its own tracking flag.
+        assert by_id[device.id]["is_untracked"] is False
+        assert by_id[other.id]["is_untracked"] is True
+
     def test_get_single_save_with_device_id(
         self, client, access_token: str, save: Save, device: Device
     ):
@@ -276,6 +379,7 @@ class TestSaveUploadWithSync:
         assert response.status_code == status.HTTP_200_OK
         data = response.json()
         assert data["device_syncs"] == []
+        assert data["origin_device_id"] is None
 
     @mock.patch(
         "endpoints.saves.fs_asset_handler.write_file", new_callable=mock.AsyncMock
@@ -316,6 +420,130 @@ class TestSaveUploadWithSync:
         assert len(data["device_syncs"]) == 1
         assert data["device_syncs"][0]["device_id"] == device.id
         assert data["device_syncs"][0]["is_untracked"] is False
+        # The creating device is recorded as the save's origin. Its name is
+        # resolvable from device_syncs (or /api/devices), so it is not repeated.
+        assert data["origin_device_id"] == device.id
+        origin_sync = next(
+            s for s in data["device_syncs"] if s["device_id"] == device.id
+        )
+        assert origin_sync["device_name"] == device.name
+
+    @mock.patch(
+        "endpoints.saves.fs_asset_handler.write_file", new_callable=mock.AsyncMock
+    )
+    @mock.patch("endpoints.saves.scan_save", new_callable=mock.AsyncMock)
+    def test_origin_device_persists_for_other_caller(
+        self,
+        mock_scan,
+        _mock_write,
+        client,
+        access_token: str,
+        rom: Rom,
+        platform: Platform,
+        admin_user: User,
+        device: Device,
+    ):
+        mock_scan.return_value = Save(
+            file_name="origin.sav",
+            file_name_no_tags="origin",
+            file_name_no_ext="origin",
+            file_extension="sav",
+            file_path=f"{platform.slug}/saves",
+            file_size_bytes=100,
+            rom_id=rom.id,
+            user_id=admin_user.id,
+        )
+
+        # Device A creates the save.
+        created = client.post(
+            f"/api/saves?rom_id={rom.id}&device_id={device.id}",
+            files={"saveFile": ("origin.sav", BytesIO(b"data"), "application/octet")},
+            headers={"Authorization": f"Bearer {access_token}"},
+        ).json()
+        save_id = created["id"]
+
+        # Device B later syncs the current version (download path) and so is also
+        # "current", but it did not create the save.
+        other = db_device_handler.add_device(
+            Device(id="downloader-device", user_id=admin_user.id, name="Downloader")
+        )
+        db_device_save_sync_handler.upsert_sync(
+            device_id=other.id, save_id=save_id, synced_at=None
+        )
+
+        response = client.get(
+            f"/api/saves/{save_id}?device_id={other.id}",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        by_id = {s["device_id"]: s for s in data["device_syncs"]}
+        # Both devices read as current, but origin still points at the creator.
+        assert by_id[device.id]["is_current"] is True
+        assert by_id[other.id]["is_current"] is True
+        assert data["origin_device_id"] == device.id
+        assert by_id[device.id]["device_name"] == device.name
+
+    @mock.patch(
+        "endpoints.saves.fs_asset_handler.write_file", new_callable=mock.AsyncMock
+    )
+    @mock.patch("endpoints.saves.scan_save", new_callable=mock.AsyncMock)
+    def test_origin_device_unchanged_on_update(
+        self,
+        mock_scan,
+        _mock_write,
+        client,
+        access_token: str,
+        rom: Rom,
+        platform: Platform,
+        admin_user: User,
+        device: Device,
+    ):
+        def scanned():
+            return Save(
+                file_name="origin.sav",
+                file_name_no_tags="origin",
+                file_name_no_ext="origin",
+                file_extension="sav",
+                file_path=f"{platform.slug}/saves",
+                file_size_bytes=100,
+                rom_id=rom.id,
+                user_id=admin_user.id,
+            )
+
+        # Device A creates the save.
+        mock_scan.return_value = scanned()
+        created = client.post(
+            f"/api/saves?rom_id={rom.id}&device_id={device.id}",
+            files={"saveFile": ("origin.sav", BytesIO(b"v1"), "application/octet")},
+            headers={"Authorization": f"Bearer {access_token}"},
+        ).json()
+        assert created["origin_device_id"] == device.id
+
+        other = db_device_handler.add_device(
+            Device(id="updater-device", user_id=admin_user.id, name="Updater")
+        )
+
+        # A second upload of the same save by another device updates content but
+        # must not reassign origin away from the creator.
+        mock_scan.return_value = scanned()
+        updated = client.post(
+            f"/api/saves?rom_id={rom.id}&device_id={other.id}&overwrite=true",
+            files={"saveFile": ("origin.sav", BytesIO(b"v2"), "application/octet")},
+            headers={"Authorization": f"Bearer {access_token}"},
+        ).json()
+        assert updated["id"] == created["id"]
+        assert updated["origin_device_id"] == device.id
+
+        # And an update with no device at all leaves origin intact.
+        mock_scan.return_value = scanned()
+        no_device = client.post(
+            f"/api/saves?rom_id={rom.id}&overwrite=true",
+            files={"saveFile": ("origin.sav", BytesIO(b"v3"), "application/octet")},
+            headers={"Authorization": f"Bearer {access_token}"},
+        ).json()
+        assert no_device["origin_device_id"] == device.id
 
     def test_upload_save_with_invalid_device_id_returns_404(
         self,

backend/tests/handler/database/test_device_save_sync_handler.py

@@ -66,6 +66,86 @@ class TestGetSyncsForDeviceAndSaves:
         assert result == []
 
 
+class TestGetSyncsForSaves:
+    def test_returns_syncs_across_devices_with_name(
+        self, admin_user: User, rom: Rom, save: Save
+    ):
+        dev_a = db_device_handler.add_device(
+            Device(id="multi-dev-a", user_id=admin_user.id, name="Device A")
+        )
+        dev_b = db_device_handler.add_device(
+            Device(id="multi-dev-b", user_id=admin_user.id, name="Device B")
+        )
+        db_device_save_sync_handler.upsert_sync(dev_a.id, save.id)
+        db_device_save_sync_handler.upsert_sync(dev_b.id, save.id)
+
+        result = db_device_save_sync_handler.get_syncs_for_saves([save.id])
+
+        assert set(result.keys()) == {save.id}
+        by_device = {sync.device_id: name for sync, name in result[save.id]}
+        assert by_device == {dev_a.id: "Device A", dev_b.id: "Device B"}
+
+    def test_filters_to_requested_saves(self, admin_user: User, rom: Rom):
+        device = db_device_handler.add_device(
+            Device(id="multi-dev-c", user_id=admin_user.id, name="Device C")
+        )
+        saves = []
+        for i in range(3):
+            s = db_save_handler.add_save(
+                Save(
+                    rom_id=rom.id,
+                    user_id=admin_user.id,
+                    file_name=f"multi_{i}.sav",
+                    file_name_no_tags=f"multi_{i}",
+                    file_name_no_ext=f"multi_{i}",
+                    file_extension="sav",
+                    emulator="emu",
+                    file_path=f"{rom.platform_slug}/saves",
+                    file_size_bytes=100,
+                )
+            )
+            saves.append(s)
+            db_device_save_sync_handler.upsert_sync(device.id, s.id)
+
+        result = db_device_save_sync_handler.get_syncs_for_saves(
+            [saves[0].id, saves[2].id]
+        )
+
+        assert set(result.keys()) == {saves[0].id, saves[2].id}
+
+    def test_empty_save_ids_returns_empty(self, admin_user: User):
+        result = db_device_save_sync_handler.get_syncs_for_saves([])
+        assert result == {}
+
+
+class TestOriginDeviceCascade:
+    def test_origin_device_id_nulled_on_device_delete(self, admin_user: User, rom: Rom):
+        device = db_device_handler.add_device(
+            Device(id="origin-dev", user_id=admin_user.id, name="Origin")
+        )
+        save = db_save_handler.add_save(
+            Save(
+                rom_id=rom.id,
+                user_id=admin_user.id,
+                file_name="origin_cascade.sav",
+                file_name_no_tags="origin_cascade",
+                file_name_no_ext="origin_cascade",
+                file_extension="sav",
+                emulator="emu",
+                file_path=f"{rom.platform_slug}/saves",
+                file_size_bytes=100,
+                origin_device_id=device.id,
+            )
+        )
+        assert save.origin_device_id == device.id
+
+        db_device_handler.delete_device(device.id, admin_user.id)
+
+        refreshed = db_save_handler.get_save(user_id=admin_user.id, id=save.id)
+        assert refreshed is not None
+        assert refreshed.origin_device_id is None
+
+
 class TestUpsertSync:
     def test_creates_new_sync(self, admin_user: User, rom: Rom, save: Save):
         device = db_device_handler.add_device(