Sync of several updates of different attributes of one record causes miss of all updates except one

Hello Paul. I have found one issue. It looks like it works as designed, but it causes break in my app and unsync.

I have entity TableSessionTable. I have the following logic changing records in this entity:

Session is created with open time and default totalAmount value (-1).
When session is being closed, two updates are performed in different functions:
- totalAmount, discount or both can be set to some integer value.
- closedTime is set to some Date value.

I noticed that when I did the following steps, it causes loss of one of updates:

Create session.
Run sync. All good.
Close session, totalAmount is set and close time is set.
Run sync.

I have debugged Seam function applyLocalChangesToServer and found that it is happening because of this condition:

        for record in insertedOrUpdatedCKRecords ?? [] {
            let recordName = record.recordID.recordName
            if let currentRecord = changedRecords[recordName] { 
// In the following code Seam checks if this record is already in changed records array and skips it.
                if let currentDate = currentRecord.modificationDate,
                    let newDate = record.modificationDate {
                    if newDate > currentDate {
                        changedRecords[recordName] = record
                    }
                }
            } else {
                changedRecords[recordName] = record
            }
        }

Here is output for each of three records (I have performed re-play debugging after did screenshots above, that's why recordID does not match with screenshots above. Don't be confused please).

Sync Started
(lldb) po record
<CKRecord: 0x7f93bff8f510; recordID=6FC63F77-6CEB-46CD-9DEC-4F3F7BAF4309:(SMStoreCloudStore_CustomZone:__defaultOwner__), recordChangeTag=c4, values={
    totalAmount = 0;
}, recordType=TableSessionTable>
{
    creatorUserRecordID -> <CKRecordID: 0x60c005e39a60; recordName=__defaultOwner__, zoneID=_defaultZone:__defaultOwner__>
    lastModifiedUserRecordID -> <CKRecordID: 0x60c005e398e0; recordName=__defaultOwner__, zoneID=_defaultZone:__defaultOwner__>
    creationDate -> 2017-11-30 18:37:06 +0000
    **modificationDate -> 2017-11-30 18:37:06 +0000**
    modifiedByDevice -> iGlockPro15
    totalAmount (modified) -> 0 (type f)
}

(lldb) po record
<CKRecord: 0x7f93bff8fb10; recordID=D8EE9C3B-A76C-424A-8142-C06AD69B7D09:(SMStoreCloudStore_CustomZone:__defaultOwner__), recordChangeTag=c5, values={
    closeTime = "2017-11-30 18:38:16 +0000";
}, recordType=GuestsTable>
{
    creatorUserRecordID -> <CKRecordID: 0x60c005e39600; recordName=__defaultOwner__, zoneID=_defaultZone:__defaultOwner__>
    lastModifiedUserRecordID -> <CKRecordID: 0x60c005e39900; recordName=__defaultOwner__, zoneID=_defaultZone:__defaultOwner__>
    creationDate -> 2017-11-30 18:37:06 +0000
    modificationDate -> 2017-11-30 18:37:06 +0000
    modifiedByDevice -> iGlockPro15
    closeTime (modified) -> 2017-11-30 18:38:16 +0000
}

(lldb) po record
<CKRecord: 0x7f93bff95dc0; recordID=6FC63F77-6CEB-46CD-9DEC-4F3F7BAF4309:(SMStoreCloudStore_CustomZone:__defaultOwner__), recordChangeTag=c4, values={
    closeTime = "2017-11-30 18:38:16 +0000";
}, recordType=TableSessionTable>
{
    creatorUserRecordID -> <CKRecordID: 0x60c005e271c0; recordName=__defaultOwner__, zoneID=_defaultZone:__defaultOwner__>
    lastModifiedUserRecordID -> <CKRecordID: 0x60c005e2a220; recordName=__defaultOwner__, zoneID=_defaultZone:__defaultOwner__>
    creationDate -> 2017-11-30 18:37:06 +0000
    **modificationDate -> 2017-11-30 18:37:06 +0000**
    modifiedByDevice -> iGlockPro15
    closeTime (modified) -> 2017-11-30 18:38:16 +0000
}

(lldb) po changedRecords.count
2

(lldb)

I had fix it by changing logic a bit, so all attributes are changed at once now and it works as expected:

Can you please advise if it can be fixed in Seam?

