-
Notifications
You must be signed in to change notification settings - Fork 87
Fix user modification timestamp. #386
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2424,7 +2424,9 @@ | |
| self.lastKnownServerRecord = lastKnownServerRecord | ||
| self._lastKnownServerRecordAllFields = lastKnownServerRecord | ||
| if let lastKnownServerRecord { | ||
| self.userModificationTime = lastKnownServerRecord.userModificationTime | ||
| self.userModificationTime = #sql(""" | ||
| max(\(self.userModificationTime), \(lastKnownServerRecord.userModificationTime)) | ||
| """) | ||
|
Comment on lines
+2427
to
+2429
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the actual fix. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -262,6 +262,71 @@ | |
| """ | ||
| } | ||
| } | ||
|
|
||
| @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) | ||
| @Test(.printTimestamps(true), .printRecordChangeTag) | ||
| func editBetweenBatchAndSentRecordZoneChanges() async throws { | ||
| try await userDatabase.userWrite { db in | ||
| try db.seed { | ||
| RemindersList(id: 1, title: "Personal") | ||
| } | ||
| } | ||
| try await syncEngine.processPendingRecordZoneChanges(scope: .private) | ||
|
|
||
| try await withDependencies { | ||
| $0.currentTime.now += 1 | ||
| } operation: { | ||
| try await userDatabase.userWrite { db in | ||
| try RemindersList.find(1).update { $0.title = "Personal 2" }.execute(db) | ||
| } | ||
|
|
||
| let changes = try await syncEngine.sendPendingRecordZoneChanges(scope: .private) | ||
|
|
||
| try await withDependencies { | ||
| $0.currentTime.now += 1 | ||
| } operation: { | ||
| try await userDatabase.userWrite { db in | ||
| try RemindersList.find(1).update { $0.title = "Personal 3" }.execute(db) | ||
| } | ||
| await changes.receive() | ||
| try await syncEngine.processPendingRecordZoneChanges(scope: .private) | ||
|
|
||
| try await userDatabase.read { db in | ||
| expectNoDifference( | ||
| try RemindersList.fetchAll(db), | ||
| [RemindersList(id: 1, title: "Personal 3")] | ||
| ) | ||
| } | ||
| assertInlineSnapshot(of: syncEngine.container, as: .customDump) { | ||
| """ | ||
| MockCloudContainer( | ||
| privateCloudDatabase: MockCloudDatabase( | ||
| databaseScope: .private, | ||
| storage: [ | ||
| [0]: CKRecord( | ||
| recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), | ||
| recordType: "remindersLists", | ||
| parent: nil, | ||
| share: nil, | ||
| recordChangeTag: 3, | ||
| id: 1, | ||
| id🗓️: 0, | ||
| title: "Personal 3", | ||
| title🗓️: 2, | ||
| 🗓️: 2 | ||
|
Comment on lines
+315
to
+316
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we can write a failing test that shows data not properly syncing (we need to better support multiple sync engines in a test), but this at least shows the edit was made at the right time. Without the changes in this PR this timestamp was 1. |
||
| ) | ||
| ] | ||
| ), | ||
| sharedCloudDatabase: MockCloudDatabase( | ||
| databaseScope: .shared, | ||
| storage: [] | ||
| ) | ||
| ) | ||
| """ | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
|
|
||
| extension CKRecord { | ||
| @TaskLocal static var printTimestamps = false | ||
| @TaskLocal static var printRecordChangeTag = false | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted a way to get visibility into record change tags. |
||
| } | ||
|
|
||
| @available(macOS 14, iOS 17, tvOS 17, watchOS 10, *) | ||
|
|
@@ -50,14 +51,18 @@ | |
| let nonEncryptedKeys = Set(allKeys()) | ||
| .subtracting(encryptedValues.allKeys()) | ||
| .subtracting(["_recordChangeTag"]) | ||
| var baseChildren = [ | ||
| ("recordID", recordID as Any), | ||
| ("recordType", recordType as Any), | ||
| ("parent", parent as Any), | ||
| ("share", share as Any), | ||
| ] | ||
| if Self.printRecordChangeTag { | ||
| baseChildren.append(("recordChangeTag", _recordChangeTag as Any)) | ||
| } | ||
| return Mirror( | ||
| self, | ||
| children: [ | ||
| ("recordID", recordID as Any), | ||
| ("recordType", recordType as Any), | ||
| ("parent", parent as Any), | ||
| ("share", share as Any), | ||
| ] | ||
| children: baseChildren | ||
| + keys | ||
| .map { | ||
| $0.hasPrefix(CKRecord.userModificationTimeKey) | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need this bit of infrastructure in the mocks to wiggle in between the moment a batch is sent out to iCloud and the moment the
sentRecordZoneChangesdelegate method is called.