fix(backups): fix orphaned snapshots erroring out

selfprivacy_api/graphql/queries/backup.py

@@ -34,6 +34,24 @@ class BackupConfiguration:
     location_id: typing.Optional[str]
 
 
+# TODO: Ideally this should not be done in API but making an internal Service requires more work
+# than to make an API record about a service
+def tombstone_service(service_id: str) -> Service:
+    return Service(
+        id=service_id,
+        display_name=f"{service_id} (Orphaned)",
+        description="",
+        svg_icon="",
+        is_movable=False,
+        is_required=False,
+        is_enabled=False,
+        status=ServiceStatusEnum.OFF,
+        url=None,
+        can_be_backed_up=False,
+        backup_description="",
+    )
+
+
 @strawberry.type
 class Backup:
     @strawberry.field
@@ -55,27 +73,21 @@ class Backup:
         result = []
         snapshots = Backups.get_all_snapshots()
         for snap in snapshots:
+            api_service = None
             service = get_service_by_id(snap.service_name)
+
             if service is None:
-                service = Service(
-                    id=snap.service_name,
-                    display_name=f"{snap.service_name} (Orphaned)",
-                    description="",
-                    svg_icon="",
-                    is_movable=False,
-                    is_required=False,
-                    is_enabled=False,
-                    status=ServiceStatusEnum.OFF,
-                    url=None,
-                    dns_records=None,
-                    can_be_backed_up=False,
-                    backup_description="",
-                )
+                api_service = tombstone_service(snap.service_name)
             else:
-                service = service_to_graphql_service(service)
+                api_service = service_to_graphql_service(service)
+            if api_service is None:
+                raise NotImplementedError(
+                    f"Could not construct API Service record for:{snap.service_name}. This should be unreachable and is a bug if you see it."
+                )
+
             graphql_snap = SnapshotInfo(
                 id=snap.id,
-                service=service,
+                service=api_service,
                 created_at=snap.created_at,
                 reason=snap.reason,
             )

tests/conftest.py

@@ -208,5 +208,7 @@ def dummy_service(
     service.enable()
     yield service
 
-    # cleanup because apparently it matters wrt tasks
-    services.services.remove(service)
+    # Cleanup because apparently it matters wrt tasks
+    # Some tests may remove it from the list intentionally, this is fine
+    if service in services.services:
+        services.services.remove(service)

tests/test_graphql/test_api_backup.py

@@ -3,6 +3,8 @@ from tests.test_backup import backups
 from tests.common import generate_backup_query
 
 
+import selfprivacy_api.services as all_services
+from selfprivacy_api.services import get_service_by_id
 from selfprivacy_api.graphql.common_types.service import service_to_graphql_service
 from selfprivacy_api.graphql.common_types.backup import (
     _AutobackupQuotas,
@@ -143,6 +145,7 @@ allSnapshots {
     id
     service {
         id
+        displayName
     }
     createdAt
     reason
@@ -306,6 +309,20 @@ def test_snapshots_empty(authorized_client, dummy_service, backups):
     assert snaps == []
 
 
+def test_snapshots_orphaned_service(authorized_client, dummy_service, backups):
+    api_backup(authorized_client, dummy_service)
+    snaps = api_snapshots(authorized_client)
+    assert len(snaps) == 1
+
+    all_services.services.remove(dummy_service)
+    assert get_service_by_id(dummy_service.get_id()) is None
+
+    snaps = api_snapshots(authorized_client)
+    assert len(snaps) == 1
+    assert "Orphaned" in snaps[0]["service"]["displayName"]
+    assert dummy_service.get_id() in snaps[0]["service"]["displayName"]
+
+
 def test_start_backup(authorized_client, dummy_service, backups):
     response = api_backup(authorized_client, dummy_service)
     data = get_data(response)["backup"]["startBackup"]