Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@
: sharedCloudDatabase

let rootRecord: CKRecord? = database.state.withValue {
$0.storage[share.recordID.zoneID]?.records.values.first { record in
record.share?.recordID == share.recordID
}
$0.storage[share.recordID.zoneID]?.entries.values.first { entry in
entry.record.share?.recordID == share.recordID
}?.record
}

return ShareMetadata(
Expand Down
80 changes: 53 additions & 27 deletions Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@
lastRecordChangeTag += 1
return lastRecordChangeTag
}

mutating func saveRecord(_ record: CKRecord) {
guard var existingEntry = storage[record.recordID.zoneID]?.entries[record.recordID]
else {
storage[record.recordID.zoneID]?.entries[record.recordID] =
RecordEntry(record: record, history: [:])
return
}
if let existingRecordChangeTag = existingEntry.record._recordChangeTag {
existingEntry.history[existingRecordChangeTag] = existingEntry.record.copy() as? CKRecord
}
existingEntry.record = record
storage[record.recordID.zoneID]?.entries[record.recordID] = existingEntry
}
}

struct AssetID: Hashable {
Expand All @@ -27,7 +41,12 @@

package struct Zone {
package var zone: CKRecordZone
package var records: [CKRecord.ID: CKRecord] = [:]
package var entries: [CKRecord.ID: RecordEntry] = [:]
}

package struct RecordEntry {
package var record: CKRecord
package var history: [Int: CKRecord]
Comment on lines -30 to +49
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beefed this up so that we don't hold just a record at a record ID, but also a history that maps record change tag to past record.

}

package init(databaseScope: CKDatabase.Scope) {
Expand All @@ -49,7 +68,7 @@
let record = try state.withValue { state in
guard let zone = state.storage[recordID.zoneID]
else { throw CKError(.zoneNotFound) }
guard let record = zone.records[recordID]
guard let record = zone.entries[recordID]?.record
else { throw CKError(.unknownItem) }
guard let record = record.copy() as? CKRecord
else { fatalError("Could not copy CKRecord.") }
Expand Down Expand Up @@ -118,7 +137,7 @@
$0.share?.recordID == share.recordID
})
let shareWasPreviouslySaved =
state.storage[share.recordID.zoneID]?.records[share.recordID] != nil
state.storage[share.recordID.zoneID]?.entries[share.recordID] != nil
guard shareWasPreviouslySaved || isSavingRootRecord
else {
saveResults[recordToSave.recordID] = .failure(CKError(.invalidArguments))
Expand All @@ -141,14 +160,14 @@
continue
}

let existingRecord = state.storage[recordToSave.recordID.zoneID]?.records[
let existingRecord = state.storage[recordToSave.recordID.zoneID]?.entries[
recordToSave.recordID
]
]?.record

func saveRecordToDatabase() {
let hasReferenceViolation =
recordToSave.parent.map { parent in
state.storage[parent.recordID.zoneID]?.records[parent.recordID] == nil
state.storage[parent.recordID.zoneID]?.entries[parent.recordID] == nil
&& !recordsToSave.contains { $0.recordID == parent.recordID }
}
?? false
Expand All @@ -161,12 +180,12 @@
func root(of record: CKRecord) -> CKRecord {
guard let parent = record.parent
else { return record }
return (state.storage[parent.recordID.zoneID]?.records[parent.recordID]).map(
root
) ?? record
return (state.storage[parent.recordID.zoneID]?.entries[parent.recordID]?.record)
.map(root) ?? record
}
func share(for rootRecord: CKRecord) -> CKShare? {
for (_, record) in state.storage[rootRecord.recordID.zoneID]?.records ?? [:] {
for (_, entry) in state.storage[rootRecord.recordID.zoneID]?.entries ?? [:] {
let record = entry.record
guard record.recordID == rootRecord.share?.recordID
else { continue }
return record as? CKShare
Expand Down Expand Up @@ -199,20 +218,20 @@
}

// TODO: This should merge copy's values to more accurately reflect reality
state.storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] = copy
state.saveRecord(copy)
saveResults[recordToSave.recordID] = .success(copy)

// NB: "Touch" parent records when saving a child:
if let parent = recordToSave.parent,
// If the parent isn't also being saved in this batch.
!recordsToSave.contains(where: { $0.recordID == parent.recordID }),
// And if the parent is in the database.
let parentRecord = state.storage[parent.recordID.zoneID]?.records[parent.recordID]?
.copy()
as? CKRecord
let parentRecord = state.storage[parent.recordID.zoneID]?.entries[parent.recordID]?
.record
.copy() as? CKRecord
{
parentRecord._recordChangeTag = state.nextRecordChangeTag()
state.storage[parent.recordID.zoneID]?.records[parent.recordID] = parentRecord
state.saveRecord(parentRecord)
}
}

Expand All @@ -225,12 +244,17 @@
precondition(existingRecord._recordChangeTag != nil)
saveRecordToDatabase()
} else {
let ancestorRecord =
state.storage[recordToSave.recordID.zoneID]?.entries[recordToSave.recordID]?
.history[recordToSaveChangeTag]
?? (existingRecord.copy() as? CKRecord ?? existingRecord)
saveResults[recordToSave.recordID] = .failure(
CKError(
.serverRecordChanged,
userInfo: [
CKRecordChangedErrorServerRecordKey: existingRecord.copy() as Any,
CKRecordChangedErrorClientRecordKey: recordToSave.copy(),
CKRecordChangedErrorAncestorRecordKey: ancestorRecord as Any,
]
)
)
Expand Down Expand Up @@ -271,8 +295,8 @@
continue
}
let hasReferenceViolation = !Set(
state.storage[recordIDToDelete.zoneID]?.records.values
.compactMap { $0.parent?.recordID == recordIDToDelete ? $0.recordID : nil }
state.storage[recordIDToDelete.zoneID]?.entries.values
.compactMap { $0.record.parent?.recordID == recordIDToDelete ? $0.record.recordID : nil }
?? []
)
.subtracting(recordIDsToDelete)
Expand All @@ -283,8 +307,9 @@
deleteResults[recordIDToDelete] = .failure(CKError(.referenceViolation))
continue
}
let recordToDelete = state.storage[recordIDToDelete.zoneID]?.records[recordIDToDelete]
state.storage[recordIDToDelete.zoneID]?.records[recordIDToDelete] = nil
let recordToDelete = state.storage[recordIDToDelete.zoneID]?.entries[recordIDToDelete]?
.record
state.storage[recordIDToDelete.zoneID]?.entries[recordIDToDelete] = nil
deleteResults[recordIDToDelete] = .success(())
if let recordType = recordToDelete?.recordType {
state.deletedRecords.append((recordIDToDelete, recordType))
Expand All @@ -297,18 +322,19 @@
shareToDelete.recordID.zoneID.ownerName == CKCurrentUserDefaultName
{
func deleteRecords(referencing recordID: CKRecord.ID) {
for recordToDelete in (state.storage[recordIDToDelete.zoneID]?.records ?? [:]).values
for entryToDelete in (state.storage[recordIDToDelete.zoneID]?.entries ?? [:]).values
{
let record = entryToDelete.record
guard
recordToDelete.share?.recordID == recordID
|| recordToDelete.parent?.recordID == recordID
record.share?.recordID == recordID
|| record.parent?.recordID == recordID
else {
continue
}
state.storage[recordIDToDelete.zoneID]?.records[recordToDelete.recordID] = nil
deleteResults[recordToDelete.recordID] = .success(())
state.deletedRecords.append((recordIDToDelete, recordToDelete.recordType))
deleteRecords(referencing: recordToDelete.recordID)
state.storage[recordIDToDelete.zoneID]?.entries[record.recordID] = nil
deleteResults[record.recordID] = .success(())
state.deletedRecords.append((recordIDToDelete, record.recordType))
deleteRecords(referencing: record.recordID)
}
}
deleteRecords(referencing: shareToDelete.recordID)
Expand Down Expand Up @@ -348,7 +374,7 @@
deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed))
}
// All storage changes are reverted in zone.
state.storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:]
state.storage[zoneID]?.entries = previousStorage[zoneID]?.entries ?? [:]
}
return (saveResults: saveResults, deleteResults: deleteResults)
}
Expand Down
3 changes: 1 addition & 2 deletions Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
zoneIDs.reduce(into: [CKRecord]()) {
accum,
zoneID in
accum += ((state.storage[zoneID]?.records.values).map { Array($0) } ?? [])
accum += (state.storage[zoneID]?.entries.values.map(\.record) ?? [])
.filter {
precondition(
$0._recordChangeTag != nil,
Expand Down Expand Up @@ -201,7 +201,6 @@
extension SyncEngine {
package struct SendRecordsCallback {
fileprivate let operation: @Sendable () async -> Void
@discardableResult
package func receive() async {
await operation()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,9 +672,9 @@
"""
}
assertInlineSnapshot(
of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.records[
of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.entries[
Reminder.recordID(for: 1)
],
]?.record,
as: .customDump
) {
"""
Expand Down Expand Up @@ -767,9 +767,9 @@
"""
}
assertInlineSnapshot(
of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.records[
of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.entries[
Reminder.recordID(for: 1)
],
]?.record,
as: .customDump
) {
"""
Expand Down
69 changes: 69 additions & 0 deletions Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,75 @@
}
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func serverRecordChangedIncludesAncestorRecord() async throws {
let recordID = CKRecord.ID(recordName: "1")
let record = CKRecord(recordType: "A", recordID: recordID)
record["value"] = 1

let (saveResults, _) = try syncEngine.private.database.modifyRecords(saving: [record])
#expect(saveResults.values.count(where: { (try? $0.get()) != nil }) == 1)

let clientRecord = try syncEngine.private.database.record(for: recordID)

let serverRecord = try syncEngine.private.database.record(for: recordID)
serverRecord["value"] = 2
_ = try syncEngine.private.database.modifyRecords(saving: [serverRecord])

clientRecord["value"] = 3
let (conflictResults, _) = try syncEngine.private.database.modifyRecords(
saving: [clientRecord]
)
let error = #expect(throws: CKError.self) {
try conflictResults[recordID]?.get()
}
#expect(error?.code == .serverRecordChanged)

let ancestor = error?.userInfo[CKRecordChangedErrorAncestorRecordKey] as? CKRecord
let server = error?.userInfo[CKRecordChangedErrorServerRecordKey] as? CKRecord
let client = error?.userInfo[CKRecordChangedErrorClientRecordKey] as? CKRecord
#expect(ancestor?["value"] as? Int == 1)
#expect(server?["value"] as? Int == 2)
#expect(client?["value"] as? Int == 3)
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func serverRecordChangedUsesChangeTagAncestor() async throws {
let recordID = CKRecord.ID(recordName: "1")
let record = CKRecord(recordType: "A", recordID: recordID)
record["value"] = 1

_ = try syncEngine.private.database.modifyRecords(saving: [record])

let clientRecord = try syncEngine.private.database.record(for: recordID)
let clientChangeTag = clientRecord._recordChangeTag

let serverRecordV2 = try syncEngine.private.database.record(for: recordID)
serverRecordV2["value"] = 2
_ = try syncEngine.private.database.modifyRecords(saving: [serverRecordV2])

let serverRecordV3 = try syncEngine.private.database.record(for: recordID)
serverRecordV3["value"] = 3
_ = try syncEngine.private.database.modifyRecords(saving: [serverRecordV3])

clientRecord["value"] = 99
let (conflictResults, _) = try syncEngine.private.database.modifyRecords(
saving: [clientRecord]
)
let error = #expect(throws: CKError.self) {
try conflictResults[recordID]?.get()
}
#expect(error?.code == .serverRecordChanged)

let ancestor = error?.userInfo[CKRecordChangedErrorAncestorRecordKey] as? CKRecord
let server = error?.userInfo[CKRecordChangedErrorServerRecordKey] as? CKRecord
let client = error?.userInfo[CKRecordChangedErrorClientRecordKey] as? CKRecord
#expect(ancestor?._recordChangeTag == clientChangeTag)
#expect(ancestor?["value"] as? Int == 1)
#expect(server?["value"] as? Int == 3)
#expect(client?["value"] as? Int == 99)
}

@Test func limitExceeded_modifyRecords() async throws {
let remindersListRecord = CKRecord(
recordType: RemindersList.tableName,
Expand Down
2 changes: 1 addition & 1 deletion Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@
"storage": state
.value
.storage
.flatMap { _, value in value.records.values }
.flatMap { _, value in value.entries.values.map(\.record) }
.sorted {
($0.recordType, $0.recordID.recordName) < ($1.recordType, $1.recordID.recordName)
},
Expand Down
2 changes: 1 addition & 1 deletion Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ extension SyncEngine {
let recordsToDeleteByID = Dictionary(
grouping: syncEngine.database.state.withValue { state in
recordIDsToDelete.compactMap {
recordID in state.storage[recordID.zoneID]?.records[recordID]
recordID in state.storage[recordID.zoneID]?.entries[recordID]?.record
}
},
by: \.recordID
Expand Down