jessesquires / JSQDataSourcesKit

⚠️ Deprecated ⚠️
https://www.jessesquires.com/blog/2020/04/14/deprecating-jsqdatasourceskit/
Other
682 stars 46 forks source link

Crash: when a section contains only a single cell, and you change the property that will move the cell to another section. #12

Closed alexzzp closed 8 years ago

alexzzp commented 9 years ago

I have been using JSQDataSourcesKit for a collectionview that is backed by CoreData, I create a fetchResultController with following sort of setting:

    fetchRequest.predicate = NSPredicate(format: "whoOwn.name = %@", self.owner!.name)
    fetchRequest.sortDescriptors = [NSSortDescriptor(key: "createTime", ascending: false)]
    let fetchResultVC = NSFetchedResultsController(fetchRequest: fetchRequest,
        managedObjectContext: DKCoreData.shared().mangeObjectContext,
        sectionNameKeyPath: "sectionDate", cacheName:TFPhotoCollectionVC.sectionCacheName)

Everything is fine when the collectionview is display. In my app, user can tap a photo and segue to another view controller, in that controller, user may change the model's "createTime" property (the sectionNameKeyPath property "sectionDate" will change as well), when the change did happen, the app crash, because of CoreData operation error like this:

    An exception was caught from the delegate of NSFetchedResultsController during a call to -controllerDidChangeContent:.  cannot move an item from a deleted section (0) with userInfo (null)

I figure out some code may be missing in the processing of "didChangeContent" of "CollectionViewFetchedResultsDelegateProvider", in code, i find following code:

    - (void)controllerDidChangeContent:(NSFetchedResultsController *)controller
    {
        NSMutableArray *moves = _objectChanges[@(NSFetchedResultsChangeMove)];
        if (moves.count > 0) {
            NSMutableArray *updatedMoves = [[NSMutableArray alloc] initWithCapacity:moves.count];

            NSMutableIndexSet *insertSections = _sectionChanges[@(NSFetchedResultsChangeInsert)];
            NSMutableIndexSet *deleteSections = _sectionChanges[@(NSFetchedResultsChangeDelete)];
            for (NSArray *move in moves) {
                NSIndexPath *fromIP = move[0];
                NSIndexPath *toIP = move[1];

                if ([deleteSections containsIndex:fromIP.section]) {
                    if (![insertSections containsIndex:toIP.section]) {
                        NSMutableArray *changeSet = _objectChanges[@(NSFetchedResultsChangeInsert)];
                        if (changeSet == nil) {
                            changeSet = [[NSMutableArray alloc] initWithObjects:toIP, nil];
                            _objectChanges[@(NSFetchedResultsChangeInsert)] = changeSet;
                        } else {
                            [changeSet addObject:toIP];
                        }
                    }
                } else if ([insertSections containsIndex:toIP.section]) {
                    NSMutableArray *changeSet = _objectChanges[@(NSFetchedResultsChangeDelete)];
                    if (changeSet == nil) {
                        changeSet = [[NSMutableArray alloc] initWithObjects:fromIP, nil];
                        _objectChanges[@(NSFetchedResultsChangeDelete)] = changeSet;
                    } else {
                        [changeSet addObject:fromIP];
                    }
                } else {
                    [updatedMoves addObject:move];
                }
            }

            if (updatedMoves.count > 0) {
                _objectChanges[@(NSFetchedResultsChangeMove)] = updatedMoves;
            } else {
                [_objectChanges removeObjectForKey:@(NSFetchedResultsChangeMove)];
            }
        }

        NSMutableArray *deletes = _objectChanges[@(NSFetchedResultsChangeDelete)];
        if (deletes.count > 0) {
            NSMutableIndexSet *deletedSections = _sectionChanges[@(NSFetchedResultsChangeDelete)];
            [deletes filterUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(NSIndexPath *evaluatedObject, NSDictionary *bindings) {
                return ![deletedSections containsIndex:evaluatedObject.section];
            }]];
        }

        NSMutableArray *inserts = _objectChanges[@(NSFetchedResultsChangeInsert)];
        if (inserts.count > 0) {
            NSMutableIndexSet *insertedSections = _sectionChanges[@(NSFetchedResultsChangeInsert)];
            [inserts filterUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(NSIndexPath *evaluatedObject, NSDictionary *bindings) {
                return ![insertedSections containsIndex:evaluatedObject.section];
            }]];
        }

        UICollectionView *collectionView = self.collectionView;

        [collectionView performBatchUpdates:^{
            NSIndexSet *deletedSections = _sectionChanges[@(NSFetchedResultsChangeDelete)];
            if (deletedSections.count > 0) {
                [collectionView deleteSections:deletedSections];
            }

            NSIndexSet *insertedSections = _sectionChanges[@(NSFetchedResultsChangeInsert)];
            if (insertedSections.count > 0) {
                [collectionView insertSections:insertedSections];
            }

            NSArray *deletedItems = _objectChanges[@(NSFetchedResultsChangeDelete)];
            if (deletedItems.count > 0) {
                [collectionView deleteItemsAtIndexPaths:deletedItems];
            }

            NSArray *insertedItems = _objectChanges[@(NSFetchedResultsChangeInsert)];
            if (insertedItems.count > 0) {
                [collectionView insertItemsAtIndexPaths:insertedItems];
            }

            NSArray *reloadItems = _objectChanges[@(NSFetchedResultsChangeUpdate)];
            if (reloadItems.count > 0) {
                [collectionView reloadItemsAtIndexPaths:reloadItems];
            }

            NSArray *moveItems = _objectChanges[@(NSFetchedResultsChangeMove)];
            for (NSArray *paths in moveItems) {
                [collectionView moveItemAtIndexPath:paths[0] toIndexPath:paths[1]];
            }
        } completion:nil];

        _objectChanges = nil;
        _sectionChanges = nil;
    }

