From ed3f1fe0188409fd268aa13c772470e6e08c7e8b Mon Sep 17 00:00:00 2001 From: Major Date: Thu, 28 May 2026 14:16:26 +0200 Subject: [PATCH] feat(sync): skip redundant library sync when other user has access When a user lacks library access permissions (403 from Immich API), check if any other user in the account has access before syncing. If another user has access, skip the sync to avoid redundant API calls and reduce load on shared library permissions. This caches the access state and retries the API call if it was previously denied, allowing for permission changes to take effect on the next sync cycle. Change-Type: feature Scope: sync --- backend/database.go | 9 ++++++ backend/database_test.go | 50 +++++++++++++++++++++++++++++ backend/interfaces.go | 1 + backend/syncService.go | 15 +++++++++ backend/syncService_test.go | 64 +++++++++++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+) diff --git a/backend/database.go b/backend/database.go index 62b3d94..2ba760c 100644 --- a/backend/database.go +++ b/backend/database.go @@ -85,6 +85,15 @@ func (d *Database) setSyncState(ctx context.Context, userID, key, value string) return err } +func (d *Database) hasAnyOtherUserWithLibraryAccess(ctx context.Context, excludeUserID string) (bool, error) { + var exists bool + err := d.db.QueryRowContext(ctx, + `SELECT EXISTS(SELECT 1 FROM syncState WHERE key = 'hasLibraryAccess' AND value = 'true' AND userID != ?)`, + excludeUserID, + ).Scan(&exists) + return exists, err +} + func (d *Database) deleteSyncState(ctx context.Context, userID, key string) { if _, err := d.db.ExecContext(ctx, "DELETE FROM syncState WHERE userID = ? AND key = ?", userID, key); err != nil { log.Printf("[DB] Failed to delete sync state %s for user %s: %v", key, userID, err) diff --git a/backend/database_test.go b/backend/database_test.go index 91576b9..03b2bac 100644 --- a/backend/database_test.go +++ b/backend/database_test.go @@ -1067,3 +1067,53 @@ func TestDeleteUserSyncData(t *testing.T) { t.Errorf("expected other user syncState preserved, got %v", otherSync) } } + +func TestHasAnyOtherUserWithLibraryAccess(t *testing.T) { + ctx := context.Background() + db := newTestDB(t) + otherUserID := "other-user-id" + if err := db.createUser(ctx, otherUserID, "other@example.com", "hashed"); err != nil { + t.Fatalf("create other user: %v", err) + } + + has, err := db.hasAnyOtherUserWithLibraryAccess(ctx, testUserID) + if err != nil { + t.Fatalf("empty state: %v", err) + } + if has { + t.Errorf("empty state: expected false, got true") + } + + if err := db.setSyncState(ctx, testUserID, "hasLibraryAccess", "true"); err != nil { + t.Fatalf("set excluded user: %v", err) + } + has, err = db.hasAnyOtherUserWithLibraryAccess(ctx, testUserID) + if err != nil { + t.Fatalf("only excluded set: %v", err) + } + if has { + t.Errorf("only excluded user has access: expected false, got true") + } + + if err := db.setSyncState(ctx, otherUserID, "hasLibraryAccess", "false"); err != nil { + t.Fatalf("set other false: %v", err) + } + has, err = db.hasAnyOtherUserWithLibraryAccess(ctx, testUserID) + if err != nil { + t.Fatalf("other false: %v", err) + } + if has { + t.Errorf("other user has value=false: expected false, got true") + } + + if err := db.setSyncState(ctx, otherUserID, "hasLibraryAccess", "true"); err != nil { + t.Fatalf("set other true: %v", err) + } + has, err = db.hasAnyOtherUserWithLibraryAccess(ctx, testUserID) + if err != nil { + t.Fatalf("other true: %v", err) + } + if !has { + t.Errorf("other user has value=true: expected true, got false") + } +} diff --git a/backend/interfaces.go b/backend/interfaces.go index fe5dce0..dcc33c7 100644 --- a/backend/interfaces.go +++ b/backend/interfaces.go @@ -9,6 +9,7 @@ type SyncStore interface { getSyncState(ctx context.Context, userID, key string) (*string, error) setSyncState(ctx context.Context, userID, key, value string) error deleteSyncState(ctx context.Context, userID, key string) + hasAnyOtherUserWithLibraryAccess(ctx context.Context, excludeUserID string) (bool, error) upsertAssets(ctx context.Context, userID string, assets []AssetRow) error batchUpdateStackInfo(ctx context.Context, userID string, updates []stackUpdateRow) (int, error) computeFrequentLocationClusters(ctx context.Context, userID string) ([]FrequentLocationRow, error) diff --git a/backend/syncService.go b/backend/syncService.go index 22269b8..4c2d516 100644 --- a/backend/syncService.go +++ b/backend/syncService.go @@ -533,6 +533,21 @@ func (s *SyncService) fetchAndReplaceAlbumAssets(ctx context.Context, userID str } func (s *SyncService) syncLibraries(ctx context.Context, userID string, immich SyncImmichAPI) error { + hasAccess, err := s.db.getSyncState(ctx, userID, "hasLibraryAccess") + if err != nil { + return fmt.Errorf("failed to check library access: %w", err) + } + if hasAccess != nil && *hasAccess == "false" { + otherHasAccess, err := s.db.hasAnyOtherUserWithLibraryAccess(ctx, userID) + if err != nil { + return fmt.Errorf("failed to check other users library access: %w", err) + } + if otherHasAccess { + return nil + } + // no early return: retry the API in case access was granted since the last 403. + } + log.Printf("[Sync] Syncing libraries for user %s...", userID) apiCtx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/backend/syncService_test.go b/backend/syncService_test.go index c4f296d..81f787d 100644 --- a/backend/syncService_test.go +++ b/backend/syncService_test.go @@ -957,6 +957,70 @@ func TestSyncLibrariesDeletesStale(t *testing.T) { } } +func TestSyncLibrariesSkipsWhenOtherUserHasAccess(t *testing.T) { + ctx := context.Background() + + factory, immich := newMockImmichFactoryNoRetry(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/libraries" { + t.Errorf("unexpected call to /api/libraries: should have been skipped") + } + http.NotFound(w, r) + }) + + db := newTestDB(t) + if err := db.setSyncState(ctx, testUserID, "hasLibraryAccess", "false"); err != nil { + t.Fatalf("set hasLibraryAccess for testUserID: %v", err) + } + if err := db.setSyncState(ctx, "otherUser", "hasLibraryAccess", "true"); err != nil { + t.Fatalf("set hasLibraryAccess for otherUser: %v", err) + } + + svc := newSyncService(db, factory, newNominatimClient(10*time.Second)) + if err := svc.syncLibraries(ctx, testUserID, immich); err != nil { + t.Fatalf("syncLibraries: %v", err) + } +} + +func TestSyncLibrariesRetriesWhenNoOneHasAccess(t *testing.T) { + ctx := context.Background() + + factory, immich := newMockImmichFactory(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/libraries" { + json.NewEncoder(w).Encode([]ImmichLibraryResponse{ + {ID: "lib1", Name: "Photos", AssetCount: 100}, + }) + return + } + http.NotFound(w, r) + }) + + db := newTestDB(t) + if err := db.setSyncState(ctx, testUserID, "hasLibraryAccess", "false"); err != nil { + t.Fatalf("set hasLibraryAccess: %v", err) + } + + svc := newSyncService(db, factory, newNominatimClient(10*time.Second)) + if err := svc.syncLibraries(ctx, testUserID, immich); err != nil { + t.Fatalf("syncLibraries: %v", err) + } + + libs, err := db.getLibraries(ctx) + if err != nil { + t.Fatalf("getLibraries: %v", err) + } + if len(libs) != 1 { + t.Fatalf("expected 1 library, got %d", len(libs)) + } + + hasAccess, err := db.getSyncState(ctx, testUserID, "hasLibraryAccess") + if err != nil { + t.Fatalf("get hasLibraryAccess: %v", err) + } + if hasAccess == nil || *hasAccess != "true" { + t.Errorf("expected hasLibraryAccess=true after successful retry, got %v", hasAccess) + } +} + func TestSyncAlbumsSuccess(t *testing.T) { ctx := context.Background()