Parent/Child Contexts #76
Conversation
to avoid race conditions when running subsequent tests and XCTAssert statements
# Conflicts: # Pod/Classes/VOKCoreDataManager.m
|
|
||
| func testDeletingObjectsOnTempContextGetsSavedToMainContext() { | ||
| //get a temp context, delete from temp, save to main, verify deleted on main | ||
| let manager = VOKCoreDataManager.sharedInstance() |
There was a problem hiding this comment.
Would it make sense to create properties out of manager and tempContext and do this configuration in setUp? - looks like it is pretty much the same for all tests
| @@ -39,36 +39,4 @@ | |||
| [VOKManagedObjectMap mapWithForeignKeyPath:inputKeyPath coreDataKey:VOKKeyForSelf(coreDataSelectorSymbol)] | |||
| #endif | |||
|
|
|||
|
|
|||
| ////////////////////////// DEPRECATED MACROS ////////////////////////// | |||
There was a problem hiding this comment.
were these recently deprecated?
There was a problem hiding this comment.
Yes, they were deprecated in #73 (v 2.4.1).
| @@ -90,7 +92,11 @@ - (NSManagedObjectContext *)managedObjectContext | |||
| if (!_managedObjectContext) { | |||
| NSAssert([NSOperationQueue currentQueue] == [NSOperationQueue mainQueue], | |||
| @"Must be on the main queue when initializing main context"); | |||
| _managedObjectContext = [self managedObjectContextWithConcurrencyType:NSMainQueueConcurrencyType]; | |||
| self.privateRootContext = [self managedObjectContextWithConcurrencyType:NSPrivateQueueConcurrencyType | |||
| parentContext:nil]; | |||
There was a problem hiding this comment.
Colons got misaligned here with the longer property name
There was a problem hiding this comment.
Also: should this be moved into it's own lazy-loading accessor method? I could go either way on that one.
| return true | ||
| } | ||
|
|
||
| if let rootContext = self.manager.managedObjectContext.parentContext { |
There was a problem hiding this comment.
I'd make this a guard let and XCTFail in the else clause.
|
That's it from me |
| @@ -12,8 +12,39 @@ import Vokoder | |||
|
|
|||
| let grandMilwaukeeIdentifier = 30096 | |||
|
|
|||
| struct CTAData { | |||
|
|
|||
| static func allStopDictionaries() -> [[String: AnyObject]] { | |||
There was a problem hiding this comment.
How bout adding a private typealias to this file to use for these:
private typealias JSONObject = [String: AnyObject]There was a problem hiding this comment.
I started doing this in another project and decided to remove it. Seeing JSONObject everywhere was just confusing because at a glance, it looked like it was supposed to be a class or struct somewhere.
| */ | ||
| - (void)saveMainContextAndWait; | ||
|
|
||
| /** | ||
| Provides a managed object context for scratch work or background processing using the same persistent store coordinator as the main context. As with all managed object contexts, it is not thread-safe. | ||
| Provides a managed object context for scratch work or background processing as a child of the main context. As with all managed object contexts, it is not thread-safe. | ||
| Create the context and do work on the same queue. You are responsible for retaining temporary contexts yourself. |
There was a problem hiding this comment.
If the idea is that this is a private-queue context, then I think the thread/queue it's created on doesn't so much matter as only using it via performBlock: and performBlockAndWait:.
| */ | ||
| - (NSManagedObjectContext *)temporaryContext; | ||
|
|
||
| /** | ||
| This provides a way for an application with heavy amounts of Core Data threading and writing to maintain object graph integrety by assuring that only one context is being written to at once. | ||
| @param writeBlock Do not save or merge this context, it will be done for you. Do not use GCD or thread jumping inside this block. | ||
| @param writeBlock Do not save or merge this context, it will be done for you. |
There was a problem hiding this comment.
I'd think the no-GCD-no-thread-jumping still applies?
|
That's it for me. |
|
updates pushed |
calling saveMainContextAndWait in tearDown was causing a deadlock on Travis (that was reproducable when limited my machine to 1 CPU). The resetCoreData call in setUp for the following test was deadlocking when trying to remove the peristent store. I suspect due to the root context still in the process of saving changes.
|
SLGTM. Does that address all of your concerns, @vokal-isaac? |
|
I may have missed it, but I think these two haven't been addressed: |
|
LGTM (discussed in person). |
|
merged, tagged, and |
Based on the guidelines from Marcus Zarra in this blog post, I changed the way managed object contexts are setup to use a parent/child relationship.
There is now a private "root" context that saves to the persistent store coordinator. The main context is a child of that root context. Temporary contexts are children of the main context. When saving, a context's parent contexts are also saved.
Although this doesn't affect the public interface much, it is a major change of how the stack is set up, so I've bumped the version to 3.0.
@vokal/ios-developers, @seanwolter code review?
I'm happy to add more tests to verify that the parent/child relationship works as expected if anyone has suggestions.