Elgato_dark/IGListKit
1
0
Fork 0
mirror of https://github.com/Instagram/IGListKit synced 2026-05-23 17:28:22 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Resolve crash when using preferItemReloadsForSectionReloads and there is section movements

Browse source 
Summary:
section moves are extremely unsafe and crashy.

If we still use the `preferItemReloadsForSectionReloads` on, the item reloads won't be correct when we applies the same logic for cases without section moves.

It leads to crash:

```
2018-09-04 23:28:12.184875-0700 xctest[51440:12103918] *** 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 (0 inserted, 1 deleted) and plus or minus the number of items moved into or out of that section (0 moved in, 0 moved out).'
```

even if we change to use -reloadSections: instead, we still get crash like the following:

```
2018-09-04 23:21:14.129961-0700 Direct[45654:12078189] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'attempt to perform a delete and a move from the same section (25)'
```

Thus let's only add the moves.count == 0 check before using the prefer item reloads over section reload optimization.

Reviewed By: calimarkus

Differential Revision: D9661723

fbshipit-source-id: 8c3ddcd5178cb1aa58ccd9ae274cd8e5198f7609
This commit is contained in:
Zhisheng Huang 2018-09-05 14:02:09 -07:00 committed by Facebook Github Bot
parent eddb0a5f21
commit e6032c8107
2 changed files with 139 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
Source/IGListAdapterUpdater.m
View file