code before [collectionView performBatchUpdates:^{}] is missing in JSQDataSourcesKit, any problem?

alexzzp commented 9 years ago

I have no idea about the code before * [collectionView performBatchUpdates:^{}] * , any idea?

jessesquires commented 9 years ago

Hey @alexzzp

I figure out some code may be missing in the processing of "didChangeContent"

Basically, what this code is doing is processing and batching up all the changes. We do this here (L76-L103). Then we apply the changes via applyObjectChanges() and applySectionChanges() here. Swift is just much cleaner and less verbose. Also see my comment here. Many things we able to change/improve when moving to Swift and iOS 8+.

Everything is fine when the collectionview is display. In my app, user can tap a photo and segue to another view controller, in that controller, user may change the model's "createTime" property (the sectionNameKeyPath property "sectionDate" will change as well), when the change did happen, the app crash, because of CoreData operation error like this:

An exception was caught from the delegate of NSFetchedResultsController during a call to -controllerDidChangeContent:.  cannot move an item from a deleted section (0) with userInfo (null)

It's hard to say if this is a bug in the library, or with your implementation. It could be either or both.

Can you reproduce this in the demo project, or provide an example project that demonstrates this?

alexzzp commented 9 years ago

hi, it is easy to repeart the error in the example project of "JSQDataSourcesKit" just add followings code to "FetchedCollectionViewController.swift"

func collectionView(collectionView: UICollectionView, didSelectItemAtIndexPath indexPath: NSIndexPath) {
    if let t = self.dataSourceProvider?.fetchedResultsController.objectAtIndexPath(indexPath) as? Thing {

        let newCategory = Category.random.rawValue
        let newNumber = Int32(arc4random_uniform(10000))
        println("change t at indexpath (\(indexPath.section), \(indexPath.row)) from \(t.category), \(t.number) to \(newCategory), \(newNumber)")
        t.category = newCategory
        t.number = newNumber

        collectionView.deselectItemAtIndexPath(indexPath, animated: false)
    }
}

With above code added, you will change the "category" and "num" property in the "FetchedCollectionViewController", then the cell may be moved to a new position (in the same section or not), everything will be find for a while when you try to keep trying tapping on the cells, but sooner or latter you will come across some error messages like followings:

