MR3 using ClassicWithBackgroundCoordinatorSQLiteMagicalRecordStack merges with saveWithBlock: but not saveWithBlockAndWait:

[chadbag]

I am using ClassicWithBackgroundCoordinatorSQLiteMagicalRecordStack in MR3. When changes are made using the private context, and one of the [MagicalRecord saveWithBlock:...] methods, the changes are merged into the main context. However, with [MagicalRecord saveWithBlockAndWait:...] methods, they are not.

I found the comment: //TODO: need to setup backgroundContext -> context merges via NSNC, and unsubscribe automatically in the ClassicWithBackgroundCoordinatorSQLiteMagicalRecordStack.m file in - (NSManagedObjectContext *) newConfinementContext . So as a test, I added [[self context] MR_observeContextDidSave:backgroundContext]; before the return in that method, and it now merges correctly. However, of course, there is no auto unsubscribing from the observation notification so that a lot of needless notification observation registrations remain on the main context for private contexts that have disappeared.

I noticed in the ClassicSQLiteMagicalRecordStack.m which is inherited by the ClassicWithBackgroundCoordinatorSQLiteMagicalRecordStack defines a method - (void) saveWithBlock:(void (^)(NSManagedObjectContext *))block identifier:(NSString *)contextWorkingName completion:(MRSaveCompletionHandler)completion (as do higher up the chain subclasses) which in the case of the Classic* stacks does the right thing for the saveWithBlock: methods, which are written calling the stacks saveWIthBlock:identifier:completion methods such that the merge happens. However, saveWIthBlockAndWait: is implemented directly and does not go through a stack-oriented method so that seems to be why the merge does not happen with saveWithBlockAndWait:. It seems to me that if the saveWithBlock:identifier:completion: were rewritten to include a wait parameter such that both the async and synchronous methods for saving all ran through the stack-based save methods, then the merge would work in both cases, without having to come up with some sort of other way to store which contexts were being observed as notification objects for changes in order to facilitate merges.

Hopefully the above is understandable. Before I go and attempt to make these changes, I wanted some feedback on whether this was a reasonable assessment and approach to take.

[lprhodes]

Did you have any luck with this? Trying to use ClassicWithBackgroundCoordinatorSQLiteMagicalRecordStack but noticed the same thing.

[chadbag]

I did make the changes I mentioned in my post above in my own local codebase and the ClassicWithBackgroundCoordinatorSQLiteMagicalRecordStack and it seems to be working now with both sorts of saves. I am still testing and have not check the other stacks to see if they would need to be updated to work with the changes I made to make this work.

I basically made both sorts of save (wait and non wait) go through the same save routine and either wait or not so that the proper setup could be done to observe the changes.

I don't have a fork of MR or anything in order to generate pull requests but I could make my modified files available for anyone to test, critique, or use.

[lprhodes]

Thanks for getting back to me - it'd be great if you could share the files please. Are you still needing to force the NSFetchedResultsController a nudge to refresh the data? (I saw on stack overflow).

[chadbag]

I don't remember about the NSFetchedResultsController. I'll have to go over my code. However, I did see in a WWDC video, where the Apple engineer suggested refetching when you have two separate persistent store controllers using the same store so I think that is what I did. It was two weeks ago when I did this :-) When I am in the office tomorrow I will identify which files I changed and make sure the other 'stacks are covered and then share the files.

[lprhodes]

Thanks a lot - really appreciate it. Not personally using the other stacks but useful! WWDC 2013 211? Downloading at the moment :) — Thanks

Luke Rhodes

On Wed, Sep 10, 2014 at 7:47 PM, chadbag notifications@github.com wrote:

I don't remember about the NSFetchedResultsController. I'll have to go over my code. However, I did see in a WWDC video, where the Apple engineer suggested refetching when you have two separate persistent store controllers using the same store so I think that is what I did. It was two weeks ago when I did this :-) When I am in the office tomorrow I will identify which files I changed and make sure the other 'stacks are covered and then share the files.

Reply to this email directly or view it on GitHub: https://github.com/magicalpanda/MagicalRecord/issues/840#issuecomment-55163409

[chadbag]

I don't use the other stacks either but one of the root classes was modified to make this work so if you use this "branch" and later want to try and use one of the other stacks you would get crashes. And hopefully something like this can be added to the core MR code so it should be complete...

[chadbag]

OK, sorry for the delay -- got caught up in some other stuff in the office.

The files I changes are in http://www.shire.net/MagicalRecord/

I basically wrote a single choke point that both saveWithBlock: and saveWithBlockAndWait: pass through and the implemented a generic version and a specific one for the ClassicWithBackground... stack.

[lprhodes]

Thanks!

I ended up just creating another method within ClassicSQLiteMagicalRecordStack:

- (BOOL) saveWithBlockAndWait:(void(^)(NSManagedObjectContext *localContext))block error:(NSError **)error
{
    NSString *contextWorkingName = NSStringFromSelector(_cmd);

    NSParameterAssert(block);

    MRLogVerbose(@"Dispatching save request: %@", contextWorkingName);
    MRLogVerbose(@"%@ save starting", contextWorkingName);

    NSManagedObjectContext *localContext = [self newConfinementContext];
    NSManagedObjectContext *mainContext = [self context];

    [mainContext MR_observeContextDidSave:localContext];
    [mainContext setMergePolicy:NSMergeByPropertyStoreTrumpMergePolicy];
    [localContext MR_setWorkingName:contextWorkingName];

    block(localContext);

    [localContext MR_saveWithOptions:MRSaveSynchronously completion:nil];
    [mainContext MR_stopObservingContextDidSave:localContext];
}

It pretty much matches up with the existing - (void) saveWithBlock:(void (^)(NSManagedObjectContext *))block identifier:(NSString *)contextWorkingName completion:(MRSaveCompletionHandler)completion; method but without the async. I've not had any issues but it could be a time bomb as I've not had time to put much thought into it.

[chadbag]

Basically the same thing. So that there would only be one set of code to set up and tear down the "observing" I pushed them through the same choke point. It is a little bit messier but relies on one piece of code to do the observing. Your way is simpler, but has two sets of code that does the "observing", meaning it has to be changed in two locations if you decide to change the logic. 6 of one, half dozen of the other.

[tomwilsn]

I just started playing around with converting an application to ClassicWithBackgroundCoordinatorSQLiteMagicalRecordStack (attempting to avoid NSFetchedResultsController deadlocks ugh) and had this same issue. The fix above seems to work but has anyone else put any thought into if this what we should actually be doing? :)

[lprhodes]

It matches what’s described in the WWDC core data sessions for this use case and I’ve been pleased with the implementation (and logic behind it). I’ve just had my head in this area of Magical Record again for the last couple of days and all’s looking good.

The only other amend I had to make was adding the following to fix a merge issue for updates from private/containment contexts:

NSSet *updated = [notification.userInfo objectForKey:NSUpdatedObjectsKey];

for (NSManagedObject *anObject in updated) {
    [[self objectWithID:anObject.objectID] willAccessValueForKey:nil];
}

That’s in the MR_mergeChangesFromNotification method of NSManagedObjectContext+MagicalObserving.m just before mergeChangesFromContextDidSaveNotification: