diff --git a/CHANGELOG.md b/CHANGELOG.md index fedd5b98..69bee606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag - Experimental fix to get the `UICollectionView` for batch updating immediately before applying the update. [Ryan Nystrom](https://github.com/rnystrom) (tbd) + +- `[IGListAdapterUpdater performBatchUpdatesWithCollectionViewBlock:]` and `[IGListAdapterUpdater performReloadDataWithCollectionViewBlock:]` clean state and run completion blocks if their `UICollectionView` is nil. [Brandon Darin](https://github.com/jbd1030) (tbd) + 3.4.0 ----- diff --git a/Source/IGListAdapterUpdater.m b/Source/IGListAdapterUpdater.m index 6aff2726..19c42afa 100644 --- a/Source/IGListAdapterUpdater.m +++ b/Source/IGListAdapterUpdater.m @@ -44,13 +44,7 @@ - (void)performReloadDataWithCollectionViewBlock:(IGListCollectionViewBlock)collectionViewBlock { IGAssertMainThread(); - - // bail early if the collection view has been deallocated in the time since the update was queued - UICollectionView *collectionView = collectionViewBlock(); - if (collectionView == nil) { - return; - } - + id delegate = self.delegate; void (^reloadUpdates)(void) = self.reloadUpdates; IGListBatchUpdates *batchUpdates = self.batchUpdates; @@ -58,6 +52,22 @@ [self cleanStateBeforeUpdates]; + void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) { + for (IGListUpdatingCompletion block in completionBlocks) { + block(finished); + } + + self.state = IGListBatchUpdateStateIdle; + }; + + // bail early if the collection view has been deallocated in the time since the update was queued + UICollectionView *collectionView = collectionViewBlock(); + if (collectionView == nil) { + [self _cleanStateAfterUpdates]; + executeCompletionBlocks(NO); + return; + } + // item updates must not send mutations to the collection view while we are reloading self.state = IGListBatchUpdateStateExecutingBatchUpdateBlock; @@ -86,23 +96,13 @@ [collectionView layoutIfNeeded]; [delegate listAdapterUpdater:self didReloadDataWithCollectionView:collectionView]; - for (IGListUpdatingCompletion block in completionBlocks) { - block(YES); - } - - self.state = IGListBatchUpdateStateIdle; + executeCompletionBlocks(YES); } - (void)performBatchUpdatesWithCollectionViewBlock:(IGListCollectionViewBlock)collectionViewBlock { IGAssertMainThread(); IGAssert(self.state == IGListBatchUpdateStateIdle, @"Should not call batch updates when state isn't idle"); - // bail early if the collection view has been deallocated in the time since the update was queued - UICollectionView *collectionView = collectionViewBlock(); - if (collectionView == nil) { - return; - } - // create local variables so we can immediately clean our state but pass these items into the batch update block id delegate = self.delegate; NSArray *fromObjects = [self.fromObjects copy]; @@ -112,6 +112,26 @@ const BOOL animated = self.queuedUpdateIsAnimated; IGListBatchUpdates *batchUpdates = self.batchUpdates; + // clean up all state so that new updates can be coalesced while the current update is in flight + [self cleanStateBeforeUpdates]; + + void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) { + self.applyingUpdateData = nil; + self.state = IGListBatchUpdateStateIdle; + + for (IGListUpdatingCompletion block in completionBlocks) { + block(finished); + } + }; + + // bail early if the collection view has been deallocated in the time since the update was queued + UICollectionView *collectionView = collectionViewBlock(); + if (collectionView == nil) { + [self _cleanStateAfterUpdates]; + executeCompletionBlocks(NO); + return; + } + NSArray *toObjects = nil; if (toObjectsBlock != nil) { toObjects = objectsWithDuplicateIdentifiersRemoved(toObjectsBlock()); @@ -125,9 +145,6 @@ } #endif - // clean up all state so that new updates can be coalesced while the current update is in flight - [self cleanStateBeforeUpdates]; - void (^executeUpdateBlocks)(void) = ^{ self.state = IGListBatchUpdateStateExecutingBatchUpdateBlock; @@ -152,15 +169,6 @@ self.state = IGListBatchUpdateStateExecutedBatchUpdateBlock; }; - void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) { - self.applyingUpdateData = nil; - self.state = IGListBatchUpdateStateIdle; - - for (IGListUpdatingCompletion block in completionBlocks) { - block(finished); - } - }; - void (^reloadDataFallback)(void) = ^{ executeUpdateBlocks(); [self _cleanStateAfterUpdates]; diff --git a/Tests/IGListAdapterE2ETests.m b/Tests/IGListAdapterE2ETests.m index c3b36d63..d16fadce 100644 --- a/Tests/IGListAdapterE2ETests.m +++ b/Tests/IGListAdapterE2ETests.m @@ -1881,4 +1881,96 @@ [self waitForExpectationsWithTimeout:30 handler:nil]; } +- (void)test_whenCollectionViewBecomesNilDuringPerformUpdates_thatStateCleanedCorrectly { + [self setupWithObjects:@[ + genTestObject(@1, @1) + ]]; + self.adapter.experiments |= IGListExperimentGetCollectionViewAtUpdate; + + // perform update on listAdapter + XCTestExpectation *expectation1 = genExpectation; + [self.adapter performUpdatesAnimated:NO completion:^(BOOL finished) { + [expectation1 fulfill]; + }]; + [self waitForExpectationsWithTimeout:30 handler:nil]; + + // update the underlying contents before performing another update + self.dataSource.objects = @[ + genTestObject(@1, @1), + genTestObject(@2, @1) + ]; + + // perform update, but set the listAdapter's collectionView to nil during the update + XCTestExpectation *expectation2 = genExpectation; + [self.adapter performUpdatesAnimated:NO completion:^(BOOL finished) { + [expectation2 fulfill]; + }]; + self.adapter.collectionView = nil; + [self waitForExpectationsWithTimeout:30 handler:nil]; + + // add a new collectionView to the listAdapter + UICollectionView *collectionView2 = [[UICollectionView alloc] initWithFrame:self.window.frame collectionViewLayout:[UICollectionViewFlowLayout new]]; + [self.window addSubview:collectionView2]; + self.adapter.collectionView = collectionView2; + + // update the underlying contents before performing update + self.dataSource.objects = @[ + genTestObject(@1, @1), + genTestObject(@2, @1), + genTestObject(@3, @1) + ]; + + // perform update on listAdapter (now with a non-nil collectionView) + XCTestExpectation *expectation3 = genExpectation; + [self.adapter performUpdatesAnimated:NO completion:^(BOOL finished) { + [expectation3 fulfill]; + }]; + [self waitForExpectationsWithTimeout:30 handler:nil]; +} + +- (void)test_whenCollectionViewBecomesNilDuringReloadData_thatStateCleanedCorrectly { + [self setupWithObjects:@[ + genTestObject(@1, @1) + ]]; + self.adapter.experiments |= IGListExperimentGetCollectionViewAtUpdate; + + // reload data on listAdapter + XCTestExpectation *expectation1 = genExpectation; + [self.adapter reloadDataWithCompletion:^(BOOL finished) { + [expectation1 fulfill]; + }]; + [self waitForExpectationsWithTimeout:30 handler:nil]; + + // update the underlying contents before reloading again + self.dataSource.objects = @[ + genTestObject(@1, @1), + genTestObject(@2, @1) + ]; + + // reload data, but set the listAdapter's collectionView to nil during the update + XCTestExpectation *expectation2 = genExpectation; + [self.adapter reloadDataWithCompletion:^(BOOL finished) { + [expectation2 fulfill]; + }]; + self.adapter.collectionView = nil; + [self waitForExpectationsWithTimeout:30 handler:nil]; + + // add a new collectionView to the listAdapter + UICollectionView *collectionView2 = [[UICollectionView alloc] initWithFrame:self.window.frame collectionViewLayout:[UICollectionViewFlowLayout new]]; + [self.window addSubview:collectionView2]; + self.adapter.collectionView = collectionView2; + self.dataSource.objects = @[ + genTestObject(@1, @1), + genTestObject(@2, @1), + genTestObject(@3, @1) + ]; + + // reload data on listAdapter (now with a non-nil collectionView) + XCTestExpectation *expectation3 = genExpectation; + [self.adapter reloadDataWithCompletion:^(BOOL finished) { + [expectation3 fulfill]; + }]; + [self waitForExpectationsWithTimeout:30 handler:nil]; +} + @end