search

Sorix / CloudCore

Framework that enables syncing between iCloud (CloudKit) and Core Data
MIT License
153 stars 40 forks source link

Error getting relationships from CloudKit #27

Open juanip027 opened 6 years ago

juanip027 commented 6 years ago

Some relations are broken (nullified) in CoreData when you get data from CloudKit.

juanip027 commented 6 years ago

I think the problem is there's no dependency order in the recordChangedBlock of CKFetchRecordZoneChangesOperation, then if both records are new, the block may be called with a reference to another record that might not exist yet in CoreData.

Just created a pull request with a possible fix: It stores the records until fetch is completed, and then tries to process them in relationship order when the topological sort in the entity graph is possible. If it's not possible then it performs the update operations many times so all the relationships are fullfilled.

Bests

Sorix commented 6 years ago

@juanip027 it's rather hard to test that, could you confirm that your pull request fixed that issue for you?

juanip027 commented 6 years ago

Hi Vasily, yes it worked for both my model and the example app model too. Also there is minimum chance to add new bugs in this pull request because it only sort the entities to process them but the update process keeps the same.

Scytalion commented 6 years ago

Hi juanip027, I have tried your pr changes. For my object graph it is not always working. I am not sure, but I think the dependencyGraph of a Core Data model is never a DAG, because Core Data needs for every relationship a reverse one. So the if let topologicalSort = dependencyGraph.topologicalSort() { part will never be called.

When more than a few objects are changed, your solution to iterate cycleLength times for every changed record is producing a lot overhead.

I can add a pr with an alternate solution for the relationship problem. In it I am tracking the relationship and entity if a managedObject is not found and insert them after all NSManagedObjects from CloudKit are created.

juanip027 commented 6 years ago

Hi Oleg, Apple recommends that you specify inverse relationships but in the model they are optional so it can be a DAG, and in this case the topologicalSort is the best solution. Regarding the idea to iterate cycleLength times, it worked for my needs since I don't have a big database and the overhead is not an issue for me but your alternate solution looks good.

Sorix commented 6 years ago

@Scytalion Could you open pull request with your solution? I will try to add specific performance tests and check real difference between solutions.

Scytalion commented 6 years ago

@juanip027 yeah you are right. If it is DAG your solution would be best. @Sorix I will add a pr with my changes.