Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Deleted objects remains in the context because of root context mergeChangesFromContextDidSaveNotification #613

Open
adams79 opened this Issue · 11 comments

3 participants

@adams79

Due to observer added to rootsavingcontext NSManagedObjectContextDidSaveNotification all changes from default context are MERGED in default context (!!!) and this causes unwanted behaviors in iOS 6 - the objects still remains in the context.
I can’t figure out how to prevent the notification to be fired if the changes are initially made in the defaultcontext.

@joergsimon

I have the same issue. any branch where this is already fixed?

another question: to my understanding, if there are only contexts in line going trough the root context, we do not need that merging at all. I understand that you might want to do that differently, but if you do not have only the basic setup the easiest fix would be just removing that notification.

I am currently trying to come up with a clear solution, but I am in a dead end:
The notification is send from the root saving context back to the default context wo did propagate the changes at first. However, I do not think I can find out from what child context of the root saving context the changes happened, don't I?

Also I just wander why exactly that leads to the problem, I thought if the changes are just replayed in the context it should not lead that deleted objects lurk around, because it should just delete them (as it does not trigger a save or so)?

@adams79

Hi,
I've a fork that actually solve this (https://github.com/adams79/MagicalRecord). I've made also a pull request...

Best

@joergsimon

ok, sad that the request is still open.

btw., just solved it too ;) . Now, how did you do your workaround? (I do not really see you commit in the fork)

I basically gave the saveWithOptions a child param, and in the block look if I am the root and my child is the default, and in that case disable the observing for that one single save.
Currently that code is in the source of my pod, so I would need to distill it out if that still makes sense...

@adams79
@joergsimon

so, I now also did send a pull request with my own version. Maybe they will react ;) .
If someone can tell me how to get the GHUnit test to run with Xcode 5 (always used OCUnit) especially with the project provided on the 2.2 branch, I would give it another try to write some tests to it ;)

@tonyarnold
Owner

I've completely overhauled the test targets for the project — you can use XCTest with Expecta now. Would you mind taking a look and writing this test so that I can see the bug in action? (assuming I haven't already merged your changes)

@tonyarnold
Owner

Actually, don't stress — I can see #647. It would still be good to get a test against this, because I don't completely follow what's going wrong here and it sounds a bit subtle.

@tonyarnold
Owner

Would you mind quickly seeing if what's in develop fixes this for you? I found some problems with the save logic that I'd missed in 2.1/2.2 and I have an inkling it might help.

Essentially, changes could be doubly propagated to the main context (via the child/parent context merge, and then again from the merge notification). Now the only thing that changes the default/main context is the merge notification — the contexts are not setup as parent/child.

As long as you make all of your changes in the root saving context (which using the standard saveWithBlock: etc methods will for for you out of the box), you should be fine. The idea is that you treat the default/main context as read-only for your UI components, fetched results controllers, fetches, etc. This should also speed things up a bit, as saves go straight to disk from the background root saving context.

Let me know how you get on — hopefully this has resolved the underlying problem.

@tonyarnold tonyarnold added the Bug label
@tonyarnold tonyarnold added this to the 2.3.0 milestone
@tonyarnold tonyarnold self-assigned this
@joergsimon

hi, sorry, was an extra busy week. I will enjoy my weekend, and then take a look into it at the start of next week, if it is ok. I should also have time to add an unit test for it then.

@tonyarnold
Owner

Of course it's OK! Early next week is completely fine.

@tonyarnold
Owner

@joergsimon any chance I could get you to test with the code in develop? I'd like to release 2.3.0 in the next week or so and move onto working on version 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.