Skip to content

Fix CloudKit data loss on delete-then-reinsert #421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jsutula wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix CloudKit data loss on delete-then-reinsert #421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bdd291d
Fix CloudKit data loss on delete-then-reinsert
jsutula Mar 13, 2026
a3049c7
Structurally enforce ID-based deduplication in MockSyncEngineState
jsutula Mar 13, 2026
afc96c1
Strengthen ChangeSupersessionTests
jsutula Mar 14, 2026
d39ab5f
Ensure all fields get fresh timestamps on reinsertion
jsutula Mar 15, 2026
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
60 changes: 46 additions & 14 deletions Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@
package final class MockSyncEngineState: CKSyncEngineStateProtocol {
package let changeTag = LockIsolated(0)
package let _pendingRecordZoneChanges = LockIsolated<
OrderedSet<CKSyncEngine.PendingRecordZoneChange>
>([]
OrderedDictionary<CKRecord.ID, CKSyncEngine.PendingRecordZoneChange>
>([:]
)
package let _pendingDatabaseChanges = LockIsolated<
OrderedSet<CKSyncEngine.PendingDatabaseChange>
>([])
OrderedDictionary<CKRecordZone.ID, CKSyncEngine.PendingDatabaseChange>
>([:])
private let fileID: StaticString
private let filePath: StaticString
private let line: UInt
Expand All @@ -160,11 +160,11 @@
}

package var pendingRecordZoneChanges: [CKSyncEngine.PendingRecordZoneChange] {
_pendingRecordZoneChanges.withValue { Array($0) }
_pendingRecordZoneChanges.withValue { Array($0.values) }
}

package var pendingDatabaseChanges: [CKSyncEngine.PendingDatabaseChange] {
_pendingDatabaseChanges.withValue { Array($0) }
_pendingDatabaseChanges.withValue { Array($0.values) }
}

package func removePendingChanges() {
Expand All @@ -173,26 +173,58 @@
}

package func add(pendingRecordZoneChanges: [CKSyncEngine.PendingRecordZoneChange]) {
self._pendingRecordZoneChanges.withValue {
$0.append(contentsOf: pendingRecordZoneChanges)
self._pendingRecordZoneChanges.withValue { dict in
for change in pendingRecordZoneChanges {
switch change {
case .saveRecord(let id), .deleteRecord(let id):
dict.updateValue(change, forKey: id)
@unknown default:
fatalError("Unsupported pendingRecordZoneChange: \(change)")
}
}
}
}

package func remove(pendingRecordZoneChanges: [CKSyncEngine.PendingRecordZoneChange]) {
self._pendingRecordZoneChanges.withValue {
$0.subtract(pendingRecordZoneChanges)
self._pendingRecordZoneChanges.withValue { dict in
for change in pendingRecordZoneChanges {
switch change {
case .saveRecord(let id), .deleteRecord(let id):
if dict[id] == change { dict.removeValue(forKey: id) }
@unknown default:
fatalError("Unsupported pendingRecordZoneChange: \(change)")
}
}
}
}

package func add(pendingDatabaseChanges: [CKSyncEngine.PendingDatabaseChange]) {
self._pendingDatabaseChanges.withValue {
$0.append(contentsOf: pendingDatabaseChanges)
self._pendingDatabaseChanges.withValue { dict in
for change in pendingDatabaseChanges {
switch change {
case .saveZone(let zone):
dict.updateValue(change, forKey: zone.zoneID)
case .deleteZone(let zoneID):
dict.updateValue(change, forKey: zoneID)
@unknown default:
fatalError("Unsupported pendingDatabaseChange: \(change)")
}
}
}
}

package func remove(pendingDatabaseChanges: [CKSyncEngine.PendingDatabaseChange]) {
self._pendingDatabaseChanges.withValue {
$0.subtract(pendingDatabaseChanges)
self._pendingDatabaseChanges.withValue { dict in
for change in pendingDatabaseChanges {
switch change {
case .saveZone(let zone):
if dict[zone.zoneID] == change { dict.removeValue(forKey: zone.zoneID) }
case .deleteZone(let zoneID):
if dict[zoneID] == change { dict.removeValue(forKey: zoneID) }
@unknown default:
fatalError("Unsupported pendingDatabaseChange: \(change)")
}
}
}
}
}
Expand Down
35 changes: 35 additions & 0 deletions Sources/SQLiteData/CloudKit/Internal/Triggers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,17 @@
defaultZone: defaultZone,
privateTables: privateTables
)
SyncMetadata
.where {
$0.recordPrimaryKey.eq(#sql("\(new.primaryKey)"))
&& $0.recordType.eq(tableName)
&& $0._isDeleted
}
.update {
$0._isDeleted = false
$0.userModificationTime = $currentTime()
$0._lastKnownServerRecordAllFields = #bind(nil)
Copy link
Author

@jsutula jsutula Mar 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

One note here: clearing _lastKnownServerRecordAllFields may be too blunt an instrument to get the desired result of fresh timestamps for every field. It does guarantee that the next set of changes sent to the server will have fresh timestamps for all fields, but I believe also introduces a window of time (after reinsert but before successful submission of changes) where concurrent modifications from another device could be fetched and applied with no delta-filtering baseline, potentially overwriting locally-reinserted values.

A more direct solution may call for a separate field on SyncMetadata, something like _isReinserted: Bool. Or better yet: roll up _isDeleted and _isReinserted as cases in a new optional enum field PendingStatus. The status being reinserted could then be used to control outbound change inclusion behavior (include changes for all fields, each modified at the row's userModificationTime) and inbound filtering behavior (only take server field value if its modified time is greater than or equal to local row's userModificationTime). I'll wait for feedback before pursuing this in case there may be other options.

}
}
)
}
Expand Down Expand Up @@ -242,6 +253,7 @@
afterZoneUpdateTrigger(),
afterUpdateTrigger(for: syncEngine),
afterSoftDeleteTrigger(for: syncEngine),
afterUndeleteTrigger(for: syncEngine),
]
}

Expand Down Expand Up @@ -348,6 +360,29 @@
}
)
}

fileprivate static func afterUndeleteTrigger(
for syncEngine: SyncEngine
) -> TemporaryTrigger<Self> {
createTemporaryTrigger(
"\(String.sqliteDataCloudKitSchemaName)_after_undelete_on_sqlitedata_icloud_metadata",
ifNotExists: true,
after: .update(of: \._isDeleted) { _, new in
Values(
syncEngine.$didUpdate(
recordName: new.recordName,
zoneName: new.zoneName,
ownerName: new.ownerName,
oldZoneName: new.zoneName,
oldOwnerName: new.ownerName,
descendantRecordNames: #bind(nil)
)
)
} when: { old, new in
old._isDeleted && !new._isDeleted && !SyncEngine.$isSynchronizing
}
)
}
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
Expand Down
1 change: 1 addition & 0 deletions Sources/SQLiteData/CloudKit/SyncEngine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,7 @@

let record =
allFields
?? metadata.lastKnownServerRecord
?? CKRecord(
recordType: metadata.recordType,
recordID: recordID
Expand Down
Loading