From bf65e6d13dc133f0a9abb19bfe768d0492b75811 Mon Sep 17 00:00:00 2001 From: Stefan Hausotte Date: Tue, 31 Mar 2026 23:09:18 +0200 Subject: [PATCH] fix: multi-instance persistent cache --- Forji/Forji/App/ContentView.swift | 70 ++++++++++--------- Forji/Forji/App/ForjiApp.swift | 28 ++++++-- Forji/Forji/Helpers/TaggedItem.swift | 52 +++++++++++++- .../MergedNotificationsOverviewView.swift | 15 +++- .../Views/MergedRepositoryListView.swift | 15 +++- .../Views/MergedSearchableOverviewView.swift | 12 +++- Forji/ForjiTests/TaggedItemTests.swift | 68 ++++++++++++++++++ 7 files changed, 212 insertions(+), 48 deletions(-) diff --git a/Forji/Forji/App/ContentView.swift b/Forji/Forji/App/ContentView.swift index 03f6835..72b0a9f 100644 --- a/Forji/Forji/App/ContentView.swift +++ b/Forji/Forji/App/ContentView.swift @@ -20,7 +20,7 @@ struct ContentView: View { @AppStorage("appearance") private var appearance: AppAppearance = .system @AppStorage("defaultAllInstances") private var defaultAllInstances = false @State var authService: AuthenticationService - @State private var multiInstanceManager: MultiInstanceManager? + @Binding var multiInstanceManager: MultiInstanceManager? @Query(filter: #Predicate { $0.isDefault }) private var defaultInstances: [ForgejoInstance] @Query private var allInstances: [ForgejoInstance] @State private var hasAttemptedAutoLogin = false @@ -44,6 +44,7 @@ struct ContentView: View { multiInstanceManager?.disconnect() multiInstanceManager = nil } + .task { await completeMultiInstanceRestore(manager: manager) } } else if authService.isAuthenticated, authService.client != nil { HomeView(authService: authService) .task { await completeSessionRestore() } @@ -69,6 +70,12 @@ struct ContentView: View { } } + private func completeMultiInstanceRestore(manager: MultiInstanceManager) async { + guard !sessionFullyRestored else { return } + await manager.connect(instances: allInstances) + sessionFullyRestored = true + } + private func completeSessionRestore() async { guard !sessionFullyRestored, let instance = authService.currentInstance else { return } do { @@ -104,48 +111,47 @@ struct ContentView: View { #endif if defaultAllInstances, allInstances.count > 1 { - let manager = MultiInstanceManager() - manager.bootstrap(instances: allInstances) - if manager.isConnected { - multiInstanceManager = manager - hasAttemptedAutoLogin = true - Task { - await manager.connect(instances: allInstances) - } - return - } + await attemptMultiInstanceLogin() + } else { + await attemptSingleInstanceLogin() + } + hasAttemptedAutoLogin = true + } - await manager.connect(instances: allInstances) - if manager.isConnected { - multiInstanceManager = manager - } - hasAttemptedAutoLogin = true + private func attemptMultiInstanceLogin() async { + if multiInstanceManager != nil { + // Already bootstrapped synchronously in ForjiApp.init return } + let manager = MultiInstanceManager() + await manager.connect(instances: allInstances) + if manager.isConnected { + multiInstanceManager = manager + } + } - if let defaultInstance = defaultInstances.first { - if authService.bootstrapSession(instance: defaultInstance) { - hasAttemptedAutoLogin = true - do { - try await authService.restoreSession(instance: defaultInstance) - defaultInstance.lastUsed = Date() - try? modelContext.save() - sessionFullyRestored = true - } catch { - await authService.logout(modelContext: modelContext) - } - return - } + private func attemptSingleInstanceLogin() async { + guard let defaultInstance = defaultInstances.first else { return } + if authService.bootstrapSession(instance: defaultInstance) { do { try await authService.restoreSession(instance: defaultInstance) defaultInstance.lastUsed = Date() try? modelContext.save() + sessionFullyRestored = true } catch { - // Auto-login failed — fall through to instance list + await authService.logout(modelContext: modelContext) } + return + } + + do { + try await authService.restoreSession(instance: defaultInstance) + defaultInstance.lastUsed = Date() + try? modelContext.save() + } catch { + // Auto-login failed — fall through to instance list } - hasAttemptedAutoLogin = true } #if DEBUG @@ -191,7 +197,7 @@ struct ContentView: View { #if DEBUG #Preview { - ContentView(authService: AuthenticationService()) + ContentView(authService: AuthenticationService(), multiInstanceManager: .constant(nil)) .modelContainer(for: ForgejoInstance.self, inMemory: true) .environment(NavigationState.shared) } diff --git a/Forji/Forji/App/ForjiApp.swift b/Forji/Forji/App/ForjiApp.swift index ed49852..d502181 100644 --- a/Forji/Forji/App/ForjiApp.swift +++ b/Forji/Forji/App/ForjiApp.swift @@ -10,6 +10,7 @@ struct ForjiApp: App { let sharedModelContainer: ModelContainer @State private var bootstrappedAuthService: AuthenticationService + @State private var bootstrappedMultiInstanceManager: MultiInstanceManager? init() { let schema = Schema([ForgejoInstance.self]) @@ -23,6 +24,7 @@ struct ForjiApp: App { sharedModelContainer = container let auth = AuthenticationService() + var multiManager: MultiInstanceManager? #if DEBUG let devServerURL = UserDefaults.standard.string(forKey: "dev_serverURL") ?? "" @@ -40,20 +42,34 @@ struct ForjiApp: App { ) let allDescriptor = FetchDescriptor() let allInstances = (try? context.fetch(allDescriptor)) ?? [] - let instance = (try? context.fetch(defaultDescriptor).first) - ?? (allInstances.count == 1 ? allInstances.first : nil) - if let instance { - auth.bootstrapSession(instance: instance) + + let defaultAllInstances = UserDefaults.standard.bool(forKey: "defaultAllInstances") + if defaultAllInstances, allInstances.count > 1 { + let manager = MultiInstanceManager() + manager.bootstrap(instances: allInstances) + if manager.isConnected { + multiManager = manager + } + } else { + let instance = (try? context.fetch(defaultDescriptor).first) + ?? (allInstances.count == 1 ? allInstances.first : nil) + if let instance { + auth.bootstrapSession(instance: instance) + } } } _bootstrappedAuthService = State(initialValue: auth) + _bootstrappedMultiInstanceManager = State(initialValue: multiManager) } var body: some Scene { WindowGroup { - ContentView(authService: bootstrappedAuthService) - .environment(NavigationState.shared) + ContentView( + authService: bootstrappedAuthService, + multiInstanceManager: $bootstrappedMultiInstanceManager, + ) + .environment(NavigationState.shared) } .modelContainer(sharedModelContainer) .onChange(of: scenePhase) { _, newPhase in diff --git a/Forji/Forji/Helpers/TaggedItem.swift b/Forji/Forji/Helpers/TaggedItem.swift index c23509e..64b8a3f 100644 --- a/Forji/Forji/Helpers/TaggedItem.swift +++ b/Forji/Forji/Helpers/TaggedItem.swift @@ -2,12 +2,14 @@ import Foundation struct TaggedItem: Identifiable { let id: String + let sourceKey: String let item: T let instanceName: String - let authService: AuthenticationService + var authService: AuthenticationService init(item: T, sourceKey: String, instanceName: String, authService: AuthenticationService) { id = "\(sourceKey):\(item.id)" + self.sourceKey = sourceKey self.item = item self.instanceName = instanceName self.authService = authService @@ -34,3 +36,51 @@ struct TaggedItem: Identifiable { return merged } } + +// MARK: - Codable (for disk cache) + +extension TaggedItem: Codable where T: Codable { + enum CodingKeys: String, CodingKey { + case id, sourceKey, item, instanceName + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + id = try container.decode(String.self, forKey: .id) + sourceKey = try container.decode(String.self, forKey: .sourceKey) + item = try container.decode(T.self, forKey: .item) + instanceName = try container.decode(String.self, forKey: .instanceName) + authService = AuthenticationService() + } + + func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(id, forKey: .id) + try container.encode(sourceKey, forKey: .sourceKey) + try container.encode(item, forKey: .item) + try container.encode(instanceName, forKey: .instanceName) + } +} + +extension PaginationState where Item: Identifiable { + /// Replace placeholder auth services on cached TaggedItems with live ones from the manager. + /// Items whose source is no longer connected are removed. + func rehydrate(from manager: MultiInstanceManager) where Item == TaggedItem { + let sources = manager.connectionSources() + rehydrate(from: sources) + } + + /// Replace placeholder auth services using the given connection sources. + /// Items whose sourceKey has no matching source are removed. + func rehydrate(from sources: [ConnectionSource]) where Item == TaggedItem { + let lookup = Dictionary( + uniqueKeysWithValues: sources.map { ($0.sourceKey, $0.authService) }, + ) + items = items.compactMap { item in + guard let auth = lookup[item.sourceKey] else { return nil } + var rehydrated = item + rehydrated.authService = auth + return rehydrated + } + } +} diff --git a/Forji/Forji/Views/MergedNotificationsOverviewView.swift b/Forji/Forji/Views/MergedNotificationsOverviewView.swift index acf7ac4..f1caa9c 100644 --- a/Forji/Forji/Views/MergedNotificationsOverviewView.swift +++ b/Forji/Forji/Views/MergedNotificationsOverviewView.swift @@ -4,9 +4,18 @@ import SwiftUI struct MergedNotificationsOverviewView: View { let manager: MultiInstanceManager - @State private var pagination = PaginationState>() + @State private var pagination: PaginationState> @State private var statusFilter: String = "unread" + init(manager: MultiInstanceManager) { + self.manager = manager + let state = PaginationState>() + state.cacheName = "merged_notifications" + state.loadFromCache() + state.rehydrate(from: manager) + _pagination = State(initialValue: state) + } + private var statusTypes: [String] { switch statusFilter { case "unread": ["unread"] @@ -123,13 +132,13 @@ struct MergedNotificationsOverviewView: View { @discardableResult private func reloadNotifications(clearItems: Bool = false) -> Task { - pagination.reload(clearItems: clearItems) { page, limit in + pagination.reloadAndCache(clearItems: clearItems) { page, limit in try await fetchFromAllInstances(page: page, limit: limit) } } private func loadMore() async { - await pagination.loadMore { page, limit in + await pagination.loadMoreAndCache { page, limit in try await fetchFromAllInstances(page: page, limit: limit) } } diff --git a/Forji/Forji/Views/MergedRepositoryListView.swift b/Forji/Forji/Views/MergedRepositoryListView.swift index 0c8668b..9ddb636 100644 --- a/Forji/Forji/Views/MergedRepositoryListView.swift +++ b/Forji/Forji/Views/MergedRepositoryListView.swift @@ -4,12 +4,21 @@ import SwiftUI struct MergedRepositoryListView: View { let manager: MultiInstanceManager - @State private var pagination = PaginationState>() + @State private var pagination: PaginationState> @AppStorage("repoFilterShowArchived") private var showArchived = true @AppStorage("repoFilterShowMirrors") private var showMirrors = true @State private var searchText = "" @State private var searchTask: Task? + init(manager: MultiInstanceManager) { + self.manager = manager + let state = PaginationState>() + state.cacheName = "merged_repos" + state.loadFromCache() + state.rehydrate(from: manager) + _pagination = State(initialValue: state) + } + private var hasNonDefaultFilters: Bool { !showArchived || !showMirrors } @@ -118,13 +127,13 @@ struct MergedRepositoryListView: View { @discardableResult private func reloadItems(clearItems: Bool = false) -> Task { - pagination.reload(clearItems: clearItems) { page, limit in + pagination.reloadAndCache(clearItems: clearItems) { page, limit in try await fetchFromAllInstances(page: page, limit: limit) } } private func loadMore() async { - await pagination.loadMore { page, limit in + await pagination.loadMoreAndCache { page, limit in try await fetchFromAllInstances(page: page, limit: limit) } } diff --git a/Forji/Forji/Views/MergedSearchableOverviewView.swift b/Forji/Forji/Views/MergedSearchableOverviewView.swift index 3645119..fb565ca 100644 --- a/Forji/Forji/Views/MergedSearchableOverviewView.swift +++ b/Forji/Forji/Views/MergedSearchableOverviewView.swift @@ -43,7 +43,7 @@ struct MergedSearchableOverviewView: let manager: MultiInstanceManager let config: MergedSearchableConfig - @State private var pagination = PaginationState>() + @State private var pagination: PaginationState> @State private var stateFilter: IssueFilterState = .open @State private var searchText = "" @State private var searchTask: Task? @@ -65,6 +65,12 @@ struct MergedSearchableOverviewView: rowContent = row detailContent = detail createContent = createView + + let state = PaginationState>() + state.cacheName = "merged_\(config.issueType)" + state.loadFromCache() + state.rehydrate(from: manager) + _pagination = State(initialValue: state) } var body: some View { @@ -186,13 +192,13 @@ struct MergedSearchableOverviewView: @discardableResult private func reloadItems(clearItems: Bool = false) -> Task { - pagination.reload(clearItems: clearItems) { page, limit in + pagination.reloadAndCache(clearItems: clearItems) { page, limit in try await fetchFromAllInstances(page: page, limit: limit) } } private func loadMore() async { - await pagination.loadMore { page, limit in + await pagination.loadMoreAndCache { page, limit in try await fetchFromAllInstances(page: page, limit: limit) } } diff --git a/Forji/ForjiTests/TaggedItemTests.swift b/Forji/ForjiTests/TaggedItemTests.swift index 9df0999..34a24b2 100644 --- a/Forji/ForjiTests/TaggedItemTests.swift +++ b/Forji/ForjiTests/TaggedItemTests.swift @@ -122,4 +122,72 @@ struct TaggedItemTests { let result: [TaggedItem] = TaggedItem.mergeAndDeduplicate(batches: [], sources: []) #expect(result.isEmpty) } + + // MARK: - Codable Round-Trip + + @Test func codableRoundTripPreservesFields() throws { + let issue = makeIssue(id: 3, title: "Cached") + let tagged = TaggedItem( + item: issue, sourceKey: "https://srv.com:admin", + instanceName: "Server", authService: makeAuth() + ) + + let data = try JSONEncoder().encode([tagged]) + let decoded = try JSONDecoder().decode([TaggedItem].self, from: data) + + #expect(decoded.count == 1) + #expect(decoded[0].id == "https://srv.com:admin:3") + #expect(decoded[0].sourceKey == "https://srv.com:admin") + #expect(decoded[0].item.title == "Cached") + #expect(decoded[0].instanceName == "Server") + } + + @Test func codableDecodedAuthServiceIsPlaceholder() throws { + let issue = makeIssue(id: 1) + let tagged = TaggedItem( + item: issue, sourceKey: "key", + instanceName: "name", authService: makeAuth() + ) + + let data = try JSONEncoder().encode([tagged]) + let decoded = try JSONDecoder().decode([TaggedItem].self, from: data) + + #expect(decoded[0].authService.isAuthenticated == false) + #expect(decoded[0].authService.client == nil) + } + + // MARK: - Rehydrate + + @MainActor + @Test func rehydrateReplacesAuthService() { + let liveAuth = AuthenticationService() + let source = ConnectionSource( + sourceKey: "https://a.com:u", name: "A", + client: ForgejoClient(serverURL: "https://a.com", username: "u", token: "t"), + authService: liveAuth + ) + let issue = makeIssue(id: 1) + let pagination = PaginationState>() + pagination.items = [ + TaggedItem(item: issue, sourceKey: "https://a.com:u", instanceName: "A", authService: makeAuth()), + ] + + pagination.rehydrate(from: [source]) + + #expect(pagination.items.count == 1) + #expect(pagination.items[0].authService === liveAuth) + } + + @MainActor + @Test func rehydrateRemovesItemsWithNoMatchingSource() { + let issue = makeIssue(id: 1) + let pagination = PaginationState>() + pagination.items = [ + TaggedItem(item: issue, sourceKey: "https://removed.com:u", instanceName: "Gone", authService: makeAuth()), + ] + + pagination.rehydrate(from: [ConnectionSource]()) + + #expect(pagination.items.isEmpty) + } }