1  change t at indexpath (0, 1) from Blue, 5636 to Blue, 9949
2  change t at indexpath (0, 3) from Blue, 9949 to Green, 2316
3  change t at indexpath (1, 4) from Green, 2316 to Green, 9123
4  change t at indexpath (1, 7) from Green, 9123 to Blue, 4758
5  change t at indexpath (0, 1) from Blue, 4758 to Red, 1870
6  change t at indexpath (2, 0) from Red, 1572 to Green, 5442
7  change t at indexpath (2, 0) from Red, 1611 to Green, 375
8  change t at indexpath (1, 1) from Green, 375 to Blue, 3606
9  change t at indexpath (0, 1) from Blue, 3606 to Green, 2811
10 change t at indexpath (1, 4) from Green, 2811 to Red, 1333
11 change t at indexpath (2, 0) from Red, 1333 to Red, 3113
12 change t at indexpath (2, 1) from Red, 3113 to Red, 8871
13 change t at indexpath (2, 1) from Red, 4207 to Blue, 8912
14 change t at indexpath (1, 5) from Green, 5442 to Blue, 4337
15 change t at indexpath (1, 4) from Green, 4693 to Green, 8097
16 change t at indexpath (1, 5) from Green, 8097 to Red, 5322
17 change t at indexpath (1, 4) from Green, 5505 to Red, 5858
18 change t at indexpath (1, 1) from Green, 963 to Blue, 3369
19 change t at indexpath (1, 4) from Green, 9741 to Red, 170
20 change t at indexpath (1, 1) from Green, 1239 to Blue, 4622
21 change t at indexpath (1, 1) from Green, 1857 to Green, 9121
22 change t at indexpath (1, 0) from Green, 172 to Blue, 657
23 change t at indexpath (1, 1) from Green, 9121 to Blue, 9750
24 change t at indexpath (1, 0) from Green, 8266 to Blue, 9105
   2015-07-01 23:51:12.185 Example[8803:277008] *** Assertion failure in -[UICollectionView _endItemAnimationsWithInvalidationContext:tentativelyForReordering:], /SourceCache/UIKit_Sim/UIKit-3347.44.2/UICollectionView.m:3863
   2015-07-01 23:51:12.185 Example[8803:277008] CoreData: error: Serious application error.  An exception was caught from the delegate of NSFetchedResultsController during a call to -controllerDidChangeContent:.  cannot move an item from a deleted section (1) with userInfo (null)
25 change t at indexpath (1, 0) from Red, 170 to Red, 3466
   2015-07-01 23:51:16.250 Example[8803:277008] *** Assertion failure in -[UICollectionViewData validateLayoutInRect:], /SourceCache/UIKit_Sim/UIKit-3347.44.2/UICollectionViewData.m:426
   2015-07-01 23:51:16.250 Example[8803:277008] CoreData: error: Serious application error.  An exception was caught from the delegate of NSFetchedResultsController during a call to -controllerDidChangeContent:.  UICollectionView received layout attributes for a cell with an index path that does not exist: <NSIndexPath: 0xc000000000000096> {length = 2, path = 2 - 0} with userInfo (null)
26 change t at indexpath (1, 0) from Red, 1870 to Green, 8637
27 change t at indexpath (1, 0) from Green, 8637 to Blue, 713
28 change t at indexpath (1, 0) from Red, 3466 to Green, 4643
alexzzp commented 9 years ago

The problem seems occurs when a section contains only a single cell, and you change the property that will move the cell to another section.

alexzzp commented 9 years ago

the problem only occurs in the collectionview, similar code will not cause any error in FetchedTableViewController

svenbacia commented 9 years ago

Hi, I had the same issue. The crash occurred in my case when I wanted to update a cell and move another one at the same time.

I was able to reproduce it with a simple UICollectionView without CoreData and JSQDataSourcesKit. I already reported a bug a few weeks ago and it seems like the bug was fixed (my radar got closed). So with the next update it should work again.

A workaround for this issue is by replacing line 131-133 in FetchedResultsDelegate.swift with collectionView?.reloadData()

alexzzp commented 9 years ago

A workaround for this issue is by replacing line 131-133 in FetchedResultsDelegate.swift with collectionView?.reloadData() ?

replace

    if let first = indexes.first, last = indexes.last {
        collectionView?.moveItemAtIndexPath(first, toIndexPath: last)
    }

to a simple line:

    collectionView?.reloadData()

it doen't work

svenbacia commented 9 years ago

Oh sorry, I meant collectionView?.reloadItemsAtIndexPaths(indexes) instead of collectionView?.reloadData().

jessesquires commented 9 years ago

awesome. thanks so much @alexzzp and @svenbacia !

i'll get this fixed soon.