Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
82 changes: 55 additions & 27 deletions Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@
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,
let existingRecordCopy = existingEntry.record.copy() as? CKRecord
{
existingEntry.history[existingRecordChangeTag] = existingRecordCopy
}
existingEntry.record = record
storage[record.recordID.zoneID]?.entries[record.recordID] = existingEntry
}
}

struct AssetID: Hashable {
Expand All @@ -27,7 +43,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
Copy Markdown
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 +70,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 +139,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 +162,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 +182,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 +220,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 +246,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 +297,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 +309,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 +324,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 +376,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
32 changes: 32 additions & 0 deletions Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,38 @@
}
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func serverRecordChangedIncludesAncestorRecord() async throws {
let recordID = CKRecord.ID(recordName: "ancestor-test")
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)
}

@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