drewmccormack / ensembles

A synchronization framework for Core Data.
MIT License
1.63k stars 132 forks source link

Strange merging error #280

Open timbuchwaldt opened 5 years ago

timbuchwaldt commented 5 years ago

During development I see a very strange error: When I insert data into core data it's synced perfectly fine. If I update some part of the object (the title) on my Mac though I sometimes see fields becoming nil - but only specific ones. For me that is the two binary fields I have. Even stranger: At times I see data from other CoreData objects appearing instead of nil - and in a consistent manner - so it's the preview + actual binary data from the same wrong document appearing under the title I just changed.

All of this strangeness resolves if I remove the app on my phone and re-install it.

General logic is the following: I subscribe to CDEMonitoredManagedObjectContextDidSave and sync on my Mac. On the iOS side I react to CDEICloudFileSystemDidDownloadFiles, then trigger the usual merge on the main thread. I also implement didSaveMergeChangesWith to update my main context from the delegate callback, which I also do in the main thread. Logging the notification received shows data not being nil or anything.

drewmccormack commented 5 years ago

TBH this doesn’t sound like it would be done by Ensembles, unless you made strange migrations in your CD store.

I would double check your own custom code for the binary data. Maybe a bug in there or not properly handled KVC.

Kind regards, Drew

On 29 Sep 2018, at 12:01, Tim Buchwaldt notifications@github.com wrote:

During development I see a very strange error: When I insert data into core data it's synced perfectly fine. If I update some part of the object (the title) on my Mac though I sometimes see fields becoming nil - but only specific ones. For me that is the two binary fields I have. Even stranger: At times I see data from other CoreData objects appearing instead of nil - and in a consistent manner - so it's the preview + actual binary data from the same wrong document appearing under the title I just changed.

All of this strangeness resolves if I remove the app on my phone and re-install it.

General logic is the following: I subscribe to CDEMonitoredManagedObjectContextDidSave and sync on my Mac. On the iOS side I react to CDEICloudFileSystemDidDownloadFiles, then trigger the usual merge on the main thread. I also implement didSaveMergeChangesWith to update my main context from the delegate callback, which I also do in the main thread. Logging the notification received shows data not being nil or anything.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

timbuchwaldt commented 5 years ago

More testing revealed: 99.999% my error in implementing globalIdentifiersForManagedObjects in a slighly dumb way (by using a value that might not actually exist on all types I support to be synced).

Thanks again for the swift replies @drewmccormack - looking forward to licensing v2 if the project I'm building ever makes any money :)

timbuchwaldt commented 5 years ago

Okay.. more tests revealed a flaw in my testing: It seems like I only did a full re-setup after disabling "allow external storage" on my CoreData Entities.

Switching that back on and setting up again leads to exactly the errors described. Cleaning, setting up again with "allow external storage" disabled -> 0 issues.

FYI: I'm using NSPersistentContainer, don't know if that changes anything but I guess it really shouldn't.

drewmccormack commented 5 years ago

What is the current status exactly? What errors are you seeing? Are you completely removing all data from the cloud after making any changes in the model or global Id handling? Leaving the old data will mess things up.

If you do want to leave data in the cloud, such as in production, you must provide a new explicit CD model version. You can’t use that new stuff where it automatically figures out the versioning.

Kind regards, Drew

On 29 Sep 2018, at 12:52, Tim Buchwaldt notifications@github.com wrote:

Reopened #280.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

timbuchwaldt commented 5 years ago

Status: If I enable external storage of my binary fields altering the containing objects on one device destroys the other devices state by either making the fields become nil or contain other objects binary data. In case it shows other objects data it shows both fields from the same other object.

Please note I don't alter the binary data in any way. It's set on the first insert to the app and from there on is meant to remain stable.

Between tries I clear the cloud data as well as all local data, so there should be no trace left. Currently this is pre-alpha development, so I just rm -rf ~/Library/Mobile Documents/iCloud/$appid

drewmccormack commented 5 years ago

Turning on the binary data will change your data model. You need to add a new model version to do this, or completely clear all ensembles data with a deleech, and by removing all files from iCloud Drive. Changing the model without adding a new version will indeed lead too data loss.

Kind regards, Drew

On 29 Sep 2018, at 16:18, Tim Buchwaldt notifications@github.com wrote:

Status: If I enable external storage of my binary fields altering the containing objects on one device destroys the other devices state by either making the fields become nil or contain other objects binary data. In case it shows other objects data it shows both fields from the same other object.

Please note I don't alter the binary data in any way. It's set on the first insert to the app and from there on is meant to remain stable.

Between tries I clear the cloud data as well as all local data, so there should be no trace left. Currently this is pre-alpha development, so I just rm -rf ~/Library/Mobile Documents/iCloud/$appid

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

timbuchwaldt commented 5 years ago

I did that. After every Change I either bumped the model version or cleaned all data.

Sent from my iPhone

On 29. Sep 2018, at 16:46, Drew McCormack notifications@github.com wrote:

Turning on the binary data will change your data model. You need to add a new model version to do this, or completely clear all ensembles data with a deleech, and by removing all files from iCloud Drive. Changing the model without adding a new version will indeed lead too data loss.

Kind regards, Drew

On 29 Sep 2018, at 16:18, Tim Buchwaldt notifications@github.com wrote:

Status: If I enable external storage of my binary fields altering the containing objects on one device destroys the other devices state by either making the fields become nil or contain other objects binary data. In case it shows other objects data it shows both fields from the same other object.

Please note I don't alter the binary data in any way. It's set on the first insert to the app and from there on is meant to remain stable.

Between tries I clear the cloud data as well as all local data, so there should be no trace left. Currently this is pre-alpha development, so I just rm -rf ~/Library/Mobile Documents/iCloud/$appid

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.