Skip to content

Commit 341468a

Browse files
mbrandonwmbrandonw
authored
Handle 'batchRequestFailed' errors (#328)
* Update mock database to respect atomic modifications and zones. * wip * clean up * wip * wip * clean up * wip * wip * wip * clean up
1 parent f4047e2 commit 341468a

10 files changed

Lines changed: 367 additions & 37 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
: sharedCloudDatabase
4949

5050
let rootRecord: CKRecord? = database.storage.withValue {
51-
$0[share.recordID.zoneID]?.values.first { record in
51+
$0[share.recordID.zoneID]?.records.values.first { record in
5252
record.share?.recordID == share.recordID
5353
}
5454
}

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

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,22 @@
44

55
@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
66
package final class MockCloudDatabase: CloudDatabase {
7-
package let storage = LockIsolated<[CKRecordZone.ID: [CKRecord.ID: CKRecord]]>([:])
7+
package let storage = LockIsolated<[CKRecordZone.ID: Zone]>([:])
88
let assets = LockIsolated<[AssetID: Data]>([:])
99
package let databaseScope: CKDatabase.Scope
1010
let _container = IsolatedWeakVar<MockCloudContainer>()
11-
1211
let dataManager = Dependency(\.dataManager)
1312

1413
struct AssetID: Hashable {
1514
let recordID: CKRecord.ID
1615
let key: String
1716
}
1817

18+
package struct Zone {
19+
package var zone: CKRecordZone
20+
package var records: [CKRecord.ID: CKRecord] = [:]
21+
}
22+
1923
package init(databaseScope: CKDatabase.Scope) {
2024
self.databaseScope = databaseScope
2125
}
@@ -34,7 +38,7 @@
3438
else { throw ckError(forAccountStatus: accountStatus) }
3539
guard let zone = storage[recordID.zoneID]
3640
else { throw CKError(.zoneNotFound) }
37-
guard let record = zone[recordID]
41+
guard let record = zone.records[recordID]
3842
else { throw CKError(.unknownItem) }
3943
guard let record = record.copy() as? CKRecord
4044
else { fatalError("Could not copy CKRecord.") }
@@ -81,6 +85,7 @@
8185
else { throw ckError(forAccountStatus: accountStatus) }
8286

8387
return storage.withValue { storage in
88+
let previousStorage = storage
8489
var saveResults: [CKRecord.ID: Result<CKRecord, any Error>] = [:]
8590
var deleteResults: [CKRecord.ID: Result<Void, any Error>] = [:]
8691

@@ -91,7 +96,8 @@
9196
let isSavingRootRecord = recordsToSave.contains(where: {
9297
$0.share?.recordID == share.recordID
9398
})
94-
let shareWasPreviouslySaved = storage[share.recordID.zoneID]?[share.recordID] != nil
99+
let shareWasPreviouslySaved =
100+
storage[share.recordID.zoneID]?.records[share.recordID] != nil
95101
guard shareWasPreviouslySaved || isSavingRootRecord
96102
else {
97103
saveResults[recordToSave.recordID] = .failure(CKError(.invalidArguments))
@@ -114,12 +120,14 @@
114120
continue
115121
}
116122

117-
let existingRecord = storage[recordToSave.recordID.zoneID]?[recordToSave.recordID]
123+
let existingRecord = storage[recordToSave.recordID.zoneID]?.records[
124+
recordToSave.recordID
125+
]
118126

119127
func saveRecordToDatabase() {
120128
let hasReferenceViolation =
121129
recordToSave.parent.map { parent in
122-
storage[parent.recordID.zoneID]?[parent.recordID] == nil
130+
storage[parent.recordID.zoneID]?.records[parent.recordID] == nil
123131
&& !recordsToSave.contains { $0.recordID == parent.recordID }
124132
}
125133
?? false
@@ -132,10 +140,12 @@
132140
func root(of record: CKRecord) -> CKRecord {
133141
guard let parent = record.parent
134142
else { return record }
135-
return (storage[parent.recordID.zoneID]?[parent.recordID]).map(root) ?? record
143+
return (storage[parent.recordID.zoneID]?.records[parent.recordID]).map(
144+
root
145+
) ?? record
136146
}
137147
func share(for rootRecord: CKRecord) -> CKShare? {
138-
for (_, record) in storage[rootRecord.recordID.zoneID] ?? [:] {
148+
for (_, record) in storage[rootRecord.recordID.zoneID]?.records ?? [:] {
139149
guard record.recordID == rootRecord.share?.recordID
140150
else { continue }
141151
return record as? CKShare
@@ -169,7 +179,7 @@
169179
}
170180

171181
// TODO: This should merge copy's values to more accurately reflect reality
172-
storage[recordToSave.recordID.zoneID]?[recordToSave.recordID] = copy
182+
storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] = copy
173183
saveResults[recordToSave.recordID] = .success(copy)
174184
}
175185

@@ -228,7 +238,7 @@
228238
continue
229239
}
230240
let hasReferenceViolation = !Set(
231-
storage[recordIDToDelete.zoneID]?.values
241+
storage[recordIDToDelete.zoneID]?.records.values
232242
.compactMap { $0.parent?.recordID == recordIDToDelete ? $0.recordID : nil }
233243
?? []
234244
)
@@ -240,8 +250,8 @@
240250
deleteResults[recordIDToDelete] = .failure(CKError(.referenceViolation))
241251
continue
242252
}
243-
let recordToDelete = storage[recordIDToDelete.zoneID]?[recordIDToDelete]
244-
storage[recordIDToDelete.zoneID]?[recordIDToDelete] = nil
253+
let recordToDelete = storage[recordIDToDelete.zoneID]?.records[recordIDToDelete]
254+
storage[recordIDToDelete.zoneID]?.records[recordIDToDelete] = nil
245255
deleteResults[recordIDToDelete] = .success(())
246256

247257
// NB: If deleting a share that the current user owns, delete the shared records and all
@@ -251,21 +261,56 @@
251261
shareToDelete.recordID.zoneID.ownerName == CKCurrentUserDefaultName
252262
{
253263
func deleteRecords(referencing recordID: CKRecord.ID) {
254-
for recordToDelete in (storage[recordIDToDelete.zoneID] ?? [:]).values {
264+
for recordToDelete in (storage[recordIDToDelete.zoneID]?.records ?? [:]).values {
255265
guard
256266
recordToDelete.share?.recordID == recordID
257267
|| recordToDelete.parent?.recordID == recordID
258268
else {
259269
continue
260270
}
261-
storage[recordIDToDelete.zoneID]?[recordToDelete.recordID] = nil
271+
storage[recordIDToDelete.zoneID]?.records[recordToDelete.recordID] = nil
262272
deleteRecords(referencing: recordToDelete.recordID)
263273
}
264274
}
265275
deleteRecords(referencing: shareToDelete.recordID)
266276
}
267277
}
268278

279+
guard atomically
280+
else {
281+
return (saveResults: saveResults, deleteResults: deleteResults)
282+
}
283+
284+
let affectedZones = Set(
285+
recordsToSave.map(\.recordID.zoneID) + recordIDsToDelete.map(\.zoneID)
286+
)
287+
for zoneID in affectedZones {
288+
let saveResultsInZone = saveResults.filter { recordID, _ in recordID.zoneID == zoneID }
289+
let deleteResultsInZone = deleteResults.filter { recordID, _ in
290+
recordID.zoneID == zoneID
291+
}
292+
let saveSuccessRecordIDs = saveResultsInZone.compactMap { recordID, result in
293+
(try? result.get()) == nil ? nil : recordID
294+
}
295+
let deleteSuccessRecordIDs = deleteResultsInZone.compactMap { recordID, result in
296+
(try? result.get()) == nil ? nil : recordID
297+
}
298+
guard
299+
saveSuccessRecordIDs.count != saveResultsInZone.count
300+
|| deleteSuccessRecordIDs.count != deleteResultsInZone.count
301+
else {
302+
continue
303+
}
304+
// Every successful save and deletion becomes a '.batchRequestFailed'.
305+
for saveSuccessRecordID in saveSuccessRecordIDs {
306+
saveResults[saveSuccessRecordID] = .failure(CKError(.batchRequestFailed))
307+
}
308+
for deleteSuccessRecordID in deleteSuccessRecordIDs {
309+
deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed))
310+
}
311+
// All storage changes are reverted in zone.
312+
storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:]
313+
}
269314
return (saveResults: saveResults, deleteResults: deleteResults)
270315
}
271316
}
@@ -286,7 +331,8 @@
286331
var deleteResults: [CKRecordZone.ID: Result<Void, any Error>] = [:]
287332

