From cbfc8063dfc764a5488252e5da98380aa531a0ae Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Thu, 24 Sep 2026 15:16:43 +0800 Subject: [PATCH] refactor: icloud sync --- FrameworkTests/CloudSyncLifecycleTests.swift | 107 +++++++++++++++ FrameworkTests/CloudSyncSafetyFixture.swift | 27 +++- FrameworkTests/CloudSyncSafetyTests.swift | 36 +++++ .../Cloud/CloudSyncEventHandling.swift | 124 ++++++++++++++++++ .../Cloud/CloudSyncRecordReceiving.swift | 24 ++++ .../Cloud/HistorySyncCoordinator.swift | 83 +++--------- .../Cloud/MetricsSyncCoordinator.swift | 75 ++--------- .../Cloud/SettingsSyncCoordinator.swift | 90 +++---------- .../Cloud/VaultSyncCoordinator.swift | 87 +++--------- TFMTests/ArchitectureBoundaryTests.swift | 16 ++- 10 files changed, 396 insertions(+), 273 deletions(-) create mode 100644 Frameworks/TermindInfrastructure/Cloud/CloudSyncEventHandling.swift create mode 100644 Frameworks/TermindInfrastructure/Cloud/CloudSyncRecordReceiving.swift diff --git a/FrameworkTests/CloudSyncLifecycleTests.swift b/FrameworkTests/CloudSyncLifecycleTests.swift index 9d0651c7..a8d0bdff 100644 --- a/FrameworkTests/CloudSyncLifecycleTests.swift +++ b/FrameworkTests/CloudSyncLifecycleTests.swift @@ -8,6 +8,113 @@ import Testing @Suite @MainActor struct CloudSyncLifecycleTests { + @Test + func sharedEventDispatcherRoutesCallbacksAndRejectsRetiredEngines() async throws { + let fixture = LifecycleFixture() + fixture.driver.start() + await fixture.driver.operation?.value + let activeEngine = fixture.network.nextEngine + let record = CKRecord( + recordType: "Test", + recordID: CKRecord.ID(recordName: "remote", zoneID: fixture.driver.zoneID)) + var receivedRecordNames: [String] = [] + var sentCallbackCount = 0 + let receive: @MainActor @Sendable ([CKRecord], [CKRecord.ID]) throws -> Void = { + receivedRecordNames.append(contentsOf: $0.map(\.recordID.recordName)) + #expect($1.isEmpty) + } + let sent: @MainActor @Sendable (CloudSyncSentChanges) throws -> Void = { _ in sentCallbackCount += 1 } + + fixture.driver.handle( + .fetchedRecordZoneChanges(records: [record], deletions: []), engine: activeEngine, + receiveRemoteRecords: receive, handleSentChanges: sent) + fixture.driver.handle( + .sentRecordZoneChanges(.init(savedRecords: [], failedRecordSaves: [])), engine: activeEngine, + receiveRemoteRecords: receive, handleSentChanges: sent) + fixture.driver.handle( + .fetchedDatabaseChanges(deletedZoneIDs: [fixture.driver.zoneID]), engine: activeEngine, + receiveRemoteRecords: receive, handleSentChanges: sent) + + #expect(receivedRecordNames == ["remote"]) + #expect(sentCallbackCount == 1) + #expect(fixture.journal.resets == ["environment", "zone"]) + + fixture.driver.pause() + fixture.network.nextEngine = LifecycleEngine() + fixture.driver.start() + await fixture.driver.operation?.value + fixture.driver.handle( + .fetchedRecordZoneChanges(records: [record], deletions: []), engine: activeEngine, + receiveRemoteRecords: receive, handleSentChanges: sent) + #expect(receivedRecordNames == ["remote"]) + } + + @Test + func sharedEventDispatcherFailsClosedAndFetchFailureKeepsUploadsFenced() async { + let fixture = LifecycleFixture() + fixture.driver.start() + await fixture.driver.operation?.value + let engine = fixture.network.nextEngine + let unusedReceive: @MainActor @Sendable ([CKRecord], [CKRecord.ID]) throws -> Void = { _, _ in } + let unusedSent: @MainActor @Sendable (CloudSyncSentChanges) throws -> Void = { _ in } + + fixture.driver.handle( + .willFetchChanges, engine: engine, receiveRemoteRecords: unusedReceive, + handleSentChanges: unusedSent) + fixture.driver.handle( + .didFetchRecordZoneChanges(CKError(.networkFailure)), engine: engine, + receiveRemoteRecords: unusedReceive, handleSentChanges: unusedSent) + fixture.driver.handle( + .didFetchChanges, engine: engine, receiveRemoteRecords: unusedReceive, + handleSentChanges: unusedSent) + #expect(fixture.driver.accepts(engine)) + #expect(!fixture.driver.isReadyToSend) + #expect(!fixture.journal.uploadsAuthorized) + #expect(fixture.journal.recoveryRequired) + + let record = CKRecord(recordType: "Test") + fixture.driver.handle( + .fetchedRecordZoneChanges(records: [record], deletions: []), engine: engine, + receiveRemoteRecords: { _, _ in throw CocoaError(.fileWriteUnknown) }, + handleSentChanges: unusedSent) + #expect(fixture.driver.engine == nil) + #expect(!fixture.journal.uploadsAuthorized) + guard case .failed = fixture.driver.status else { + Issue.record("A category callback failure must fail the shared dispatcher closed") + return + } + } + + @Test + func sharedFailedSaveHandlerPreservesConflictZoneRetryAndTerminalSemantics() async throws { + let fixture = LifecycleFixture() + fixture.driver.start() + await fixture.driver.operation?.value + let serverRecord = CKRecord(recordType: "Test") + var conflictRecord: CKRecord? + try fixture.driver.handleFailedSave( + CKError(.serverRecordChanged, userInfo: [CKRecordChangedErrorServerRecordKey: serverRecord]) + ) { records, isConflict in + conflictRecord = records.first + #expect(isConflict) + } + #expect(conflictRecord === serverRecord) + #expect(throws: CKError.self) { + try fixture.driver.handleFailedSave(CKError(.serverRecordChanged)) { _, _ in } + } + + try fixture.driver.handleFailedSave(CKError(.zoneNotFound)) { _, _ in } + #expect(fixture.journal.resets == ["environment", "zone"]) + try fixture.driver.handleFailedSave(CKError(.networkFailure)) { _, _ in } + #expect(fixture.driver.terminalError == nil) + try fixture.driver.handleFailedSave(CKError(.permissionFailure)) { _, _ in } + #expect(fixture.driver.terminalError != nil) + guard case .failed = fixture.driver.status else { + Issue.record("A terminal save error must remain visible") + return + } + } + @Test func uploadBatchesNeverExceedCloudKitRequestLimit() async throws { let fixture = LifecycleFixture() diff --git a/FrameworkTests/CloudSyncSafetyFixture.swift b/FrameworkTests/CloudSyncSafetyFixture.swift index 55487263..0d0b9478 100644 --- a/FrameworkTests/CloudSyncSafetyFixture.swift +++ b/FrameworkTests/CloudSyncSafetyFixture.swift @@ -24,6 +24,12 @@ private struct SyncSafetyAdapter { var pendingNames: () throws -> Set var markTombstone: () throws -> Void var failingMerge: (CloudSyncSafetyStore) throws -> Void + var remoteApplyCount: () -> Int +} + +@MainActor +private final class RemoteApplyCounter { + var value = 0 } /// Real journals, coordinators, record coding and batch providers; only CloudKit transport is fake. @@ -46,6 +52,7 @@ final class CloudSyncSafetyFixture { var pendingNames: () throws -> Set { adapter.pendingNames } var markTombstone: () throws -> Void { adapter.markTombstone } var failingMerge: (CloudSyncSafetyStore) throws -> Void { adapter.failingMerge } + var remoteApplyCount: Int { adapter.remoteApplyCount() } init(category: SyncSafetyCategory, url: URL? = nil, environment: String = "Test") throws { container = try TermindPersistence.makeContainer(inMemory: url == nil, url: url) @@ -86,6 +93,8 @@ final class CloudSyncSafetyFixture { let coordinator = VaultSyncCoordinator( context: context, defaults: defaults, environmentIdentifier: environment, makeTransport: { _, _ in network.transport }) + let counter = RemoteApplyCounter() + coordinator.didApplyRemote = { _, _ in counter.value += 1 } return SyncSafetyAdapter( lifecycle: coordinator.lifecycle, recordNames: envelopes.map(\.key.recordName), receive: { try coordinator.receiveRemoteRecords($0, deletions: $1) }, @@ -110,7 +119,8 @@ final class CloudSyncSafetyFixture { try safety.clearQuarantine(recordName: envelope.key.recordName) throw CocoaError(.fileWriteUnknown) }) - }) + }, + remoteApplyCount: { counter.value }) } private static func history( @@ -131,6 +141,8 @@ final class CloudSyncSafetyFixture { let coordinator = HistorySyncCoordinator( persistence: persistence, defaults: defaults, environmentIdentifier: environment, makeTransport: { _, _ in network.transport }, makeLegacyTransport: { _, _ in legacy.transport }) + let counter = RemoteApplyCounter() + coordinator.didApplyRemote = { _ in counter.value += 1 } return SyncSafetyAdapter( lifecycle: coordinator.lifecycle, recordNames: envelopes.map(\.key.recordName), receive: { try coordinator.receiveRemoteRecords($0, deletions: $1) }, @@ -154,7 +166,8 @@ final class CloudSyncSafetyFixture { try safety.clearQuarantine(recordName: envelope.key.recordName) throw CocoaError(.fileWriteUnknown) }) - }) + }, + remoteApplyCount: { counter.value }) } private static func settings( @@ -169,6 +182,8 @@ final class CloudSyncSafetyFixture { let coordinator = SettingsSyncCoordinator( persistence: persistence, secrets: secrets, defaults: defaults, environmentIdentifier: environment, makeTransport: { _, _ in network.transport }) + let counter = RemoteApplyCounter() + coordinator.didApplyRemote = { _, _ in counter.value += 1 } return SyncSafetyAdapter( lifecycle: coordinator.lifecycle, recordNames: envelopes.map(\.key.recordName), receive: { try coordinator.receiveRemoteRecords($0, deletions: $1) }, @@ -193,7 +208,8 @@ final class CloudSyncSafetyFixture { try safety.clearQuarantine(recordName: envelope.key.recordName) throw CocoaError(.fileWriteUnknown) }) - }) + }, + remoteApplyCount: { counter.value }) } private static func metrics( @@ -210,6 +226,8 @@ final class CloudSyncSafetyFixture { let coordinator = MetricsSyncCoordinator( persistence: persistence, defaults: defaults, environmentIdentifier: environment, makeTransport: { _, _ in network.transport }) + let counter = RemoteApplyCounter() + coordinator.didApplyRemote = { _, _ in counter.value += 1 } return SyncSafetyAdapter( lifecycle: coordinator.lifecycle, recordNames: envelopes.map(\.key.recordName), receive: { try coordinator.receiveRemoteRecords($0, deletions: $1) }, @@ -234,7 +252,8 @@ final class CloudSyncSafetyFixture { try safety.clearQuarantine(recordName: envelope.key.recordName) throw CocoaError(.fileWriteUnknown) }) - }) + }, + remoteApplyCount: { counter.value }) } func start() async throws { diff --git a/FrameworkTests/CloudSyncSafetyTests.swift b/FrameworkTests/CloudSyncSafetyTests.swift index bfb98572..e78016b9 100644 --- a/FrameworkTests/CloudSyncSafetyTests.swift +++ b/FrameworkTests/CloudSyncSafetyTests.swift @@ -137,6 +137,40 @@ struct CloudSyncSafetyTests { #expect(fixture.record(record.recordID) == nil) } + @Test(arguments: SyncSafetyCategory.allCases) + func unreadableRepresentationWinsForTheSameIDInEitherBatchOrder(category: SyncSafetyCategory) async throws { + for unreadableFirst in [false, true] { + let fixture = try CloudSyncSafetyFixture(category: category) + try await fixture.start() + let id = fixture.recordIDs[0] + let supported = try #require(fixture.record(id)) + let future = try fixture.futureRecord() + let records = unreadableFirst ? [future, supported] : [supported, future] + + try fixture.receive(records, []) + + #expect(try fixture.safety().isUploadBlocked(recordName: id.recordName)) + #expect(fixture.record(id) == nil) + #expect(fixture.remoteApplyCount == 0) + fixture.stop() + } + } + + @Test + func quarantinePersistenceFailureEscapesTheSharedDecoder() throws { + let record = CKRecord(recordType: "Test") + var logged = false + + #expect(throws: CocoaError.self) { + _ = try decodeCloudSyncRecords( + [record], included: { _ in true }, + decode: { _ -> String in throw CKError(.badDatabase) }, + quarantine: { _, _ in throw CocoaError(.fileWriteUnknown) }, + logFailure: { _ in logged = true }) + } + #expect(logged) + } + @Test(arguments: SyncSafetyCategory.allCases) func newRecordTypesAreIgnoredButCannotBypassAConflictForAKnownID(category: SyncSafetyCategory) async throws { let fixture = try CloudSyncSafetyFixture(category: category) @@ -147,8 +181,10 @@ struct CloudSyncSafetyTests { for key in source.allKeys() { unknown[key] = source[key] } try fixture.receive([unknown], []) #expect(try fixture.safety().blockedRecordNames().isEmpty) + #expect(fixture.remoteApplyCount == 0) try fixture.conflict(CKError(.serverRecordChanged, userInfo: [CKRecordChangedErrorServerRecordKey: unknown])) #expect(try fixture.safety().isUploadBlocked(recordName: unknown.recordID.recordName)) #expect(fixture.record(unknown.recordID) == nil) + #expect(fixture.remoteApplyCount == 0) } } diff --git a/Frameworks/TermindInfrastructure/Cloud/CloudSyncEventHandling.swift b/Frameworks/TermindInfrastructure/Cloud/CloudSyncEventHandling.swift new file mode 100644 index 00000000..08df1b73 --- /dev/null +++ b/Frameworks/TermindInfrastructure/Cloud/CloudSyncEventHandling.swift @@ -0,0 +1,124 @@ +import CloudKit + +struct CloudSyncSentChanges { + struct FailedSave { + var record: CKRecord + var error: CKError + } + + var savedRecords: [CKRecord] + var failedRecordSaves: [FailedSave] + + init(savedRecords: [CKRecord], failedRecordSaves: [FailedSave]) { + self.savedRecords = savedRecords + self.failedRecordSaves = failedRecordSaves + } + + init(_ changes: CKSyncEngine.Event.SentRecordZoneChanges) { + self.init( + savedRecords: changes.savedRecords, + failedRecordSaves: changes.failedRecordSaves.map { FailedSave(record: $0.record, error: $0.error) }) + } +} + +/// A constructible projection of the CKSyncEngine events used by writable sync categories. +/// Keeping this value separate from CloudKit's event constructors gives the shared dispatcher a deterministic test seam. +enum CloudSyncEventInput { + case stateUpdate(CKSyncEngine.State.Serialization) + case accountChange(CKSyncEngine.Event.AccountChange.ChangeType) + case fetchedRecordZoneChanges(records: [CKRecord], deletions: [CKRecord.ID]) + case fetchedDatabaseChanges(deletedZoneIDs: [CKRecordZone.ID]) + case sentRecordZoneChanges(CloudSyncSentChanges) + case willFetchChanges + case willSendChanges + case didFetchRecordZoneChanges(CKError?) + case didFetchChanges + case didSendChanges + case ignored + + init(_ event: CKSyncEngine.Event) { + switch event { + case .stateUpdate(let update): + self = .stateUpdate(update.stateSerialization) + case .accountChange(let change): + self = .accountChange(change.changeType) + case .fetchedRecordZoneChanges(let changes): + self = .fetchedRecordZoneChanges( + records: changes.modifications.map(\.record), deletions: changes.deletions.map(\.recordID)) + case .fetchedDatabaseChanges(let changes): + self = .fetchedDatabaseChanges(deletedZoneIDs: changes.deletions.map(\.zoneID)) + case .sentRecordZoneChanges(let changes): + self = .sentRecordZoneChanges(CloudSyncSentChanges(changes)) + case .willFetchChanges: + self = .willFetchChanges + case .willSendChanges: + self = .willSendChanges + case .didFetchRecordZoneChanges(let changes): + self = .didFetchRecordZoneChanges(changes.error) + case .didFetchChanges: + self = .didFetchChanges + case .didSendChanges: + self = .didSendChanges + default: + self = .ignored + } + } +} + +extension CloudSyncLifecycle { + func handle( + _ event: CloudSyncEventInput, engine: any CloudSyncEngineSession, + receiveRemoteRecords: @MainActor ([CKRecord], [CKRecord.ID]) throws -> Void, + handleSentChanges: @MainActor (CloudSyncSentChanges) throws -> Void + ) { + guard accepts(engine) else { return } + do { + switch event { + case .stateUpdate(let serialization): + try saveEngineState(serialization) + case .accountChange(let change): + try handleAccountChange(change) + case .fetchedRecordZoneChanges(let records, let deletions): + try receiveRemoteRecords(records, deletions) + case .fetchedDatabaseChanges(let deletedZoneIDs): + if deletedZoneIDs.contains(zoneID) { try handleZoneDeletion() } + case .sentRecordZoneChanges(let changes): + try handleSentChanges(changes) + case .willFetchChanges: + try beginFetch() + case .willSendChanges: + try beginSend() + case .didFetchRecordZoneChanges(let error): + if let error { + guard Self.isRetryable(error) else { throw error } + noteFetchFailure(error) + } + case .didFetchChanges: + try finishFetch() + try updateUpToDateStatus() + case .didSendChanges: + try finishSend() + try updateUpToDateStatus() + case .ignored: + break + } + } catch { + failClosed(error) + } + } + + func handleFailedSave( + _ error: CKError, receiveRemoteRecords: @MainActor ([CKRecord], Bool) throws -> Void + ) throws { + if error.code == .serverRecordChanged { + guard let serverRecord = error.serverRecord else { throw error } + try receiveRemoteRecords([serverRecord], true) + } else if error.code == .zoneNotFound { + try handleZoneDeletion() + } else if !Self.isRetryable(error) { + logFailure(error, stage: "save") + terminalError = error.localizedDescription + status = .failed(error.localizedDescription) + } + } +} diff --git a/Frameworks/TermindInfrastructure/Cloud/CloudSyncRecordReceiving.swift b/Frameworks/TermindInfrastructure/Cloud/CloudSyncRecordReceiving.swift new file mode 100644 index 00000000..6cea88ba --- /dev/null +++ b/Frameworks/TermindInfrastructure/Cloud/CloudSyncRecordReceiving.swift @@ -0,0 +1,24 @@ +import CloudKit + +/// Decodes a remote batch while making an unreadable representation win over every readable representation of the +/// same record name. Callers keep category filtering and transactional merge work at the coordinator boundary. +func decodeCloudSyncRecords( + _ records: [CKRecord], + included: (CKRecord) -> Bool, + decode: (CKRecord) throws -> Envelope, + quarantine: (CKRecord, any Error) throws -> Void, + logFailure: (any Error) -> Void +) throws -> [(record: CKRecord, envelope: Envelope)] { + var decoded: [(record: CKRecord, envelope: Envelope)] = [] + var rejectedRecordNames = Set() + for record in records where included(record) { + do { + decoded.append((record, try decode(record))) + } catch { + logFailure(error) + try quarantine(record, error) + rejectedRecordNames.insert(record.recordID.recordName) + } + } + return decoded.filter { !rejectedRecordNames.contains($0.record.recordID.recordName) } +} diff --git a/Frameworks/TermindInfrastructure/Cloud/HistorySyncCoordinator.swift b/Frameworks/TermindInfrastructure/Cloud/HistorySyncCoordinator.swift index 26d34470..692b5599 100644 --- a/Frameworks/TermindInfrastructure/Cloud/HistorySyncCoordinator.swift +++ b/Frameworks/TermindInfrastructure/Cloud/HistorySyncCoordinator.swift @@ -203,7 +203,10 @@ public final class HistorySyncCoordinator: NSObject, CKSyncEngineDelegate, @unch } public nonisolated func handleEvent(_ event: CKSyncEngine.Event, syncEngine: CKSyncEngine) async { - await handle(event, engine: syncEngine) + await lifecycle.handle( + CloudSyncEventInput(event), engine: syncEngine, + receiveRemoteRecords: { try receiveRemoteRecords($0, deletions: $1) }, + handleSentChanges: handleSentChanges) } public nonisolated func nextRecordZoneChangeBatch( @@ -248,63 +251,24 @@ public final class HistorySyncCoordinator: NSObject, CKSyncEngineDelegate, @unch } } - private func handle(_ event: CKSyncEngine.Event, engine: CKSyncEngine) async { - guard lifecycle.accepts(engine) else { return } - do { - switch event { - case .stateUpdate(let update): - try lifecycle.saveEngineState(update.stateSerialization) - case .accountChange(let change): - try lifecycle.handleAccountChange(change.changeType) - case .fetchedRecordZoneChanges(let changes): - try receiveRemoteRecords( - changes.modifications.map(\.record), deletions: changes.deletions.map(\.recordID)) - case .fetchedDatabaseChanges(let changes): - if changes.deletions.contains(where: { $0.zoneID == zoneID }) { - try lifecycle.handleZoneDeletion() - } - case .sentRecordZoneChanges(let changes): - try handleSentChanges(changes) - case .willFetchChanges: - try lifecycle.beginFetch() - case .willSendChanges: - try lifecycle.beginSend() - case .didFetchRecordZoneChanges(let changes): - if let error = changes.error { - guard CloudSyncLifecycle.isRetryable(error) else { throw error } - lifecycle.noteFetchFailure(error) - } - case .didFetchChanges: - try lifecycle.finishFetch() - try lifecycle.updateUpToDateStatus() - case .didSendChanges: - try lifecycle.finishSend() - try lifecycle.updateUpToDateStatus() - default: - break - } - } catch { lifecycle.failClosed(error) } - } - func receiveRemoteRecords( _ records: [CKRecord], deletions: [CKRecord.ID] = [], isConflict: Bool = false ) throws { guard let scope = lifecycle.scope else { throw CancellationError() } let safety = CloudSyncSafetyStore(context: context, scope: scope) - var decoded: [(CKRecord, HistoryEnvelope)] = [] - var rejected = Set() - for record in records - where record.recordID.zoneID == zoneID && (isConflict || Self.recordTypes.contains(record.recordType)) { - if let key = HistoryRecordKey(recordName: record.recordID.recordName), !includes(key) { continue } - do { - decoded.append((record, try Self.envelope(from: record))) - } catch { - lifecycle.logFailure(error, stage: "decode-quarantine") - try CloudSyncRecordCoding.quarantine(record, error: error, in: safety, nativeFields: true) - rejected.insert(record.recordID.recordName) - } - } - decoded.removeAll { rejected.contains($0.0.recordID.recordName) } + let decoded: [(record: CKRecord, envelope: HistoryEnvelope)] = try decodeCloudSyncRecords( + records, + included: { record in + guard record.recordID.zoneID == zoneID, + isConflict || Self.recordTypes.contains(record.recordType) + else { return false } + return HistoryRecordKey(recordName: record.recordID.recordName).map(includes) != false + }, + decode: Self.envelope, + quarantine: { + try CloudSyncRecordCoding.quarantine($0, error: $1, in: safety, nativeFields: true) + }, + logFailure: { lifecycle.logFailure($0, stage: "decode-quarantine") }) let identical = try Set( decoded.compactMap { _, envelope -> HistoryRecordKey? in guard isConflict else { return nil } @@ -339,7 +303,7 @@ public final class HistorySyncCoordinator: NSObject, CKSyncEngineDelegate, @unch if !changed.isEmpty { didApplyRemote?(changed) } } - private func handleSentChanges(_ changes: CKSyncEngine.Event.SentRecordZoneChanges) throws { + private func handleSentChanges(_ changes: CloudSyncSentChanges) throws { guard let scope = lifecycle.scope else { throw CancellationError() } for record in changes.savedRecords where record.recordID.zoneID == zoneID { guard try !lifecycle.store.safety.isUploadBlocked(scope, record.recordID.recordName) else { continue } @@ -354,15 +318,8 @@ public final class HistorySyncCoordinator: NSObject, CKSyncEngineDelegate, @unch } func handleFailedSave(_ error: CKError) throws { - if error.code == .serverRecordChanged { - guard let serverRecord = error.serverRecord else { throw error } - try receiveRemoteRecords([serverRecord], isConflict: true) - } else if error.code == .zoneNotFound { - try lifecycle.handleZoneDeletion() - } else if !CloudSyncLifecycle.isRetryable(error) { - lifecycle.logFailure(error, stage: "save") - lifecycle.terminalError = error.localizedDescription - status = .failed(error.localizedDescription) + try lifecycle.handleFailedSave(error) { + try receiveRemoteRecords($0, isConflict: $1) } } diff --git a/Frameworks/TermindInfrastructure/Cloud/MetricsSyncCoordinator.swift b/Frameworks/TermindInfrastructure/Cloud/MetricsSyncCoordinator.swift index 605b991f..f18feb16 100644 --- a/Frameworks/TermindInfrastructure/Cloud/MetricsSyncCoordinator.swift +++ b/Frameworks/TermindInfrastructure/Cloud/MetricsSyncCoordinator.swift @@ -119,7 +119,10 @@ public final class MetricsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unch } public nonisolated func handleEvent(_ event: CKSyncEngine.Event, syncEngine: CKSyncEngine) async { - await handle(event, engine: syncEngine) + await lifecycle.handle( + CloudSyncEventInput(event), engine: syncEngine, + receiveRemoteRecords: { try receiveRemoteRecords($0, deletions: $1) }, + handleSentChanges: handleSentChanges) } public nonisolated func nextRecordZoneChangeBatch( @@ -154,62 +157,17 @@ public final class MetricsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unch } } - private func handle(_ event: CKSyncEngine.Event, engine: CKSyncEngine) async { - guard lifecycle.accepts(engine) else { return } - do { - switch event { - case .stateUpdate(let update): - try lifecycle.saveEngineState(update.stateSerialization) - case .accountChange(let change): - try lifecycle.handleAccountChange(change.changeType) - case .fetchedRecordZoneChanges(let changes): - try receiveRemoteRecords( - changes.modifications.map(\.record), deletions: changes.deletions.map(\.recordID)) - case .fetchedDatabaseChanges(let changes): - if changes.deletions.contains(where: { $0.zoneID == zoneID }) { - try lifecycle.handleZoneDeletion() - } - case .sentRecordZoneChanges(let changes): - try handleSentChanges(changes) - case .willFetchChanges: - try lifecycle.beginFetch() - case .willSendChanges: - try lifecycle.beginSend() - case .didFetchRecordZoneChanges(let changes): - if let error = changes.error { - guard CloudSyncLifecycle.isRetryable(error) else { throw error } - lifecycle.noteFetchFailure(error) - } - case .didFetchChanges: - try lifecycle.finishFetch() - try lifecycle.updateUpToDateStatus() - case .didSendChanges: - try lifecycle.finishSend() - try lifecycle.updateUpToDateStatus() - default: - break - } - } catch { lifecycle.failClosed(error) } - } - func receiveRemoteRecords( _ records: [CKRecord], deletions: [CKRecord.ID] = [], isConflict: Bool = false ) throws { guard let scope = lifecycle.scope else { throw CancellationError() } let safety = CloudSyncSafetyStore(context: context, scope: scope) - var decoded: [(CKRecord, MetricsEnvelope)] = [] - var rejected = Set() - for record in records - where record.recordID.zoneID == zoneID && (isConflict || record.recordType == Self.recordType) { - do { - decoded.append((record, try Self.envelope(from: record))) - } catch { - lifecycle.logFailure(error, stage: "decode-quarantine") - try CloudSyncRecordCoding.quarantine(record, error: error, in: safety) - rejected.insert(record.recordID.recordName) - } - } - decoded.removeAll { rejected.contains($0.0.recordID.recordName) } + let decoded: [(record: CKRecord, envelope: MetricsEnvelope)] = try decodeCloudSyncRecords( + records, + included: { $0.recordID.zoneID == zoneID && (isConflict || $0.recordType == Self.recordType) }, + decode: Self.envelope, + quarantine: { try CloudSyncRecordCoding.quarantine($0, error: $1, in: safety) }, + logFailure: { lifecycle.logFailure($0, stage: "decode-quarantine") }) let identical = try Set( decoded.compactMap { _, envelope -> MetricsRecordKey? in guard isConflict else { return nil } @@ -240,7 +198,7 @@ public final class MetricsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unch if !result.acceptedKeys.isEmpty { didApplyRemote?(result.snapshot, result.acceptedKeys) } } - private func handleSentChanges(_ changes: CKSyncEngine.Event.SentRecordZoneChanges) throws { + private func handleSentChanges(_ changes: CloudSyncSentChanges) throws { guard let scope = lifecycle.scope else { throw CancellationError() } let safety = CloudSyncSafetyStore(context: context, scope: scope) for record in changes.savedRecords where record.recordID.zoneID == zoneID { @@ -257,15 +215,8 @@ public final class MetricsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unch } func handleFailedSave(_ error: CKError) throws { - if error.code == .serverRecordChanged { - guard let serverRecord = error.serverRecord else { throw error } - try receiveRemoteRecords([serverRecord], isConflict: true) - } else if error.code == .zoneNotFound { - try lifecycle.handleZoneDeletion() - } else if !CloudSyncLifecycle.isRetryable(error) { - lifecycle.logFailure(error, stage: "save") - lifecycle.terminalError = error.localizedDescription - status = .failed(error.localizedDescription) + try lifecycle.handleFailedSave(error) { + try receiveRemoteRecords($0, isConflict: $1) } } diff --git a/Frameworks/TermindInfrastructure/Cloud/SettingsSyncCoordinator.swift b/Frameworks/TermindInfrastructure/Cloud/SettingsSyncCoordinator.swift index 318fc035..d0bcd539 100644 --- a/Frameworks/TermindInfrastructure/Cloud/SettingsSyncCoordinator.swift +++ b/Frameworks/TermindInfrastructure/Cloud/SettingsSyncCoordinator.swift @@ -159,7 +159,10 @@ public final class SettingsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unc public nonisolated func handleEvent( _ event: CKSyncEngine.Event, syncEngine: CKSyncEngine ) async { - await handle(event, engine: syncEngine) + await lifecycle.handle( + CloudSyncEventInput(event), engine: syncEngine, + receiveRemoteRecords: { try receiveRemoteRecords($0, deletions: $1) }, + handleSentChanges: handleSentChanges) } public nonisolated func nextRecordZoneChangeBatch( @@ -200,72 +203,22 @@ public final class SettingsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unc } } - @MainActor - private func handle(_ event: CKSyncEngine.Event, engine: CKSyncEngine) async { - guard lifecycle.accepts(engine) else { return } - do { - switch event { - case .stateUpdate(let update): - try lifecycle.saveEngineState(update.stateSerialization) - - case .accountChange(let change): - try lifecycle.handleAccountChange(change.changeType) - - case .fetchedRecordZoneChanges(let changes): - try receiveRemoteRecords( - changes.modifications.map(\.record), deletions: changes.deletions.map(\.recordID)) - - case .fetchedDatabaseChanges(let changes): - if changes.deletions.contains(where: { $0.zoneID == zoneID }) { - try lifecycle.handleZoneDeletion() - } - - case .sentRecordZoneChanges(let changes): - try handleSentChanges(changes) - - case .willFetchChanges: - try lifecycle.beginFetch() - case .willSendChanges: - try lifecycle.beginSend() - case .didFetchRecordZoneChanges(let changes): - if let error = changes.error { - guard CloudSyncLifecycle.isRetryable(error) else { throw error } - lifecycle.noteFetchFailure(error) - } - case .didFetchChanges: - try lifecycle.finishFetch() - try lifecycle.updateUpToDateStatus() - case .didSendChanges: - try lifecycle.finishSend() - try lifecycle.updateUpToDateStatus() - - default: - break - } - } catch { lifecycle.failClosed(error) } - } - func receiveRemoteRecords( _ records: [CKRecord], deletions: [CKRecord.ID] = [], isConflict: Bool = false ) throws { guard let scope = lifecycle.scope else { throw CancellationError() } let safety = CloudSyncSafetyStore(context: context, scope: scope) - var decoded: [(CKRecord, SettingsEnvelope)] = [] - var rejected = Set() - for record in records - where record.recordID.zoneID == zoneID && (isConflict || Self.acceptsRecordType(record.recordType)) { - if let key = SettingsRecordKey(recordName: record.recordID.recordName), !key.kind.participatesInSync { - continue - } - do { - decoded.append((record, try Self.envelope(from: record))) - } catch { - lifecycle.logFailure(error, stage: "decode-quarantine") - try CloudSyncRecordCoding.quarantine(record, error: error, in: safety) - rejected.insert(record.recordID.recordName) - } - } - decoded.removeAll { rejected.contains($0.0.recordID.recordName) } + let decoded: [(record: CKRecord, envelope: SettingsEnvelope)] = try decodeCloudSyncRecords( + records, + included: { record in + guard record.recordID.zoneID == zoneID, + isConflict || Self.acceptsRecordType(record.recordType) + else { return false } + return SettingsRecordKey(recordName: record.recordID.recordName)?.kind.participatesInSync != false + }, + decode: Self.envelope, + quarantine: { try CloudSyncRecordCoding.quarantine($0, error: $1, in: safety) }, + logFailure: { lifecycle.logFailure($0, stage: "decode-quarantine") }) let identical = try Set( decoded.compactMap { _, envelope -> SettingsRecordKey? in guard isConflict else { return nil } @@ -301,7 +254,7 @@ public final class SettingsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unc if !result.acceptedKeys.isEmpty { didApplyRemote?(result.snapshot, result.acceptedKeys) } } - private func handleSentChanges(_ changes: CKSyncEngine.Event.SentRecordZoneChanges) throws { + private func handleSentChanges(_ changes: CloudSyncSentChanges) throws { guard let scope = lifecycle.scope else { throw CancellationError() } let safety = CloudSyncSafetyStore(context: context, scope: scope) for record in changes.savedRecords where record.recordID.zoneID == zoneID { @@ -318,15 +271,8 @@ public final class SettingsSyncCoordinator: NSObject, CKSyncEngineDelegate, @unc } func handleFailedSave(_ error: CKError) throws { - if error.code == .serverRecordChanged { - guard let serverRecord = error.serverRecord else { throw error } - try receiveRemoteRecords([serverRecord], isConflict: true) - } else if error.code == .zoneNotFound { - try lifecycle.handleZoneDeletion() - } else if !CloudSyncLifecycle.isRetryable(error) { - lifecycle.logFailure(error, stage: "save") - lifecycle.terminalError = error.localizedDescription - status = .failed(error.localizedDescription) + try lifecycle.handleFailedSave(error) { + try receiveRemoteRecords($0, isConflict: $1) } } diff --git a/Frameworks/TermindInfrastructure/Cloud/VaultSyncCoordinator.swift b/Frameworks/TermindInfrastructure/Cloud/VaultSyncCoordinator.swift index 07c2a2f7..c78361bc 100644 --- a/Frameworks/TermindInfrastructure/Cloud/VaultSyncCoordinator.swift +++ b/Frameworks/TermindInfrastructure/Cloud/VaultSyncCoordinator.swift @@ -124,7 +124,10 @@ public final class VaultSyncCoordinator: NSObject, CKSyncEngineDelegate, @unchec public nonisolated func handleEvent( _ event: CKSyncEngine.Event, syncEngine: CKSyncEngine ) async { - await handle(event, engine: syncEngine) + await lifecycle.handle( + CloudSyncEventInput(event), engine: syncEngine, + receiveRemoteRecords: { try receiveRemoteRecords($0, deletions: $1) }, + handleSentChanges: handleSentChanges) } public nonisolated func nextRecordZoneChangeBatch( @@ -168,72 +171,21 @@ public final class VaultSyncCoordinator: NSObject, CKSyncEngineDelegate, @unchec } } - @MainActor - private func handle(_ event: CKSyncEngine.Event, engine: CKSyncEngine) async { - guard lifecycle.accepts(engine) else { return } - do { - switch event { - case .stateUpdate(let update): - try lifecycle.saveEngineState(update.stateSerialization) - - case .accountChange(let change): - try lifecycle.handleAccountChange(change.changeType) - - case .fetchedRecordZoneChanges(let changes): - try receiveRemoteRecords( - changes.modifications.map(\.record), deletions: changes.deletions.map(\.recordID)) - - case .fetchedDatabaseChanges(let changes): - if changes.deletions.contains(where: { $0.zoneID == zoneID }) { - try lifecycle.handleZoneDeletion() - } - - case .sentRecordZoneChanges(let changes): - try handleSentChanges(changes) - - case .willFetchChanges: - try lifecycle.beginFetch() - case .willSendChanges: - try lifecycle.beginSend() - case .didFetchRecordZoneChanges(let changes): - if let error = changes.error { - guard CloudSyncLifecycle.isRetryable(error) else { throw error } - lifecycle.noteFetchFailure(error) - } - case .didFetchChanges: - try lifecycle.finishFetch() - try lifecycle.updateUpToDateStatus() - case .didSendChanges: - try lifecycle.finishSend() - try lifecycle.updateUpToDateStatus() - - default: - break - } - } catch { lifecycle.failClosed(error) } - } - /// Shared by fetched records and serverRecordChanged replies; never clears a block before the merge commits. func receiveRemoteRecords( _ records: [CKRecord], deletions: [CKRecord.ID] = [], isConflict: Bool = false ) throws { guard let scope = lifecycle.scope else { throw CancellationError() } let safety = CloudSyncSafetyStore(context: context, scope: scope) - var decoded: [(CKRecord, VaultEnvelope)] = [] - var rejected = Set() - for record in records - where record.recordID.zoneID == zoneID && (isConflict || record.recordType == Self.recordType) - && HistoryRecordKey(recordName: record.recordID.recordName) == nil - { - do { - decoded.append((record, try Self.envelope(from: record))) - } catch { - lifecycle.logFailure(error, stage: "decode-quarantine") - try CloudSyncRecordCoding.quarantine(record, error: error, in: safety) - rejected.insert(record.recordID.recordName) - } - } - decoded.removeAll { rejected.contains($0.0.recordID.recordName) } + let decoded: [(record: CKRecord, envelope: VaultEnvelope)] = try decodeCloudSyncRecords( + records, + included: { + $0.recordID.zoneID == zoneID && (isConflict || $0.recordType == Self.recordType) + && HistoryRecordKey(recordName: $0.recordID.recordName) == nil + }, + decode: Self.envelope, + quarantine: { try CloudSyncRecordCoding.quarantine($0, error: $1, in: safety) }, + logFailure: { lifecycle.logFailure($0, stage: "decode-quarantine") }) let identical = try Set( decoded.compactMap { _, envelope -> VaultRecordKey? in guard isConflict else { return nil } @@ -264,7 +216,7 @@ public final class VaultSyncCoordinator: NSObject, CKSyncEngineDelegate, @unchec if !result.acceptedKeys.isEmpty { didApplyRemote?(result.snapshot, result.acceptedKeys) } } - private func handleSentChanges(_ changes: CKSyncEngine.Event.SentRecordZoneChanges) throws { + private func handleSentChanges(_ changes: CloudSyncSentChanges) throws { guard let scope = lifecycle.scope else { throw CancellationError() } let safety = CloudSyncSafetyStore(context: context, scope: scope) for record in changes.savedRecords @@ -284,15 +236,8 @@ public final class VaultSyncCoordinator: NSObject, CKSyncEngineDelegate, @unchec } func handleFailedSave(_ error: CKError) throws { - if error.code == .serverRecordChanged { - guard let serverRecord = error.serverRecord else { throw error } - try receiveRemoteRecords([serverRecord], isConflict: true) - } else if error.code == .zoneNotFound { - try lifecycle.handleZoneDeletion() - } else if !CloudSyncLifecycle.isRetryable(error) { - lifecycle.logFailure(error, stage: "save") - lifecycle.terminalError = error.localizedDescription - status = .failed(error.localizedDescription) + try lifecycle.handleFailedSave(error) { + try receiveRemoteRecords($0, isConflict: $1) } } diff --git a/TFMTests/ArchitectureBoundaryTests.swift b/TFMTests/ArchitectureBoundaryTests.swift index 3e2ac449..92d37933 100644 --- a/TFMTests/ArchitectureBoundaryTests.swift +++ b/TFMTests/ArchitectureBoundaryTests.swift @@ -224,16 +224,30 @@ struct ArchitectureBoundaryTests { } #expect(source.contains("lifecycle.makeChangeBatch(")) #expect(source.contains("lifecycle.canUpload(")) + #expect(source.contains("lifecycle.handle(")) + #expect(source.contains("lifecycle.handleFailedSave(")) + #expect(source.contains("decodeCloudSyncRecords(")) #expect(source.contains("CloudSyncRecordCoding.quarantine(")) - #expect(source.contains("isConflict: true")) #expect(!source.contains("try? journal.outgoingEntry")) #expect(!source.contains("CKSyncEngine.RecordZoneChangeBatch(")) } let lifecycle = try String(contentsOf: root.appending(path: "CloudSyncLifecycle.swift"), encoding: .utf8) + let eventHandling = try String( + contentsOf: root.appending(path: "CloudSyncEventHandling.swift"), encoding: .utf8) + let recordReceiving = try String( + contentsOf: root.appending(path: "CloudSyncRecordReceiving.swift"), encoding: .utf8) #expect(lifecycle.contains("maximumRecordsPerRequest = 250")) #expect(lifecycle.contains("records.count == Self.maximumRecordsPerRequest")) #expect(!lifecycle.split(separator: "\n").contains("import SwiftData")) #expect(!lifecycle.split(separator: "\n").contains("import TermindPersistence")) + #expect(!eventHandling.split(separator: "\n").contains("import SwiftData")) + #expect(!eventHandling.split(separator: "\n").contains("import TermindPersistence")) + #expect(eventHandling.contains("guard accepts(engine) else { return }")) + #expect(eventHandling.contains("failClosed(error)")) + #expect(eventHandling.contains("guard let serverRecord = error.serverRecord else { throw error }")) + #expect(eventHandling.contains("try receiveRemoteRecords([serverRecord], true)")) + #expect(recordReceiving.contains("try quarantine(record, error)")) + #expect(recordReceiving.contains("rejectedRecordNames.contains")) for file in files where file.lastPathComponent != "CloudSyncLifecycle.swift" { let source = try String(contentsOf: file, encoding: .utf8) #expect(!source.contains("CKSyncEngine.RecordZoneChangeBatch("))