This "bug" is very dangerous for guys like me, who preferred manual sync instead of automatic one. I checked - if user changes different attributes of one record several times, only one change will be uploaded to iCloud. In the same time CoreData will contain actual information. In this case I believe the only way to handle this properly is to remove all data from Cloud before sync and load all the data to server, like it is performed during the first sync.

Or this cycle can be improved by comparing not only modification date, but by checking what attributes were changed. In this case different records can be loaded to cloud one-by-one I believe.

@tifroz Hello, have you faced issue like described above? I saw your pull request, as I understood, you fixed similar issue but when sync is performed from server to client.

PR #57 fixes some sync issues going both ways. Hard to tell for sure if it will fix your problem specifically, but now that the PR was merged (it was merged just yesterday) it's easy enough for you to give it a try and report back?

@tifroz Yep, I've checked. It's still the same, as described above. Unfortunately I am not so experienced to fix in on my own :(

I have uploaded sample project for your convenience here.

Steps to reproduce:

You will have to setup iCloud container, nothing special.
Run app.
If it's the first time - click Fill database.
Click Open table.
Open any row.
Click Run manual sync - this will run sync.
Once synced - check CloudKit dashboard for uploaded values.
Click on Refresh date and save - this will change the first attribute of the record.
Click on Refresh name and save - this will change the second attribute of the record.
Click on Run manual sync and check uploaded values once synced. You will see that only one change was loaded to iCloud.

This issue looks dangerous even for auto sync, because everybody can have issues with access to Internet and this can cause an issue. Another showstopper for me :(

@paulw11 @tifroz Hello gentlemen!

I did some investigation and it looks like issue is not so complicated as I thought. :)

I have checked function recordsForUpdatedObjects that returns array of CKRecords and was thinking to put the following logic here:

Find all updates for each record.
Merge them depending on time of insert.
Return CKRecord for each LocalStoreRecordID that will contain all changed properties.

But here I found that Seam3 does not guarantee that sequence of updates will be satisfied because there is no system or Seam3 attribute in entities that can be used to check what record was the latest one. I thought that I can use Z_PK CoreData attribute from sm_LocalStore_ChangedProperties. I have searched the Internet and found that sorting by this attribute is not a good idea and CoreData does not guarantee it's sequence.

That's why I have found that there are two possible solutions for this issue:

Complicated, let's say, strategic solution.
- Add new attribute to sm_LocalStore_ChangedProperties with type Date and call it "creationDate"
- Implement logic that was described above to check all updates for each record and create cumulative CKRecord that will contain all information about changed attribute of record. This solution will require a lot of work and looks dangerous for me.

Easy, let's say, tactical solution. I have changed logic of this function a bit and added dictionary: var changedRecords = [String:Set<String>]() This dictionary will contain all changed properties for each recordID. And then sync will be performed properly. Full code:

func recordsForUpdatedObjects(backingContext context: NSManagedObjectContext) throws -> [CKRecord]? {
    let fetchRequest = NSFetchRequest<NSFetchRequestResult>(entityName: SMStore.SMLocalStoreChangeSetEntityName)
    fetchRequest.predicate = NSPredicate(format: "%K == %@ || %K == %@", SMStoreChangeSetHandler.SMLocalStoreChangeTypeAttributeName, NSNumber(value: SMLocalStoreRecordChangeType.recordInserted.rawValue as Int16), SMStoreChangeSetHandler.SMLocalStoreChangeTypeAttributeName, NSNumber(value: SMLocalStoreRecordChangeType.recordUpdated.rawValue as Int16))
    let results = try context.fetch(fetchRequest)
    // We cannot rely on CoreData Z_PK and cannot sort by it. So we have no chance to understand changes sequence for sure without additional TimeStamp field
    // That's why let's implement hacky solution - add all changedKeys to CKRecord for sync
    // Dictionary will contain all recordIDs of changed records and changedKeys that have been ever changed for each record
    var changedRecords = [String:Set<String>]()

    if !results.isEmpty {
        var changedPropertiesSet = Set<String>()
        for result in results as! [NSManagedObject] {
            let recordId = result.value(forKey: SMStore.SMLocalStoreRecordIDAttributeName) as! String

            let changedPropertyKeys = result.value(forKey: SMStore.SMLocalStoreRecordChangedPropertiesAttributeName) as? String
            if changedPropertyKeys != nil && changedPropertyKeys!.isEmpty == false {
                if changedPropertyKeys!.range(of: ",") != nil {
                    var changedPropertyKeysArray: [String]?
                    changedPropertyKeysArray = changedPropertyKeys!.components(separatedBy: ",")
                    for changedPropertyKeyElement in changedPropertyKeysArray! {
                        changedPropertiesSet.insert(changedPropertyKeyElement)
                    }
                    changedRecords[recordId] = changedPropertiesSet
                } else {
                    changedPropertiesSet.insert(changedPropertyKeys!)
                    changedRecords[recordId] = changedPropertiesSet
                }
            }
        }
    }

    var ckRecords: [CKRecord] = [CKRecord]()
    if !results.isEmpty {
        let recordIDSubstitution = "recordIDString"
        let predicate = NSPredicate(format: "%K == $recordIDString", SMStore.SMLocalStoreRecordIDAttributeName)
        for result in results as! [NSManagedObject] {
            result.setValue(NSNumber(value: true as Bool), forKey: SMStoreChangeSetHandler.SMLocalStoreChangeQueuedAttributeName)
            let entityName: String = result.value(forKey: SMStoreChangeSetHandler.SMLocalStoreEntityNameAttributeName) as! String
            let recordIDString: String = result.value(forKey: SMStore.SMLocalStoreRecordIDAttributeName) as! String
            let fetchRequest = NSFetchRequest<NSFetchRequestResult>(entityName: entityName)
            fetchRequest.predicate = predicate.withSubstitutionVariables([recordIDSubstitution:recordIDString])
            fetchRequest.fetchLimit = 1
            let objects = try context.fetch(fetchRequest)
            if objects.count > 0 {
                let object: NSManagedObject = objects.last as! NSManagedObject

                // Use all properties for CKRecord creation
                var changedPropertyKeysArray: [String]?
                if let changedPropertyKeysSet = changedRecords[recordIDString] {
                    for property in changedPropertyKeysSet {
                        changedPropertyKeysArray?.append(property)
                    }
                }

                if let ckRecord = object.createOrUpdateCKRecord(usingValuesOfChangedKeys: changedPropertyKeysArray) {
                    ckRecords.append(ckRecord)
                }
            }
        }
    }
    print(changedRecords)
    print(ckRecords.count)
    try context.saveIfHasChanges()
    return ckRecords
}

I did some smoke test with my example project (link can be found above) and performed testing with my project. Here is some evidence.

Test case 1 - change several attributes several times, one attribute per each change.

Create record and sync.
Change two fields several times.
Run sync and check the result. Output:

Sync Started ["CCC6D847-9285-4AC8-A055-2BC50FCE10D9": Set(["closeTime", "guestName"])] 4 More records coming No more records coming Sync Performed Sync performed successfully

SUCCESS

Test case 2 - change one attribute one or several times. Bulk change of several attributes in one time.

Create record and sync.
Change one attribute several times. Bulk change of several attributes in one time.
Run sync and check result. Output:

Sync Started ["7A53E154-C194-4186-966C-204A1115F33B": Set(["itemName", "itemDescription", "itemPrice"])] 3 More records coming No more records coming Sync Performed Sync performed successfully

SUCCESS

Test case 3 - change one attribute one or several times. Bulk change of several attributes in one time. Another one change of one attribute.

Check initial state.
Change one attribute one or several times.
Bulk change of several attributes in one time.
Change one attribute and run sync.

Check result.

Output: Sync Started ["7A53E154-C194-4186-966C-204A1115F33B": Set(["itemName", "itemDescription", "itemPrice"])] 4 More records coming No more records coming Sync Performed Sync performed successfully

SUCCESS

For me it looks like pretty valid solution. It still looks hacky for me, probably there could be better way to do it. But I'm a beginner in iOS Development.

May I ask you to review this code and do some tests on your side, if you are interested in this fix? If you confirm that this solution can be used, I will be happy to create my very first Pull Request :) I will really appreciate your opinion!