288333
for recordZoneToSave in recordZonesToSave {
289-
storage[recordZoneToSave.zoneID] = storage[recordZoneToSave.zoneID] ?? [:]
334+
storage[recordZoneToSave.zoneID] =
335+
storage[recordZoneToSave.zoneID] ?? Zone(zone: recordZoneToSave)
290336
saveResults[recordZoneToSave.zoneID] = .success(recordZoneToSave)
291337
}
292338

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
}
4949
records = zoneIDs.reduce(into: [CKRecord]()) { accum, zoneID in
5050
accum += database.storage.withValue {
51-
($0[zoneID]?.values).map { Array($0) } ?? []
51+
($0[zoneID]?.records.values).map { Array($0) } ?? []
5252
}
5353
}
5454
await parentSyncEngine.handleEvent(
@@ -177,6 +177,7 @@
177177
package func processPendingRecordZoneChanges(
178178
options: CKSyncEngine.SendChangesOptions = CKSyncEngine.SendChangesOptions(),
179179
scope: CKDatabase.Scope,
180+
forceAtomicByZone: Bool? = nil,
180181
fileID: StaticString = #fileID,
181182
filePath: StaticString = #filePath,
182183
line: UInt = #line,
@@ -208,7 +209,7 @@
208209
return
209210
}
210211

211-
let batch = await nextRecordZoneChangeBatch(
212+
var batch = await nextRecordZoneChangeBatch(
212213
reason: .scheduled,
213214
options: options,
214215
syncEngine: {
@@ -224,14 +225,17 @@
224225
}
225226
}()
226227
)
228+
if let forceAtomicByZone {
229+
batch?.atomicByZone = forceAtomicByZone
230+
}
227231
guard let batch
228232
else { return }
229233

230234
let (saveResults, deleteResults) = try syncEngine.database.modifyRecords(
231235
saving: batch.recordsToSave,
232236
deleting: batch.recordIDsToDelete,
233237
savePolicy: .ifServerRecordUnchanged,
234-
atomically: true
238+
atomically: batch.atomicByZone
235239
)
236240

237241
var savedRecords: [CKRecord] = []
@@ -263,9 +267,15 @@
263267
syncEngine.state.remove(
264268
pendingRecordZoneChanges: savedRecords.map { .saveRecord($0.recordID) }
265269
)
270+
syncEngine.state.remove(
271+
pendingRecordZoneChanges: failedRecordSaves.map { .saveRecord($0.record.recordID) }
272+
)
266273
syncEngine.state.remove(
267274
pendingRecordZoneChanges: deletedRecordIDs.map { .deleteRecord($0) }
268275
)
276+
syncEngine.state.remove(
277+
pendingRecordZoneChanges: failedRecordDeletes.keys.map { .deleteRecord($0) }
278+
)
269279

270280
await syncEngine.parentSyncEngine
271281
.handleEvent(

‎Sources/SQLiteData/CloudKit/SyncEngine.swift‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -912,8 +912,7 @@
912912
parentForeignKey: parentForeignKey,
913913
defaultZone: defaultZone,
914914
privateTables: privateTables
915-
)
916-
{
915+
) {
917916
try trigger.execute(db)
918917
}
919918
}
@@ -929,7 +928,7 @@
929928
defaultZone: defaultZone,
930929
privateTables: privateTables
931930
)
932-
.reversed() {
931+
.reversed() {
933932
try trigger.drop().execute(db)
934933
}
935934
}
@@ -1696,8 +1695,12 @@
16961695
try await open(table)
16971696
}
16981697

