From a9635c8ee75f30bb685cf720fb47301d274bf2de Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 1 Dec 2025 15:06:39 -0600 Subject: [PATCH 01/10] Update mock database to respect atomic modifications and zones. --- .../CloudKitDemo/CountersListFeature.swift | 49 ++++++++++++++++ Examples/CloudKitDemo/Schema.swift | 6 +- .../Internal/MockCloudContainer.swift | 2 +- .../CloudKit/Internal/MockCloudDatabase.swift | 56 ++++++++++++++----- .../CloudKit/Internal/MockSyncEngine.swift | 4 +- Sources/SQLiteData/CloudKit/SyncEngine.swift | 6 +- .../ForeignKeyConstraintTests.swift | 4 +- .../MockCloudDatabaseTests.swift | 51 +++++++++++++++++ .../Internal/CloudKit+CustomDump.swift | 2 +- .../Internal/CloudKitTestHelpers.swift | 2 +- 10 files changed, 160 insertions(+), 22 deletions(-) diff --git a/Examples/CloudKitDemo/CountersListFeature.swift b/Examples/CloudKitDemo/CountersListFeature.swift index 98b904e2..7a76ed04 100644 --- a/Examples/CloudKitDemo/CountersListFeature.swift +++ b/Examples/CloudKitDemo/CountersListFeature.swift @@ -1,11 +1,13 @@ import CloudKit import SQLiteData +import SwiftData import SwiftUI import SwiftUINavigation struct CountersListView: View { @FetchAll var counters: [Counter] @Dependency(\.defaultDatabase) var database + @Dependency(\.defaultSyncEngine) var syncEngine var body: some View { List { @@ -33,8 +35,55 @@ struct CountersListView: View { } } } + ToolbarItem { + Button("Trigger") { + Task { await trigger() } + } + } + } + } + +func trigger() async { + await withErrorReporting { + // Step 1: Create a new counter + let newCounter = try await database.write { db in + try Counter.insert { Counter.Draft() } + .returning(\.self) + .fetchOne(db)! + } + + // Step 2: Force sending all data to iCloud + try await syncEngine.sendChanges() + + // Step 3: Grab the CKRecord for the newly inserted counter + let newCounterLastKnownServerRecord = try await database.read { db in + try SyncMetadata.find(newCounter.syncMetadataID).select(\.lastKnownServerRecord) + .fetchOne(db)!! + } + + // Step 4: Make a change to the counter directly on iCloud + let container = CKContainer(identifier: ModelConfiguration(groupContainer: .automatic).cloudKitContainerIdentifier!) + let serverCounter = try await container.privateCloudDatabase.record(for: newCounterLastKnownServerRecord.recordID) + serverCounter.encryptedValues["count"] = Int.random(in: 1...1_000) + let (saveResults, _) = try await container.privateCloudDatabase.modifyRecords(saving: [serverCounter], deleting: []) + + // Step 5: Make two changes to the local database: 1) decrement any counter besides the one + // created above (should succeed), and 2) increment the counter just created (should + // fail due to conflict) + try await database.write { db in + try Counter + .where { $0.id.neq(newCounter.id) } + .update { $0.count -= 1 } + .execute(db) + try Counter + .find(newCounter.id) + .update { $0.count += 1 } + .execute(db) } + + try await syncEngine.sendChanges() } +} func deleteRows(at indexSet: IndexSet) { withErrorReporting { diff --git a/Examples/CloudKitDemo/Schema.swift b/Examples/CloudKitDemo/Schema.swift index 4721e583..e297d909 100644 --- a/Examples/CloudKitDemo/Schema.swift +++ b/Examples/CloudKitDemo/Schema.swift @@ -11,7 +11,11 @@ nonisolated struct Counter: Identifiable { extension DependencyValues { mutating func bootstrapDatabase() throws { @Dependency(\.context) var context - let database = try SQLiteData.defaultDatabase() + var configuration = Configuration() + configuration.prepareDatabase { + try $0.attachMetadatabase() + } + let database = try SQLiteData.defaultDatabase(configuration: configuration) logger.debug( """ App database diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift index f0421c82..bb043eda 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift @@ -48,7 +48,7 @@ : sharedCloudDatabase let rootRecord: CKRecord? = database.storage.withValue { - $0[share.recordID.zoneID]?.values.first { record in + $0[share.recordID.zoneID]?.records.values.first { record in record.share?.recordID == share.recordID } } diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index 7edfeccb..a70dc8d7 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -4,11 +4,10 @@ @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) package final class MockCloudDatabase: CloudDatabase { - package let storage = LockIsolated<[CKRecordZone.ID: [CKRecord.ID: CKRecord]]>([:]) + package let storage = LockIsolated<[CKRecordZone.ID: Zone]>([:]) let assets = LockIsolated<[AssetID: Data]>([:]) package let databaseScope: CKDatabase.Scope let _container = IsolatedWeakVar() - let dataManager = Dependency(\.dataManager) struct AssetID: Hashable { @@ -16,6 +15,11 @@ let key: String } + package struct Zone { + package var zone: CKRecordZone + package var records: [CKRecord.ID: CKRecord] = [:] + } + package init(databaseScope: CKDatabase.Scope) { self.databaseScope = databaseScope } @@ -34,7 +38,7 @@ else { throw ckError(forAccountStatus: accountStatus) } guard let zone = storage[recordID.zoneID] else { throw CKError(.zoneNotFound) } - guard let record = zone[recordID] + guard let record = zone.records[recordID] else { throw CKError(.unknownItem) } guard let record = record.copy() as? CKRecord else { fatalError("Could not copy CKRecord.") } @@ -80,7 +84,7 @@ guard accountStatus == .available else { throw ckError(forAccountStatus: accountStatus) } - return storage.withValue { storage in + var (saveResults, deleteResults) = storage.withValue { storage in var saveResults: [CKRecord.ID: Result] = [:] var deleteResults: [CKRecord.ID: Result] = [:] @@ -91,7 +95,7 @@ let isSavingRootRecord = recordsToSave.contains(where: { $0.share?.recordID == share.recordID }) - let shareWasPreviouslySaved = storage[share.recordID.zoneID]?[share.recordID] != nil + let shareWasPreviouslySaved = storage[share.recordID.zoneID]?.records[share.recordID] != nil guard shareWasPreviouslySaved || isSavingRootRecord else { saveResults[recordToSave.recordID] = .failure(CKError(.invalidArguments)) @@ -105,12 +109,12 @@ continue } - let existingRecord = storage[recordToSave.recordID.zoneID]?[recordToSave.recordID] + let existingRecord = storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] func saveRecordToDatabase() { let hasReferenceViolation = recordToSave.parent.map { parent in - storage[parent.recordID.zoneID]?[parent.recordID] == nil + storage[parent.recordID.zoneID]?.records[parent.recordID] == nil && !recordsToSave.contains { $0.recordID == parent.recordID } } ?? false @@ -123,10 +127,12 @@ func root(of record: CKRecord) -> CKRecord { guard let parent = record.parent else { return record } - return (storage[parent.recordID.zoneID]?[parent.recordID]).map(root) ?? record + return (storage[parent.recordID.zoneID]?.records[parent.recordID]).map( + root + ) ?? record } func share(for rootRecord: CKRecord) -> CKShare? { - for (_, record) in storage[rootRecord.recordID.zoneID] ?? [:] { + for (_, record) in storage[rootRecord.recordID.zoneID]?.records ?? [:] { guard record.recordID == rootRecord.share?.recordID else { continue } return record as? CKShare @@ -160,7 +166,7 @@ } // TODO: This should merge copy's values to more accurately reflect reality - storage[recordToSave.recordID.zoneID]?[recordToSave.recordID] = copy + storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] = copy saveResults[recordToSave.recordID] = .success(copy) } @@ -219,7 +225,7 @@ continue } let hasReferenceViolation = !Set( - storage[recordIDToDelete.zoneID]?.values + storage[recordIDToDelete.zoneID]?.records.values .compactMap { $0.parent?.recordID == recordIDToDelete ? $0.recordID : nil } ?? [] ) @@ -231,12 +237,36 @@ deleteResults[recordIDToDelete] = .failure(CKError(.referenceViolation)) continue } - storage[recordIDToDelete.zoneID]?[recordIDToDelete] = nil + storage[recordIDToDelete.zoneID]?.records[recordIDToDelete] = nil deleteResults[recordIDToDelete] = .success(()) } return (saveResults: saveResults, deleteResults: deleteResults) } + + // NB: atomic is per zone setting + // NB: check if zone has atomic capability + if atomically { + let saveSuccessRecordIDs = saveResults.compactMap { recordID, result in + (try? result.get()) == nil ? nil : recordID + } + let deleteSuccessRecordIDs = deleteResults.compactMap { recordID, result in + (try? result.get()) == nil ? nil : recordID + } + if + saveSuccessRecordIDs.count != saveResults.count + || deleteSuccessRecordIDs.count != deleteResults.count + { + for saveSuccessRecordID in saveSuccessRecordIDs { + saveResults[saveSuccessRecordID] = .failure(CKError(.batchRequestFailed)) + } + for deleteSuccessRecordID in deleteSuccessRecordIDs { + deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed)) + } + } + } + + return (saveResults: saveResults, deleteResults: deleteResults) } package func modifyRecordZones( @@ -255,7 +285,7 @@ var deleteResults: [CKRecordZone.ID: Result] = [:] for recordZoneToSave in recordZonesToSave { - storage[recordZoneToSave.zoneID] = storage[recordZoneToSave.zoneID] ?? [:] + storage[recordZoneToSave.zoneID] = storage[recordZoneToSave.zoneID] ?? Zone(zone: recordZoneToSave) saveResults[recordZoneToSave.zoneID] = .success(recordZoneToSave) } diff --git a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift index 31f1c02c..7165282f 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift @@ -47,7 +47,7 @@ } records = zoneIDs.reduce(into: [CKRecord]()) { accum, zoneID in accum += database.storage.withValue { - ($0[zoneID]?.values).map { Array($0) } ?? [] + ($0[zoneID]?.records.values).map { Array($0) } ?? [] } } await parentSyncEngine.handleEvent( @@ -222,7 +222,7 @@ saving: batch.recordsToSave, deleting: batch.recordIDsToDelete, savePolicy: .ifServerRecordUnchanged, - atomically: true + atomically: batch.atomicByZone ) var savedRecords: [CKRecord] = [] diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 0a985f33..67fcfb15 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -1684,8 +1684,12 @@ try await open(table) } + case .batchRequestFailed: + fatalError() + break + case .networkFailure, .networkUnavailable, .zoneBusy, .serviceUnavailable, - .notAuthenticated, .operationCancelled, .batchRequestFailed, + .notAuthenticated, .operationCancelled, .internalError, .partialFailure, .badContainer, .requestRateLimited, .missingEntitlement, .invalidArguments, .resultsTruncated, .assetFileNotFound, .assetFileModified, .incompatibleVersion, .constraintViolation, .changeTokenExpired, diff --git a/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift b/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift index d9369f95..bf6d011c 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift @@ -668,7 +668,7 @@ """ } assertInlineSnapshot( - of: syncEngine.private.database.storage[syncEngine.defaultZone.zoneID]?[ + of: syncEngine.private.database.storage[syncEngine.defaultZone.zoneID]?.records[ Reminder.recordID(for: 1) ], as: .customDump @@ -763,7 +763,7 @@ """ } assertInlineSnapshot( - of: syncEngine.private.database.storage[syncEngine.defaultZone.zoneID]?[ + of: syncEngine.private.database.storage[syncEngine.defaultZone.zoneID]?.records[ Reminder.recordID(for: 1) ], as: .customDump diff --git a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift index 01533b59..fbb4e58c 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift @@ -421,6 +421,57 @@ } #expect(error?.code == .unknownItem) } + + @Test func batchRequestFailed() async throws { + let record1ID = CKRecord.ID(recordName: "1") + let record2ID = CKRecord.ID(recordName: "2") + + do { + let record1 = CKRecord(recordType: "record1", recordID: record1ID) + let record2 = CKRecord(recordType: "record2", recordID: record2ID) + let (saveResults, _) = try syncEngine.private.database.modifyRecords(saving: [ + record1, record2, + ]) + #expect(saveResults.values.count(where: { (try? $0.get()) != nil }) == 2) + } + + let freshRecord2 = try syncEngine.private.database.record(for: record2ID) + do { + let freshRecord1 = try syncEngine.private.database.record(for: record1ID) + freshRecord1["isOn"] = true + freshRecord2["isOn"] = true + let (saveResults, _) = try syncEngine.private.database.modifyRecords( + saving: [freshRecord1, freshRecord2] + ) + #expect(saveResults.values.count(where: { (try? $0.get()) != nil }) == 2) + } + + do { + let freshRecord1 = try syncEngine.private.database.record(for: record1ID) + freshRecord1["isOn"] = true + freshRecord2["isOn"] = false + let (saveResults, _) = try syncEngine.private.database.modifyRecords( + saving: [freshRecord1, freshRecord2] + ) + #expect( + saveResults.compactMapValues { ($0.error as? CKError)?.code } == [ + record1ID: .batchRequestFailed, + record2ID: .serverRecordChanged + ] + ) + } + } } } #endif + +extension Result { + fileprivate var error: Failure? { + do { + _ = try get() + return nil + } catch { + return error + } + } +} diff --git a/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift b/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift index 3514158e..5874e5d6 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift @@ -239,7 +239,7 @@ "databaseScope": databaseScope, "storage": storage .value - .flatMap { _, value in value.values } + .flatMap { _, value in value.records.values } .sorted { ($0.recordType, $0.recordID.recordName) < ($1.recordType, $1.recordID.recordName) }, diff --git a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift index 70c082c0..6ab734cf 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift @@ -75,7 +75,7 @@ extension SyncEngine { let syncEngine = syncEngine(for: scope) let recordsToDeleteByID = Dictionary( grouping: syncEngine.database.storage.withValue { storage in - recordIDsToDelete.compactMap { recordID in storage[recordID.zoneID]?[recordID] } + recordIDsToDelete.compactMap { recordID in storage[recordID.zoneID]?.records[recordID] } }, by: \.recordID ) From 154e52907ab3375653d071c52162772c68051a97 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 10 Dec 2025 14:37:27 -0600 Subject: [PATCH 02/10] wip --- .../CloudKit/Internal/MockCloudDatabase.swift | 67 +++++++----- .../CloudKit/Internal/MockSyncEngine.swift | 7 ++ Sources/SQLiteData/CloudKit/SyncEngine.swift | 39 +++++-- .../CloudKitTests/AtomicTests.swift | 101 ++++++++++++++++++ .../MockCloudDatabaseTests.swift | 11 -- .../Internal/BaseCloudKitTests.swift | 15 ++- .../Internal/CloudKitTestHelpers.swift | 6 +- .../Internal/ResultExtensions.swift | 10 ++ .../SQLiteDataTests/Internal/TestScopes.swift | 23 ++++ 9 files changed, 228 insertions(+), 51 deletions(-) create mode 100644 Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift create mode 100644 Tests/SQLiteDataTests/Internal/ResultExtensions.swift diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index faa1d6ee..e096677d 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -85,6 +85,7 @@ else { throw ckError(forAccountStatus: accountStatus) } var (saveResults, deleteResults) = storage.withValue { storage in + let previousStorage = storage var saveResults: [CKRecord.ID: Result] = [:] var deleteResults: [CKRecord.ID: Result] = [:] @@ -95,7 +96,8 @@ let isSavingRootRecord = recordsToSave.contains(where: { $0.share?.recordID == share.recordID }) - let shareWasPreviouslySaved = storage[share.recordID.zoneID]?.records[share.recordID] != nil + let shareWasPreviouslySaved = + storage[share.recordID.zoneID]?.records[share.recordID] != nil guard shareWasPreviouslySaved || isSavingRootRecord else { saveResults[recordToSave.recordID] = .failure(CKError(.invalidArguments)) @@ -118,7 +120,9 @@ continue } - let existingRecord = storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] + let existingRecord = storage[recordToSave.recordID.zoneID]?.records[ + recordToSave.recordID + ] func saveRecordToDatabase() { let hasReferenceViolation = @@ -246,7 +250,7 @@ deleteResults[recordIDToDelete] = .failure(CKError(.referenceViolation)) continue } - let recordToDelete = storage[recordIDToDelete.zoneID]?[recordIDToDelete] + let recordToDelete = storage[recordIDToDelete.zoneID]?.records[recordIDToDelete] storage[recordIDToDelete.zoneID]?.records[recordIDToDelete] = nil deleteResults[recordIDToDelete] = .success(()) @@ -257,14 +261,14 @@ shareToDelete.recordID.zoneID.ownerName == CKCurrentUserDefaultName { func deleteRecords(referencing recordID: CKRecord.ID) { - for recordToDelete in (storage[recordIDToDelete.zoneID] ?? [:]).values { + for recordToDelete in (storage[recordIDToDelete.zoneID]?.records ?? [:]).values { guard recordToDelete.share?.recordID == recordID || recordToDelete.parent?.recordID == recordID else { continue } - storage[recordIDToDelete.zoneID]?[recordToDelete.recordID] = nil + storage[recordIDToDelete.zoneID]?.records[recordToDelete.recordID] = nil deleteRecords(referencing: recordToDelete.recordID) } } @@ -272,29 +276,39 @@ } } - return (saveResults: saveResults, deleteResults: deleteResults) - } + // NB: atomic is per zone setting + // NB: check if zone has atomic capability + if atomically { + let zones = Set(recordsToSave.map(\.recordID.zoneID) + recordIDsToDelete.map(\.zoneID)) + for zoneID in zones { + let saveResultsInZone = saveResults.filter { recordID, _ in recordID.zoneID == zoneID } + let deleteResultsInZone = deleteResults.filter { recordID, _ in + recordID.zoneID == zoneID + } + let saveSuccessRecordIDs = saveResultsInZone.compactMap { recordID, result in + (try? result.get()) == nil ? nil : recordID + } + let deleteSuccessRecordIDs = deleteResultsInZone.compactMap { recordID, result in + (try? result.get()) == nil ? nil : recordID + } + if saveSuccessRecordIDs.count != saveResultsInZone.count + || deleteSuccessRecordIDs.count != deleteResultsInZone.count + { + for saveSuccessRecordID in saveSuccessRecordIDs { + print("!!!") + saveResults[saveSuccessRecordID] = .failure(CKError(.batchRequestFailed)) + } + for deleteSuccessRecordID in deleteSuccessRecordIDs { + print("!!!") + deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed)) + } - // NB: atomic is per zone setting - // NB: check if zone has atomic capability - if atomically { - let saveSuccessRecordIDs = saveResults.compactMap { recordID, result in - (try? result.get()) == nil ? nil : recordID - } - let deleteSuccessRecordIDs = deleteResults.compactMap { recordID, result in - (try? result.get()) == nil ? nil : recordID - } - if - saveSuccessRecordIDs.count != saveResults.count - || deleteSuccessRecordIDs.count != deleteResults.count - { - for saveSuccessRecordID in saveSuccessRecordIDs { - saveResults[saveSuccessRecordID] = .failure(CKError(.batchRequestFailed)) - } - for deleteSuccessRecordID in deleteSuccessRecordIDs { - deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed)) + storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:] + } } } + + return (saveResults: saveResults, deleteResults: deleteResults) } return (saveResults: saveResults, deleteResults: deleteResults) @@ -316,7 +330,8 @@ var deleteResults: [CKRecordZone.ID: Result] = [:] for recordZoneToSave in recordZonesToSave { - storage[recordZoneToSave.zoneID] = storage[recordZoneToSave.zoneID] ?? Zone(zone: recordZoneToSave) + storage[recordZoneToSave.zoneID] = + storage[recordZoneToSave.zoneID] ?? Zone(zone: recordZoneToSave) saveResults[recordZoneToSave.zoneID] = .success(recordZoneToSave) } diff --git a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift index fe25e6a6..80213ca2 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift @@ -260,12 +260,19 @@ fatalError("Mocks should only raise 'CKError' values.") } } + print("!!!") syncEngine.state.remove( pendingRecordZoneChanges: savedRecords.map { .saveRecord($0.recordID) } ) + syncEngine.state.remove( + pendingRecordZoneChanges: failedRecordSaves.map { .saveRecord($0.record.recordID) } + ) syncEngine.state.remove( pendingRecordZoneChanges: deletedRecordIDs.map { .deleteRecord($0) } ) + syncEngine.state.remove( + pendingRecordZoneChanges: failedRecordDeletes.keys.map { .deleteRecord($0) } + ) await syncEngine.parentSyncEngine .handleEvent( diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 5101112f..2495d0aa 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -38,6 +38,7 @@ private let notificationsObserver = LockIsolated<(any NSObjectProtocol)?>(nil) private let activityCounts = LockIsolated(ActivityCounts()) private let startTask = LockIsolated?>(nil) + private let atomicByZone: Bool /// The error message used when a write occurs to a record for which the current user does not /// have permission. @@ -215,10 +216,12 @@ logger: Logger, delegate: (any SyncEngineDelegate)?, tables: [any SynchronizableTable], - privateTables: [any SynchronizableTable] = [] + privateTables: [any SynchronizableTable] = [], + atomicByZone: Bool = false ) throws { let allTables = Set((tables + privateTables).map(HashableSynchronizedTable.init)) .map(\.type) + self.atomicByZone = atomicByZone self.tables = allTables self.privateTables = privateTables self.delegate = delegate @@ -618,6 +621,7 @@ false } } + print("!!!") syncEngines.withValue { $0.private?.state.add(pendingRecordZoneChanges: changesByIsPrivate[true] ?? []) $0.shared?.state.add(pendingRecordZoneChanges: changesByIsPrivate[false] ?? []) @@ -1079,7 +1083,7 @@ } #endif - let batch = await syncEngine.recordZoneChangeBatch(pendingChanges: changes) { recordID in + var batch = await syncEngine.recordZoneChangeBatch(pendingChanges: changes) { recordID in guard let (metadata, allFields) = await withErrorReporting( .sqliteDataCloudKitFailure, @@ -1178,6 +1182,7 @@ } return await open(table) } + batch?.atomicByZone = atomicByZone return batch } @@ -1577,10 +1582,10 @@ var newPendingRecordZoneChanges: [CKSyncEngine.PendingRecordZoneChange] = [] var newPendingDatabaseChanges: [CKSyncEngine.PendingDatabaseChange] = [] - defer { - syncEngine.state.add(pendingDatabaseChanges: newPendingDatabaseChanges) - syncEngine.state.add(pendingRecordZoneChanges: newPendingRecordZoneChanges) - } +// defer { +// syncEngine.state.add(pendingDatabaseChanges: newPendingDatabaseChanges) +// syncEngine.state.add(pendingRecordZoneChanges: newPendingRecordZoneChanges) +// } for (failedRecord, error) in failedRecordSaves { func clearServerRecord() async { await withErrorReporting(.sqliteDataCloudKitFailure) { @@ -1697,7 +1702,18 @@ } case .batchRequestFailed: - fatalError() +// print("?!?!?!") +// await withErrorReporting { +// try await metadatabase.write { db in +// try PendingRecordZoneChange.insert { +// PendingRecordZoneChange.init(.saveRecord(failedRecord.recordID)) +// } +// .execute(db) +// } +// } + //[.failed(.serverRecordChanged), .failed(.batchRequestFailed)] + //syncEngine.state.add(pendingRecordZoneChanges: [.saveRecord(failedRecord.recordID)]) + newPendingRecordZoneChanges.append(.saveRecord(failedRecord.recordID)) break case .networkFailure, .networkUnavailable, .zoneBusy, .serviceUnavailable, @@ -1740,6 +1756,15 @@ if enqueuedUnsyncedRecordID { await handleFetchedRecordZoneChanges(syncEngine: syncEngine) } + + await withErrorReporting { + try await enqueueLocallyPendingChanges() + } + +// defer { + syncEngine.state.add(pendingDatabaseChanges: newPendingDatabaseChanges) + syncEngine.state.add(pendingRecordZoneChanges: newPendingRecordZoneChanges) +// } } private func cacheShare(_ share: CKShare) async throws { diff --git a/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift b/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift new file mode 100644 index 00000000..188875a9 --- /dev/null +++ b/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift @@ -0,0 +1,101 @@ +#if canImport(CloudKit) + import CloudKit + import ConcurrencyExtras + import CustomDump + import InlineSnapshotTesting + import OrderedCollections + import SQLiteData + import SQLiteDataTestSupport + import SnapshotTestingCustomDump + import Testing + + extension BaseCloudKitTests { + @MainActor + final class AtomicTests: BaseCloudKitTests, @unchecked Sendable { + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test(.atomicByZone(true)) func basics() async throws { + try await userDatabase.userWrite { db in + try db.seed { + RemindersList(id: 1, title: "Personal") + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let remindersListRecord = try syncEngine.private.database.record( + for: RemindersList.recordID(for: 1) + ) + remindersListRecord.setValue("My stuff", forKey: "title", at: 1) + let (saveResults, _) = try syncEngine.private.database.modifyRecords(saving: [remindersListRecord]) + #expect(saveResults.values.allSatisfy { $0.error == nil }) + + try await withDependencies { + $0.currentTime.now = 2 + } operation: { + try await userDatabase.userWrite { db in + try RemindersList.find(1).update { $0.title = "Stuff" }.execute(db) + try RemindersList.insert { RemindersList(id: 2, title: "Business") }.execute(db) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + title: "My stuff" + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } + + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + title: "Stuff" + ), + [1]: CKRecord( + recordID: CKRecord.ID(2:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 2, + title: "Business" + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } + } + } + } +#endif diff --git a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift index b1d8e6de..245121d5 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift @@ -648,14 +648,3 @@ } } #endif - -extension Result { - fileprivate var error: Failure? { - do { - _ = try get() - return nil - } catch { - return error - } - } -} diff --git a/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift b/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift index 4aec1bc8..cfbc5f3e 100644 --- a/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift +++ b/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift @@ -73,7 +73,8 @@ class BaseCloudKitTests: @unchecked Sendable { ModelB.self, ModelC.self, privateTables: RemindersListPrivate.self, - startImmediately: _StartImmediatelyTrait.startImmediately + startImmediately: _StartImmediatelyTrait.startImmediately, + atomicByZone: _AtomicByZoneTrait.atomicByZone ) if _StartImmediatelyTrait.startImmediately, _AccountStatusScope.accountStatus == .available @@ -167,7 +168,8 @@ extension SyncEngine { delegate: (any SyncEngineDelegate)? = nil, tables: repeat (each T1).Type, privateTables: repeat (each T2).Type, - startImmediately: Bool = true + startImmediately: Bool = true, + atomicByZone: Bool = false ) async throws where repeat (each T1).PrimaryKey.QueryOutput: IdentifierStringConvertible, @@ -189,7 +191,8 @@ extension SyncEngine { delegate: delegate, tables: allTables, privateTables: allPrivateTables, - startImmediately: startImmediately + startImmediately: startImmediately, + atomicByZone: atomicByZone ) } convenience init( @@ -198,7 +201,8 @@ extension SyncEngine { delegate: (any SyncEngineDelegate)? = nil, tables: [any SynchronizableTable], privateTables: [any SynchronizableTable] = [], - startImmediately: Bool = true + startImmediately: Bool = true, + atomicByZone: Bool = false ) async throws { try self.init( container: container, @@ -221,7 +225,8 @@ extension SyncEngine { logger: Logger(.disabled), delegate: delegate, tables: tables, - privateTables: privateTables + privateTables: privateTables, + atomicByZone: atomicByZone ) try setUpSyncEngine() if startImmediately { diff --git a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift index fcfdc9f4..207fd4c8 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift @@ -65,7 +65,8 @@ extension SyncEngine { func modifyRecords( scope: CKDatabase.Scope, saving recordsToSave: [CKRecord] = [], - deleting recordIDsToDelete: [CKRecord.ID] = [] + deleting recordIDsToDelete: [CKRecord.ID] = [], + atomically: Bool = true ) throws -> ModifyRecordsCallback< ( saveResults: [CKRecord.ID: Result], @@ -83,7 +84,8 @@ extension SyncEngine { let (saveResults, deleteResults) = try syncEngine.database.modifyRecords( saving: recordsToSave, - deleting: recordIDsToDelete + deleting: recordIDsToDelete, + atomically: atomically ) return ModifyRecordsCallback { diff --git a/Tests/SQLiteDataTests/Internal/ResultExtensions.swift b/Tests/SQLiteDataTests/Internal/ResultExtensions.swift new file mode 100644 index 00000000..31c785eb --- /dev/null +++ b/Tests/SQLiteDataTests/Internal/ResultExtensions.swift @@ -0,0 +1,10 @@ +extension Result { + var error: Failure? { + do { + _ = try get() + return nil + } catch { + return error + } + } +} diff --git a/Tests/SQLiteDataTests/Internal/TestScopes.swift b/Tests/SQLiteDataTests/Internal/TestScopes.swift index 7c1c34e6..2c884b95 100644 --- a/Tests/SQLiteDataTests/Internal/TestScopes.swift +++ b/Tests/SQLiteDataTests/Internal/TestScopes.swift @@ -52,6 +52,29 @@ } } + struct _AtomicByZoneTrait: SuiteTrait, TestScoping, TestTrait { + @TaskLocal static var atomicByZone = false + let atomicByZone: Bool + init(atomicByZone: Bool = false) { + self.atomicByZone = atomicByZone + } + func provideScope( + for test: Test, + testCase: Test.Case?, + performing function: () async throws -> Void + ) async throws { + try await Self.$atomicByZone.withValue(atomicByZone) { + try await function() + } + } + } + + extension Trait where Self == _AtomicByZoneTrait { + static func atomicByZone(_ atomicByZone: Bool) -> Self { + Self(atomicByZone: atomicByZone) + } + } + struct _AttachMetadatabaseTrait: SuiteTrait, TestScoping, TestTrait { @TaskLocal static var attachMetadatabase = false let attachMetadatabase: Bool From b13a93855fae545db0bc2c9c144fc412ee4f6be7 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Fri, 12 Dec 2025 11:01:54 -0600 Subject: [PATCH 03/10] clean up --- .../CloudKit/Internal/MockCloudDatabase.swift | 2 -- .../CloudKit/Internal/MockSyncEngine.swift | 1 - Sources/SQLiteData/CloudKit/SyncEngine.swift | 25 +++---------------- 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index e096677d..67a5a4ee 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -295,11 +295,9 @@ || deleteSuccessRecordIDs.count != deleteResultsInZone.count { for saveSuccessRecordID in saveSuccessRecordIDs { - print("!!!") saveResults[saveSuccessRecordID] = .failure(CKError(.batchRequestFailed)) } for deleteSuccessRecordID in deleteSuccessRecordIDs { - print("!!!") deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed)) } diff --git a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift index 80213ca2..16cb7966 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift @@ -260,7 +260,6 @@ fatalError("Mocks should only raise 'CKError' values.") } } - print("!!!") syncEngine.state.remove( pendingRecordZoneChanges: savedRecords.map { .saveRecord($0.recordID) } ) diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 2495d0aa..e8a36892 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -621,7 +621,6 @@ false } } - print("!!!") syncEngines.withValue { $0.private?.state.add(pendingRecordZoneChanges: changesByIsPrivate[true] ?? []) $0.shared?.state.add(pendingRecordZoneChanges: changesByIsPrivate[false] ?? []) @@ -1582,10 +1581,10 @@ var newPendingRecordZoneChanges: [CKSyncEngine.PendingRecordZoneChange] = [] var newPendingDatabaseChanges: [CKSyncEngine.PendingDatabaseChange] = [] -// defer { -// syncEngine.state.add(pendingDatabaseChanges: newPendingDatabaseChanges) -// syncEngine.state.add(pendingRecordZoneChanges: newPendingRecordZoneChanges) -// } + defer { + syncEngine.state.add(pendingDatabaseChanges: newPendingDatabaseChanges) + syncEngine.state.add(pendingRecordZoneChanges: newPendingRecordZoneChanges) + } for (failedRecord, error) in failedRecordSaves { func clearServerRecord() async { await withErrorReporting(.sqliteDataCloudKitFailure) { @@ -1702,17 +1701,6 @@ } case .batchRequestFailed: -// print("?!?!?!") -// await withErrorReporting { -// try await metadatabase.write { db in -// try PendingRecordZoneChange.insert { -// PendingRecordZoneChange.init(.saveRecord(failedRecord.recordID)) -// } -// .execute(db) -// } -// } - //[.failed(.serverRecordChanged), .failed(.batchRequestFailed)] - //syncEngine.state.add(pendingRecordZoneChanges: [.saveRecord(failedRecord.recordID)]) newPendingRecordZoneChanges.append(.saveRecord(failedRecord.recordID)) break @@ -1760,11 +1748,6 @@ await withErrorReporting { try await enqueueLocallyPendingChanges() } - -// defer { - syncEngine.state.add(pendingDatabaseChanges: newPendingDatabaseChanges) - syncEngine.state.add(pendingRecordZoneChanges: newPendingRecordZoneChanges) -// } } private func cacheShare(_ share: CKShare) async throws { From 177f45fe443da53fe21dbd7183e3bf321f1fb1ce Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Fri, 12 Dec 2025 11:34:12 -0600 Subject: [PATCH 04/10] wip --- .../CloudKitDemo/CountersListFeature.swift | 49 ------------------- Examples/CloudKitDemo/Schema.swift | 6 +-- Sources/SQLiteData/CloudKit/SyncEngine.swift | 44 ++++++++++++----- .../CloudKitTests/AtomicTests.swift | 36 +++++++++++++- 4 files changed, 69 insertions(+), 66 deletions(-) diff --git a/Examples/CloudKitDemo/CountersListFeature.swift b/Examples/CloudKitDemo/CountersListFeature.swift index 7a76ed04..98b904e2 100644 --- a/Examples/CloudKitDemo/CountersListFeature.swift +++ b/Examples/CloudKitDemo/CountersListFeature.swift @@ -1,13 +1,11 @@ import CloudKit import SQLiteData -import SwiftData import SwiftUI import SwiftUINavigation struct CountersListView: View { @FetchAll var counters: [Counter] @Dependency(\.defaultDatabase) var database - @Dependency(\.defaultSyncEngine) var syncEngine var body: some View { List { @@ -35,55 +33,8 @@ struct CountersListView: View { } } } - ToolbarItem { - Button("Trigger") { - Task { await trigger() } - } - } - } - } - -func trigger() async { - await withErrorReporting { - // Step 1: Create a new counter - let newCounter = try await database.write { db in - try Counter.insert { Counter.Draft() } - .returning(\.self) - .fetchOne(db)! - } - - // Step 2: Force sending all data to iCloud - try await syncEngine.sendChanges() - - // Step 3: Grab the CKRecord for the newly inserted counter - let newCounterLastKnownServerRecord = try await database.read { db in - try SyncMetadata.find(newCounter.syncMetadataID).select(\.lastKnownServerRecord) - .fetchOne(db)!! - } - - // Step 4: Make a change to the counter directly on iCloud - let container = CKContainer(identifier: ModelConfiguration(groupContainer: .automatic).cloudKitContainerIdentifier!) - let serverCounter = try await container.privateCloudDatabase.record(for: newCounterLastKnownServerRecord.recordID) - serverCounter.encryptedValues["count"] = Int.random(in: 1...1_000) - let (saveResults, _) = try await container.privateCloudDatabase.modifyRecords(saving: [serverCounter], deleting: []) - - // Step 5: Make two changes to the local database: 1) decrement any counter besides the one - // created above (should succeed), and 2) increment the counter just created (should - // fail due to conflict) - try await database.write { db in - try Counter - .where { $0.id.neq(newCounter.id) } - .update { $0.count -= 1 } - .execute(db) - try Counter - .find(newCounter.id) - .update { $0.count += 1 } - .execute(db) } - - try await syncEngine.sendChanges() } -} func deleteRows(at indexSet: IndexSet) { withErrorReporting { diff --git a/Examples/CloudKitDemo/Schema.swift b/Examples/CloudKitDemo/Schema.swift index e297d909..4721e583 100644 --- a/Examples/CloudKitDemo/Schema.swift +++ b/Examples/CloudKitDemo/Schema.swift @@ -11,11 +11,7 @@ nonisolated struct Counter: Identifiable { extension DependencyValues { mutating func bootstrapDatabase() throws { @Dependency(\.context) var context - var configuration = Configuration() - configuration.prepareDatabase { - try $0.attachMetadatabase() - } - let database = try SQLiteData.defaultDatabase(configuration: configuration) + let database = try SQLiteData.defaultDatabase() logger.debug( """ App database diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index e8a36892..487b4406 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -915,8 +915,7 @@ parentForeignKey: parentForeignKey, defaultZone: defaultZone, privateTables: privateTables - ) - { + ) { try trigger.execute(db) } } @@ -932,7 +931,7 @@ defaultZone: defaultZone, privateTables: privateTables ) - .reversed() { + .reversed() { try trigger.drop().execute(db) } } @@ -1722,20 +1721,43 @@ } } + for (failedRecordID, error) in failedRecordDeletes { + } + + // + // + let enqueuedUnsyncedRecordID = await withErrorReporting(.sqliteDataCloudKitFailure) { try await userDatabase.write { db in var enqueuedUnsyncedRecordID = false for (failedRecordID, error) in failedRecordDeletes { - guard - error.code == .referenceViolation - else { continue } - try UnsyncedRecordID.insert(or: .ignore) { - UnsyncedRecordID(recordID: failedRecordID) + switch error.code { + case .referenceViolation: + enqueuedUnsyncedRecordID = true + try UnsyncedRecordID.insert(or: .ignore) { + UnsyncedRecordID(recordID: failedRecordID) + } + .execute(db) + syncEngine.state.remove(pendingRecordZoneChanges: [.deleteRecord(failedRecordID)]) + break + case .batchRequestFailed: + syncEngine.state.add(pendingRecordZoneChanges: [.deleteRecord(failedRecordID)]) + break + case .networkFailure, .networkUnavailable, .zoneBusy, .serviceUnavailable, + .notAuthenticated, .operationCancelled, .internalError, .partialFailure, + .badContainer, .requestRateLimited, .missingEntitlement, .invalidArguments, + .resultsTruncated, .assetFileNotFound, .assetFileModified, .incompatibleVersion, + .constraintViolation, .changeTokenExpired, .badDatabase, .quotaExceeded, + .limitExceeded, .userDeletedZone, .tooManyParticipants, .alreadyShared, + .managedAccountRestricted, .participantMayNeedVerification, .serverResponseLost, + .assetNotAvailable, .accountTemporarilyUnavailable, .permissionFailure, + .unknownItem, .serverRecordChanged, .serverRejectedRequest, .zoneNotFound, + .participantAlreadyInvited: + break + @unknown default: + break } - .execute(db) - syncEngine.state.remove(pendingRecordZoneChanges: [.deleteRecord(failedRecordID)]) - enqueuedUnsyncedRecordID = true } return enqueuedUnsyncedRecordID } diff --git a/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift b/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift index 188875a9..8250f590 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift @@ -11,9 +11,10 @@ extension BaseCloudKitTests { @MainActor + @Suite(.atomicByZone(true)) final class AtomicTests: BaseCloudKitTests, @unchecked Sendable { @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test(.atomicByZone(true)) func basics() async throws { + @Test func editConflictAndNewRecord() async throws { try await userDatabase.userWrite { db in try db.seed { RemindersList(id: 1, title: "Personal") @@ -96,6 +97,39 @@ """ } } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func deleteConflictAndNewRecord() async throws { + try await userDatabase.userWrite { db in + try db.seed { + RemindersList(id: 1, title: "Personal") + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let remindersListRecord = try syncEngine.private.database.record( + for: RemindersList.recordID(for: 1) + ) + remindersListRecord.setValue("My stuff", forKey: "title", at: 1) + let (saveResults, _) = try syncEngine.private.database.modifyRecords(saving: [remindersListRecord]) + #expect(saveResults.values.allSatisfy { $0.error == nil }) + + try await withDependencies { + $0.currentTime.now = 2 + } operation: { + try await userDatabase.userWrite { db in + try RemindersList.find(1).delete().execute(db) + try RemindersList.insert { RemindersList(id: 2, title: "Business") }.execute(db) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + assertInlineSnapshot(of: container, as: .customDump) + + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + assertInlineSnapshot(of: container, as: .customDump) + } } } #endif From 59966cc9865ac2faf68e72b50853a3dbeed15769 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 14 Dec 2025 16:54:33 -0600 Subject: [PATCH 05/10] wip --- .../CloudKit/Internal/MockSyncEngine.swift | 4 +- Sources/SQLiteData/CloudKit/SyncEngine.swift | 8 +- .../CloudKitTests/AtomicTests.swift | 89 ++++++++++++++++--- .../Internal/BaseCloudKitTests.swift | 15 ++-- .../SQLiteDataTests/Internal/TestScopes.swift | 23 ----- 5 files changed, 88 insertions(+), 51 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift index 16cb7966..680b299d 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift @@ -177,6 +177,7 @@ package func processPendingRecordZoneChanges( options: CKSyncEngine.SendChangesOptions = CKSyncEngine.SendChangesOptions(), scope: CKDatabase.Scope, + forceAtomicByZone: Bool = false, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, @@ -208,7 +209,7 @@ return } - let batch = await nextRecordZoneChangeBatch( + var batch = await nextRecordZoneChangeBatch( reason: .scheduled, options: options, syncEngine: { @@ -224,6 +225,7 @@ } }() ) + batch?.atomicByZone = forceAtomicByZone guard let batch else { return } diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 487b4406..dac9f6d7 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -38,7 +38,6 @@ private let notificationsObserver = LockIsolated<(any NSObjectProtocol)?>(nil) private let activityCounts = LockIsolated(ActivityCounts()) private let startTask = LockIsolated?>(nil) - private let atomicByZone: Bool /// The error message used when a write occurs to a record for which the current user does not /// have permission. @@ -216,12 +215,10 @@ logger: Logger, delegate: (any SyncEngineDelegate)?, tables: [any SynchronizableTable], - privateTables: [any SynchronizableTable] = [], - atomicByZone: Bool = false + privateTables: [any SynchronizableTable] = [] ) throws { let allTables = Set((tables + privateTables).map(HashableSynchronizedTable.init)) .map(\.type) - self.atomicByZone = atomicByZone self.tables = allTables self.privateTables = privateTables self.delegate = delegate @@ -1081,7 +1078,7 @@ } #endif - var batch = await syncEngine.recordZoneChangeBatch(pendingChanges: changes) { recordID in + let batch = await syncEngine.recordZoneChangeBatch(pendingChanges: changes) { recordID in guard let (metadata, allFields) = await withErrorReporting( .sqliteDataCloudKitFailure, @@ -1180,7 +1177,6 @@ } return await open(table) } - batch?.atomicByZone = atomicByZone return batch } diff --git a/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift b/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift index 8250f590..b1e54c4a 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift @@ -11,7 +11,6 @@ extension BaseCloudKitTests { @MainActor - @Suite(.atomicByZone(true)) final class AtomicTests: BaseCloudKitTests, @unchecked Sendable { @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Test func editConflictAndNewRecord() async throws { @@ -37,7 +36,10 @@ try RemindersList.insert { RemindersList(id: 2, title: "Business") }.execute(db) } } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) + try await syncEngine.processPendingRecordZoneChanges( + scope: .private, + forceAtomicByZone: true + ) assertInlineSnapshot(of: container, as: .customDump) { """ @@ -63,7 +65,10 @@ """ } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) + try await syncEngine.processPendingRecordZoneChanges( + scope: .private, + forceAtomicByZone: true + ) assertInlineSnapshot(of: container, as: .customDump) { """ @@ -99,13 +104,17 @@ } @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test func deleteConflictAndNewRecord() async throws { + @Test func editConflictAndDeleteRecord() async throws { try await userDatabase.userWrite { db in try db.seed { RemindersList(id: 1, title: "Personal") + RemindersList(id: 2, title: "Business") } } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) + try await syncEngine.processPendingRecordZoneChanges( + scope: .private, + forceAtomicByZone: true + ) let remindersListRecord = try syncEngine.private.database.record( for: RemindersList.recordID(for: 1) @@ -118,17 +127,75 @@ $0.currentTime.now = 2 } operation: { try await userDatabase.userWrite { db in - try RemindersList.find(1).delete().execute(db) - try RemindersList.insert { RemindersList(id: 2, title: "Business") }.execute(db) + try RemindersList.find(1).update { $0.title = "Stuff" }.execute(db) + try RemindersList.find(2).delete().execute(db) } } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) + try await syncEngine.processPendingRecordZoneChanges( + scope: .private, + forceAtomicByZone: true + ) - assertInlineSnapshot(of: container, as: .customDump) + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + title: "My stuff" + ), + [1]: CKRecord( + recordID: CKRecord.ID(2:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 2, + title: "Business" + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) + try await syncEngine.processPendingRecordZoneChanges( + scope: .private, + forceAtomicByZone: true + ) - assertInlineSnapshot(of: container, as: .customDump) + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + title: "Stuff" + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } } } } diff --git a/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift b/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift index cfbc5f3e..4aec1bc8 100644 --- a/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift +++ b/Tests/SQLiteDataTests/Internal/BaseCloudKitTests.swift @@ -73,8 +73,7 @@ class BaseCloudKitTests: @unchecked Sendable { ModelB.self, ModelC.self, privateTables: RemindersListPrivate.self, - startImmediately: _StartImmediatelyTrait.startImmediately, - atomicByZone: _AtomicByZoneTrait.atomicByZone + startImmediately: _StartImmediatelyTrait.startImmediately ) if _StartImmediatelyTrait.startImmediately, _AccountStatusScope.accountStatus == .available @@ -168,8 +167,7 @@ extension SyncEngine { delegate: (any SyncEngineDelegate)? = nil, tables: repeat (each T1).Type, privateTables: repeat (each T2).Type, - startImmediately: Bool = true, - atomicByZone: Bool = false + startImmediately: Bool = true ) async throws where repeat (each T1).PrimaryKey.QueryOutput: IdentifierStringConvertible, @@ -191,8 +189,7 @@ extension SyncEngine { delegate: delegate, tables: allTables, privateTables: allPrivateTables, - startImmediately: startImmediately, - atomicByZone: atomicByZone + startImmediately: startImmediately ) } convenience init( @@ -201,8 +198,7 @@ extension SyncEngine { delegate: (any SyncEngineDelegate)? = nil, tables: [any SynchronizableTable], privateTables: [any SynchronizableTable] = [], - startImmediately: Bool = true, - atomicByZone: Bool = false + startImmediately: Bool = true ) async throws { try self.init( container: container, @@ -225,8 +221,7 @@ extension SyncEngine { logger: Logger(.disabled), delegate: delegate, tables: tables, - privateTables: privateTables, - atomicByZone: atomicByZone + privateTables: privateTables ) try setUpSyncEngine() if startImmediately { diff --git a/Tests/SQLiteDataTests/Internal/TestScopes.swift b/Tests/SQLiteDataTests/Internal/TestScopes.swift index 2c884b95..7c1c34e6 100644 --- a/Tests/SQLiteDataTests/Internal/TestScopes.swift +++ b/Tests/SQLiteDataTests/Internal/TestScopes.swift @@ -52,29 +52,6 @@ } } - struct _AtomicByZoneTrait: SuiteTrait, TestScoping, TestTrait { - @TaskLocal static var atomicByZone = false - let atomicByZone: Bool - init(atomicByZone: Bool = false) { - self.atomicByZone = atomicByZone - } - func provideScope( - for test: Test, - testCase: Test.Case?, - performing function: () async throws -> Void - ) async throws { - try await Self.$atomicByZone.withValue(atomicByZone) { - try await function() - } - } - } - - extension Trait where Self == _AtomicByZoneTrait { - static func atomicByZone(_ atomicByZone: Bool) -> Self { - Self(atomicByZone: atomicByZone) - } - } - struct _AttachMetadatabaseTrait: SuiteTrait, TestScoping, TestTrait { @TaskLocal static var attachMetadatabase = false let attachMetadatabase: Bool From dfc677299fffd9e0cb0706d80359605983b6cd92 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 14 Dec 2025 17:01:11 -0600 Subject: [PATCH 06/10] clean up --- .../CloudKit/Internal/MockCloudDatabase.swift | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index 67a5a4ee..075e3a0a 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -276,39 +276,43 @@ } } - // NB: atomic is per zone setting - // NB: check if zone has atomic capability - if atomically { - let zones = Set(recordsToSave.map(\.recordID.zoneID) + recordIDsToDelete.map(\.zoneID)) - for zoneID in zones { - let saveResultsInZone = saveResults.filter { recordID, _ in recordID.zoneID == zoneID } - let deleteResultsInZone = deleteResults.filter { recordID, _ in - recordID.zoneID == zoneID - } - let saveSuccessRecordIDs = saveResultsInZone.compactMap { recordID, result in - (try? result.get()) == nil ? nil : recordID - } - let deleteSuccessRecordIDs = deleteResultsInZone.compactMap { recordID, result in - (try? result.get()) == nil ? nil : recordID - } - if saveSuccessRecordIDs.count != saveResultsInZone.count - || deleteSuccessRecordIDs.count != deleteResultsInZone.count - { - for saveSuccessRecordID in saveSuccessRecordIDs { - saveResults[saveSuccessRecordID] = .failure(CKError(.batchRequestFailed)) - } - for deleteSuccessRecordID in deleteSuccessRecordIDs { - deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed)) - } + guard atomically + else { + return (saveResults: saveResults, deleteResults: deleteResults) + } - storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:] - } + let affectedZones = Set( + recordsToSave.map(\.recordID.zoneID) + recordIDsToDelete.map(\.zoneID) + ) + for zoneID in affectedZones { + let saveResultsInZone = saveResults.filter { recordID, _ in recordID.zoneID == zoneID } + let deleteResultsInZone = deleteResults.filter { recordID, _ in + recordID.zoneID == zoneID + } + let saveSuccessRecordIDs = saveResultsInZone.compactMap { recordID, result in + (try? result.get()) == nil ? nil : recordID } + let deleteSuccessRecordIDs = deleteResultsInZone.compactMap { recordID, result in + (try? result.get()) == nil ? nil : recordID + } + guard + saveSuccessRecordIDs.count != saveResultsInZone.count + || deleteSuccessRecordIDs.count != deleteResultsInZone.count + else { + continue + } + // Every successful save and deletion becomes a '.batchRequestFailed'. + for saveSuccessRecordID in saveSuccessRecordIDs { + saveResults[saveSuccessRecordID] = .failure(CKError(.batchRequestFailed)) + } + for deleteSuccessRecordID in deleteSuccessRecordIDs { + deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed)) + } + // All storage changes are reverted in zone. + storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:] } - return (saveResults: saveResults, deleteResults: deleteResults) } - return (saveResults: saveResults, deleteResults: deleteResults) } From db63845aab1ae34805fa8adce111ff63340624f5 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 14 Dec 2025 17:05:33 -0600 Subject: [PATCH 07/10] wip --- Sources/SQLiteData/CloudKit/SyncEngine.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index dac9f6d7..0012aa19 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -1717,12 +1717,6 @@ } } - for (failedRecordID, error) in failedRecordDeletes { - } - - // - // - let enqueuedUnsyncedRecordID = await withErrorReporting(.sqliteDataCloudKitFailure) { try await userDatabase.write { db in From c3d2b7fbefa2556738c9ff38f87afce522c24de2 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 15 Dec 2025 07:35:27 -0600 Subject: [PATCH 08/10] wip --- Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index 075e3a0a..c8138bf0 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -84,7 +84,7 @@ guard accountStatus == .available else { throw ckError(forAccountStatus: accountStatus) } - var (saveResults, deleteResults) = storage.withValue { storage in + return storage.withValue { storage in let previousStorage = storage var saveResults: [CKRecord.ID: Result] = [:] var deleteResults: [CKRecord.ID: Result] = [:] @@ -313,7 +313,6 @@ } return (saveResults: saveResults, deleteResults: deleteResults) } - return (saveResults: saveResults, deleteResults: deleteResults) } package func modifyRecordZones( From 9b4ef7c7b2c00c64ee62561ed035a08bd7f5d90d Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 15 Dec 2025 10:08:03 -0600 Subject: [PATCH 09/10] wip --- Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift index 680b299d..336cb6c1 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift @@ -177,7 +177,7 @@ package func processPendingRecordZoneChanges( options: CKSyncEngine.SendChangesOptions = CKSyncEngine.SendChangesOptions(), scope: CKDatabase.Scope, - forceAtomicByZone: Bool = false, + forceAtomicByZone: Bool? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, @@ -225,7 +225,9 @@ } }() ) - batch?.atomicByZone = forceAtomicByZone + if let forceAtomicByZone { + batch?.atomicByZone = forceAtomicByZone + } guard let batch else { return } From 6f005fb2ba67ab6f85c2792a77279c338a3465af Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 15 Dec 2025 10:09:19 -0600 Subject: [PATCH 10/10] clean up --- Sources/SQLiteData/CloudKit/SyncEngine.swift | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 0012aa19..f4b97a0b 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -1756,10 +1756,6 @@ if enqueuedUnsyncedRecordID { await handleFetchedRecordZoneChanges(syncEngine: syncEngine) } - - await withErrorReporting { - try await enqueueLocallyPendingChanges() - } } private func cacheShare(_ share: CKShare) async throws {