diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index c8138bf0..21c3c8a0 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -64,6 +64,9 @@ guard accountStatus == .available else { throw ckError(forAccountStatus: accountStatus) } + guard ids.count < 200 + else { throw CKError(.limitExceeded) } + var results: [CKRecord.ID: Result] = [:] for id in ids { results[id] = Result { try record(for: id) } @@ -84,6 +87,11 @@ guard accountStatus == .available else { throw ckError(forAccountStatus: accountStatus) } + guard (recordsToSave.count + recordIDsToDelete.count) < 200 + else { + throw CKError(.limitExceeded) + } + return storage.withValue { storage in let previousStorage = storage var saveResults: [CKRecord.ID: Result] = [:] diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index c9488697..5d094197 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -24,7 +24,7 @@ package let tables: [any SynchronizableTable] package let privateTables: [any SynchronizableTable] let tablesByName: [String: any SynchronizableTable] - private let tablesByOrder: [String: Int] + package let tablesByOrder: [String: Int] let foreignKeysByTableName: [String: [ForeignKey]] package let syncEngines = LockIsolated(SyncEngines()) package let defaultZone: CKRecordZone @@ -217,7 +217,7 @@ tables: [any SynchronizableTable], privateTables: [any SynchronizableTable] = [] ) throws { - let allTables = Set((tables + privateTables).map(HashableSynchronizedTable.init)) + let allTables = OrderedSet((tables + privateTables).map(HashableSynchronizedTable.init)) .map(\.type) self.tables = allTables self.privateTables = privateTables @@ -1419,11 +1419,11 @@ ) async { let deletedRecordIDsByRecordType = OrderedDictionary( grouping: deletions.sorted { lhs, rhs in - guard - let lhsIndex = tablesByOrder[lhs.recordType], - let rhsIndex = tablesByOrder[rhs.recordType] - else { return true } - return lhsIndex > rhsIndex + topologicallyAscending( + lhsTableName: lhs.recordType, + rhsTableName: rhs.recordType, + rootFirst: false + ) }, by: \.recordType ) @@ -1490,18 +1490,31 @@ .execute(db) } } - let results = try await syncEngine.database.records(for: Array(unsyncedRecordIDs)) + let batchSize = 150 + let orderedUnsyncedRecordIDs = unsyncedRecordIDs.sorted { + topologicallyAscending( + lhsTableName: $0.tableName, + rhsTableName: $1.tableName, + rootFirst: true + ) + } var unsyncedRecords: [CKRecord] = [] - for (recordID, result) in results { - switch result { - case .success(let record): - unsyncedRecords.append(record) - case .failure(let error as CKError) where error.code == .unknownItem: - try await userDatabase.write { db in - try UnsyncedRecordID.find(recordID).delete().execute(db) + for start in stride(from: 0, to: orderedUnsyncedRecordIDs.count, by: batchSize) { + let recordIDsBatch = orderedUnsyncedRecordIDs + .dropFirst(start) + .prefix(batchSize) + let results = try await syncEngine.database.records(for: Array(recordIDsBatch)) + for (recordID, result) in results { + switch result { + case .success(let record): + unsyncedRecords.append(record) + case .failure(let error as CKError) where error.code == .unknownItem: + try await userDatabase.write { db in + try UnsyncedRecordID.find(recordID).delete().execute(db) + } + case .failure: + continue } - case .failure: - continue } } return unsyncedRecords @@ -1509,13 +1522,11 @@ ?? [CKRecord]() let modifications = (modifications + unsyncedRecords).sorted { lhs, rhs in - guard - let lhsRecordType = lhs.recordID.tableName, - let lhsIndex = tablesByOrder[lhsRecordType], - let rhsRecordType = rhs.recordID.tableName, - let rhsIndex = tablesByOrder[rhsRecordType] - else { return true } - return lhsIndex < rhsIndex + topologicallyAscending( + lhsTableName: lhs.recordType, + rhsTableName: rhs.recordType, + rootFirst: true + ) } enum ShareOrReference { @@ -1563,6 +1574,27 @@ } } + private func topologicallyAscending( + lhsTableName: String?, + rhsTableName: String?, + rootFirst: Bool + ) -> Bool { + switch (lhsTableName, rhsTableName) { + case (nil, nil), (nil, _): + return false + case (_, nil): + return true + case let (.some(lhs), .some(rhs)): + let lhsIndex = tablesByOrder[lhs] ?? (rootFirst ? .max : .min) + let rhsIndex = tablesByOrder[rhs] ?? (rootFirst ? .max : .min) + guard lhsIndex != rhsIndex + else { + return lhs < rhs + } + return rootFirst ? lhsIndex < rhsIndex : lhsIndex > rhsIndex + } + } + package func handleSentRecordZoneChanges( savedRecords: [CKRecord] = [], failedRecordSaves: [(record: CKRecord, error: CKError)] = [], @@ -2292,10 +2324,12 @@ tablesByName: [String: any SynchronizableTable] ) throws -> [String: Int] { let tableDependencies = try userDatabase.read { db in - var dependencies: [HashableSynchronizedTable: [any SynchronizableTable]] = [:] + var dependencies: OrderedDictionary = [:] for table in tables { func open(_: some SynchronizableTable) throws -> [String] { - try PragmaForeignKeyList.select(\.table) + try PragmaForeignKeyList + .order(by: \.table) + .select(\.table) .fetchAll(db) } let toTables = try open(table) diff --git a/Tests/SQLiteDataTests/CloudKitTests/FetchRecordZoneChangesTests.swift b/Tests/SQLiteDataTests/CloudKitTests/FetchRecordZoneChangesTests.swift index 505930e4..13e85616 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/FetchRecordZoneChangesTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/FetchRecordZoneChangesTests.swift @@ -739,6 +739,12 @@ } #expect(error?.message == SyncEngine.invalidRecordNameError) } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func syncInvalidRecordID() async throws { + let record = CKRecord(recordType: "foo", recordID: CKRecord.ID(recordName: "bar")) + try await syncEngine.modifyRecords(scope: .private, saving: [record]).notify() + } } } #endif diff --git a/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift b/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift index bf6d011c..5bd8f224 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift @@ -869,6 +869,70 @@ """ } } + + /* + * Create a parent record in CloudKit database but do not sync to client. + * Create many child records in CloudKit database and **do** sync to client. + * Sync parent record to client. + * => Cached unsaved child records should be batched so as to not run into 'limitExceeded' + errors + */ + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func batchAssociations() async throws { + + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + let remindersListModification = try syncEngine.modifyRecords( + scope: .private, + saving: [remindersListRecord] + ) + + let reminderRecords = (1...500).map { index in + let reminderRecord = CKRecord( + recordType: Reminder.tableName, + recordID: Reminder.recordID(for: index) + ) + reminderRecord.setValue(index, forKey: "id", at: now) + reminderRecord.setValue("Reminder #\(index)", forKey: "title", at: now) + reminderRecord.setValue(1, forKey: "remindersListID", at: now) + reminderRecord.parent = CKRecord.Reference( + record: remindersListRecord, + action: .none + ) + return reminderRecord + } + + try await syncEngine.modifyRecords( + scope: .private, + saving: Array(reminderRecords[0...100]) + ).notify() + try await syncEngine.modifyRecords( + scope: .private, + saving: Array(reminderRecords[101...200]) + ).notify() + try await syncEngine.modifyRecords( + scope: .private, + saving: Array(reminderRecords[201...300]) + ).notify() + try await syncEngine.modifyRecords( + scope: .private, + saving: Array(reminderRecords[301...400]) + ).notify() + try await syncEngine.modifyRecords( + scope: .private, + saving: Array(reminderRecords[401...499]) + ).notify() + await remindersListModification.notify() + + try await userDatabase.read { db in + try #expect(RemindersList.fetchCount(db) == 1) + try #expect(Reminder.fetchCount(db) == 500) + } + } } } #endif diff --git a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift index 9ba9ad8e..ecd50cc2 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift @@ -649,6 +649,74 @@ ) } } + + @Test func limitExceeded_modifyRecords() async throws { + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + + let reminderRecords = (1...400).map { index in + let reminderRecord = CKRecord( + recordType: Reminder.tableName, + recordID: Reminder.recordID(for: index) + ) + reminderRecord.setValue(index, forKey: "id", at: now) + reminderRecord.setValue("Reminder #\(index)", forKey: "title", at: now) + reminderRecord.setValue(1, forKey: "remindersListID", at: now) + reminderRecord.parent = CKRecord.Reference( + record: remindersListRecord, + action: .none + ) + return reminderRecord + } + + let error = #expect(throws: CKError.self) { + _ = try syncEngine.private.database.modifyRecords( + saving: reminderRecords + [remindersListRecord] + ) + } + #expect(error?.code == .limitExceeded) + } + + @Test func records_limitExceeded() async throws { + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + + let reminderRecords = (1...400).map { index in + let reminderRecord = CKRecord( + recordType: Reminder.tableName, + recordID: Reminder.recordID(for: index) + ) + reminderRecord.setValue(index, forKey: "id", at: now) + reminderRecord.setValue("Reminder #\(index)", forKey: "title", at: now) + reminderRecord.setValue(1, forKey: "remindersListID", at: now) + reminderRecord.parent = CKRecord.Reference( + record: remindersListRecord, + action: .none + ) + return reminderRecord + } + + _ = try syncEngine.private.database.modifyRecords(saving: [remindersListRecord]) + _ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[0...100])) + _ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[101...200])) + _ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[201...300])) + _ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[301...399])) + + let error = await #expect(throws: CKError.self) { + _ = try await syncEngine.private.database.records( + for: [remindersListRecord.recordID] + reminderRecords.map(\.recordID) + ) + } + #expect(error?.code == .limitExceeded) + } } } #endif diff --git a/Tests/SQLiteDataTests/CloudKitTests/TopologicalTableSortingTests.swift b/Tests/SQLiteDataTests/CloudKitTests/TopologicalTableSortingTests.swift new file mode 100644 index 00000000..a829086f --- /dev/null +++ b/Tests/SQLiteDataTests/CloudKitTests/TopologicalTableSortingTests.swift @@ -0,0 +1,30 @@ +#if canImport(CloudKit) + import CloudKit + import SQLiteData + import Testing + + extension BaseCloudKitTests { + @MainActor + final class TopologicalTableSortingTests: BaseCloudKitTests, @unchecked Sendable { + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func tablesByOrder() async throws { + #expect( + syncEngine.tablesByOrder == [ + "remindersLists": 0, + "reminders": 1, + "remindersListAssets": 2, + "tags": 3, + "reminderTags": 4, + "parents": 5, + "childWithOnDeleteSetNulls": 6, + "childWithOnDeleteSetDefaults": 7, + "modelAs": 8, + "modelBs": 9, + "modelCs": 10, + "remindersListPrivates": 11, + ] + ) + } + } + } +#endif