1698+
case .batchRequestFailed:
1699+
newPendingRecordZoneChanges.append(.saveRecord(failedRecord.recordID))
1700+
break
1701+
16991702
case .networkFailure, .networkUnavailable, .zoneBusy, .serviceUnavailable,
1700-
.notAuthenticated, .operationCancelled, .batchRequestFailed,
1703+
.notAuthenticated, .operationCancelled,
17011704
.internalError, .partialFailure, .badContainer, .requestRateLimited, .missingEntitlement,
17021705
.invalidArguments, .resultsTruncated, .assetFileNotFound,
17031706
.assetFileModified, .incompatibleVersion, .constraintViolation, .changeTokenExpired,
@@ -1719,15 +1722,32 @@
17191722
try await userDatabase.write { db in
17201723
var enqueuedUnsyncedRecordID = false
17211724
for (failedRecordID, error) in failedRecordDeletes {
1722-
guard
1723-
error.code == .referenceViolation
1724-
else { continue }
1725-
try UnsyncedRecordID.insert(or: .ignore) {
1726-
UnsyncedRecordID(recordID: failedRecordID)
1725+
switch error.code {
1726+
case .referenceViolation:
1727+
enqueuedUnsyncedRecordID = true
1728+
try UnsyncedRecordID.insert(or: .ignore) {
1729+
UnsyncedRecordID(recordID: failedRecordID)
1730+
}
1731+
.execute(db)
1732+
syncEngine.state.remove(pendingRecordZoneChanges: [.deleteRecord(failedRecordID)])
1733+
break
1734+
case .batchRequestFailed:
1735+
syncEngine.state.add(pendingRecordZoneChanges: [.deleteRecord(failedRecordID)])
1736+
break
1737+
case .networkFailure, .networkUnavailable, .zoneBusy, .serviceUnavailable,
1738+
.notAuthenticated, .operationCancelled, .internalError, .partialFailure,
1739+
.badContainer, .requestRateLimited, .missingEntitlement, .invalidArguments,
1740+
.resultsTruncated, .assetFileNotFound, .assetFileModified, .incompatibleVersion,
1741+
.constraintViolation, .changeTokenExpired, .badDatabase, .quotaExceeded,
1742+
.limitExceeded, .userDeletedZone, .tooManyParticipants, .alreadyShared,
1743+
.managedAccountRestricted, .participantMayNeedVerification, .serverResponseLost,
1744+
.assetNotAvailable, .accountTemporarilyUnavailable, .permissionFailure,
1745+
.unknownItem, .serverRecordChanged, .serverRejectedRequest, .zoneNotFound,
1746+
.participantAlreadyInvited:
1747+
break
1748+
@unknown default:
1749+
break
17271750
}
1728-
.execute(db)
1729-
syncEngine.state.remove(pendingRecordZoneChanges: [.deleteRecord(failedRecordID)])
1730-
enqueuedUnsyncedRecordID = true
17311751
}
17321752
return enqueuedUnsyncedRecordID
17331753
}

0 commit comments

Comments
 (0)