From f038930c39364685cdcebbd9e8713967a5ad92f8 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 21 Sep 2026 13:47:02 +0700 Subject: [PATCH] refactor: Refactor GitFlow sync to use LocalDataStore and async operations --- .../GitFlowConfigurationSyncController.swift | 88 ++++-- macgit/App/GitProviderAccountController.swift | 16 +- macgit/App/RepositoryBookmarkController.swift | 282 ++++++++---------- .../RepositoryCommitRuleSyncController.swift | 43 +-- .../App/RepositoryVisibilityController.swift | 8 +- macgit/App/macgitApp.swift | 60 ++-- .../GitProviderAccountPreferenceStore.swift | 53 ++-- macgit/Services/GitProviderSSHKeyStore.swift | 33 +- .../Services/LegacyLocalDataMigration.swift | 76 +++++ macgit/Services/LocalDataError.swift | 13 + macgit/Services/LocalDataStore.swift | 84 ++++++ macgit/Services/LocalDataTransaction.swift | 26 ++ .../LocalGitProviderAccountStore.swift | 167 +++++------ macgit/Services/LocalSQLiteDatabase.swift | 133 +++++++++ macgit/Services/RepoSettingsStore.swift | 36 +-- .../Services/RepositoryVisibilityCache.swift | 64 ++-- .../Views/Common/LocalDataLoadingView.swift | 28 ++ ...MainWindowView+ProtectedBranchCommit.swift | 14 +- ...indowView+ProviderAccountPreferences.swift | 7 +- .../MainWindow/MainWindowView+Sheets.swift | 46 +-- macgit/Views/MainWindow/MainWindowView.swift | 1 - macgit/Views/MainWindow/RepoPickerView.swift | 18 +- .../GitFlowConfigurationSyncTests.swift | 23 +- ...tProviderAccountPreferenceStoreTests.swift | 30 +- macgitTests/GitProviderSSHKeyStoreTests.swift | 26 +- macgitTests/LocalDataMigrationTests.swift | 232 ++++++++++++++ macgitTests/LocalDataStoreTestFixture.swift | 30 ++ .../LocalGitProviderAccountStoreTests.swift | 16 +- macgitTests/RepoSettingsStoreTests.swift | 15 +- macgitTests/RepositoryBookmarkTests.swift | 13 +- ...ositoryCommitRuleSyncControllerTests.swift | 51 +++- .../RepositoryVisibilityControllerTests.swift | 14 +- 32 files changed, 1179 insertions(+), 567 deletions(-) create mode 100644 macgit/Services/LegacyLocalDataMigration.swift create mode 100644 macgit/Services/LocalDataError.swift create mode 100644 macgit/Services/LocalDataStore.swift create mode 100644 macgit/Services/LocalDataTransaction.swift create mode 100644 macgit/Services/LocalSQLiteDatabase.swift create mode 100644 macgit/Views/Common/LocalDataLoadingView.swift create mode 100644 macgitTests/LocalDataMigrationTests.swift create mode 100644 macgitTests/LocalDataStoreTestFixture.swift diff --git a/macgit/App/GitFlowConfigurationSyncController.swift b/macgit/App/GitFlowConfigurationSyncController.swift index 63950cea..b2d80c49 100644 --- a/macgit/App/GitFlowConfigurationSyncController.swift +++ b/macgit/App/GitFlowConfigurationSyncController.swift @@ -34,24 +34,54 @@ final class GitFlowConfigurationSyncController: ObservableObject { private let cloudStore: GitFlowConfigurationCloudStore? private let localStore: GitFlowConfigurationStore private let identityResolver: any RepositoryRemoteIdentityResolving - private let userDefaults: UserDefaults - private let pendingUploadsKey: String + private let dataStore: LocalDataStore + private var operations: [URL: (UUID, Task)] = [:] init( cloudStore: GitFlowConfigurationCloudStore?, localStore: GitFlowConfigurationStore = GitFlowConfigurationStore(), identityResolver: any RepositoryRemoteIdentityResolving = RepositoryRemoteIdentityResolver(), - userDefaults: UserDefaults = .standard, - pendingUploadsKey: String = "dev.thanhtran.macgit.gitFlowConfiguration.pendingUploads" + dataStore: LocalDataStore? = nil ) { self.cloudStore = cloudStore self.localStore = localStore self.identityResolver = identityResolver - self.userDefaults = userDefaults - self.pendingUploadsKey = pendingUploadsKey + self.dataStore = dataStore ?? .shared } - func reconcile( + // File-backed configuration and its SQLite outbox cannot share a transaction. + // Serialize their workflows per repository so a download cannot interleave a save. + private func serialized(in repositoryURL: URL, operation: @escaping () async throws -> T) async throws -> T { + let previous = operations[repositoryURL]?.1 + let id = UUID() + let task = Task { + await previous?.value + try await dataStore.prepare() + return try await operation() + } + operations[repositoryURL] = (id, Task { _ = try? await task.value }) + defer { if operations[repositoryURL]?.0 == id { operations[repositoryURL] = nil } } + return try await task.value + } + + func reconcile(repositoryURL: URL, fallbackConfiguration: GitFlowConfiguration, uid: String?) async -> GitFlowConfigurationSyncOutcome { + guard uid != nil, cloudStore != nil else { return .unchanged } + do { + return try await serialized(in: repositoryURL) { + await self.reconcileNow(repositoryURL: repositoryURL, fallbackConfiguration: fallbackConfiguration, uid: uid) + } + } catch { + return GitFlowConfigurationSyncOutcome(configuration: nil, warningMessage: error.localizedDescription) + } + } + + func save(_ configuration: GitFlowConfiguration, repositoryURL: URL, uid: String?) async throws -> String? { + try await serialized(in: repositoryURL) { + try await self.saveNow(configuration, repositoryURL: repositoryURL, uid: uid) + } + } + + private func reconcileNow( repositoryURL: URL, fallbackConfiguration: GitFlowConfiguration, uid: String? @@ -68,7 +98,7 @@ final class GitFlowConfigurationSyncController: ObservableObject { let uploadID = pendingUploadID(uid: uid, repositoryID: identity.documentID) do { - if isPendingUpload(uploadID) { + if let pendingVersion = pendingVersion(uploadID) { if case .value(let localConfiguration) = localResult { try await upload( localConfiguration, @@ -76,10 +106,10 @@ final class GitFlowConfigurationSyncController: ObservableObject { uid: uid, cloudStore: cloudStore ) - clearPendingUpload(uploadID) + try await clearPendingUpload(uploadID, version: pendingVersion) return .unchanged } - clearPendingUpload(uploadID) + try await clearPendingUpload(uploadID, version: pendingVersion) } if let cloudConfiguration = try await cloudStore.configuration( @@ -87,7 +117,8 @@ final class GitFlowConfigurationSyncController: ObservableObject { uid: uid ) { let latestLocalResult = await localStore.loadResult(in: repositoryURL) - if isPendingUpload(uploadID) || localConfigurationChanged( + let pendingVersion = pendingVersion(uploadID) + if pendingVersion != nil || localConfigurationChanged( from: localResult, to: latestLocalResult ) { @@ -98,7 +129,7 @@ final class GitFlowConfigurationSyncController: ObservableObject { uid: uid, cloudStore: cloudStore ) - clearPendingUpload(uploadID) + if let pendingVersion { try await clearPendingUpload(uploadID, version: pendingVersion) } } return .unchanged } @@ -140,7 +171,7 @@ final class GitFlowConfigurationSyncController: ObservableObject { } } - func save( + private func saveNow( _ configuration: GitFlowConfiguration, repositoryURL: URL, uid: String? @@ -152,7 +183,7 @@ final class GitFlowConfigurationSyncController: ObservableObject { return nil } let uploadID = pendingUploadID(uid: uid, repositoryID: identity.documentID) - markPendingUpload(uploadID) + let pendingVersion = try await markPendingUpload(uploadID) do { try await upload( @@ -161,7 +192,7 @@ final class GitFlowConfigurationSyncController: ObservableObject { uid: uid, cloudStore: cloudStore ) - clearPendingUpload(uploadID) + try await clearPendingUpload(uploadID, version: pendingVersion) return nil } catch { return "Git Flow was saved locally, but its configuration could not sync: \(error.localizedDescription)" @@ -202,23 +233,22 @@ final class GitFlowConfigurationSyncController: ObservableObject { "\(uid)|\(repositoryID)" } - private func isPendingUpload(_ id: String) -> Bool { - pendingUploadIDs.contains(id) + private func pendingVersion(_ id: String) -> String? { + try? dataStore.value(String.self, in: "gitFlowPending", id: id) } - private func markPendingUpload(_ id: String) { - var ids = pendingUploadIDs - ids.insert(id) - userDefaults.set(Array(ids), forKey: pendingUploadsKey) - } - - private func clearPendingUpload(_ id: String) { - var ids = pendingUploadIDs - ids.remove(id) - userDefaults.set(Array(ids), forKey: pendingUploadsKey) + private func markPendingUpload(_ id: String) async throws -> String { + let version = UUID().uuidString + try await dataStore.transaction { transaction in + try transaction.set(version, in: "gitFlowPending", id: id) + } + return version } - private var pendingUploadIDs: Set { - Set(userDefaults.stringArray(forKey: pendingUploadsKey) ?? []) + private func clearPendingUpload(_ id: String, version: String) async throws { + try await dataStore.transaction { transaction in + guard try transaction.value(String.self, in: "gitFlowPending", id: id) == version else { return } + transaction.remove(in: "gitFlowPending", id: id) + } } } diff --git a/macgit/App/GitProviderAccountController.swift b/macgit/App/GitProviderAccountController.swift index 3b0424f1..197cce3d 100644 --- a/macgit/App/GitProviderAccountController.swift +++ b/macgit/App/GitProviderAccountController.swift @@ -44,7 +44,7 @@ final class GitProviderAccountController: ObservableObject { init( store: GitProviderAccountStore, tokenVault: GitProviderTokenVault, - sshKeyStore: GitProviderSSHKeyStore = UserDefaultsGitProviderSSHKeyStore(), + sshKeyStore: GitProviderSSHKeyStore? = nil, sshAuthService: GitProviderSSHAuthenticating = GitProviderSSHAuthService(), authService: GitProviderAuthenticating? = nil, configuration: GitHubProviderAuthConfiguration? = nil, @@ -57,7 +57,7 @@ final class GitProviderAccountController: ObservableObject { ) { self.store = store self.tokenVault = tokenVault - self.sshKeyStore = sshKeyStore + self.sshKeyStore = sshKeyStore ?? SQLiteGitProviderSSHKeyStore() self.sshAuthService = sshAuthService self.authService = authService self.configuration = configuration @@ -90,7 +90,7 @@ final class GitProviderAccountController: ObservableObject { hasSameProviderIdentity($0, previousAccount) }) { try? tokenVault.deleteToken(for: previousAccount) - try? sshKeyStore.deleteKey(for: previousAccount) + try? await sshKeyStore.deleteKey(for: previousAccount) } } @@ -210,7 +210,7 @@ final class GitProviderAccountController: ObservableObject { errorMessage = nil do { try tokenVault.deleteToken(for: account) - try sshKeyStore.deleteKey(for: account) + try await sshKeyStore.deleteKey(for: account) try await store.delete(accountID: account.id) accounts.removeAll { $0.id == account.id } } catch { @@ -290,9 +290,9 @@ final class GitProviderAccountController: ObservableObject { do { if transportProtocol == .ssh, let sshKey { - try sshKeyStore.saveKey(sshKey, for: updatedAccount) + try await sshKeyStore.saveKey(sshKey, for: updatedAccount) } else { - try sshKeyStore.deleteKey(for: updatedAccount) + try await sshKeyStore.deleteKey(for: updatedAccount) } try await store.save(updatedAccount) publish(updatedAccount) @@ -348,11 +348,11 @@ final class GitProviderAccountController: ObservableObject { ) try validateAccountCreation(for: account) - try sshKeyStore.saveKey(key, for: account) + try await sshKeyStore.saveKey(key, for: account) do { try await store.save(account) } catch { - try? sshKeyStore.deleteKey(for: account) + try? await sshKeyStore.deleteKey(for: account) throw error } diff --git a/macgit/App/RepositoryBookmarkController.swift b/macgit/App/RepositoryBookmarkController.swift index 70d3a273..da16e038 100644 --- a/macgit/App/RepositoryBookmarkController.swift +++ b/macgit/App/RepositoryBookmarkController.swift @@ -38,240 +38,194 @@ enum RepositoryBookmarkError: LocalizedError { @MainActor final class RepositoryBookmarkController: ObservableObject { - @Published private(set) var bookmarks: [RepositoryBookmark] + @Published private(set) var bookmarks: [RepositoryBookmark] = [] + @Published private(set) var localPaths: [String: String] = [:] @Published private(set) var syncingBookmarkIDs: Set = [] @Published private(set) var errorMessage: String? private let cloudStore: RepositoryBookmarkCloudStore? - private let userDefaults: UserDefaults - private let bookmarksKey: String - private let localPathsKey: String - private let pendingUploadsKey: String - private let pendingDeletesKey: String - - @Published private(set) var localPaths: [String: String] - private var pendingUploads: Set - private var pendingDeletes: Set + private let dataStore: LocalDataStore private var activeUID: String? private var observation: ObservationToken? - init( - cloudStore: RepositoryBookmarkCloudStore?, - userDefaults: UserDefaults = .standard, - keyPrefix: String = "dev.thanhtran.macgit.repositoryBookmarks" - ) { + init(cloudStore: RepositoryBookmarkCloudStore?, dataStore: LocalDataStore? = nil) { self.cloudStore = cloudStore - self.userDefaults = userDefaults - bookmarksKey = "\(keyPrefix).items" - localPathsKey = "\(keyPrefix).localPaths" - pendingUploadsKey = "\(keyPrefix).pendingUploads" - pendingDeletesKey = "\(keyPrefix).pendingDeletes" - - if let data = userDefaults.data(forKey: bookmarksKey), - let decoded = try? JSONDecoder().decode([RepositoryBookmark].self, from: data) { - bookmarks = decoded - } else { - bookmarks = [] - } - localPaths = userDefaults.dictionary(forKey: localPathsKey) as? [String: String] ?? [:] - pendingUploads = Set(userDefaults.stringArray(forKey: pendingUploadsKey) ?? []) - pendingDeletes = Set(userDefaults.stringArray(forKey: pendingDeletesKey) ?? []) + self.dataStore = dataStore ?? .shared } - deinit { - observation?.cancel() + deinit { observation?.cancel() } + + func load() throws { + bookmarks = try dataStore.values(RepositoryBookmark.self, in: "bookmarks").values.sorted { + $0.name.localizedCaseInsensitiveCompare($1.name) == .orderedAscending + } + localPaths = try dataStore.values(String.self, in: "bookmarkPaths") } func updateAccount(_ account: AccountSnapshot?) async { - let uid = account?.uid - guard uid != activeUID else { return } - - observation?.cancel() - observation = nil - activeUID = uid - guard let uid, let cloudStore else { return } - - await flushPendingChanges(uid: uid, cloudStore: cloudStore) - do { + try await dataStore.prepare() + try load() + let uid = account?.uid + guard uid != activeUID else { return } + observation?.cancel() + observation = nil + activeUID = uid + guard let uid, let cloudStore else { return } + await flushPendingChanges(uid: uid, cloudStore: cloudStore) + guard activeUID == uid else { return } let cloudBookmarks = try await cloudStore.bookmarks(uid: uid) guard activeUID == uid else { return } - applyCloudBookmarks(cloudBookmarks) + try await applyCloudBookmarks(cloudBookmarks, uid: uid) + guard activeUID == uid else { return } observation = cloudStore.observe(uid: uid) { [weak self] result in Task { @MainActor [weak self] in guard let self, self.activeUID == uid else { return } - switch result { - case .success(let bookmarks): - self.applyCloudBookmarks(bookmarks) - case .failure(let error): - self.errorMessage = error.localizedDescription - } + do { + try await self.applyCloudBookmarks(result.get(), uid: uid) + } catch { self.errorMessage = error.localizedDescription } } } - } catch { - guard activeUID == uid else { return } - errorMessage = error.localizedDescription - } + } catch { errorMessage = error.localizedDescription } } - func bookmark(forID id: String) -> RepositoryBookmark? { - bookmarks.first { $0.id == id } - } + func bookmark(forID id: String) -> RepositoryBookmark? { bookmarks.first { $0.id == id } } func bookmark(remoteURLString: String) -> RepositoryBookmark? { - guard let identity = RepositoryBookmarkIdentity.resolve(remoteURLString: remoteURLString) else { - return nil - } + guard let identity = RepositoryBookmarkIdentity.resolve(remoteURLString: remoteURLString) else { return nil } return bookmarks.first { $0.canonicalKey == identity.canonicalKey } } func localURL(for bookmark: RepositoryBookmark) -> URL? { - guard let path = localPaths[bookmark.id] else { return nil } - return URL(fileURLWithPath: path, isDirectory: true) + localPaths[bookmark.id].map { URL(fileURLWithPath: $0, isDirectory: true) } } - func bookmarkID(linkedTo url: URL) -> String? { - localPaths.first { $0.value == url.path }?.key - } + func bookmarkID(linkedTo url: URL) -> String? { localPaths.first { $0.value == url.path }?.key } func addBookmark(for repositoryURL: URL) async throws -> RepositoryBookmark { let remoteURLString = try await bookmarkRemoteURL(in: repositoryURL) guard let identity = RepositoryBookmarkIdentity.resolve(remoteURLString: remoteURLString) else { throw RepositoryBookmarkError.unsupportedRemote } - - if let existing = bookmarks.first(where: { $0.canonicalKey == identity.canonicalKey }) { - link(existing, to: repositoryURL) - return existing + let bookmark = try await dataStore.transaction { transaction in + let existing = try transaction.values(RepositoryBookmark.self, in: "bookmarks").values.first { $0.canonicalKey == identity.canonicalKey } + let bookmark = existing ?? RepositoryBookmark(identity: identity) + try transaction.set(bookmark, in: "bookmarks", id: bookmark.id) + try transaction.set(repositoryURL.path, in: "bookmarkPaths", id: bookmark.id) + if existing == nil { + transaction.remove(in: "bookmarkDeletes", id: bookmark.id) + try transaction.set(UUID().uuidString, in: "bookmarkUploads", id: bookmark.id) + } + return bookmark } - - let bookmark = RepositoryBookmark(identity: identity) - bookmarks.append(bookmark) - localPaths[bookmark.id] = repositoryURL.path - pendingDeletes.remove(bookmark.id) - pendingUploads.insert(bookmark.id) - persist() - await upload(bookmark) + try load() + if let uid = activeUID, let cloudStore { await upload(bookmark, uid: uid, cloudStore: cloudStore) } return bookmark } func removeBookmark(_ bookmark: RepositoryBookmark) async { - bookmarks.removeAll { $0.id == bookmark.id } - localPaths.removeValue(forKey: bookmark.id) - pendingUploads.remove(bookmark.id) - pendingDeletes.insert(bookmark.id) - persist() - - guard let uid = activeUID, let cloudStore else { return } - syncingBookmarkIDs.insert(bookmark.id) do { - try await cloudStore.delete(bookmarkID: bookmark.id, uid: uid) - pendingDeletes.remove(bookmark.id) - persist() - } catch { - errorMessage = error.localizedDescription - } - syncingBookmarkIDs.remove(bookmark.id) + try await dataStore.transaction { transaction in + transaction.remove(in: "bookmarks", id: bookmark.id) + transaction.remove(in: "bookmarkPaths", id: bookmark.id) + transaction.remove(in: "bookmarkUploads", id: bookmark.id) + try transaction.set(UUID().uuidString, in: "bookmarkDeletes", id: bookmark.id) + } + try load() + if let uid = activeUID, let cloudStore { await deleteFromCloud(bookmark.id, uid: uid, cloudStore: cloudStore) } + } catch { errorMessage = error.localizedDescription } } - func link(_ bookmark: RepositoryBookmark, to repositoryURL: URL) { - localPaths[bookmark.id] = repositoryURL.path - persist() + func link(_ bookmark: RepositoryBookmark, to repositoryURL: URL) async throws { + try await dataStore.transaction { transaction in + try transaction.set(repositoryURL.path, in: "bookmarkPaths", id: bookmark.id) + } + try load() } func validateAndLink(_ bookmark: RepositoryBookmark, to repositoryURL: URL) async throws { let remoteURLString = try await bookmarkRemoteURL(in: repositoryURL) guard let identity = RepositoryBookmarkIdentity.resolve(remoteURLString: remoteURLString), - identity.canonicalKey == bookmark.canonicalKey else { - throw RepositoryBookmarkError.folderDoesNotMatch - } - link(bookmark, to: repositoryURL) + identity.canonicalKey == bookmark.canonicalKey else { throw RepositoryBookmarkError.folderDoesNotMatch } + try await link(bookmark, to: repositoryURL) } - func unlinkLocalFolder(for bookmark: RepositoryBookmark) { - localPaths.removeValue(forKey: bookmark.id) - persist() + func unlinkLocalFolder(for bookmark: RepositoryBookmark) async { + do { + try await dataStore.transaction { $0.remove(in: "bookmarkPaths", id: bookmark.id) } + try load() + } catch { errorMessage = error.localizedDescription } } - func clearError() { - errorMessage = nil - } + func clearError() { errorMessage = nil } private func bookmarkRemoteURL(in repositoryURL: URL) async throws -> String { let origin = await GitStatusService.shared.remoteURL(remote: "origin", in: repositoryURL) - if !origin.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { - return origin - } - + if !origin.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { return origin } for remote in await GitStatusService.shared.remotes(in: repositoryURL) { let remoteURL = await GitStatusService.shared.remoteURL(remote: remote, in: repositoryURL) - if !remoteURL.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { - return remoteURL - } + if !remoteURL.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { return remoteURL } } throw RepositoryBookmarkError.noRemote } - private func upload(_ bookmark: RepositoryBookmark) async { - guard let uid = activeUID, let cloudStore else { return } - syncingBookmarkIDs.insert(bookmark.id) - do { - try await cloudStore.save(bookmark, uid: uid) - pendingUploads.remove(bookmark.id) - persist() - } catch { - errorMessage = error.localizedDescription + private func acknowledge(_ collection: String, id: String, version: String, uid: String) async throws { + try await dataStore.transaction { transaction in + guard self.activeUID == uid, + try transaction.value(String.self, in: collection, id: id) == version else { return } + transaction.remove(in: collection, id: id) } - syncingBookmarkIDs.remove(bookmark.id) } - private func flushPendingChanges( - uid: String, - cloudStore: RepositoryBookmarkCloudStore - ) async { - for bookmarkID in pendingDeletes { - do { - try await cloudStore.delete(bookmarkID: bookmarkID, uid: uid) - pendingDeletes.remove(bookmarkID) - } catch { - errorMessage = error.localizedDescription - } - } - - for bookmark in bookmarks where pendingUploads.contains(bookmark.id) { - do { - try await cloudStore.save(bookmark, uid: uid) - pendingUploads.remove(bookmark.id) - } catch { - errorMessage = error.localizedDescription - } - } - persist() + private func upload(_ bookmark: RepositoryBookmark, uid: String, cloudStore: RepositoryBookmarkCloudStore) async { + guard activeUID == uid else { return } + syncingBookmarkIDs.insert(bookmark.id) + defer { syncingBookmarkIDs.remove(bookmark.id) } + do { + guard let version = try dataStore.value(String.self, in: "bookmarkUploads", id: bookmark.id) else { return } + try await cloudStore.save(bookmark, uid: uid) + try await acknowledge("bookmarkUploads", id: bookmark.id, version: version, uid: uid) + } catch { errorMessage = error.localizedDescription } } - private func applyCloudBookmarks(_ cloudBookmarks: [RepositoryBookmark]) { - let pendingLocal = bookmarks.filter { pendingUploads.contains($0.id) } - var merged = Dictionary(uniqueKeysWithValues: cloudBookmarks.map { ($0.id, $0) }) - for bookmark in pendingLocal { - merged[bookmark.id] = bookmark - } - for deletedID in pendingDeletes { - merged.removeValue(forKey: deletedID) - } - bookmarks = merged.values.sorted { - $0.name.localizedCaseInsensitiveCompare($1.name) == .orderedAscending - } - let validIDs = Set(bookmarks.map(\.id)) - localPaths = localPaths.filter { validIDs.contains($0.key) } - persist() + private func deleteFromCloud(_ id: String, uid: String, cloudStore: RepositoryBookmarkCloudStore) async { + guard activeUID == uid else { return } + syncingBookmarkIDs.insert(id) + defer { syncingBookmarkIDs.remove(id) } + do { + guard let version = try dataStore.value(String.self, in: "bookmarkDeletes", id: id) else { return } + try await cloudStore.delete(bookmarkID: id, uid: uid) + try await acknowledge("bookmarkDeletes", id: id, version: version, uid: uid) + } catch { errorMessage = error.localizedDescription } } - private func persist() { - if let data = try? JSONEncoder().encode(bookmarks) { - userDefaults.set(data, forKey: bookmarksKey) + private func flushPendingChanges(uid: String, cloudStore: RepositoryBookmarkCloudStore) async { + do { + for id in try dataStore.values(String.self, in: "bookmarkDeletes").keys { + await deleteFromCloud(id, uid: uid, cloudStore: cloudStore) + } + for bookmark in bookmarks { await upload(bookmark, uid: uid, cloudStore: cloudStore) } + } catch { errorMessage = error.localizedDescription } + } + + private func applyCloudBookmarks(_ cloudBookmarks: [RepositoryBookmark], uid: String) async throws { + try await dataStore.transaction { transaction in + guard self.activeUID == uid else { return } + let pendingUploads = try transaction.values(String.self, in: "bookmarkUploads") + let pendingDeletes = try transaction.values(String.self, in: "bookmarkDeletes") + var merged = Dictionary(cloudBookmarks.map { ($0.id, $0) }, uniquingKeysWith: { _, last in last }) + for (id, bookmark) in try transaction.values(RepositoryBookmark.self, in: "bookmarks") where pendingUploads[id] != nil { + merged[id] = bookmark + } + for id in pendingDeletes.keys { merged[id] = nil } + for id in try transaction.values(RepositoryBookmark.self, in: "bookmarks").keys where merged[id] == nil { + transaction.remove(in: "bookmarks", id: id) + } + for (id, bookmark) in merged { try transaction.set(bookmark, in: "bookmarks", id: id) } + for id in try transaction.values(String.self, in: "bookmarkPaths").keys where merged[id] == nil { + transaction.remove(in: "bookmarkPaths", id: id) + } } - userDefaults.set(localPaths, forKey: localPathsKey) - userDefaults.set(Array(pendingUploads), forKey: pendingUploadsKey) - userDefaults.set(Array(pendingDeletes), forKey: pendingDeletesKey) + try load() } } diff --git a/macgit/App/RepositoryCommitRuleSyncController.swift b/macgit/App/RepositoryCommitRuleSyncController.swift index 3e29d2cb..b33b9e1a 100644 --- a/macgit/App/RepositoryCommitRuleSyncController.swift +++ b/macgit/App/RepositoryCommitRuleSyncController.swift @@ -4,18 +4,17 @@ import Combine @MainActor final class RepositoryCommitRuleSyncController: ObservableObject { - private let defaults: UserDefaults + private let dataStore: LocalDataStore private let localStore: RepoSettingsStore private let resolver: any RepositoryRemoteIdentityResolving - private let pendingKey = "dev.thanhtran.macgit.repositoryCommitRules.pending" private var sessionID = UUID() private var activeUID: String? private var activePath: String? private var runningSessions: Set = [] - init(defaults: UserDefaults = .standard, localStore: RepoSettingsStore = .shared, + init(localStore: RepoSettingsStore = .shared, resolver: any RepositoryRemoteIdentityResolving = RepositoryRemoteIdentityResolver()) { - self.defaults = defaults + self.dataStore = localStore.dataStore self.localStore = localStore self.resolver = resolver } @@ -27,11 +26,12 @@ final class RepositoryCommitRuleSyncController: ObservableObject { sessionID = UUID() } - func markChanged(_ value: Bool, uid: String?, repositoryURL: URL) { + func markChanged(_ value: Bool, uid: String?, repositoryURL: URL) async throws { guard let uid else { return } - var pending = pendingValues - pending[key(uid: uid, path: repositoryURL.path)] = value - defaults.set(pending, forKey: pendingKey) + let id = key(uid: uid, path: repositoryURL.path) + try await dataStore.transaction { transaction in + try transaction.set(value, in: "commitRulePending", id: id) + } } func reconcile(repositoryURL: URL, uid: String?, cloud: any RepositoryCommitRuleCloudStore, @@ -44,27 +44,36 @@ final class RepositoryCommitRuleSyncController: ObservableObject { let pendingID = key(uid: uid, path: repositoryURL.path) guard let identity = await resolver.identity(in: repositoryURL), session == sessionID else { return nil } do { + try await dataStore.prepare() + guard session == sessionID else { return nil } let initial = localValue(repositoryURL) let remote = try await cloud.load(identity: identity, uid: uid) guard session == sessionID else { return nil } // Edits made while offline or while loading always win over an older download. if pendingValues[pendingID] == nil, let remote { - if localValue(repositoryURL) == initial { - var settings = localStore.settings(for: repositoryURL.path, currentBranch: nil, remotes: []) + let applied = try await dataStore.transaction { transaction in + guard session == self.sessionID, + try transaction.value(Bool.self, in: "commitRulePending", id: pendingID) == nil else { return false } + var settings = try transaction.value(RepoSettings.self, in: "repoSettings", id: repositoryURL.path) + ?? RepoSettings.defaults(currentBranch: nil, remotes: []) + guard settings.skipProtectedBranchCommitWarnings == initial else { return false } settings.skipProtectedBranchCommitWarnings = remote - localStore.update(for: repositoryURL.path, settings: settings) + try transaction.set(settings, in: "repoSettings", id: repositoryURL.path) + return true + } + if applied, session == sessionID, pendingValues[pendingID] == nil, localValue(repositoryURL) == remote { onApplied(remote) } } else if pendingValues[pendingID] == nil { - markChanged(initial, uid: uid, repositoryURL: repositoryURL) + try await markChanged(initial, uid: uid, repositoryURL: repositoryURL) } while session == sessionID, let value = pendingValues[pendingID] { try await cloud.save(value, identity: identity, uid: uid) guard session == sessionID else { return nil } - var pending = pendingValues - if pending[pendingID] == value { - pending.removeValue(forKey: pendingID) - defaults.set(pending, forKey: pendingKey) + try await dataStore.transaction { transaction in + guard session == self.sessionID, + try transaction.value(Bool.self, in: "commitRulePending", id: pendingID) == value else { return } + transaction.remove(in: "commitRulePending", id: pendingID) } } return nil @@ -75,7 +84,7 @@ final class RepositoryCommitRuleSyncController: ObservableObject { } private var pendingValues: [String: Bool] { - defaults.dictionary(forKey: pendingKey) as? [String: Bool] ?? [:] + (try? dataStore.values(Bool.self, in: "commitRulePending")) ?? [:] } private func key(uid: String, path: String) -> String { "\(uid)|\(path)" } diff --git a/macgit/App/RepositoryVisibilityController.swift b/macgit/App/RepositoryVisibilityController.swift index 094372b5..d3a782d7 100644 --- a/macgit/App/RepositoryVisibilityController.swift +++ b/macgit/App/RepositoryVisibilityController.swift @@ -150,7 +150,7 @@ final class RepositoryVisibilityController: ObservableObject { guard let service = services[repository.provider] else { return .unknown } if let anonymousResult = try? await service.visibility(for: repository, token: nil) { - save(anonymousResult, repository: repository) + await save(anonymousResult, repository: repository) return anonymousResult } @@ -169,7 +169,7 @@ final class RepositoryVisibilityController: ObservableObject { continue } if let authenticatedResult = try? await service.visibility(for: repository, token: token) { - save(authenticatedResult, repository: repository) + await save(authenticatedResult, repository: repository) return authenticatedResult } } @@ -179,8 +179,8 @@ final class RepositoryVisibilityController: ObservableObject { private func save( _ visibility: RepositoryVisibility, repository: GitRepositoryIdentity - ) { - cache.save(visibility, for: repository, resolvedAt: .now) + ) async { + await cache.save(visibility, for: repository, resolvedAt: .now) } private func matchingAccounts( diff --git a/macgit/App/macgitApp.swift b/macgit/App/macgitApp.swift index ba1d6d1a..2f5f48bc 100644 --- a/macgit/App/macgitApp.swift +++ b/macgit/App/macgitApp.swift @@ -127,7 +127,7 @@ struct macgitApp: App { .gitlab: GitLabRepositoryVisibilityService(), ], tokenVault: providerTokenVault, - cache: UserDefaultsRepositoryVisibilityCache() + cache: SQLiteRepositoryVisibilityCache() ) ) _repositoryBookmarkController = StateObject( @@ -210,35 +210,37 @@ struct macgitApp: App { request: RepositoryWindowRequest?, isWelcomeWindow: Bool = false ) -> some View { - ContentView( - request: request, - isWelcomeWindow: isWelcomeWindow, - accountController: accountController, - providerAccountController: providerAccountController, - aiProviderController: aiProviderController - ) - .environmentObject(appState) - .environmentObject(appUpdateController) - .environmentObject(featureAccessController) - .environmentObject(repositoryVisibilityController) - .environmentObject(repositoryBookmarkController) - .environmentObject(gitFlowConfigurationSyncController) - .preferredColorScheme(appState.appearance.colorScheme) - .task { - appUpdateController.start() - } - .onChange(of: accountController.account?.uid, initial: true) { _, uid in - aiProviderController.managedUsageController?.setSession(uid: uid) - Task { await aiProviderController.refreshAvailability() } - } - .onChange(of: accountController.entitlement) { _, _ in - Task { await aiProviderController.refreshAvailability() } - } - .onReceive(NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification)) { _ in - Task { - await aiProviderController.managedUsageController?.refresh() + LocalDataLoadingView { + ContentView( + request: request, + isWelcomeWindow: isWelcomeWindow, + accountController: accountController, + providerAccountController: providerAccountController, + aiProviderController: aiProviderController + ) + .environmentObject(appState) + .environmentObject(appUpdateController) + .environmentObject(featureAccessController) + .environmentObject(repositoryVisibilityController) + .environmentObject(repositoryBookmarkController) + .environmentObject(gitFlowConfigurationSyncController) + .preferredColorScheme(appState.appearance.colorScheme) + .task { + appUpdateController.start() } - } + .onChange(of: accountController.account?.uid, initial: true) { _, uid in + aiProviderController.managedUsageController?.setSession(uid: uid) + Task { await aiProviderController.refreshAvailability() } + } + .onChange(of: accountController.entitlement) { _, _ in + Task { await aiProviderController.refreshAvailability() } + } + .onReceive(NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification)) { _ in + Task { + await aiProviderController.managedUsageController?.refresh() + } + } + } } var body: some Scene { diff --git a/macgit/Services/GitProviderAccountPreferenceStore.swift b/macgit/Services/GitProviderAccountPreferenceStore.swift index 90f059a3..e85b6d67 100644 --- a/macgit/Services/GitProviderAccountPreferenceStore.swift +++ b/macgit/Services/GitProviderAccountPreferenceStore.swift @@ -32,49 +32,42 @@ enum GitProviderAccountPreferenceKey { } } +@MainActor final class GitProviderAccountPreferenceStore { static let shared = GitProviderAccountPreferenceStore() - - private let userDefaults: UserDefaults - private let key: String - private var accountIDsByRemoteIdentity: [String: String] - - init( - userDefaults: UserDefaults = .standard, - key: String = "dev.thanhtran.macgit.providerAccountPreferences" - ) { - self.userDefaults = userDefaults - self.key = key - accountIDsByRemoteIdentity = userDefaults.dictionary(forKey: key) as? [String: String] ?? [:] - } + private let dataStore: LocalDataStore + init(dataStore: LocalDataStore? = nil) { self.dataStore = dataStore ?? .shared } var preferences: [String: String] { - accountIDsByRemoteIdentity + (try? dataStore.values(String.self, in: "providerPreferences")) ?? [:] } func accountID(for identity: GitRemoteIdentity) -> String? { - accountIDsByRemoteIdentity[GitProviderAccountPreferenceKey.make(for: identity)] + preferences[GitProviderAccountPreferenceKey.make(for: identity)] } - func update(accountID: String?, for identity: GitRemoteIdentity) { - update(accountID: accountID, forPreferenceKey: GitProviderAccountPreferenceKey.make(for: identity)) + func update(accountID: String?, for identity: GitRemoteIdentity) async throws { + try await update(accountID: accountID, forPreferenceKey: GitProviderAccountPreferenceKey.make(for: identity)) } - func update(accountID: String?, forPreferenceKey preferenceKey: String) { - if let accountID, !accountID.isEmpty { - accountIDsByRemoteIdentity[preferenceKey] = accountID - } else { - accountIDsByRemoteIdentity.removeValue(forKey: preferenceKey) + func update(accountID: String?, forPreferenceKey preferenceKey: String) async throws { + try await dataStore.transaction { transaction in + if let accountID, !accountID.isEmpty { + try transaction.set(accountID, in: "providerPreferences", id: preferenceKey) + } else { + transaction.remove(in: "providerPreferences", id: preferenceKey) + } } - save() } - func replacePreferences(_ preferences: [String: String]) { - accountIDsByRemoteIdentity = preferences.filter { !$0.value.isEmpty } - save() - } - - private func save() { - userDefaults.set(accountIDsByRemoteIdentity, forKey: key) + func replacePreferences(_ preferences: [String: String]) async throws { + try await dataStore.transaction { transaction in + for key in try transaction.values(String.self, in: "providerPreferences").keys { + transaction.remove(in: "providerPreferences", id: key) + } + for (key, value) in preferences where !value.isEmpty { + try transaction.set(value, in: "providerPreferences", id: key) + } + } } } diff --git a/macgit/Services/GitProviderSSHKeyStore.swift b/macgit/Services/GitProviderSSHKeyStore.swift index 72021197..399ec1ef 100644 --- a/macgit/Services/GitProviderSSHKeyStore.swift +++ b/macgit/Services/GitProviderSSHKeyStore.swift @@ -24,8 +24,8 @@ struct GitProviderSSHKey: Equatable, Codable { protocol GitProviderSSHKeyStore { func key(for account: GitProviderAccount) throws -> GitProviderSSHKey? - func saveKey(_ key: GitProviderSSHKey, for account: GitProviderAccount) throws - func deleteKey(for account: GitProviderAccount) throws + func saveKey(_ key: GitProviderSSHKey, for account: GitProviderAccount) async throws + func deleteKey(for account: GitProviderAccount) async throws } enum GitProviderSSHKeyStoreKey { @@ -40,28 +40,23 @@ enum GitProviderSSHKeyStoreKey { } } -struct UserDefaultsGitProviderSSHKeyStore: GitProviderSSHKeyStore { - private let defaults: UserDefaults - private let encoder = JSONEncoder() - private let decoder = JSONDecoder() - - init(defaults: UserDefaults = .standard) { - self.defaults = defaults - } +struct SQLiteGitProviderSSHKeyStore: GitProviderSSHKeyStore { + private let dataStore: LocalDataStore + init(dataStore: LocalDataStore? = nil) { self.dataStore = dataStore ?? .shared } func key(for account: GitProviderAccount) throws -> GitProviderSSHKey? { - guard let data = defaults.data(forKey: GitProviderSSHKeyStoreKey.storageKey(for: account)) else { - return nil - } - return try decoder.decode(GitProviderSSHKey.self, from: data) + try dataStore.value(GitProviderSSHKey.self, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)) } - func saveKey(_ key: GitProviderSSHKey, for account: GitProviderAccount) throws { - let data = try encoder.encode(key) - defaults.set(data, forKey: GitProviderSSHKeyStoreKey.storageKey(for: account)) + func saveKey(_ key: GitProviderSSHKey, for account: GitProviderAccount) async throws { + try await dataStore.transaction { transaction in + try transaction.set(key, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)) + } } - func deleteKey(for account: GitProviderAccount) throws { - defaults.removeObject(forKey: GitProviderSSHKeyStoreKey.storageKey(for: account)) + func deleteKey(for account: GitProviderAccount) async throws { + try await dataStore.transaction { transaction in + transaction.remove(in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)) + } } } diff --git a/macgit/Services/LegacyLocalDataMigration.swift b/macgit/Services/LegacyLocalDataMigration.swift new file mode 100644 index 00000000..43e93326 --- /dev/null +++ b/macgit/Services/LegacyLocalDataMigration.swift @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Foundation + +enum LegacyLocalDataMigration { + // Leave the original defaults intact for recovery. The database migration + // marker prevents stale defaults from resurrecting subsequently deleted rows. + static func records(from defaults: UserDefaults) throws -> [String: [String: Data]] { + var transaction = LocalDataTransaction(records: [:]) + func decode(_ type: T.Type, key: String) throws -> T? { + guard defaults.object(forKey: key) != nil else { return nil } + guard let data = defaults.data(forKey: key), + let value = try? JSONDecoder().decode(type, from: data) else { + throw LocalDataError.invalidLegacyData(key) + } + return value + } + func dictionary(_ type: T.Type, key: String) throws -> [String: T] { + guard defaults.object(forKey: key) != nil else { return [:] } + guard let values = defaults.dictionary(forKey: key) as? [String: T] else { + throw LocalDataError.invalidLegacyData(key) + } + return values + } + func strings(key: String) throws -> [String] { + guard defaults.object(forKey: key) != nil else { return [] } + guard let values = defaults.stringArray(forKey: key) else { throw LocalDataError.invalidLegacyData(key) } + return values + } + let prefix = "dev.thanhtran.macgit." + for bookmark in try decode([RepositoryBookmark].self, key: prefix + "repositoryBookmarks.items") ?? [] { + try transaction.set(bookmark, in: "bookmarks", id: bookmark.id) + } + for (id, path) in try dictionary(String.self, key: prefix + "repositoryBookmarks.localPaths") { + try transaction.set(path, in: "bookmarkPaths", id: id) + } + for id in try strings(key: prefix + "repositoryBookmarks.pendingUploads") { + try transaction.set(UUID().uuidString, in: "bookmarkUploads", id: id) + } + for id in try strings(key: prefix + "repositoryBookmarks.pendingDeletes") { + try transaction.set(UUID().uuidString, in: "bookmarkDeletes", id: id) + } + for account in try decode([GitProviderAccount].self, key: prefix + "localGitProviderAccounts") ?? [] { + try transaction.set(account, in: "providerAccounts", id: account.id) + } + for account in try decode([GitProviderAccount].self, key: prefix + "localGitProviderAccountPendingDeletions") ?? [] { + try transaction.set(account, in: "providerDeletions", id: account.id) + } + for (uid, identities) in try dictionary([String].self, key: prefix + "localGitProviderAccountSyncedIdentities") { + try transaction.set(identities, in: "providerSyncedIdentities", id: uid) + } + for (id, settings) in try decode([String: RepoSettings].self, key: prefix + "repoSettings") ?? [:] { + try transaction.set(settings, in: "repoSettings", id: id) + } + for (id, accountID) in try dictionary(String.self, key: prefix + "providerAccountPreferences") { + try transaction.set(accountID, in: "providerPreferences", id: id) + } + for key in defaults.dictionaryRepresentation().keys where key.hasPrefix("gitProviderSSHKey.") { + if let value = try decode(GitProviderSSHKey.self, key: key) { + try transaction.set(value, in: "sshPaths", id: key) + } + } + for (id, value) in try dictionary(Bool.self, key: prefix + "repositoryCommitRules.pending") { + try transaction.set(value, in: "commitRulePending", id: id) + } + for id in try strings(key: prefix + "gitFlowConfiguration.pendingUploads") { + try transaction.set(UUID().uuidString, in: "gitFlowPending", id: id) + } + // This cache is disposable. A corrupt cache must not prevent migration of user data. + if let values = try? decode([String: CachedRepositoryVisibility].self, key: prefix + "repositoryVisibility") { + for (id, value) in values where Date().timeIntervalSince(value.resolvedAt) <= 30 * 24 * 60 * 60 { + try transaction.set(value, in: "repositoryVisibility", id: id) + } + } + return transaction.records + } +} diff --git a/macgit/Services/LocalDataError.swift b/macgit/Services/LocalDataError.swift new file mode 100644 index 00000000..3e9db796 --- /dev/null +++ b/macgit/Services/LocalDataError.swift @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Foundation + +enum LocalDataError: LocalizedError { + case notReady + case invalidLegacyData(String) + var errorDescription: String? { + switch self { + case .notReady: "Local data is still loading." + case .invalidLegacyData(let key): "Saved data could not be migrated (\(key)). The original data has been retained." + } + } +} diff --git a/macgit/Services/LocalDataStore.swift b/macgit/Services/LocalDataStore.swift new file mode 100644 index 00000000..0b5db19a --- /dev/null +++ b/macgit/Services/LocalDataStore.swift @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Combine +import Foundation + +/// UI reads use this snapshot. SQLite I/O is isolated to LocalSQLiteDatabase. +/// All writers share one queue, including transactions spanning several stores. +@MainActor +final class LocalDataStore: ObservableObject { + static let shared: LocalDataStore = { + // XCTest hosts must never migrate or mutate the user's real database. + if FirebaseBootstrap.isRunningUnitTests { + let id = UUID().uuidString + return LocalDataStore( + databaseURL: FileManager.default.temporaryDirectory.appending(path: "CommitPlusTests-\(id)/store.sqlite"), + userDefaults: UserDefaults(suiteName: "CommitPlusTests.\(id)")! + ) + } + return LocalDataStore() + }() + @Published private(set) var isReady = false + @Published private(set) var errorMessage: String? + private let database: LocalSQLiteDatabase + private let defaults: UserDefaults + private var records: [String: [String: Data]] = [:] + private var loading: Task? + private var writer: Task? + + init(databaseURL: URL = URL.applicationSupportDirectory.appending(path: "Commit+/LocalData/store.sqlite"), + userDefaults: UserDefaults = .standard) { + database = LocalSQLiteDatabase(url: databaseURL) + defaults = userDefaults + } + + func prepare() async throws { + if isReady { return } + if let loading { return try await loading.value } + let task = Task { @MainActor in + let needsImport = try await database.needsLegacyImport() + let legacy = needsImport ? try LegacyLocalDataMigration.records(from: defaults) : nil + records = try await database.load(importing: legacy) + isReady = true + errorMessage = nil + } + loading = task + do { + try await task.value + loading = nil + } catch { + loading = nil + errorMessage = "Could not open local data: \(error.localizedDescription)" + throw error + } + } + + func value(_ type: T.Type, in collection: String, id: String) throws -> T? { + guard isReady else { throw LocalDataError.notReady } + return try records[collection]?[id].map { try JSONDecoder().decode(type, from: $0) } + } + + func values(_ type: T.Type, in collection: String) throws -> [String: T] { + guard isReady else { throw LocalDataError.notReady } + return try (records[collection] ?? [:]).mapValues { try JSONDecoder().decode(type, from: $0) } + } + + func transaction(_ update: @escaping (inout LocalDataTransaction) throws -> T) async throws -> T { + let previous = writer + let task = Task { @MainActor in + await previous?.value + try await prepare() + var transaction = LocalDataTransaction(records: records) + let result = try update(&transaction) + try await database.commit(transaction.changes) + records = transaction.records + return result + } + writer = Task { _ = try? await task.value } + do { return try await task.value } + catch { + errorMessage = "Could not save local data: \(error.localizedDescription)" + throw error + } + } +} + diff --git a/macgit/Services/LocalDataTransaction.swift b/macgit/Services/LocalDataTransaction.swift new file mode 100644 index 00000000..5cb4d6ef --- /dev/null +++ b/macgit/Services/LocalDataTransaction.swift @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Foundation + +struct LocalDataTransaction { + private(set) var records: [String: [String: Data]] + private(set) var changes: [(String, String, Data?)] = [] + + func value(_ type: T.Type, in collection: String, id: String) throws -> T? { + try records[collection]?[id].map { try JSONDecoder().decode(type, from: $0) } + } + + func values(_ type: T.Type, in collection: String) throws -> [String: T] { + try (records[collection] ?? [:]).mapValues { try JSONDecoder().decode(type, from: $0) } + } + + mutating func set(_ value: T, in collection: String, id: String) throws { + let data = try JSONEncoder().encode(value) + records[collection, default: [:]][id] = data + changes.append((collection, id, data)) + } + + mutating func remove(in collection: String, id: String) { + records[collection]?[id] = nil + changes.append((collection, id, nil)) + } +} diff --git a/macgit/Services/LocalGitProviderAccountStore.swift b/macgit/Services/LocalGitProviderAccountStore.swift index 92a48743..4f1e3120 100644 --- a/macgit/Services/LocalGitProviderAccountStore.swift +++ b/macgit/Services/LocalGitProviderAccountStore.swift @@ -23,113 +23,100 @@ protocol GitProviderAccountLocalStore { var accountOwnerID: String { get } func accounts() throws -> [GitProviderAccount] - func save(_ account: GitProviderAccount) throws - func delete(accountID: String) throws -> GitProviderAccount? - func remove(accountID: String) throws + func save(_ account: GitProviderAccount) async throws + func delete(accountID: String) async throws -> GitProviderAccount? + func remove(accountID: String) async throws func pendingDeletions() throws -> [GitProviderAccount] - func clearPendingDeletion(_ account: GitProviderAccount) throws + func clearPendingDeletion(_ account: GitProviderAccount) async throws func syncedIdentityKeys(uid: String) -> Set - func setSyncedIdentityKeys(_ keys: Set, uid: String) + func setSyncedIdentityKeys(_ keys: Set, uid: String) async throws } @MainActor -final class UserDefaultsGitProviderAccountLocalStore: GitProviderAccountLocalStore { +final class SQLiteGitProviderAccountLocalStore: GitProviderAccountLocalStore { let accountOwnerID: String - - private let defaults: UserDefaults - private let accountsKey: String - private let pendingDeletionsKey: String - private let syncedIdentitiesKey: String - private let encoder = JSONEncoder() - private let decoder = JSONDecoder() - - init( - defaults: UserDefaults = .standard, - accountsKey: String = "dev.thanhtran.macgit.localGitProviderAccounts", - pendingDeletionsKey: String = "dev.thanhtran.macgit.localGitProviderAccountPendingDeletions", - syncedIdentitiesKey: String = "dev.thanhtran.macgit.localGitProviderAccountSyncedIdentities", - ownerIDKey: String = "dev.thanhtran.macgit.localGitProviderAccountOwnerID" - ) { - self.defaults = defaults - self.accountsKey = accountsKey - self.pendingDeletionsKey = pendingDeletionsKey - self.syncedIdentitiesKey = syncedIdentitiesKey - - if let storedOwnerID = defaults.string(forKey: ownerIDKey), !storedOwnerID.isEmpty { - accountOwnerID = storedOwnerID - } else { - let newOwnerID = "local-\(UUID().uuidString)" - defaults.set(newOwnerID, forKey: ownerIDKey) - accountOwnerID = newOwnerID + private let dataStore: LocalDataStore + + init(dataStore: LocalDataStore? = nil, defaults: UserDefaults = .standard) { + self.dataStore = dataStore ?? .shared + // Keep this installation identity stable: it also participates in credential keys. + let key = "dev.thanhtran.macgit.localGitProviderAccountOwnerID" + if let owner = defaults.string(forKey: key), !owner.isEmpty { accountOwnerID = owner } + else { + accountOwnerID = "local-\(UUID().uuidString)" + defaults.set(accountOwnerID, forKey: key) } } func accounts() throws -> [GitProviderAccount] { - guard let data = defaults.data(forKey: accountsKey) else { return [] } - return try decoder.decode([GitProviderAccount].self, from: data) + try dataStore.values(GitProviderAccount.self, in: "providerAccounts").values.sorted { $0.connectedAt < $1.connectedAt } } - func save(_ account: GitProviderAccount) throws { - var storedAccounts = try accounts() - let identity = GitProviderAccountLocalIdentity(account) - storedAccounts.removeAll { - $0.id == account.id || GitProviderAccountLocalIdentity($0) == identity + func save(_ account: GitProviderAccount) async throws { + try await dataStore.transaction { transaction in + let identity = GitProviderAccountLocalIdentity(account) + for (id, stored) in try transaction.values(GitProviderAccount.self, in: "providerAccounts") + where id == account.id || GitProviderAccountLocalIdentity(stored) == identity { + transaction.remove(in: "providerAccounts", id: id) + } + try transaction.set(account, in: "providerAccounts", id: account.id) + for (id, deleted) in try transaction.values(GitProviderAccount.self, in: "providerDeletions") + where GitProviderAccountLocalIdentity(deleted) == identity { + transaction.remove(in: "providerDeletions", id: id) + } } - storedAccounts.append(account) - try persist(storedAccounts, key: accountsKey) - - var deletions = try pendingDeletions() - deletions.removeAll { GitProviderAccountLocalIdentity($0) == identity } - try persist(deletions, key: pendingDeletionsKey) } - func delete(accountID: String) throws -> GitProviderAccount? { - var storedAccounts = try accounts() - let deletedAccount = storedAccounts.first { $0.id == accountID } - storedAccounts.removeAll { $0.id == accountID } - try persist(storedAccounts, key: accountsKey) + func delete(accountID: String) async throws -> GitProviderAccount? { + try await dataStore.transaction { transaction in + guard let account = try transaction.value(GitProviderAccount.self, in: "providerAccounts", id: accountID) else { return nil } + try Self.remove(account, from: &transaction) + for (id, deleted) in try transaction.values(GitProviderAccount.self, in: "providerDeletions") + where GitProviderAccountLocalIdentity(deleted) == GitProviderAccountLocalIdentity(account) { + transaction.remove(in: "providerDeletions", id: id) + } + try transaction.set(account, in: "providerDeletions", id: account.id) + return account + } + } - if let deletedAccount { - var deletions = try pendingDeletions() - let identity = GitProviderAccountLocalIdentity(deletedAccount) - deletions.removeAll { GitProviderAccountLocalIdentity($0) == identity } - deletions.append(deletedAccount) - try persist(deletions, key: pendingDeletionsKey) + func remove(accountID: String) async throws { + try await dataStore.transaction { transaction in + if let account = try transaction.value(GitProviderAccount.self, in: "providerAccounts", id: accountID) { + try Self.remove(account, from: &transaction) + } } - return deletedAccount } - func remove(accountID: String) throws { - var storedAccounts = try accounts() - storedAccounts.removeAll { $0.id == accountID } - try persist(storedAccounts, key: accountsKey) + private static func remove(_ account: GitProviderAccount, from transaction: inout LocalDataTransaction) throws { + transaction.remove(in: "providerAccounts", id: account.id) + transaction.remove(in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)) + for (key, value) in try transaction.values(String.self, in: "providerPreferences") where value == account.id { + transaction.remove(in: "providerPreferences", id: key) + } } func pendingDeletions() throws -> [GitProviderAccount] { - guard let data = defaults.data(forKey: pendingDeletionsKey) else { return [] } - return try decoder.decode([GitProviderAccount].self, from: data) + Array(try dataStore.values(GitProviderAccount.self, in: "providerDeletions").values) } - func clearPendingDeletion(_ account: GitProviderAccount) throws { - var deletions = try pendingDeletions() - let identity = GitProviderAccountLocalIdentity(account) - deletions.removeAll { GitProviderAccountLocalIdentity($0) == identity } - try persist(deletions, key: pendingDeletionsKey) + func clearPendingDeletion(_ account: GitProviderAccount) async throws { + try await dataStore.transaction { transaction in + for (id, deleted) in try transaction.values(GitProviderAccount.self, in: "providerDeletions") + where GitProviderAccountLocalIdentity(deleted) == GitProviderAccountLocalIdentity(account) { + transaction.remove(in: "providerDeletions", id: id) + } + } } func syncedIdentityKeys(uid: String) -> Set { - let values = defaults.dictionary(forKey: syncedIdentitiesKey) as? [String: [String]] ?? [:] - return Set(values[uid] ?? []) - } - - func setSyncedIdentityKeys(_ keys: Set, uid: String) { - var values = defaults.dictionary(forKey: syncedIdentitiesKey) as? [String: [String]] ?? [:] - values[uid] = keys.sorted() - defaults.set(values, forKey: syncedIdentitiesKey) + Set((try? dataStore.value([String].self, in: "providerSyncedIdentities", id: uid)) ?? []) } - private func persist(_ accounts: [GitProviderAccount], key: String) throws { - defaults.set(try encoder.encode(accounts), forKey: key) + func setSyncedIdentityKeys(_ keys: Set, uid: String) async throws { + try await dataStore.transaction { transaction in + try transaction.set(keys.sorted(), in: "providerSyncedIdentities", id: uid) + } } } @@ -146,7 +133,7 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { localStore: GitProviderAccountLocalStore? = nil, cloudStore: GitProviderAccountCloudStore? ) { - self.localStore = localStore ?? UserDefaultsGitProviderAccountLocalStore() + self.localStore = localStore ?? SQLiteGitProviderAccountLocalStore() self.cloudStore = cloudStore } @@ -174,12 +161,12 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { if let cloudAccount = cloudAccounts.first(where: { GitProviderAccountLocalIdentity($0) == identity }) { do { try await cloudStore.delete(accountID: cloudAccount.id, macgitUID: uid) - try localStore.clearPendingDeletion(deletedAccount) + try await localStore.clearPendingDeletion(deletedAccount) } catch { // Keep the tombstone so a stale cloud snapshot cannot restore the local deletion. } } else { - try localStore.clearPendingDeletion(deletedAccount) + try await localStore.clearPendingDeletion(deletedAccount) } } @@ -195,7 +182,7 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { if previouslySyncedIdentityKeys.contains(identityKey), !visibleCloudIdentityKeys.contains(identityKey), !pendingIdentities.contains(GitProviderAccountLocalIdentity(localAccount)) { - try localStore.remove(accountID: localAccount.id) + try await localStore.remove(accountID: localAccount.id) } } @@ -207,7 +194,7 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { }) { cloudAccountIDsByLocalAccountID[localAccount.id] = cloudAccount.id } else { - try localStore.save(cloudAccount) + try await localStore.save(cloudAccount) mergedAccounts.append(cloudAccount) cloudAccountIDsByLocalAccountID[cloudAccount.id] = cloudAccount.id } @@ -219,21 +206,23 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { syncedIdentityKeys.insert(GitProviderAccountLocalIdentity(account).storageKey) } } - localStore.setSyncedIdentityKeys(syncedIdentityKeys, uid: uid) + try await localStore.setSyncedIdentityKeys(syncedIdentityKeys, uid: uid) } func save(_ account: GitProviderAccount) async throws { - try localStore.save(account) + try await localStore.save(account) guard let cloudUID else { return } if await mirrorToCloud(account, uid: cloudUID) { var syncedKeys = localStore.syncedIdentityKeys(uid: cloudUID) syncedKeys.insert(GitProviderAccountLocalIdentity(account).storageKey) - localStore.setSyncedIdentityKeys(syncedKeys, uid: cloudUID) + // The account is already durable. A sync bookkeeping failure must not + // make the caller delete credentials as if the local save had failed. + try? await localStore.setSyncedIdentityKeys(syncedKeys, uid: cloudUID) } } func delete(accountID: String) async throws { - guard let deletedAccount = try localStore.delete(accountID: accountID) else { return } + guard let deletedAccount = try await localStore.delete(accountID: accountID) else { return } guard let cloudUID, let cloudStore else { return } let cloudAccountID = cloudAccountIDsByLocalAccountID.removeValue(forKey: accountID) ?? accountID @@ -242,10 +231,10 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { if cloudAccountID != accountID { try? await cloudStore.delete(accountID: accountID, macgitUID: cloudUID) } - try localStore.clearPendingDeletion(deletedAccount) + try await localStore.clearPendingDeletion(deletedAccount) var syncedKeys = localStore.syncedIdentityKeys(uid: cloudUID) syncedKeys.remove(GitProviderAccountLocalIdentity(deletedAccount).storageKey) - localStore.setSyncedIdentityKeys(syncedKeys, uid: cloudUID) + try await localStore.setSyncedIdentityKeys(syncedKeys, uid: cloudUID) } catch { // The local deletion is complete; retry the cloud deletion on the next sync. } diff --git a/macgit/Services/LocalSQLiteDatabase.swift b/macgit/Services/LocalSQLiteDatabase.swift new file mode 100644 index 00000000..5c3d95d2 --- /dev/null +++ b/macgit/Services/LocalSQLiteDatabase.swift @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Foundation +import SQLite3 + +/// One row per entity, with atomic changes across collections. Payloads preserve +/// the existing Codable schemas; preferences and credentials are not stored here. +actor LocalSQLiteDatabase { + private let url: URL + init(url: URL) { self.url = url } + + func needsLegacyImport() throws -> Bool { + try withDatabase { db in try !hasImported(db) } + } + + func load(importing legacy: [String: [String: Data]]?) throws -> [String: [String: Data]] { + try withDatabase { db in + if let legacy { + try transaction(db) { + if try !hasImported(db) { + for (collection, values) in legacy { + for (id, payload) in values { + try put(db, collection, id, payload) + } + } + // Read back every imported row before marking the import complete. + let imported = try read(db) + for (collection, values) in legacy { + for (id, payload) in values where imported[collection]?[id] != payload { + throw failure("Local data migration verification failed.") + } + } + try query(db, "INSERT INTO migrations (id) VALUES ('user-defaults-v1')") + } + } + } + return try read(db) + } + } + + func commit(_ changes: [(String, String, Data?)]) throws { + guard !changes.isEmpty else { return } + try withDatabase { db in + try transaction(db) { + for (collection, id, payload) in changes { + if let payload { try put(db, collection, id, payload) } + else { try query(db, "DELETE FROM records WHERE collection = ? AND id = ?", [collection, id]) } + } + } + } + } + + private func hasImported(_ db: OpaquePointer) throws -> Bool { + var found = false + try query(db, "SELECT id FROM migrations WHERE id = 'user-defaults-v1'") { _ in found = true } + return found + } + + private func put(_ db: OpaquePointer, _ collection: String, _ id: String, _ payload: Data) throws { + try query(db, "INSERT INTO records (collection, id, payload) VALUES (?, ?, ?) ON CONFLICT(collection, id) DO UPDATE SET payload = excluded.payload", + [collection, id, String(decoding: payload, as: UTF8.self)]) + } + + private func read(_ db: OpaquePointer) throws -> [String: [String: Data]] { + var result: [String: [String: Data]] = [:] + try query(db, "SELECT collection, id, payload FROM records ORDER BY collection, id") { row in + result[column(row, 0), default: [:]][column(row, 1)] = Data(column(row, 2).utf8) + } + return result + } + + private func transaction(_ db: OpaquePointer, _ operation: () throws -> Void) throws { + try query(db, "BEGIN IMMEDIATE") + do { + try operation() + try query(db, "COMMIT") + } catch { + try? query(db, "ROLLBACK") + throw error + } + } + + private func withDatabase(_ operation: (OpaquePointer) throws -> T) throws -> T { + try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) + var connection: OpaquePointer? + let status = sqlite3_open(url.path, &connection) + guard let db = connection else { throw failure("Could not open the local database.") } + defer { sqlite3_close(db) } + guard status == SQLITE_OK else { throw failure(String(cString: sqlite3_errmsg(db))) } + sqlite3_busy_timeout(db, 3_000) + var version: Int32 = 0 + try query(db, "PRAGMA user_version") { version = sqlite3_column_int($0, 0) } + guard version <= 1 else { throw failure("This local database requires a newer version of Commit+.") } + if version == 0 { + try transaction(db) { + try query(db, "CREATE TABLE IF NOT EXISTS records (collection TEXT NOT NULL, id TEXT NOT NULL, payload TEXT NOT NULL, PRIMARY KEY (collection, id))") + try query(db, "CREATE TABLE IF NOT EXISTS migrations (id TEXT PRIMARY KEY)") + try query(db, "PRAGMA user_version = 1") + } + } + return try operation(db) + } + + private func query(_ db: OpaquePointer, _ sql: String, _ values: [String] = [], + row: (OpaquePointer) throws -> Void = { _ in }) throws { + var prepared: OpaquePointer? + guard sqlite3_prepare_v2(db, sql, -1, &prepared, nil) == SQLITE_OK, let statement = prepared else { + throw failure(String(cString: sqlite3_errmsg(db))) + } + defer { sqlite3_finalize(statement) } + for (index, value) in values.enumerated() { + let bytes = value.utf8CString + let status = bytes.withUnsafeBufferPointer { + sqlite3_bind_text(statement, Int32(index + 1), $0.baseAddress, Int32($0.count - 1), unsafeBitCast(-1, to: sqlite3_destructor_type.self)) + } + guard status == SQLITE_OK else { throw failure(String(cString: sqlite3_errmsg(db))) } + } + var status = sqlite3_step(statement) + while status == SQLITE_ROW { + try row(statement) + status = sqlite3_step(statement) + } + guard status == SQLITE_DONE else { throw failure(String(cString: sqlite3_errmsg(db))) } + } + + private func column(_ statement: OpaquePointer, _ index: Int32) -> String { + guard let value = sqlite3_column_text(statement, index) else { return "" } + return String(decoding: UnsafeBufferPointer(start: value, count: Int(sqlite3_column_bytes(statement, index))), as: UTF8.self) + } + + private func failure(_ message: String) -> NSError { + NSError(domain: "LocalSQLiteDatabase", code: 1, userInfo: [NSLocalizedDescriptionKey: message]) + } +} diff --git a/macgit/Services/RepoSettingsStore.swift b/macgit/Services/RepoSettingsStore.swift index d2d7e83c..84ec2d65 100644 --- a/macgit/Services/RepoSettingsStore.swift +++ b/macgit/Services/RepoSettingsStore.swift @@ -22,37 +22,25 @@ // import Foundation +@MainActor final class RepoSettingsStore { static let shared = RepoSettingsStore() + let dataStore: LocalDataStore - private let userDefaults: UserDefaults - private let key: String - private var settings: [String: RepoSettings] - - init(userDefaults: UserDefaults = .standard, key: String = "dev.thanhtran.macgit.repoSettings") { - self.userDefaults = userDefaults - self.key = key - if let data = userDefaults.data(forKey: key), - let decoded = try? JSONDecoder().decode([String: RepoSettings].self, from: data) { - settings = decoded - } else { - settings = [:] - } - } + init(dataStore: LocalDataStore? = nil) { self.dataStore = dataStore ?? .shared } func settings(for repositoryPath: String, currentBranch: String?, remotes: [String]) -> RepoSettings { - settings[repositoryPath] ?? RepoSettings.defaults(currentBranch: currentBranch, remotes: remotes) - } - - func update(for repositoryPath: String, settings: RepoSettings) { - self.settings[repositoryPath] = settings - save() + (try? dataStore.value(RepoSettings.self, in: "repoSettings", id: repositoryPath)) + ?? RepoSettings.defaults(currentBranch: currentBranch, remotes: remotes) } - private func save() { - guard let data = try? JSONEncoder().encode(settings) else { - return + func update(for repositoryPath: String, settings: RepoSettings, pendingCommitRuleUID: String? = nil) async throws { + try await dataStore.transaction { transaction in + try transaction.set(settings, in: "repoSettings", id: repositoryPath) + if let uid = pendingCommitRuleUID { + try transaction.set(settings.skipProtectedBranchCommitWarnings, + in: "commitRulePending", id: "\(uid)|\(repositoryPath)") + } } - userDefaults.set(data, forKey: key) } } diff --git a/macgit/Services/RepositoryVisibilityCache.swift b/macgit/Services/RepositoryVisibilityCache.swift index 3b371e62..f35de4e3 100644 --- a/macgit/Services/RepositoryVisibilityCache.swift +++ b/macgit/Services/RepositoryVisibilityCache.swift @@ -69,66 +69,36 @@ protocol RepositoryVisibilityCaching { _ visibility: RepositoryVisibility, for repository: GitRepositoryIdentity, resolvedAt: Date - ) + ) async } @MainActor -final class UserDefaultsRepositoryVisibilityCache: RepositoryVisibilityCaching { - private let userDefaults: UserDefaults - private let key: String - private let encoder = JSONEncoder() - private let decoder = JSONDecoder() +final class SQLiteRepositoryVisibilityCache: RepositoryVisibilityCaching { + private let dataStore: LocalDataStore + init(dataStore: LocalDataStore? = nil) { self.dataStore = dataStore ?? .shared } - init( - userDefaults: UserDefaults = .standard, - key: String = "dev.thanhtran.macgit.repositoryVisibility" - ) { - self.userDefaults = userDefaults - self.key = key - } - - func cachedVisibility( - for repository: GitRepositoryIdentity, - maximumAge: TimeInterval, - now: Date - ) -> RepositoryVisibility? { - let record = loadAll()[CachedRepositoryVisibility.cacheKey(for: repository)] - guard let record, + func cachedVisibility(for repository: GitRepositoryIdentity, maximumAge: TimeInterval, now: Date) -> RepositoryVisibility? { + guard let record = try? dataStore.value(CachedRepositoryVisibility.self, in: "repositoryVisibility", + id: CachedRepositoryVisibility.cacheKey(for: repository)), record.visibility == .public || record.visibility == .private, now.timeIntervalSince(record.resolvedAt) >= 0, - now.timeIntervalSince(record.resolvedAt) <= maximumAge else { - return nil - } + now.timeIntervalSince(record.resolvedAt) <= maximumAge else { return nil } return record.visibility } - func save( - _ visibility: RepositoryVisibility, - for repository: GitRepositoryIdentity, - resolvedAt: Date - ) { + func save(_ visibility: RepositoryVisibility, for repository: GitRepositoryIdentity, resolvedAt: Date) async { guard visibility == .public || visibility == .private else { return } - let record = CachedRepositoryVisibility( - repository: repository, - visibility: visibility, - resolvedAt: resolvedAt - ) - var values = loadAll() - values[record.cacheKey] = record + let record = CachedRepositoryVisibility(repository: repository, visibility: visibility, resolvedAt: resolvedAt) do { - userDefaults.set(try encoder.encode(values), forKey: key) + try await dataStore.transaction { transaction in + for (id, cached) in try transaction.values(CachedRepositoryVisibility.self, in: "repositoryVisibility") + where resolvedAt.timeIntervalSince(cached.resolvedAt) > 30 * 24 * 60 * 60 { + transaction.remove(in: "repositoryVisibility", id: id) + } + try transaction.set(record, in: "repositoryVisibility", id: record.cacheKey) + } } catch { NSLog("Commit+ repository visibility cache could not be saved: %@", error.localizedDescription) } } - - private func loadAll() -> [String: CachedRepositoryVisibility] { - guard let data = userDefaults.data(forKey: key) else { return [:] } - do { - return try decoder.decode([String: CachedRepositoryVisibility].self, from: data) - } catch { - NSLog("Commit+ repository visibility cache could not be decoded: %@", error.localizedDescription) - return [:] - } - } } diff --git a/macgit/Views/Common/LocalDataLoadingView.swift b/macgit/Views/Common/LocalDataLoadingView.swift new file mode 100644 index 00000000..c16f5bdc --- /dev/null +++ b/macgit/Views/Common/LocalDataLoadingView.swift @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import SwiftUI + +struct LocalDataLoadingView: View { + @ObservedObject private var store = LocalDataStore.shared + @State private var retry = 0 + @ViewBuilder let content: () -> Content + + var body: some View { + Group { + if store.isReady { + content() + } else if let message = store.errorMessage { + ContentUnavailableView { + Label("Local Data Unavailable", systemImage: "externaldrive.badge.exclamationmark") + } description: { + Text(message) + } actions: { + Button("Try Again") { retry += 1 } + } + } else { + ProgressView("Loading local data…") + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + } + .task(id: retry) { try? await store.prepare() } + } +} diff --git a/macgit/Views/MainWindow/MainWindowView+ProtectedBranchCommit.swift b/macgit/Views/MainWindow/MainWindowView+ProtectedBranchCommit.swift index 7b931c4c..30262892 100644 --- a/macgit/Views/MainWindow/MainWindowView+ProtectedBranchCommit.swift +++ b/macgit/Views/MainWindow/MainWindowView+ProtectedBranchCommit.swift @@ -4,12 +4,14 @@ import FirebaseCore extension MainWindowView { func commitRulePreferenceChanged() { - commitRuleSyncController.markChanged( - repoSettings.skipProtectedBranchCommitWarnings, - uid: accountController.account?.uid, - repositoryURL: repositoryURL - ) - Task { await reconcileCommitRulePreference() } + let settings = repoSettings + let uid = accountController.account?.uid + Task { + do { + try await repoSettingsStore.update(for: repositoryURL.path, settings: settings, pendingCommitRuleUID: uid) + await reconcileCommitRulePreference() + } catch { syncState.showError(error.localizedDescription) } + } } func reconcileCommitRulePreference() async { diff --git a/macgit/Views/MainWindow/MainWindowView+ProviderAccountPreferences.swift b/macgit/Views/MainWindow/MainWindowView+ProviderAccountPreferences.swift index 23484625..b77cf6da 100644 --- a/macgit/Views/MainWindow/MainWindowView+ProviderAccountPreferences.swift +++ b/macgit/Views/MainWindow/MainWindowView+ProviderAccountPreferences.swift @@ -76,7 +76,12 @@ extension MainWindowView { continue } - providerAccountPreferenceStore.update(accountID: selectedAccountID, for: identity) + do { + try await providerAccountPreferenceStore.update(accountID: selectedAccountID, for: identity) + } catch { + syncState.showError(error.localizedDescription) + return nil + } resolver = providerCredentialResolver } diff --git a/macgit/Views/MainWindow/MainWindowView+Sheets.swift b/macgit/Views/MainWindow/MainWindowView+Sheets.swift index 088abcb7..fd00e5c8 100644 --- a/macgit/Views/MainWindow/MainWindowView+Sheets.swift +++ b/macgit/Views/MainWindow/MainWindowView+Sheets.swift @@ -327,24 +327,25 @@ extension MainWindowView { providerAccountPreferences: providerAccountPreferenceStore.preferences, onSave: { newSettings in let commitRuleChanged = repoSettings.skipProtectedBranchCommitWarnings != newSettings.skipProtectedBranchCommitWarnings - repoSettings = newSettings - repoSettingsStore.update(for: repositoryURL.path, settings: newSettings) - if commitRuleChanged { commitRulePreferenceChanged() } + let uid = commitRuleChanged ? accountController.account?.uid : nil Task { - try? await GitStatusService.shared.updateGitUserConfiguration( - useGlobalSettings: newSettings.useGlobalUserSettings, - name: newSettings.userName, - email: newSettings.userEmail, - in: repositoryURL - ) - } - syncState.startBackgroundSync( - repositoryURL: repositoryURL, - settings: newSettings, - globalAutoFetchEnabled: appState.autoFetchEnabled - ) - Task { - await refreshRemotePresentation(for: newSettings.defaultRemoteName) + do { + try await repoSettingsStore.update(for: repositoryURL.path, settings: newSettings, pendingCommitRuleUID: uid) + repoSettings = newSettings + try? await GitStatusService.shared.updateGitUserConfiguration( + useGlobalSettings: newSettings.useGlobalUserSettings, + name: newSettings.userName, + email: newSettings.userEmail, + in: repositoryURL + ) + syncState.startBackgroundSync( + repositoryURL: repositoryURL, + settings: newSettings, + globalAutoFetchEnabled: appState.autoFetchEnabled + ) + await refreshRemotePresentation(for: newSettings.defaultRemoteName) + if commitRuleChanged { await reconcileCommitRulePreference() } + } catch { syncState.showError(error.localizedDescription) } } }, onSaveGitFlowConfiguration: { configuration in @@ -386,11 +387,12 @@ extension MainWindowView { return branch }, onSaveProviderAccountPreferences: { preferences in - for (preferenceKey, accountID) in preferences { - providerAccountPreferenceStore.update( - accountID: accountID, - forPreferenceKey: preferenceKey - ) + Task { + do { + for (preferenceKey, accountID) in preferences { + try await providerAccountPreferenceStore.update(accountID: accountID, forPreferenceKey: preferenceKey) + } + } catch { syncState.showError(error.localizedDescription) } } }, onOpenGitIgnore: openGitIgnoreFile, diff --git a/macgit/Views/MainWindow/MainWindowView.swift b/macgit/Views/MainWindow/MainWindowView.swift index 4d251947..88b6b2c6 100644 --- a/macgit/Views/MainWindow/MainWindowView.swift +++ b/macgit/Views/MainWindow/MainWindowView.swift @@ -367,7 +367,6 @@ struct MainWindowView: View { protectedBranchCommitController.finish(decision) } .onChange(of: repoSettings.skipProtectedBranchCommitWarnings) { _, _ in - repoSettingsStore.update(for: repositoryURL.path, settings: repoSettings) commitRulePreferenceChanged() } } diff --git a/macgit/Views/MainWindow/RepoPickerView.swift b/macgit/Views/MainWindow/RepoPickerView.swift index e218b566..d3522239 100644 --- a/macgit/Views/MainWindow/RepoPickerView.swift +++ b/macgit/Views/MainWindow/RepoPickerView.swift @@ -193,7 +193,7 @@ struct RepoPickerView: View { Button("Remove", role: .destructive) { if let bookmarkID = bookmarkController.bookmarkID(linkedTo: repo.url), let bookmark = bookmarkController.bookmark(forID: bookmarkID) { - bookmarkController.unlinkLocalFolder(for: bookmark) + Task { await bookmarkController.unlinkLocalFolder(for: bookmark) } } store.remove(repo) missingRepository = nil @@ -222,7 +222,7 @@ struct RepoPickerView: View { initialRepositoryName: bookmark.name, onClone: { url in bookmarkToClone = nil - bookmarkController.link(bookmark, to: url) + linkBookmark(bookmark, to: url) store.add(url) onRepositoryOpened(url) } @@ -230,12 +230,22 @@ struct RepoPickerView: View { } .onChange(of: bookmarkController.errorMessage) { _, newValue in guard let newValue else { return } - errorMessage = "Repository bookmark Firebase sync failed. Local bookmarks remain available. \(newValue)" + errorMessage = "Could not save or sync the repository bookmark. \(newValue)" showingError = true bookmarkController.clearError() } } + private func linkBookmark(_ bookmark: RepositoryBookmark, to url: URL) { + Task { + do { try await bookmarkController.link(bookmark, to: url) } + catch { + errorMessage = error.localizedDescription + showingError = true + } + } + } + static func visibleRepositories( from repositories: [RecentRepository], searchText: String, @@ -819,7 +829,7 @@ struct RepoPickerView: View { repoIcons[repo.url] = remoteURL.isEmpty ? "code-branch" : determineRepoIconName(from: remoteURL) loadingRepoIcons.remove(repo.url) if let bookmark = bookmarkController.bookmark(remoteURLString: remoteURL) { - bookmarkController.link(bookmark, to: repo.url) + linkBookmark(bookmark, to: repo.url) } } } diff --git a/macgitTests/GitFlowConfigurationSyncTests.swift b/macgitTests/GitFlowConfigurationSyncTests.swift index 6efb1402..e40577da 100644 --- a/macgitTests/GitFlowConfigurationSyncTests.swift +++ b/macgitTests/GitFlowConfigurationSyncTests.swift @@ -105,6 +105,9 @@ final class GitFlowConfigurationSyncTests: XCTestCase { } func testNewCloneDownloadsCloudConfigurationIntoLocalCache() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() let repositoryURL = try makeRepository() defer { try? FileManager.default.removeItem(at: repositoryURL) } let identity = try XCTUnwrap( @@ -126,7 +129,8 @@ final class GitFlowConfigurationSyncTests: XCTestCase { ) let controller = GitFlowConfigurationSyncController( cloudStore: cloudStore, - identityResolver: FakeRepositoryRemoteIdentityResolver(identity: identity) + identityResolver: FakeRepositoryRemoteIdentityResolver(identity: identity), + dataStore: fixture.store ) let outcome = await controller.reconcile( @@ -149,6 +153,9 @@ final class GitFlowConfigurationSyncTests: XCTestCase { } func testExistingLocalConfigurationSeedsMissingCloudDocument() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() let repositoryURL = try makeRepository() defer { try? FileManager.default.removeItem(at: repositoryURL) } let identity = try XCTUnwrap( @@ -166,7 +173,8 @@ final class GitFlowConfigurationSyncTests: XCTestCase { let cloudStore = FakeGitFlowConfigurationCloudStore(configuration: nil) let controller = GitFlowConfigurationSyncController( cloudStore: cloudStore, - identityResolver: FakeRepositoryRemoteIdentityResolver(identity: identity) + identityResolver: FakeRepositoryRemoteIdentityResolver(identity: identity), + dataStore: fixture.store ) let outcome = await controller.reconcile( @@ -183,11 +191,11 @@ final class GitFlowConfigurationSyncTests: XCTestCase { } func testFailedSignedInSaveRetriesLocalConfigurationBeforeReadingCloud() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() let repositoryURL = try makeRepository() defer { try? FileManager.default.removeItem(at: repositoryURL) } - let suiteName = "GitFlowConfigurationSyncTests.\(UUID().uuidString)" - let defaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) - defer { defaults.removePersistentDomain(forName: suiteName) } let identity = try XCTUnwrap( RepositoryBookmarkIdentity.resolve( remoteURLString: "https://github.com/openai/codex.git" @@ -207,8 +215,7 @@ final class GitFlowConfigurationSyncTests: XCTestCase { let controller = GitFlowConfigurationSyncController( cloudStore: cloudStore, identityResolver: FakeRepositoryRemoteIdentityResolver(identity: identity), - userDefaults: defaults, - pendingUploadsKey: suiteName + dataStore: fixture.store ) let localConfiguration = GitFlowConfiguration( isEnabled: true, @@ -235,7 +242,7 @@ final class GitFlowConfigurationSyncTests: XCTestCase { XCTAssertEqual(cloudStore.configurationRequestCount, 0) XCTAssertEqual(cloudStore.savedConfiguration?.mainBranch, "trunk") XCTAssertEqual(cloudStore.savedConfiguration?.developBranch, "next") - XCTAssertEqual(defaults.stringArray(forKey: suiteName) ?? [], []) + XCTAssertTrue(try fixture.store.values(String.self, in: "gitFlowPending").isEmpty) } private func makeRepository() throws -> URL { diff --git a/macgitTests/GitProviderAccountPreferenceStoreTests.swift b/macgitTests/GitProviderAccountPreferenceStoreTests.swift index 9411b158..5af39405 100644 --- a/macgitTests/GitProviderAccountPreferenceStoreTests.swift +++ b/macgitTests/GitProviderAccountPreferenceStoreTests.swift @@ -20,36 +20,34 @@ import XCTest @testable import macgit final class GitProviderAccountPreferenceStoreTests: XCTestCase { - func testPersistsAccountPreferenceForCanonicalRemoteIdentity() throws { - let defaultsKey = "test.provider-account-preferences.\(UUID().uuidString)" - let defaults = UserDefaults.standard - defaults.removeObject(forKey: defaultsKey) - defer { defaults.removeObject(forKey: defaultsKey) } + func testPersistsAccountPreferenceForCanonicalRemoteIdentity() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() let identity = try XCTUnwrap(GitRemoteIdentityResolver.identity( from: "git@github.com:octocat/Hello-World.git" )) - let store = GitProviderAccountPreferenceStore(userDefaults: defaults, key: defaultsKey) + let store = GitProviderAccountPreferenceStore(dataStore: fixture.store) - store.update(accountID: "connection-work", for: identity) + try await store.update(accountID: "connection-work", for: identity) - let reloadedStore = GitProviderAccountPreferenceStore(userDefaults: defaults, key: defaultsKey) + let reloadedStore = GitProviderAccountPreferenceStore(dataStore: try await fixture.reopen()) XCTAssertEqual(reloadedStore.accountID(for: identity), "connection-work") } - func testRemovingPreferencePersistsAutomaticSelection() throws { - let defaultsKey = "test.provider-account-preferences.\(UUID().uuidString)" - let defaults = UserDefaults.standard - defaults.removeObject(forKey: defaultsKey) - defer { defaults.removeObject(forKey: defaultsKey) } + func testRemovingPreferencePersistsAutomaticSelection() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() let identity = try XCTUnwrap(GitRemoteIdentityResolver.identity( from: "https://gitlab.com/group/project.git" )) - let store = GitProviderAccountPreferenceStore(userDefaults: defaults, key: defaultsKey) - store.update(accountID: "connection-work", for: identity) + let store = GitProviderAccountPreferenceStore(dataStore: fixture.store) + try await store.update(accountID: "connection-work", for: identity) - store.update(accountID: nil, for: identity) + try await store.update(accountID: nil, for: identity) XCTAssertNil(store.accountID(for: identity)) XCTAssertTrue(store.preferences.isEmpty) diff --git a/macgitTests/GitProviderSSHKeyStoreTests.swift b/macgitTests/GitProviderSSHKeyStoreTests.swift index 022f085c..b51a9ebf 100644 --- a/macgitTests/GitProviderSSHKeyStoreTests.swift +++ b/macgitTests/GitProviderSSHKeyStoreTests.swift @@ -29,32 +29,32 @@ final class GitProviderSSHKeyStoreTests: XCTestCase { ) } - func testUserDefaultsStoreSavesReadsAndDeletesKey() throws { - let suiteName = "GitProviderSSHKeyStoreTests-\(UUID().uuidString)" - let defaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) - defer { defaults.removePersistentDomain(forName: suiteName) } + func testSQLiteStoreSavesReadsAndDeletesKey() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() - let store = UserDefaultsGitProviderSSHKeyStore(defaults: defaults) + let store = SQLiteGitProviderSSHKeyStore(dataStore: fixture.store) let account = makeProviderAccount() let key = GitProviderSSHKey(path: "/Users/test/.ssh/id_ed25519") - try store.saveKey(key, for: account) + try await store.saveKey(key, for: account) XCTAssertEqual(try store.key(for: account), key) - try store.deleteKey(for: account) + try await store.deleteKey(for: account) XCTAssertNil(try store.key(for: account)) } - func testDeletingMissingKeyIsIdempotent() throws { - let suiteName = "GitProviderSSHKeyStoreTests-\(UUID().uuidString)" - let defaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) - defer { defaults.removePersistentDomain(forName: suiteName) } + func testDeletingMissingKeyIsIdempotent() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() - let store = UserDefaultsGitProviderSSHKeyStore(defaults: defaults) + let store = SQLiteGitProviderSSHKeyStore(dataStore: fixture.store) - XCTAssertNoThrow(try store.deleteKey(for: makeProviderAccount())) + try await store.deleteKey(for: makeProviderAccount()) } private func makeProviderAccount() -> GitProviderAccount { diff --git a/macgitTests/LocalDataMigrationTests.swift b/macgitTests/LocalDataMigrationTests.swift new file mode 100644 index 00000000..d64b9a35 --- /dev/null +++ b/macgitTests/LocalDataMigrationTests.swift @@ -0,0 +1,232 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import SQLite3 +import XCTest +@testable import macgit + +@MainActor +final class LocalDataMigrationTests: XCTestCase { + func testImportsRelatedRecordsAndPreservesIDsAndLegacyBackup() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + let prefix = "dev.thanhtran.macgit." + let identity = try XCTUnwrap(RepositoryBookmarkIdentity.resolve(remoteURLString: "git@github.com:team/repo.git")) + let bookmark = RepositoryBookmark(identity: identity) + let account = makeAccount() + let settings = RepoSettings(defaultPullBranch: "trunk") + let ssh = GitProviderSSHKey(path: "/Users/test/.ssh/work") + let encodedAccounts = try JSONEncoder().encode([account]) + fixture.defaults.set(try JSONEncoder().encode([bookmark]), forKey: prefix + "repositoryBookmarks.items") + fixture.defaults.set([bookmark.id: "/tmp/repo"], forKey: prefix + "repositoryBookmarks.localPaths") + fixture.defaults.set([bookmark.id], forKey: prefix + "repositoryBookmarks.pendingUploads") + fixture.defaults.set(["deleted-bookmark"], forKey: prefix + "repositoryBookmarks.pendingDeletes") + fixture.defaults.set(encodedAccounts, forKey: prefix + "localGitProviderAccounts") + fixture.defaults.set(try JSONEncoder().encode([account]), forKey: prefix + "localGitProviderAccountPendingDeletions") + fixture.defaults.set(["user-a": ["github|github.com|42"]], forKey: prefix + "localGitProviderAccountSyncedIdentities") + fixture.defaults.set("original-owner", forKey: prefix + "localGitProviderAccountOwnerID") + fixture.defaults.set(try JSONEncoder().encode(["/tmp/repo": settings]), forKey: prefix + "repoSettings") + fixture.defaults.set(["remote": account.id], forKey: prefix + "providerAccountPreferences") + fixture.defaults.set(try JSONEncoder().encode(ssh), forKey: GitProviderSSHKeyStoreKey.storageKey(for: account)) + fixture.defaults.set(["user-a|/tmp/repo": false], forKey: prefix + "repositoryCommitRules.pending") + fixture.defaults.set(["user-a|remote"], forKey: prefix + "gitFlowConfiguration.pendingUploads") + fixture.defaults.set("dark", forKey: "appearance") + + try await fixture.store.prepare() + let reopened = try await fixture.reopen() + XCTAssertEqual(try reopened.value(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id), bookmark) + XCTAssertEqual(try reopened.value(String.self, in: "bookmarkPaths", id: bookmark.id), "/tmp/repo") + XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkUploads", id: bookmark.id)) + XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkDeletes", id: "deleted-bookmark")) + XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerAccounts", id: account.id), account) + XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerDeletions", id: account.id), account) + XCTAssertEqual(try reopened.value([String].self, in: "providerSyncedIdentities", id: "user-a"), ["github|github.com|42"]) + XCTAssertEqual(try reopened.value(RepoSettings.self, in: "repoSettings", id: "/tmp/repo"), settings) + XCTAssertEqual(try reopened.value(String.self, in: "providerPreferences", id: "remote"), account.id) + XCTAssertEqual(try reopened.value(GitProviderSSHKey.self, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)), ssh) + XCTAssertEqual(try reopened.value(Bool.self, in: "commitRulePending", id: "user-a|/tmp/repo"), false) + XCTAssertNotNil(try reopened.value(String.self, in: "gitFlowPending", id: "user-a|remote")) + XCTAssertEqual(SQLiteGitProviderAccountLocalStore(dataStore: reopened, defaults: fixture.defaults).accountOwnerID, "original-owner") + XCTAssertEqual(fixture.defaults.data(forKey: prefix + "localGitProviderAccounts"), encodedAccounts) + XCTAssertEqual(fixture.defaults.string(forKey: "appearance"), "dark") + XCTAssertTrue(try reopened.values(String.self, in: "appearance").isEmpty) + } + + func testCompletedImportDoesNotResurrectDeletedAccountFromDefaults() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + let account = makeAccount() + fixture.defaults.set(try JSONEncoder().encode([account]), forKey: "dev.thanhtran.macgit.localGitProviderAccounts") + try await fixture.store.prepare() + let accounts = SQLiteGitProviderAccountLocalStore(dataStore: fixture.store, defaults: fixture.defaults) + _ = try await accounts.delete(accountID: account.id) + let reopened = try await fixture.reopen() + XCTAssertNil(try reopened.value(GitProviderAccount.self, in: "providerAccounts", id: account.id)) + XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerDeletions", id: account.id), account) + // Stale or subsequently damaged defaults must no longer be consulted. + fixture.defaults.set(Data("broken".utf8), forKey: "dev.thanhtran.macgit.localGitProviderAccounts") + _ = try await fixture.reopen() + } + + func testCorruptLegacyDataLeavesImportRetryableAndDoesNotDiscardOriginal() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + let key = "dev.thanhtran.macgit.repoSettings" + let damaged = Data("{broken".utf8) + fixture.defaults.set(damaged, forKey: key) + do { try await fixture.store.prepare(); XCTFail("Corrupt user data must not become an empty store") } + catch { XCTAssertFalse(fixture.store.isReady) } + XCTAssertEqual(fixture.defaults.data(forKey: key), damaged) + let settings = RepoSettings(defaultPullBranch: "recovered") + fixture.defaults.set(try JSONEncoder().encode(["/tmp/repo": settings]), forKey: key) + try await fixture.store.prepare() + XCTAssertEqual(try fixture.store.value(RepoSettings.self, in: "repoSettings", id: "/tmp/repo"), settings) + } + + func testAccountDeleteRollsBackMetadataLinksAndTombstoneOnDiskFailure() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let account = makeAccount() + let accounts = SQLiteGitProviderAccountLocalStore(dataStore: fixture.store, defaults: fixture.defaults) + try await accounts.save(account) + try await GitProviderAccountPreferenceStore(dataStore: fixture.store).update(accountID: account.id, forPreferenceKey: "remote") + let keys = SQLiteGitProviderSSHKeyStore(dataStore: fixture.store) + try await keys.saveKey(GitProviderSSHKey(path: "/tmp/key"), for: account) + try execute("CREATE TRIGGER fail_delete BEFORE INSERT ON records WHEN NEW.collection = 'providerDeletions' BEGIN SELECT RAISE(ABORT, 'test disk failure'); END", url: fixture.databaseURL) + do { _ = try await accounts.delete(accountID: account.id); XCTFail("Write should fail") } + catch { } + XCTAssertEqual(try accounts.accounts(), [account]) + XCTAssertTrue(try accounts.pendingDeletions().isEmpty) + let reopened = try await fixture.reopen() + XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerAccounts", id: account.id), account) + XCTAssertEqual(try reopened.value(String.self, in: "providerPreferences", id: "remote"), account.id) + XCTAssertNotNil(try reopened.value(GitProviderSSHKey.self, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account))) + try execute("DROP TRIGGER fail_delete", url: fixture.databaseURL) + _ = try await accounts.delete(accountID: account.id) + XCTAssertNil(try keys.key(for: account)) + XCTAssertTrue(try fixture.store.values(String.self, in: "providerPreferences").isEmpty) + XCTAssertEqual(try accounts.pendingDeletions(), [account]) + } + + func testFailedImportRollsBackRowsAndMarkerThenRetries() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + let engine = LocalSQLiteDatabase(url: fixture.databaseURL) + let needsImport = try await engine.needsLegacyImport() + XCTAssertTrue(needsImport) + try execute("CREATE TRIGGER fail_import BEFORE INSERT ON migrations BEGIN SELECT RAISE(ABORT, 'import failure'); END", url: fixture.databaseURL) + fixture.defaults.set(try JSONEncoder().encode([makeAccount()]), forKey: "dev.thanhtran.macgit.localGitProviderAccounts") + do { try await fixture.store.prepare(); XCTFail("Import should fail") } + catch { } + let rows = try await engine.load(importing: nil) + XCTAssertTrue(rows.isEmpty) + try execute("DROP TRIGGER fail_import", url: fixture.databaseURL) + try await fixture.store.prepare() + XCTAssertEqual(try fixture.store.values(GitProviderAccount.self, in: "providerAccounts").count, 1) + } + + func testConcurrentTransactionsPreserveBothUpdates() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let first = Task { try await fixture.store.transaction { try $0.set(1, in: "counter", id: "first") } } + let second = Task { try await fixture.store.transaction { try $0.set(2, in: "counter", id: "second") } } + try await first.value + try await second.value + let reopened = try await fixture.reopen() + XCTAssertEqual(try reopened.values(Int.self, in: "counter"), ["first": 1, "second": 2]) + } + + func testRepositorySettingsAndPendingRuleCommitTogether() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + var settings = RepoSettings(defaultPullBranch: "main") + settings.skipProtectedBranchCommitWarnings = true + try await RepoSettingsStore(dataStore: fixture.store).update(for: "/tmp/repo", settings: settings, pendingCommitRuleUID: "user-a") + let reopened = try await fixture.reopen() + XCTAssertEqual(try reopened.value(RepoSettings.self, in: "repoSettings", id: "/tmp/repo"), settings) + XCTAssertEqual(try reopened.value(Bool.self, in: "commitRulePending", id: "user-a|/tmp/repo"), true) + XCTAssertNil(try reopened.value(Bool.self, in: "commitRulePending", id: "user-b|/tmp/repo")) + } + + func testBookmarkRemovalRollsBackAllRelatedRowsOnFailure() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let identity = try XCTUnwrap(RepositoryBookmarkIdentity.resolve(remoteURLString: "git@github.com:team/repo.git")) + let bookmark = RepositoryBookmark(identity: identity) + try await fixture.store.transaction { transaction in + try transaction.set(bookmark, in: "bookmarks", id: bookmark.id) + try transaction.set("/tmp/repo", in: "bookmarkPaths", id: bookmark.id) + try transaction.set("pending", in: "bookmarkUploads", id: bookmark.id) + } + let controller = RepositoryBookmarkController(cloudStore: nil, dataStore: fixture.store) + try controller.load() + try execute("CREATE TRIGGER fail_bookmark BEFORE INSERT ON records WHEN NEW.collection = 'bookmarkDeletes' BEGIN SELECT RAISE(ABORT, 'test failure'); END", url: fixture.databaseURL) + await controller.removeBookmark(bookmark) + XCTAssertNotNil(controller.errorMessage) + XCTAssertEqual(controller.bookmarks, [bookmark]) + XCTAssertEqual(controller.localURL(for: bookmark)?.path, "/tmp/repo") + let reopened = try await fixture.reopen() + XCTAssertNotNil(try reopened.value(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id)) + XCTAssertEqual(try reopened.value(String.self, in: "bookmarkUploads", id: bookmark.id), "pending") + XCTAssertNil(try reopened.value(String.self, in: "bookmarkDeletes", id: bookmark.id)) + } + + func testBookmarkCloudFailureKeepsDeletionAcrossReopenAndStaleSnapshot() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let identity = try XCTUnwrap(RepositoryBookmarkIdentity.resolve(remoteURLString: "git@github.com:team/repo.git")) + let bookmark = RepositoryBookmark(identity: identity) + try await fixture.store.transaction { transaction in + try transaction.set(bookmark, in: "bookmarks", id: bookmark.id) + try transaction.set("/tmp/repo", in: "bookmarkPaths", id: bookmark.id) + } + let cloud = MigrationBookmarkCloud(bookmarks: [bookmark]) + let controller = RepositoryBookmarkController(cloudStore: cloud, dataStore: fixture.store) + let account = AccountSnapshot(uid: "user-a", email: nil, displayName: nil, providerIDs: []) + await controller.updateAccount(account) + await controller.removeBookmark(bookmark) + XCTAssertTrue(controller.bookmarks.isEmpty) + XCTAssertNotNil(controller.errorMessage) + let reopened = try await fixture.reopen() + let second = RepositoryBookmarkController(cloudStore: cloud, dataStore: reopened) + await second.updateAccount(account) + XCTAssertTrue(second.bookmarks.isEmpty) + XCTAssertNil(second.localURL(for: bookmark)) + XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkDeletes", id: bookmark.id)) + } + + private func execute(_ sql: String, url: URL) throws { + var connection: OpaquePointer? + guard sqlite3_open(url.path, &connection) == SQLITE_OK, let db = connection else { throw LocalDataError.notReady } + defer { sqlite3_close(db) } + guard sqlite3_exec(db, sql, nil, nil, nil) == SQLITE_OK else { + throw NSError(domain: "TestSQLite", code: 1, userInfo: [NSLocalizedDescriptionKey: String(cString: sqlite3_errmsg(db))]) + } + } + + private func makeAccount() -> GitProviderAccount { + GitProviderAccount(id: "original-account-id", macgitUID: "original-owner", provider: .github, + hostURL: URL(string: "https://github.com")!, providerUserID: "42", username: "test", + displayName: nil, avatarURL: nil, scopes: [], permissions: [:], tokenStatus: .valid, + connectedAt: Date(timeIntervalSince1970: 100), lastValidatedAt: nil) + } +} + +@MainActor +private final class MigrationBookmarkCloud: RepositoryBookmarkCloudStore { + let storedBookmarks: [RepositoryBookmark] + init(bookmarks: [RepositoryBookmark]) { storedBookmarks = bookmarks } + func bookmarks(uid: String) async throws -> [RepositoryBookmark] { storedBookmarks } + func save(_ bookmark: RepositoryBookmark, uid: String) async throws { } + func delete(bookmarkID: String, uid: String) async throws { throw LocalDataError.notReady } + func observe(uid: String, onChange: @escaping (Result<[RepositoryBookmark], Error>) -> Void) -> ObservationToken { + MigrationObservationToken() + } +} + +private final class MigrationObservationToken: ObservationToken { + func cancel() { } +} diff --git a/macgitTests/LocalDataStoreTestFixture.swift b/macgitTests/LocalDataStoreTestFixture.swift new file mode 100644 index 00000000..4ea404fc --- /dev/null +++ b/macgitTests/LocalDataStoreTestFixture.swift @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Foundation +@testable import macgit + +@MainActor +final class LocalDataStoreTestFixture { + let directory: URL + let defaults: UserDefaults + let suite: String + let store: LocalDataStore + var databaseURL: URL { directory.appendingPathComponent("store.sqlite") } + + init() throws { + directory = FileManager.default.temporaryDirectory.appendingPathComponent("local-data-tests-\(UUID())") + suite = "LocalDataStoreTests.\(UUID())" + defaults = UserDefaults(suiteName: suite)! + store = LocalDataStore(databaseURL: directory.appendingPathComponent("store.sqlite"), userDefaults: defaults) + } + + func reopen() async throws -> LocalDataStore { + let reopened = LocalDataStore(databaseURL: databaseURL, userDefaults: defaults) + try await reopened.prepare() + return reopened + } + + func cleanup() { + defaults.removePersistentDomain(forName: suite) + try? FileManager.default.removeItem(at: directory) + } +} diff --git a/macgitTests/LocalGitProviderAccountStoreTests.swift b/macgitTests/LocalGitProviderAccountStoreTests.swift index f1a556fc..b2296e0b 100644 --- a/macgitTests/LocalGitProviderAccountStoreTests.swift +++ b/macgitTests/LocalGitProviderAccountStoreTests.swift @@ -121,6 +121,18 @@ final class LocalGitProviderAccountStoreTests: XCTestCase { XCTAssertTrue(accounts.isEmpty) } + func testSyncBookkeepingFailureDoesNotReportDurableAccountSaveAsFailed() async throws { + let local = FakeLocalProviderAccountStore(accounts: []) + let cloud = FakeCloudProviderAccountStore(accounts: []) + let store = LocalFirstGitProviderAccountStore(localStore: local, cloudStore: cloud) + try await store.updateCloudAccount(uid: "firebase-user") + local.syncedKeysError = TestCloudError.failed + let account = makeAccount(id: "local", ownerID: "local-owner", providerUserID: "1") + try await store.save(account) + let saved = try await store.accounts() + XCTAssertEqual(saved, [account]) + } + private func makeAccount(id: String, ownerID: String, providerUserID: String) -> GitProviderAccount { GitProviderAccount( id: id, @@ -146,6 +158,7 @@ private final class FakeLocalProviderAccountStore: GitProviderAccountLocalStore private var storedAccounts: [GitProviderAccount] private var deletions: [GitProviderAccount] = [] private var syncedKeysByUID: [String: Set] = [:] + var syncedKeysError: Error? init(accounts: [GitProviderAccount]) { storedAccounts = accounts @@ -186,7 +199,8 @@ private final class FakeLocalProviderAccountStore: GitProviderAccountLocalStore syncedKeysByUID[uid] ?? [] } - func setSyncedIdentityKeys(_ keys: Set, uid: String) { + func setSyncedIdentityKeys(_ keys: Set, uid: String) throws { + if let syncedKeysError { throw syncedKeysError } syncedKeysByUID[uid] = keys } } diff --git a/macgitTests/RepoSettingsStoreTests.swift b/macgitTests/RepoSettingsStoreTests.swift index fb5f98e4..aef651b0 100644 --- a/macgitTests/RepoSettingsStoreTests.swift +++ b/macgitTests/RepoSettingsStoreTests.swift @@ -34,13 +34,12 @@ final class RepoSettingsStoreTests: XCTestCase { XCTAssertFalse(decoded.skipProtectedBranchCommitWarnings) } - func testRepoSettingsStorePersistsSettingsPerRepositoryPath() { - let defaultsKey = "test.repo-settings.\(UUID().uuidString)" - let defaults = UserDefaults.standard - defaults.removeObject(forKey: defaultsKey) - defer { defaults.removeObject(forKey: defaultsKey) } + func testRepoSettingsStorePersistsSettingsPerRepositoryPath() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() - let store = RepoSettingsStore(userDefaults: defaults, key: defaultsKey) + let store = RepoSettingsStore(dataStore: fixture.store) let repoA = "/tmp/repo-a-\(UUID().uuidString)" let repoB = "/tmp/repo-b-\(UUID().uuidString)" @@ -49,9 +48,9 @@ final class RepoSettingsStoreTests: XCTestCase { repoASettings.pullStrategy = .rebase repoASettings.autoFetchOverride = true repoASettings.refreshOnAppActiveOverride = false - store.update(for: repoA, settings: repoASettings) + try await store.update(for: repoA, settings: repoASettings) - let freshStore = RepoSettingsStore(userDefaults: defaults, key: defaultsKey) + let freshStore = RepoSettingsStore(dataStore: try await fixture.reopen()) let loadedA = freshStore.settings(for: repoA, currentBranch: "main", remotes: ["origin"]) let loadedB = freshStore.settings(for: repoB, currentBranch: nil, remotes: ["upstream"]) diff --git a/macgitTests/RepositoryBookmarkTests.swift b/macgitTests/RepositoryBookmarkTests.swift index 57ca0f50..d729a634 100644 --- a/macgitTests/RepositoryBookmarkTests.swift +++ b/macgitTests/RepositoryBookmarkTests.swift @@ -87,14 +87,13 @@ final class RepositoryBookmarkTests: XCTestCase { } @MainActor - func testControllerKeepsLocalFolderMappingOutOfBookmarkModel() throws { - let suiteName = "RepositoryBookmarkTests.\(UUID().uuidString)" - let defaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) - defer { defaults.removePersistentDomain(forName: suiteName) } + func testControllerKeepsLocalFolderMappingOutOfBookmarkModel() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() let controller = RepositoryBookmarkController( cloudStore: nil, - userDefaults: defaults, - keyPrefix: suiteName + dataStore: fixture.store ) let identity = try XCTUnwrap( RepositoryBookmarkIdentity.resolve( @@ -104,7 +103,7 @@ final class RepositoryBookmarkTests: XCTestCase { let bookmark = RepositoryBookmark(identity: identity) let localURL = URL(fileURLWithPath: "/Users/test/Project/codex", isDirectory: true) - controller.link(bookmark, to: localURL) + try await controller.link(bookmark, to: localURL) XCTAssertEqual(controller.localURL(for: bookmark), localURL) XCTAssertFalse(bookmark.remoteURL.absoluteString.contains("/Users/test")) diff --git a/macgitTests/RepositoryCommitRuleSyncControllerTests.swift b/macgitTests/RepositoryCommitRuleSyncControllerTests.swift index 2a4c2a46..7608beb5 100644 --- a/macgitTests/RepositoryCommitRuleSyncControllerTests.swift +++ b/macgitTests/RepositoryCommitRuleSyncControllerTests.swift @@ -5,15 +5,15 @@ import XCTest @MainActor final class RepositoryCommitRuleSyncControllerTests: XCTestCase { func testCloudPreferenceAppliesWithoutReplacingOtherRepoSettings() async throws { - let suite = "commit-rule-sync-\(UUID())" - let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } - let local = RepoSettingsStore(userDefaults: defaults) + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let local = RepoSettingsStore(dataStore: fixture.store) let url = URL(fileURLWithPath: "/tmp/repo") var settings = RepoSettings.defaults(currentBranch: "main", remotes: ["origin"]) settings.userName = "Local author" - local.update(for: url.path, settings: settings) - let controller = RepositoryCommitRuleSyncController(defaults: defaults, localStore: local, resolver: CommitRuleTestIdentity()) + try await local.update(for: url.path, settings: settings) + let controller = RepositoryCommitRuleSyncController(localStore: local, resolver: CommitRuleTestIdentity()) let cloud = CommitRuleTestCloud(value: true) var applied: Bool? let warning = await controller.reconcile(repositoryURL: url, uid: "user-a", cloud: cloud) { applied = $0 } @@ -26,19 +26,19 @@ final class RepositoryCommitRuleSyncControllerTests: XCTestCase { } func testPendingOfflineChoiceSurvivesControllerRecreationAndWinsOverCloud() async throws { - let suite = "commit-rule-sync-\(UUID())" - let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } - let local = RepoSettingsStore(userDefaults: defaults) + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let local = RepoSettingsStore(dataStore: fixture.store) let url = URL(fileURLWithPath: "/tmp/repo") - let first = RepositoryCommitRuleSyncController(defaults: defaults, localStore: local, resolver: CommitRuleTestIdentity()) - first.markChanged(false, uid: "user-a", repositoryURL: url) + let first = RepositoryCommitRuleSyncController(localStore: local, resolver: CommitRuleTestIdentity()) + try await first.markChanged(false, uid: "user-a", repositoryURL: url) let cloud = CommitRuleTestCloud(value: true) cloud.failSave = true let failure = await first.reconcile(repositoryURL: url, uid: "user-a", cloud: cloud) { _ in XCTFail("Must not overwrite local edit") } XCTAssertNotNil(failure) cloud.failSave = false - let second = RepositoryCommitRuleSyncController(defaults: defaults, localStore: local, resolver: CommitRuleTestIdentity()) + let second = RepositoryCommitRuleSyncController(localStore: local, resolver: CommitRuleTestIdentity()) let warning = await second.reconcile(repositoryURL: url, uid: "user-a", cloud: cloud) { _ in XCTFail("Must upload pending edit") } XCTAssertNil(warning) XCTAssertEqual(cloud.value, false) @@ -51,6 +51,29 @@ final class RepositoryCommitRuleSyncControllerTests: XCTestCase { XCTAssertEqual(cloud.loads, 0) XCTAssertTrue(cloud.saves.isEmpty) } + + func testLocalEditWhileCloudLoadsWinsAndPreservesOtherSettings() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let local = RepoSettingsStore(dataStore: fixture.store) + let url = URL(fileURLWithPath: "/tmp/repo") + let cloud = CommitRuleTestCloud(value: true) + cloud.onLoad = { + var settings = RepoSettings.defaults(currentBranch: "work", remotes: ["upstream"]) + settings.userName = "Changed while loading" + settings.skipProtectedBranchCommitWarnings = false + try await local.update(for: url.path, settings: settings, pendingCommitRuleUID: "user-a") + } + let controller = RepositoryCommitRuleSyncController(localStore: local, resolver: CommitRuleTestIdentity()) + let warning = await controller.reconcile(repositoryURL: url, uid: "user-a", cloud: cloud) { _ in + XCTFail("An old cloud value must not replace a local edit") + } + XCTAssertNil(warning) + XCTAssertEqual(cloud.saves, [false]) + XCTAssertEqual(local.settings(for: url.path, currentBranch: nil, remotes: []).userName, "Changed while loading") + XCTAssertTrue(try fixture.store.values(Bool.self, in: "commitRulePending").isEmpty) + } } private struct CommitRuleTestIdentity: RepositoryRemoteIdentityResolving { @@ -65,9 +88,11 @@ private final class CommitRuleTestCloud: RepositoryCommitRuleCloudStore { var saves: [Bool] = [] var loads = 0 var failSave = false + var onLoad: (() async throws -> Void)? init(value: Bool?) { self.value = value } func load(identity: RepositoryBookmarkIdentity, uid: String) async throws -> Bool? { loads += 1 + try await onLoad?() return value } func save(_ skipWarnings: Bool, identity: RepositoryBookmarkIdentity, uid: String) async throws { diff --git a/macgitTests/RepositoryVisibilityControllerTests.swift b/macgitTests/RepositoryVisibilityControllerTests.swift index 1a77e343..d91610b3 100644 --- a/macgitTests/RepositoryVisibilityControllerTests.swift +++ b/macgitTests/RepositoryVisibilityControllerTests.swift @@ -116,18 +116,18 @@ final class RepositoryVisibilityControllerTests: XCTestCase { XCTAssertEqual(cache.values.values.first?.visibility, .public) } - func testUserDefaultsCachePersistsOnlyConfirmedVisibility() { - let suiteName = "RepositoryVisibilityControllerTests.\(UUID().uuidString)" - let defaults = UserDefaults(suiteName: suiteName)! - defer { defaults.removePersistentDomain(forName: suiteName) } - let cache = UserDefaultsRepositoryVisibilityCache(userDefaults: defaults) + func testSQLiteCachePersistsOnlyConfirmedVisibility() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + let cache = SQLiteRepositoryVisibilityCache(dataStore: fixture.store) let repository = githubRepository(name: "Hello-World") let now = Date(timeIntervalSince1970: 1_800_000_000) - cache.save(.unknown, for: repository, resolvedAt: now) + await cache.save(.unknown, for: repository, resolvedAt: now) XCTAssertNil(cache.cachedVisibility(for: repository, maximumAge: 900, now: now)) - cache.save(.private, for: repository, resolvedAt: now) + await cache.save(.private, for: repository, resolvedAt: now) XCTAssertEqual( cache.cachedVisibility(for: repository, maximumAge: 900, now: now), .private