I have just realised that changedPropertiesSet is never truncated. So each next record will contain properties from previous records. As I have checked - this does not cause any issues, not valid properties are ignored during CKRecord creation. I have done more beautiful implementation. Now I like it :)

func recordsForUpdatedObjects(backingContext context: NSManagedObjectContext) throws -> [CKRecord]? {
        let fetchRequest = NSFetchRequest<NSFetchRequestResult>(entityName: SMStore.SMLocalStoreChangeSetEntityName)
        fetchRequest.predicate = NSPredicate(format: "%K == %@ || %K == %@", SMStoreChangeSetHandler.SMLocalStoreChangeTypeAttributeName, NSNumber(value: SMLocalStoreRecordChangeType.recordInserted.rawValue as Int16), SMStoreChangeSetHandler.SMLocalStoreChangeTypeAttributeName, NSNumber(value: SMLocalStoreRecordChangeType.recordUpdated.rawValue as Int16))
        let results = try context.fetch(fetchRequest)
        // Dictionary will contain all recordIDs of changed records and respective changedKeys that have been changed for records after last sync.
        var changedRecords = [String:Set<String>]()

        if !results.isEmpty {
            var changedRecordIDs = Set<String>()
            for result in results as! [NSManagedObject] {
                let recordId = result.value(forKey: SMStore.SMLocalStoreRecordIDAttributeName) as! String
                changedRecordIDs.insert(recordId)
            }

            for changedRecordID in changedRecordIDs {
                var changedPropertiesSet = Set<String>()
                for result in results as! [NSManagedObject] {
                    let recordId = result.value(forKey: SMStore.SMLocalStoreRecordIDAttributeName) as! String
                    if recordId == changedRecordID {
                        let changedPropertyKeys = result.value(forKey: SMStore.SMLocalStoreRecordChangedPropertiesAttributeName) as? String
                        if changedPropertyKeys != nil && changedPropertyKeys!.isEmpty == false {
                            if changedPropertyKeys!.range(of: ",") != nil {
                                var changedPropertyKeysArray: [String]?
                                changedPropertyKeysArray = changedPropertyKeys!.components(separatedBy: ",")
                                for changedPropertyKeyElement in changedPropertyKeysArray! {
                                    changedPropertiesSet.insert(changedPropertyKeyElement)
                                }
                                changedRecords[recordId] = changedPropertiesSet
                            } else {
                                changedPropertiesSet.insert(changedPropertyKeys!)
                                changedRecords[recordId] = changedPropertiesSet
                            }
                        }
                    }
                }
            }
        }

        var ckRecords: [CKRecord] = [CKRecord]()
        if !results.isEmpty {
            let recordIDSubstitution = "recordIDString"
            let predicate = NSPredicate(format: "%K == $recordIDString", SMStore.SMLocalStoreRecordIDAttributeName)
            for result in results as! [NSManagedObject] {
                result.setValue(NSNumber(value: true as Bool), forKey: SMStoreChangeSetHandler.SMLocalStoreChangeQueuedAttributeName)
                let entityName: String = result.value(forKey: SMStoreChangeSetHandler.SMLocalStoreEntityNameAttributeName) as! String
                let recordIDString: String = result.value(forKey: SMStore.SMLocalStoreRecordIDAttributeName) as! String
                let fetchRequest = NSFetchRequest<NSFetchRequestResult>(entityName: entityName)
                fetchRequest.predicate = predicate.withSubstitutionVariables([recordIDSubstitution:recordIDString])
                fetchRequest.fetchLimit = 1
                let objects = try context.fetch(fetchRequest)
                if objects.count > 0 {
                    let object: NSManagedObject = objects.last as! NSManagedObject

                    // Use Dictionary with changed properties for each record.
                    var changedPropertyKeysArray: [String]?
                    if let changedPropertyKeysSet = changedRecords[recordIDString] {
                        for property in changedPropertyKeysSet {
                            changedPropertyKeysArray?.append(property)
                        }
                    }

                    if let ckRecord = object.createOrUpdateCKRecord(usingValuesOfChangedKeys: changedPropertyKeysArray) {
                        ckRecords.append(ckRecord)
                    }
                }
            }
        }
        try context.saveIfHasChanges()
        return ckRecords
    }

As I have checked, it works fine. Here is output of dictionary: Sync Started ["6BCB0313-852A-47D8-8CCA-5801FEB311B0": Set(["quantityOfItems"]), "F9410F70-370D-4574-8D61-688113D8DAAA": Set(["quantityOfItems"]), "E93702A3-2FD2-48D0-86CF-929A04611399": Set(["closeTime", "guestName"]), "984BB57B-0193-4D6A-BC63-2B83890BF3D5": Set(["guestName"])]

paulw11 / Seam3

Sync of several updates of different attributes of one record causes miss of all updates except one #61