Skip to content
This repository has been archived by the owner on Jun 2, 2018. It is now read-only.

Added notification observer to new instances of mainQueueContext #71

Merged
merged 2 commits into from
Dec 30, 2015
Merged

Conversation

jimabc
Copy link
Contributor

@jimabc jimabc commented Dec 29, 2015

After calling resetStore in CoreDataStack the mainQueueContext loses its ability to save changes upstream because the existing notification observer is for the previous instance.

@rcedwards
Copy link
Contributor

Ah yes thats correct.

It would be best to add this to constructMainQueueContext() and remove the one in the initializer as apposed to just adding this again in the persistent stores didSet.

i.e.

private func constructMainQueueContext() -> NSManagedObjectContext {
    var managedObjectContext: NSManagedObjectContext!
    let setup: () -> Void = {
        managedObjectContext = NSManagedObjectContext(concurrencyType: .MainQueueConcurrencyType)
        managedObjectContext.mergePolicy = NSMergePolicy(mergeType: .MergeByPropertyStoreTrumpMergePolicyType)
        managedObjectContext.parentContext = self.privateQueueContext

        NSNotificationCenter.defaultCenter().addObserver(self,
            selector: "stackMemberContextDidSaveNotification:",
            name: NSManagedObjectContextDidSaveNotification,
            object: managedObjectContext)
    }

    ...

    return managedObjectContext
}

The testPersistentStoreReset() test could also benefit from being adapted to wait for an observation of the private queue context being saved so we can ensure this case is under test. This is done in the SaveTests testBackgroundInsertAndSavePropagatesChanges

If you're interested in making these changes that would be great! Otherwise, we'll get right on it.

Thanks

@jimabc
Copy link
Contributor Author

jimabc commented Dec 29, 2015

Great, I'll make the requisite changes. Thanks!

@rcedwards
Copy link
Contributor

👍 Looks good. Thanks for the contribution!

rcedwards added a commit that referenced this pull request Dec 30, 2015
Added notification observer to new instances of mainQueueContext
@rcedwards rcedwards merged commit 418750a into bignerdranch:master Dec 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants