SwiftKickMobile / TLIndexPathTools

TLIndexPathTools is a small set of classes that can greatly simplify your table and collection views.
tlindexpathtools.com
MIT License
347 stars 60 forks source link

Invalid update caused by NSManagedObjectContext save? (on a different thread?) #22

Closed fatuhoku closed 10 years ago

fatuhoku commented 10 years ago

Thanks for TLIndexPath. I have a small nagging issue (below) that once fixed will help me adopt it for my project.

fatuhoku commented 10 years ago

Say I have a project called Foobar that uses Core Data. I use TLIndexPathTools to display a collection of Quux's using UICollectionView.

The idea is that you can add and remove Quux's to and from the collection. I set self.indexPathController.fetchRequest = ... to achieve this, as in the README.md.

I don't want the user to lose their work so I want these additions / removals to persist to disk every 0.5s, so I've implemented an auto-save feature by observing NSManagedObjectContextObjectsDidChangeNotification and saving the context every time this happens:

RACSignal *changeSignal = [[NSNotificationCenter defaultCenter] rac_addObserverForName:NSManagedObjectContextObjectsDidChangeNotification object:nil];
[[changeSignal throttle:kAutoSaveDelay] subscribeNext:^(NSNotification *notification) {
    [[NSManagedObjectContext MR_defaultContext] MR_saveToPersistentStoreWithCompletion:nil];
}];

where kAutoSaveDelay == 0.5f (seconds), which ensures I don't save too often given large numbers of changes in a small time!

The Problem

When I create a new Quux, the collection view is updated with the new cell. That's really great.

BUT, after 0.5 seconds, my autosave kicks in, and I get this error:

2014-04-24 10:15:59.868 Foobar[9260:a0b] *** Assertion failure in -[UICollectionView _endItemAnimations], /SourceCache/UIKit_Sim/UIKit-2903.2/UICollectionView.m:3716
2014-04-24 10:15:59.869 Foobar[9260:a0b] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of items in section 0.  The number of items contained in an existing section after the update (1) must be equal to the number of items contained in that section before the update (1), plus or minus the number of items inserted or deleted from that section (1 inserted, 0 deleted) and plus or minus the number of items moved into or out of that section (0 moved in, 0 moved out).'
*** First throw call stack:
(
    0   CoreFoundation                      0x0109a5e4 __exceptionPreprocess + 180
    1   libobjc.A.dylib                     0x031408b6 objc_exception_throw + 44
    2   CoreFoundation                      0x0109a448 +[NSException raise:format:arguments:] + 136
    3   Foundation                          0x0168323e -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 116
    4   UIKit                               0x02727ddc -[UICollectionView _endItemAnimations] + 14193
    5   UIKit                               0x0272d9a4 -[UICollectionView _endUpdates] + 44
    6   UIKit                               0x0272db4b -[UICollectionView performBatchUpdates:completion:] + 418
    7   Foobar                          0x0024c966 -[TLIndexPathUpdates performBatchUpdatesOnCollectionView:completion:] + 1126

Any idea why this might be happening?

fatuhoku commented 10 years ago

It's clear that

- (void)controller:(TLIndexPathController *)controller didUpdateDataModel:(TLIndexPathUpdates *)updates {
// Collection view updates

Is called twice:

The TLIndexPathUpdates calculated doesn't take into account that the insertion had already been processed, somehow.

I inserted a few more debug messages into controller:willUpdateDataModel:withDataModel: and found that BOTH models contain the same NSManagedObject, and yet when an TLIndexPathUpdates is calculated, the SAME object is reported to have been inserted. This is just silly.

The reason: an object's objectID is not a reliable way to match objects before and after saving, Yet it's being used as the default identifier:

+ (id)identifierForItem:(id)item identifierBlock:(id (^)(id))identifierBlock
{
    ...
    if (!identifier && [item isKindOfClass:[NSManagedObject class]]) {
        identifier = ((NSManagedObject *)item).objectID;
    }
    ...
}

I think the solution is almost certainly to generate your own ID for the object and supply an identifier block.

Please document this caveat. I've encountered this problem before with just using the standard NSFetchedResultsController but the manifestation of it in NSIndexPathTools is more confusing and nastier!

wtmoose commented 10 years ago

Is the MOC associated with the FRC on the main queue? It must be.

If it is, the next step would be to step through the TLIndexPathUpdates initializer the second time to find out how it identifies an insert. I can't think of how that could happen.

Tim

On Apr 24, 2014, at 5:06 AM, Hok Shun Poon notifications@github.com wrote:

It's clear that

  • (void)controller:(TLIndexPathController )controller didUpdateDataModel:(TLIndexPathUpdates )updates { // Collection view updates Is called twice:

The first time is because FRC fires off a controllerDidChangeContent:, which the TLIndexPathController handles The second time is because of the same thing The TLIndexPathUpdates calculated doesn't take into account that the insertion had already been processed, somehow.

— Reply to this email directly or view it on GitHub.

fatuhoku commented 10 years ago

I don't know whether the MOC is associated to the FRC on the main queue. I use MagicalRecord's MR_fetchRequest to retrieve the fetch request, which is set on the index path controller.

As I have noted in my latest edit, I know how it identifies an insert. It compares the objectID. NSManagedObject always start off with a temporary objectID — when the object is saved (which occurs in my case) a different permanent objectID is set.

This causes TLIndexPathController to report a discrepancy, as the objects aren't matched properly.

wtmoose commented 10 years ago

You can specify an identifierKeyPath:

-[TLIndexPathController initWithFetchRequest:managedObjectContext:sectionNameKeyPath:identifierKeyPath:cacheName:]

If your objects don't have a suitable identifier, you can obtain permanent IDs upfront from the MOC:

-[NSManagedObjectContext obtainPermanentIDsForObjects:error:]

TLIndexPathController doesn't currently support block-based object identification with Core Data, but it is something I would consider adding.

wtmoose commented 10 years ago

This seems done.

fatuhoku commented 10 years ago

Well I was hoping that this could be more clearly documented in the README. I would contribute but I think it's best if the maintainer takes lead on putting this documentation where it belongs.

It cost me a lot of digging around — headaches and heartaches to understand this issue. In any case, this thread is probably good enough.

I would suggest an FAQ section for commonly encountered errors.