@ -327,7 +327,8 @@ static NSArray<NSIndexPath *> *convertSectionReloadToItemUpdates(NSIndexSet *sec
moves = [NSSet new];
}
if (self.preferItemReloadsForSectionReloads) {
// Item reloads are not safe, if any section moves happened
if (moves.count == 0 && self.preferItemReloadsForSectionReloads) {
[reloads enumerateIndexesUsingBlock:^(NSUInteger sectionIndex, BOOL * _Nonnull stop) {
NSMutableIndexSet *localIndexSet = [NSMutableIndexSet indexSetWithIndex:sectionIndex];
if (sectionIndex < [collectionView numberOfSections]

137
Tests/IGListAdapterUpdaterTests.m
View file

@ -13,6 +13,7 @@
#import "IGListAdapterUpdaterInternal.h"
#import "IGListTestUICollectionViewDataSource.h"
#import "IGTestObject.h"
#import "IGListMoveIndexInternal.h"
#define genExpectation [self expectationWithDescription:NSStringFromSelector(_cmd)]
#define waitExpectation [self waitForExpectationsWithTimeout:30 handler:nil]
@ -689,6 +690,8 @@
[mockDelegate verify];
}
# pragma mark - preferItemReloadsFroSectionReloads
- (void)test_whenReloadIsCalledWithSameItemCount_andPreferItemReload_updateIndexPathsHappen {
self.updater.preferItemReloadsForSectionReloads = YES;
@ -753,4 +756,138 @@
[mockDelegate verify];
}
- (void)test_whenReloadIsCalledWithSectionMoveAndUpdate_andPreferItemReload_deleteInsertMoveHappens {
self.updater.preferItemReloadsForSectionReloads = YES;
IGListBatchUpdateData *expectedBatchUpdateData = [[IGListBatchUpdateData alloc] initWithInsertSections:[NSIndexSet indexSetWithIndex:0]
deleteSections:[NSIndexSet indexSetWithIndex:1]
moveSections:[NSSet setWithArray:@[[[IGListMoveIndex alloc] initWithFrom:0 to:1]]]
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]];
id mockDelegate = [OCMockObject niceMockForProtocol:@protocol(IGListAdapterUpdaterDelegate)];
self.updater.delegate = mockDelegate;
[mockDelegate setExpectationOrderMatters:YES];
[[mockDelegate expect] listAdapterUpdater:self.updater willPerformBatchUpdatesWithCollectionView:self.collectionView];
[[mockDelegate expect] listAdapterUpdater:self.updater didPerformBatchUpdates:expectedBatchUpdateData collectionView:self.collectionView];
XCTestExpectation *expectation = genExpectation;
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@2] identifier:@"id2"]];
IGListToObjectBlock to = ^NSArray *{
// move section, and also update the item for "id2"
return @[[IGSectionObject sectionWithObjects:@[@22] identifier:@"id2"],
[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"]];
};
self.dataSource.sections = from;
[self.updater performUpdateWithCollectionViewBlock:[self collectionViewBlock] fromObjects:from toObjectsBlock:to animated:NO objectTransitionBlock:self.updateBlock completion:^(BOOL finished) {
[expectation fulfill];
}];
waitExpectation;
[mockDelegate verify];
}
- (void)test_whenReloadIsCalledWithSectionMoveAndUpdate_withDifferentSectionLength_andPreferItemReload_deleteInsertMoveHappens {
self.updater.preferItemReloadsForSectionReloads = YES;
IGListBatchUpdateData *expectedBatchUpdateData = [[IGListBatchUpdateData alloc] initWithInsertSections:[NSIndexSet indexSetWithIndex:0]
deleteSections:[NSIndexSet indexSetWithIndex:1]
moveSections:[NSSet setWithArray:@[[[IGListMoveIndex alloc] initWithFrom:0 to:1]]]
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]];
id mockDelegate = [OCMockObject niceMockForProtocol:@protocol(IGListAdapterUpdaterDelegate)];
self.updater.delegate = mockDelegate;
[mockDelegate setExpectationOrderMatters:YES];
[[mockDelegate expect] listAdapterUpdater:self.updater willPerformBatchUpdatesWithCollectionView:self.collectionView];
[[mockDelegate expect] listAdapterUpdater:self.updater didPerformBatchUpdates:expectedBatchUpdateData collectionView:self.collectionView];
XCTestExpectation *expectation = genExpectation;
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1, @2, @3] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@2] identifier:@"id2"]];
IGListToObjectBlock to = ^NSArray *{
// move section, and also update the item for "id2"
return @[[IGSectionObject sectionWithObjects:@[@22] identifier:@"id2"],
[IGSectionObject sectionWithObjects:@[@1, @2, @3] identifier:@"id1"]];
};
self.dataSource.sections = from;
[self.updater performUpdateWithCollectionViewBlock:[self collectionViewBlock] fromObjects:from toObjectsBlock:to animated:NO objectTransitionBlock:self.updateBlock completion:^(BOOL finished) {
[expectation fulfill];
}];
waitExpectation;
[mockDelegate verify];
}
- (void)test_whenReloadIsCalledWithSectionMoveAndUpdate_withThreeSections_deleteInsertMoveHappens {
self.updater.preferItemReloadsForSectionReloads = YES;
IGListBatchUpdateData *expectedBatchUpdateData = [[IGListBatchUpdateData alloc] initWithInsertSections:[NSIndexSet indexSetWithIndex:0]
deleteSections:[NSIndexSet indexSetWithIndex:1]
moveSections:[NSSet setWithArray:@[[[IGListMoveIndex alloc] initWithFrom:0 to:1]]]
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]];
id mockDelegate = [OCMockObject niceMockForProtocol:@protocol(IGListAdapterUpdaterDelegate)];
self.updater.delegate = mockDelegate;
[mockDelegate setExpectationOrderMatters:YES];
[[mockDelegate expect] listAdapterUpdater:self.updater willPerformBatchUpdatesWithCollectionView:self.collectionView];
[[mockDelegate expect] listAdapterUpdater:self.updater didPerformBatchUpdates:expectedBatchUpdateData collectionView:self.collectionView];
XCTestExpectation *expectation = genExpectation;
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@2] identifier:@"id2"],
[IGSectionObject sectionWithObjects:@[@3] identifier:@"id3"]];
IGListToObjectBlock to = ^NSArray *{
// move section, and also update the items for "id2"
return @[[IGSectionObject sectionWithObjects:@[@22, @23] identifier:@"id2"],
[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@3] identifier:@"id3"]];
};
self.dataSource.sections = from;
[self.updater performUpdateWithCollectionViewBlock:[self collectionViewBlock] fromObjects:from toObjectsBlock:to animated:NO objectTransitionBlock:self.updateBlock completion:^(BOOL finished) {
[expectation fulfill];
}];
waitExpectation;
[mockDelegate verify];
}
- (void)test_whenReloadIsCalledWithSectionInsertAndUpdate_andPreferItemReload_orderHappened {
self.updater.preferItemReloadsForSectionReloads = YES;
IGListBatchUpdateData *expectedBatchUpdateData = [[IGListBatchUpdateData alloc] initWithInsertSections:[NSIndexSet indexSetWithIndex:1]
deleteSections:[NSIndexSet new]
moveSections:[NSSet new]
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[[NSIndexPath indexPathForItem:0 inSection:0]]
moveIndexPaths:@[]];
id mockDelegate = [OCMockObject niceMockForProtocol:@protocol(IGListAdapterUpdaterDelegate)];
self.updater.delegate = mockDelegate;
[mockDelegate setExpectationOrderMatters:YES];
[[mockDelegate expect] listAdapterUpdater:self.updater willPerformBatchUpdatesWithCollectionView:self.collectionView];
[[mockDelegate expect] listAdapterUpdater:self.updater didPerformBatchUpdates:expectedBatchUpdateData collectionView:self.collectionView];
XCTestExpectation *expectation = genExpectation;
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"]];
IGListToObjectBlock to = ^NSArray *{
// insert new section id2, and also update for section id1
return @[[IGSectionObject sectionWithObjects:@[@2] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@22] identifier:@"id2"]];
};
self.dataSource.sections = from;
[self.updater performUpdateWithCollectionViewBlock:[self collectionViewBlock] fromObjects:from toObjectsBlock:to animated:NO objectTransitionBlock:self.updateBlock completion:^(BOOL finished) {
[expectation fulfill];
}];
waitExpectation;
[mockDelegate verify];
}
@end