soffes / ssdatakit

Eliminate your Core Data boilerplate code
MIT License
453 stars 57 forks source link

Changes in privateQueueContext are not automatically propagated to mainQueueContext #11

Closed ploddi closed 10 years ago

ploddi commented 11 years ago

I'm using SSDataKit along with AFIncrementalStore and discover interesting behavior.

Seems like when NSFetchedResultController performing fetch, implicitly uses topmost PSC of parent/child MOC chain. I swizzle -executeRequest:withContext:error: method of NSPersistentStoreCoordinator, and it is called with privateQueueContext as context parameter, while NSFetchedResultController is initialized with mainQueueContext.

AFIncrementalStore can intercept fetch requests and change context before (or after in async manner) returning any results back to caller. In my case NSIncrementalStore method -executeRequest:withContext:error: is also called with privateQueueContext as parameter. Then AFIncrementalStore getting data from remote server and update privateQueueContext asynchronously. But this changes never automatically pulled by mainQueueContext.

Now I just override -mainQueueContext method to not use private moc as parent, but looking for more appropriate solution to pull changes back to main moc in case of async update in private moc.

soffes commented 11 years ago

@calebd can you take a look?

calebd commented 11 years ago

That's how nested contexts in CoreData work. Changes are pushed up by saves, and pulled down by fetches. This sounds like a problem with AFIncrementalStore (which I've never used).

yimingtang commented 11 years ago

My app works fine with old SSDataKit which has mainContext only. However, after updating it to nested MOC, problem happens on iOS 5.1.

http://stackoverflow.com/questions/11786436/core-data-nested-managed-object-contexts-and-frequent-deadlocks-freezes http://wbyoung.tumblr.com/post/27851725562/core-data-growing-pains

The SO post and article described bugs with nested MOC.

So I to a quick fix:


+ (NSManagedObjectContext *)mainQueueContext {
    if (!__mainQueueContext) {
        __mainQueueContext = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSMainQueueConcurrencyType];

        // Use nested MOC on iOS 6.0+
        if ([[[UIDevice currentDevice] systemVersion] compare:@"6.0" options:NSNumericSearch] != NSOrderedAscending) {
            [__mainQueueContext setParentContext:[self privateQueueContext]];
        } else {
            [__mainQueueContext setPersistentStoreCoordinator:[self persistentStoreCoordinator]];
        }
    }
    return __mainQueueContext;
}

I don't know whether it's a good solution, but it just works.

t089 commented 10 years ago

I think setting up nested core data contexts is best left to the developer. The specific setup depends on the problem you want to solve.

I for example use the following setup:

This setup works really well for me. I can use the main queue context in UI and don't need to worry about anything going on in the background. When a data import finishes the changes become visible in the UI immediately.

There are a few caveats however:

  1. It is usually not feasible to have multiple import queue contexts around at the same time. Although they share the same parent, they will not share their respective changes. Consider an operation that loads some objects: if the object already exists (i.e. it is fetched from the parent), the object is updated. If the object does not yet exist, a new one is created. If two separate import contexts do this at the same time, they both might not be able to find the object and create it. When they save, you end up with two identical objects in your main context. I solved this issue by doing all imports in a single, shared import context. This way all operations line up. This is basically a "lock" on the parent context.
  2. Since the import context is a child of the main context, a problem can arise during saving. Imagine your user is editing some objects in your app on the main queue context. He is not finished yet, so the context has unsaved changes. At the same time, some background routine calls a web service and loads some remote objects. The objects are parsed and all is fine and the import context saves to the main context. Now the main context contains not only the unsaved changes from the user but also from the import context. What do you do now? If you just save the changes after the import you also save the changes from the user. This might cause a bad user experience. What if he wanted to revert the changes? You can't tell them apart anymore. If you don't save immediately after import you risk that the user changes his mind and reverts the context. Now also the imported and still unsaved data is lost as well.

And there is certainly more… In the end: there is no simple answer and therefore I am not sure if a framework should try to give one...

kharmabum commented 10 years ago

@t089, nice overview. Thank you for sharing. With regards to your final point, this scenario can be avoiding by creating a new child MOC (of type main) off the Main Queue context (for editing an existing object) as suggested in this Cocoanetics article.

To match the scenario you described, SSDatakit might benefit from the addition of + (NSManagedObjectContext *)newMainQueueContext and + (NSManagedObjectContext *)newPrivateQueueContext helper functions (seen here) that spawn MOCs off the Main Queue context in addition to a new method on the MOC category that saves a context as well as its parent.

soffes commented 10 years ago

Having a default queue is very important. It makes initializing objects easier and more predictable. Everywhere in Data Kit, you can override the context. If you don't want to use Data Kit's context, then don't.

This is definitely a controversial topic. Everyone does things differently. Data Kit makes a decision. Personally, I think it's the best way to do thing. If you don't like it, you can setup your own and pass around the context everywhere.

What if we undeprecate mainContext and just return the mainQueueContext. If you want to override the main context, you can just return a different context in mainContext. Everyone wins?

What do you think @calebd?

kharmabum commented 10 years ago

I think Data Kit has settled on the preferred set up for the majority of cases people will likely encounter.

But I think ambiguity lies in which context (Main or Private) "should" serve as the parent to a a new private context thats been created for some batch of operations we don't want to interfere with saving and/or UI updates.

The way things are currently set up, if the parent on a new private context is set to the Private Queue context and a save occurs on this new context, then the Private Queue context persists that save. But the Main Queue context isn't currently set up to merge changes from saves on the Private Queue context (its parent). Alternatively, if the parent of a new private context is set to Main and a save occurs then our changes bubble up but currently aren't persisted without an additional save.

Personally, I think the Private context should stay as is and Data Kit should "encourage" that new MOCs be spawned off the Main context. This way the Main context gets any changes automatically but the consumer of Data Kit can decide at what time to persist all current outstanding changes [1]. However, for convenience, a - saveIncludingParent method (or something to that effect) could be included in the MOC category.

Just my current thoughts after having reviewed a bunch of Core Data material and revisiting Data Kit.

Edit:

[1] It's also a simpler design (so hopefully easier to understand and less potential for introducing future bugs).

kharmabum commented 10 years ago

I believe the alternative would be to set up the mainQueueContext to merge changes from saves on the privateQueueContext. In this case, the mainQueueContext and privateQueueContext would always be in sync, even following a save that was initiated on a child context of the privateQueueContext. Maybe this would be more intuitive to some people as it might be more natural to spawn a new private context off of an already private context or utilize the already existent privateQueueContext for a background operation. Then there would be a pre-baked option of doing background operations that get automatically persisted and passed to the mainQueueContext.

So I think either it should be left as is (which I believe to be perfectly adequate) or an opinionated method of spawning child contexts should be offered and detailed. But I think it's worth addressing explicitly.

soffes commented 10 years ago

The private context is the "primary" context. The main queue context is a child of the private context. Changes are automatically merged from the main queue context to the private queue context. This is how Data Kit is designed to work.

If you'd like it to work a different way, you are more than welcome to setup whatever works for you and not use any of the context stuff Data Kit provides for you.