Skip to content

Commit 8af16d5

Browse files
mbrandonwmbrandonw
authored
Fix 'limitExceeded' errors related to FK constraint failures. (#359)
* Fix 'limitExceeded' errors related to FK constraint failures. * revert * test explanation * emulate error for modifyRecords too * fix test * rename * added a test for tablesByOrder * clean up * add test
1 parent df360b2 commit 8af16d5

6 files changed

Lines changed: 236 additions & 26 deletions

File tree

‎Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@
6464
guard accountStatus == .available
6565
else { throw ckError(forAccountStatus: accountStatus) }
6666

67+
guard ids.count < 200
68+
else { throw CKError(.limitExceeded) }
69+
6770
var results: [CKRecord.ID: Result<CKRecord, any Error>] = [:]
6871
for id in ids {
6972
results[id] = Result { try record(for: id) }
@@ -84,6 +87,11 @@
8487
guard accountStatus == .available
8588
else { throw ckError(forAccountStatus: accountStatus) }
8689

90+
guard (recordsToSave.count + recordIDsToDelete.count) < 200
91+
else {
92+
throw CKError(.limitExceeded)
93+
}
94+
8795
return storage.withValue { storage in
8896
let previousStorage = storage
8997
var saveResults: [CKRecord.ID: Result<CKRecord, any Error>] = [:]

‎Sources/SQLiteData/CloudKit/SyncEngine.swift‎

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
package let tables: [any SynchronizableTable]
2525
package let privateTables: [any SynchronizableTable]
2626
let tablesByName: [String: any SynchronizableTable]
27-
private let tablesByOrder: [String: Int]
27+
package let tablesByOrder: [String: Int]
2828
let foreignKeysByTableName: [String: [ForeignKey]]
2929
package let syncEngines = LockIsolated<SyncEngines>(SyncEngines())
3030
package let defaultZone: CKRecordZone
@@ -217,7 +217,7 @@
217217
tables: [any SynchronizableTable],
218218
privateTables: [any SynchronizableTable] = []
219219
) throws {
220-
let allTables = Set((tables + privateTables).map(HashableSynchronizedTable.init))
220+
let allTables = OrderedSet((tables + privateTables).map(HashableSynchronizedTable.init))
221221
.map(\.type)
222222
self.tables = allTables
223223
self.privateTables = privateTables
@@ -1419,11 +1419,11 @@
14191419
) async {
14201420
let deletedRecordIDsByRecordType = OrderedDictionary(
14211421
grouping: deletions.sorted { lhs, rhs in
1422-
guard
1423-
let lhsIndex = tablesByOrder[lhs.recordType],
1424-
let rhsIndex = tablesByOrder[rhs.recordType]
1425-
else { return true }
1426-
return lhsIndex > rhsIndex
1422+
topologicallyAscending(
1423+
lhsTableName: lhs.recordType,
1424+
rhsTableName: rhs.recordType,
1425+
rootFirst: false
1426+
)
14271427
},
14281428
by: \.recordType
14291429
)
@@ -1490,32 +1490,43 @@
14901490
.execute(db)
14911491
}
14921492
}
1493-
let results = try await syncEngine.database.records(for: Array(unsyncedRecordIDs))
1493+
let batchSize = 150
1494+
let orderedUnsyncedRecordIDs = unsyncedRecordIDs.sorted {
1495+
topologicallyAscending(
1496+
lhsTableName: $0.tableName,
1497+
rhsTableName: $1.tableName,
1498+
rootFirst: true
1499+
)
1500+
}
14941501
var unsyncedRecords: [CKRecord] = []
1495-
for (recordID, result) in results {
1496-
switch result {
1497-
case .success(let record):
1498-
unsyncedRecords.append(record)
1499-
case .failure(let error as CKError) where error.code == .unknownItem:
1500-
try await userDatabase.write { db in
1501-
try UnsyncedRecordID.find(recordID).delete().execute(db)
1502+
for start in stride(from: 0, to: orderedUnsyncedRecordIDs.count, by: batchSize) {
1503+
let recordIDsBatch = orderedUnsyncedRecordIDs
1504+
.dropFirst(start)
1505+
.prefix(batchSize)
1506+
let results = try await syncEngine.database.records(for: Array(recordIDsBatch))
1507+
for (recordID, result) in results {
1508+
switch result {
1509+
case .success(let record):
1510+
unsyncedRecords.append(record)
1511+
case .failure(let error as CKError) where error.code == .unknownItem:
1512+
try await userDatabase.write { db in
1513+
try UnsyncedRecordID.find(recordID).delete().execute(db)
1514+
}
1515+
case .failure:
1516+
continue
15021517
}
1503-
case .failure:
1504-
continue
15051518
}
15061519
}
15071520
return unsyncedRecords
15081521
}
15091522
?? [CKRecord]()
15101523

15111524
let modifications = (modifications + unsyncedRecords).sorted { lhs, rhs in
1512-
guard
1513-
let lhsRecordType = lhs.recordID.tableName,
1514-
let lhsIndex = tablesByOrder[lhsRecordType],
1515-
let rhsRecordType = rhs.recordID.tableName,
1516-
let rhsIndex = tablesByOrder[rhsRecordType]
1517-
else { return true }
1518-
return lhsIndex < rhsIndex
1525+
topologicallyAscending(
1526+
lhsTableName: lhs.recordType,
1527+
rhsTableName: rhs.recordType,
1528+
rootFirst: true
1529+
)
15191530
}
15201531

15211532
enum ShareOrReference {
@@ -1563,6 +1574,27 @@
15631574
}
15641575
}
15651576

1577+
private func topologicallyAscending(
1578+
lhsTableName: String?,
1579+
rhsTableName: String?,
1580+
rootFirst: Bool
1581+
) -> Bool {
1582+
switch (lhsTableName, rhsTableName) {
1583+
case (nil, nil), (nil, _):
1584+
return false
1585+
case (_, nil):
1586+
return true
1587+
case let (.some(lhs), .some(rhs)):
1588+
let lhsIndex = tablesByOrder[lhs] ?? (rootFirst ? .max : .min)
1589+
let rhsIndex = tablesByOrder[rhs] ?? (rootFirst ? .max : .min)
1590+
guard lhsIndex != rhsIndex
1591+
else {
1592+
return lhs < rhs
1593+
}
1594+
return rootFirst ? lhsIndex < rhsIndex : lhsIndex > rhsIndex
1595+
}
1596+
}
1597+
15661598
package func handleSentRecordZoneChanges(
15671599
savedRecords: [CKRecord] = [],
15681600
failedRecordSaves: [(record: CKRecord, error: CKError)] = [],
@@ -2292,10 +2324,12 @@
22922324
tablesByName: [String: any SynchronizableTable]
22932325
) throws -> [String: Int] {
22942326
let tableDependencies = try userDatabase.read { db in
2295-
var dependencies: [HashableSynchronizedTable: [any SynchronizableTable]] = [:]
2327+
var dependencies: OrderedDictionary<HashableSynchronizedTable, [any SynchronizableTable]> = [:]
22962328
for table in tables {
22972329
func open<T>(_: some SynchronizableTable<T>) throws -> [String] {
2298-
try PragmaForeignKeyList<T>.select(\.table)
2330+
try PragmaForeignKeyList<T>
2331+
.order(by: \.table)
2332+
.select(\.table)
22992333
.fetchAll(db)
23002334
}
23012335
let toTables = try open(table)

‎Tests/SQLiteDataTests/CloudKitTests/FetchRecordZoneChangesTests.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,12 @@
739739
}
740740
#expect(error?.message == SyncEngine.invalidRecordNameError)
741741
}
742+
743+
@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
744+
@Test func syncInvalidRecordID() async throws {
745+
let record = CKRecord(recordType: "foo", recordID: CKRecord.ID(recordName: "bar"))
746+
try await syncEngine.modifyRecords(scope: .private, saving: [record]).notify()
747+
}
742748
}
743749
}
744750
#endif

‎Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,6 +869,70 @@
869869
"""
870870
}
871871
}
872+
873+
/*
874+
* Create a parent record in CloudKit database but do not sync to client.
875+
* Create many child records in CloudKit database and **do** sync to client.
876+
* Sync parent record to client.
877+
* => Cached unsaved child records should be batched so as to not run into 'limitExceeded'
878+
errors
879+
*/
880+
@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
881+
@Test func batchAssociations() async throws {
882+
883+
let remindersListRecord = CKRecord(
884+
recordType: RemindersList.tableName,
885+
recordID: RemindersList.recordID(for: 1)
886+
)
887+
remindersListRecord.setValue(1, forKey: "id", at: now)
888+
remindersListRecord.setValue("Personal", forKey: "title", at: now)
889+
let remindersListModification = try syncEngine.modifyRecords(
890+
scope: .private,
891+
saving: [remindersListRecord]
892+
)
893+
894+
let reminderRecords = (1...500).map { index in
895+
let reminderRecord = CKRecord(
896+
recordType: Reminder.tableName,
897+
recordID: Reminder.recordID(for: index)
898+
)
899+
reminderRecord.setValue(index, forKey: "id", at: now)
900+
reminderRecord.setValue("Reminder #\(index)", forKey: "title", at: now)
901+
reminderRecord.setValue(1, forKey: "remindersListID", at: now)
902+
reminderRecord.parent = CKRecord.Reference(
903+
record: remindersListRecord,
904+
action: .none
905+
)
906+
return reminderRecord
907+
}
908+
909+
try await syncEngine.modifyRecords(
910+
scope: .private,
911+
saving: Array(reminderRecords[0...100])
912+
).notify()
913+
try await syncEngine.modifyRecords(
914+
scope: .private,
915+
saving: Array(reminderRecords[101...200])
916+
).notify()
917+
try await syncEngine.modifyRecords(
918+
scope: .private,
919+
saving: Array(reminderRecords[201...300])
920+
).notify()
921+
try await syncEngine.modifyRecords(
922+
scope: .private,
923+
saving: Array(reminderRecords[301...400])
924+
).notify()
925+
try await syncEngine.modifyRecords(
926+
scope: .private,
927+
saving: Array(reminderRecords[401...499])
928+
).notify()
929+
await remindersListModification.notify()
930+
931+
try await userDatabase.read { db in
932+
try #expect(RemindersList.fetchCount(db) == 1)
933+
try #expect(Reminder.fetchCount(db) == 500)
934+
}
935+
}
872936
}
873937
}
874938
#endif

‎Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,74 @@
649649
)
650650
}
651651
}
652+
653+
@Test func limitExceeded_modifyRecords() async throws {
654+
let remindersListRecord = CKRecord(
655+
recordType: RemindersList.tableName,
656+
recordID: RemindersList.recordID(for: 1)
657+
)
658+
remindersListRecord.setValue(1, forKey: "id", at: now)
659+
remindersListRecord.setValue("Personal", forKey: "title", at: now)
660+
661+
let reminderRecords = (1...400).map { index in
662+
let reminderRecord = CKRecord(
663+
recordType: Reminder.tableName,
664+
recordID: Reminder.recordID(for: index)
665+
)
666+
reminderRecord.setValue(index, forKey: "id", at: now)
667+
reminderRecord.setValue("Reminder #\(index)", forKey: "title", at: now)
668+
reminderRecord.setValue(1, forKey: "remindersListID", at: now)
669+
reminderRecord.parent = CKRecord.Reference(
670+
record: remindersListRecord,
671+
action: .none
672+
)
673+
return reminderRecord
674+
}
675+
676+
let error = #expect(throws: CKError.self) {
677+
_ = try syncEngine.private.database.modifyRecords(
678+
saving: reminderRecords + [remindersListRecord]
679+
)
680+
}
681+
#expect(error?.code == .limitExceeded)
682+
}
683+
684+
@Test func records_limitExceeded() async throws {
685+
let remindersListRecord = CKRecord(
686+
recordType: RemindersList.tableName,
687+
recordID: RemindersList.recordID(for: 1)
688+
)
689+
remindersListRecord.setValue(1, forKey: "id", at: now)
690+
remindersListRecord.setValue("Personal", forKey: "title", at: now)
691+
692+
let reminderRecords = (1...400).map { index in
693+
let reminderRecord = CKRecord(
694+
recordType: Reminder.tableName,
695+
recordID: Reminder.recordID(for: index)
696+
)
697+
reminderRecord.setValue(index, forKey: "id", at: now)
698+
reminderRecord.setValue("Reminder #\(index)", forKey: "title", at: now)
699+
reminderRecord.setValue(1, forKey: "remindersListID", at: now)
700+
reminderRecord.parent = CKRecord.Reference(
701+
record: remindersListRecord,
702+
action: .none
703+
)
704+
return reminderRecord
705+
}
706+
707+
_ = try syncEngine.private.database.modifyRecords(saving: [remindersListRecord])
708+
_ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[0...100]))
709+
_ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[101...200]))
710+
_ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[201...300]))
711+
_ = try syncEngine.private.database.modifyRecords(saving: Array(reminderRecords[301...399]))
712+
713+
let error = await #expect(throws: CKError.self) {
714+
_ = try await syncEngine.private.database.records(
715+
for: [remindersListRecord.recordID] + reminderRecords.map(\.recordID)
716+
)
717+
}
718+
#expect(error?.code == .limitExceeded)
719+
}
652720
}
653721
}
654722
#endif

‎Tests/SQLiteDataTests/CloudKitTests/TopologicalTableSortingTests.swift‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#if canImport(CloudKit)
2+
import CloudKit
3+
import SQLiteData
4+
import Testing
5+
6+
extension BaseCloudKitTests {
7+
@MainActor
8+
final class TopologicalTableSortingTests: BaseCloudKitTests, @unchecked Sendable {
9+
@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
10+
@Test func tablesByOrder() async throws {
11+
#expect(
12+
syncEngine.tablesByOrder == [
13+
"remindersLists": 0,
14+
"reminders": 1,
15+
"remindersListAssets": 2,
16+
"tags": 3,
17+
"reminderTags": 4,
18+
"parents": 5,
19+
"childWithOnDeleteSetNulls": 6,
20+
"childWithOnDeleteSetDefaults": 7,
21+
"modelAs": 8,
22+
"modelBs": 9,
23+
"modelCs": 10,
24+
"remindersListPrivates": 11,
25+
]
26+
)
27+
}
28+
}
29+
}
30+
#endif

0 commit comments

Comments
 (0)