From 3f9dea21e0aaa30329bd36a20edb61e364ab33dd Mon Sep 17 00:00:00 2001 From: Ryan Nystrom Date: Tue, 7 Mar 2017 09:22:22 -0800 Subject: [PATCH] Track batch updates in object and explicit state Summary: Batch updates are complicated b/c its unknown when the update block will actually execute. When executing the block, we want to collect inserts/deletes/reloads/moves (item and section). Allow mutations to happen synchronously outside of the update block. Tracking state will also help with the auto-diff where we need to allow re-entrant updates. Peeling off a chunk from #494 Issue fixed: #288 - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. Closes https://github.com/Instagram/IGListKit/pull/525 Reviewed By: jessesquires Differential Revision: D4656463 Pulled By: rnystrom fbshipit-source-id: 8f4d3dc21b03d595e02ee9ee9664277e8ead0531 --- CHANGELOG.md | 2 + IGListKit.xcodeproj/project.pbxproj | 22 +++++ Source/IGListAdapterUpdater.m | 89 ++++++++----------- .../Internal/IGListAdapterUpdaterInternal.h | 10 +-- Source/Internal/IGListBatchUpdateState.h | 17 ++++ Source/Internal/IGListBatchUpdates.h | 25 ++++++ Source/Internal/IGListBatchUpdates.m | 25 ++++++ Tests/IGListAdapterE2ETests.m | 37 ++++++++ Tests/Objects/IGTestDelegateController.h | 2 +- 9 files changed, 170 insertions(+), 59 deletions(-) create mode 100644 Source/Internal/IGListBatchUpdateState.h create mode 100644 Source/Internal/IGListBatchUpdates.h create mode 100644 Source/Internal/IGListBatchUpdates.m diff --git a/CHANGELOG.md b/CHANGELOG.md index e63a522ed..19d1615cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,8 @@ This release closes the [3.0.0 milestone](https://github.com/Instagram/IGListKit - Fix a crash when reusing collection views between embedded `IGListAdapter`s. [Ryan Nystrom](https://github.com/rnystrom) [(#517)](https://github.com/Instagram/IGListKit/pull/517) +- Only collect batch updates when explicitly inside the batch update block, execute them otherwise. Fixes dropped updates. Ryan Nystrom](https://github.com/rnystrom) [(#494)](https://github.com/Instagram/IGListKit/pull/494) + 2.1.0 ----- diff --git a/IGListKit.xcodeproj/project.pbxproj b/IGListKit.xcodeproj/project.pbxproj index db31fea1d..d600e5486 100644 --- a/IGListKit.xcodeproj/project.pbxproj +++ b/IGListKit.xcodeproj/project.pbxproj @@ -168,6 +168,14 @@ 2914BEE91DCD15F400C96401 /* IGTestNibSupplementaryView.xib in Resources */ = {isa = PBXBuildFile; fileRef = 2904861C1DCD02140007F41D /* IGTestNibSupplementaryView.xib */; }; 2914BEEA1DCD15F400C96401 /* IGTestNibSupplementaryView.xib in Resources */ = {isa = PBXBuildFile; fileRef = 2904861C1DCD02140007F41D /* IGTestNibSupplementaryView.xib */; }; 294AC6321DDE4C19002FCE5D /* IGListDiffResultTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 294AC6311DDE4C19002FCE5D /* IGListDiffResultTests.m */; }; + 297278BD1E6B58560099D8EA /* IGListBatchUpdates.h in Headers */ = {isa = PBXBuildFile; fileRef = 297278BB1E6B58560099D8EA /* IGListBatchUpdates.h */; }; + 297278BE1E6B58560099D8EA /* IGListBatchUpdates.h in Headers */ = {isa = PBXBuildFile; fileRef = 297278BB1E6B58560099D8EA /* IGListBatchUpdates.h */; }; + 297278BF1E6B58560099D8EA /* IGListBatchUpdates.m in Sources */ = {isa = PBXBuildFile; fileRef = 297278BC1E6B58560099D8EA /* IGListBatchUpdates.m */; }; + 297278C01E6B58560099D8EA /* IGListBatchUpdates.m in Sources */ = {isa = PBXBuildFile; fileRef = 297278BC1E6B58560099D8EA /* IGListBatchUpdates.m */; }; + 297278C11E6B58560099D8EA /* IGListBatchUpdates.m in Sources */ = {isa = PBXBuildFile; fileRef = 297278BC1E6B58560099D8EA /* IGListBatchUpdates.m */; }; + 297278C21E6B58560099D8EA /* IGListBatchUpdates.m in Sources */ = {isa = PBXBuildFile; fileRef = 297278BC1E6B58560099D8EA /* IGListBatchUpdates.m */; }; + 297278C41E6B59D50099D8EA /* IGListBatchUpdateState.h in Headers */ = {isa = PBXBuildFile; fileRef = 297278C31E6B59D50099D8EA /* IGListBatchUpdateState.h */; }; + 297278C51E6B59D50099D8EA /* IGListBatchUpdateState.h in Headers */ = {isa = PBXBuildFile; fileRef = 297278C31E6B59D50099D8EA /* IGListBatchUpdateState.h */; }; 298DDA1F1E3B0DC800F76F50 /* IGListCollectionViewLayout.h in Headers */ = {isa = PBXBuildFile; fileRef = 298DDA1D1E3B0DC800F76F50 /* IGListCollectionViewLayout.h */; settings = {ATTRIBUTES = (Public, ); }; }; 298DDA201E3B0DC800F76F50 /* IGListCollectionViewLayout.h in Headers */ = {isa = PBXBuildFile; fileRef = 298DDA1D1E3B0DC800F76F50 /* IGListCollectionViewLayout.h */; settings = {ATTRIBUTES = (Public, ); }; }; 298DDA211E3B0DC800F76F50 /* IGListCollectionViewLayout.mm in Sources */ = {isa = PBXBuildFile; fileRef = 298DDA1E1E3B0DC800F76F50 /* IGListCollectionViewLayout.mm */; }; @@ -373,6 +381,9 @@ 2904861F1DCD02750007F41D /* IGTestNibSupplementaryView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGTestNibSupplementaryView.m; sourceTree = ""; }; 294369B01DB1B7AE0025F6E7 /* IGTestNibCell.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = IGTestNibCell.xib; sourceTree = ""; }; 294AC6311DDE4C19002FCE5D /* IGListDiffResultTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListDiffResultTests.m; sourceTree = ""; }; + 297278BB1E6B58560099D8EA /* IGListBatchUpdates.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IGListBatchUpdates.h; sourceTree = ""; }; + 297278BC1E6B58560099D8EA /* IGListBatchUpdates.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListBatchUpdates.m; sourceTree = ""; }; + 297278C31E6B59D50099D8EA /* IGListBatchUpdateState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IGListBatchUpdateState.h; sourceTree = ""; }; 298DDA1D1E3B0DC800F76F50 /* IGListCollectionViewLayout.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IGListCollectionViewLayout.h; sourceTree = ""; }; 298DDA1E1E3B0DC800F76F50 /* IGListCollectionViewLayout.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = IGListCollectionViewLayout.mm; sourceTree = ""; }; 298DDA231E3B15EE00F76F50 /* IGListCollectionViewLayoutTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListCollectionViewLayoutTests.m; sourceTree = ""; }; @@ -591,6 +602,8 @@ 0B3B92B71E08D7F5008390ED /* IGListAdapterProxy.h */, 0B3B92B81E08D7F5008390ED /* IGListAdapterProxy.m */, 0B3B92B91E08D7F5008390ED /* IGListAdapterUpdaterInternal.h */, + 297278BB1E6B58560099D8EA /* IGListBatchUpdates.h */, + 297278BC1E6B58560099D8EA /* IGListBatchUpdates.m */, 0B3B92BA1E08D7F5008390ED /* IGListDisplayHandler.h */, 0B3B92BB1E08D7F5008390ED /* IGListDisplayHandler.m */, 0B3B92BC1E08D7F5008390ED /* IGListSectionControllerInternal.h */, @@ -601,6 +614,7 @@ 0B3B92C11E08D7F5008390ED /* IGListWorkingRangeHandler.mm */, 0B3B92C21E08D7F5008390ED /* UICollectionView+IGListBatchUpdateData.h */, 0B3B92C31E08D7F5008390ED /* UICollectionView+IGListBatchUpdateData.m */, + 297278C31E6B59D50099D8EA /* IGListBatchUpdateState.h */, ); path = Internal; sourceTree = ""; @@ -754,6 +768,7 @@ isa = PBXHeadersBuildPhase; buildActionMask = 2147483647; files = ( + 297278C51E6B59D50099D8EA /* IGListBatchUpdateState.h in Headers */, 989317641E0ED45900DB93B3 /* IGListCompatibility.h in Headers */, 0B3B92FB1E08D7F5008390ED /* IGListAdapterDataSource.h in Headers */, 0B3B92E91E08D7F5008390ED /* IGListIndexSetResultInternal.h in Headers */, @@ -794,6 +809,7 @@ 0B3B92F71E08D7F5008390ED /* IGListAdapter.h in Headers */, 0B3B931B1E08D7F5008390ED /* IGListSectionType.h in Headers */, 0B3B92E31E08D7F5008390ED /* IGListMoveIndexPath.h in Headers */, + 297278BE1E6B58560099D8EA /* IGListBatchUpdates.h in Headers */, 0B3B93111E08D7F5008390ED /* IGListReloadDataUpdater.h in Headers */, 298DDA201E3B0DC800F76F50 /* IGListCollectionViewLayout.h in Headers */, 0B3B92FF1E08D7F5008390ED /* IGListAdapterUpdater.h in Headers */, @@ -807,6 +823,7 @@ isa = PBXHeadersBuildPhase; buildActionMask = 2147483647; files = ( + 297278C41E6B59D50099D8EA /* IGListBatchUpdateState.h in Headers */, 989317631E0ED45900DB93B3 /* IGListCompatibility.h in Headers */, 0B3B92FA1E08D7F5008390ED /* IGListAdapterDataSource.h in Headers */, 0B3B92E81E08D7F5008390ED /* IGListIndexSetResultInternal.h in Headers */, @@ -847,6 +864,7 @@ 0B3B92F61E08D7F5008390ED /* IGListAdapter.h in Headers */, 0B3B931A1E08D7F5008390ED /* IGListSectionType.h in Headers */, 0B3B92E21E08D7F5008390ED /* IGListMoveIndexPath.h in Headers */, + 297278BD1E6B58560099D8EA /* IGListBatchUpdates.h in Headers */, 0B3B93101E08D7F5008390ED /* IGListReloadDataUpdater.h in Headers */, 298DDA1F1E3B0DC800F76F50 /* IGListCollectionViewLayout.h in Headers */, 0B3B92FE1E08D7F5008390ED /* IGListAdapterUpdater.h in Headers */, @@ -1215,6 +1233,7 @@ 298DDA221E3B0DC800F76F50 /* IGListCollectionViewLayout.mm in Sources */, 0B3B93311E08D7F5008390ED /* IGListAdapterProxy.m in Sources */, 0B3B92CD1E08D7F5008390ED /* IGListDiff.mm in Sources */, + 297278C11E6B58560099D8EA /* IGListBatchUpdates.m in Sources */, 0B3B931F1E08D7F5008390ED /* IGListSingleSectionController.m in Sources */, 0B3B92D51E08D7F5008390ED /* IGListIndexPathResult.m in Sources */, 0B3B93371E08D7F5008390ED /* IGListDisplayHandler.m in Sources */, @@ -1237,6 +1256,7 @@ 298DDA381E3B168E00F76F50 /* IGLayoutTestItem.m in Sources */, 885FE2361DC51B76009CE2B4 /* IGListStackSectionControllerTests.m in Sources */, 885FE2311DC51B76009CE2B4 /* IGListDisplayHandlerTests.m in Sources */, + 297278C21E6B58560099D8EA /* IGListBatchUpdates.m in Sources */, 0B40C5F41E01CBCB00378109 /* IGListCollectionViewTests.m in Sources */, 298DDA3B1E3B16F800F76F50 /* IGLayoutTestDataSource.m in Sources */, 29C474901DDF460500AE68CE /* IGListSectionMapTests.m in Sources */, @@ -1294,6 +1314,7 @@ 298DDA211E3B0DC800F76F50 /* IGListCollectionViewLayout.mm in Sources */, 0B3B93301E08D7F5008390ED /* IGListAdapterProxy.m in Sources */, 0B3B92CC1E08D7F5008390ED /* IGListDiff.mm in Sources */, + 297278BF1E6B58560099D8EA /* IGListBatchUpdates.m in Sources */, 0B3B931E1E08D7F5008390ED /* IGListSingleSectionController.m in Sources */, 0B3B92D41E08D7F5008390ED /* IGListIndexPathResult.m in Sources */, 0B3B93361E08D7F5008390ED /* IGListDisplayHandler.m in Sources */, @@ -1316,6 +1337,7 @@ 298DDA391E3B168F00F76F50 /* IGLayoutTestItem.m in Sources */, 88144F1C1D870EDC007C7F66 /* IGTestStackedDataSource.m in Sources */, 88144F181D870EDC007C7F66 /* IGTestDelegateController.m in Sources */, + 297278C01E6B58560099D8EA /* IGListBatchUpdates.m in Sources */, 1F0A68C51DF8D5B9009E8ADE /* IGListCollectionViewTests.m in Sources */, 298DDA3A1E3B16F600F76F50 /* IGLayoutTestDataSource.m in Sources */, 88144F0D1D870EDC007C7F66 /* IGListDisplayHandlerTests.m in Sources */, diff --git a/Source/IGListAdapterUpdater.m b/Source/IGListAdapterUpdater.m index 391202e50..d3678d40e 100644 --- a/Source/IGListAdapterUpdater.m +++ b/Source/IGListAdapterUpdater.m @@ -29,12 +29,7 @@ - (instancetype)init { _completionBlocks = [NSMutableArray new]; _itemUpdateBlocks = [NSMutableArray new]; - _reloadSections = [NSMutableIndexSet new]; - - _deleteIndexPaths = [NSMutableSet new]; - _insertIndexPaths = [NSMutableSet new]; - _moveIndexPaths = [NSMutableSet new]; - _reloadIndexPaths = [NSMutableSet new]; + _batchUpdatesCollector = [IGListBatchUpdates new]; _allowsBackgroundReloading = YES; } @@ -65,7 +60,7 @@ - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView { NSArray *itemUpdateBlocks = [self.itemUpdateBlocks copy]; // item updates must not send mutations to the collection view while we are reloading - self.batchUpdateOrReloadInProgress = YES; + self.state = IGListBatchUpdateStateExecutingBatchUpdateBlock; if (reloadUpdates) { reloadUpdates(); @@ -77,6 +72,8 @@ - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView { for (IGListItemUpdateBlock itemUpdateBlock in itemUpdateBlocks) { itemUpdateBlock(); } + + self.state = IGListBatchUpdateStateExecutedBatchUpdateBlock; // cleanup state before reloading and calling completion blocks [self cleanupState]; @@ -88,11 +85,11 @@ - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView { [collectionView layoutIfNeeded]; [delegate listAdapterUpdater:self didReloadDataWithCollectionView:collectionView]; - self.batchUpdateOrReloadInProgress = NO; - for (IGListUpdatingCompletion block in completionBlocks) { block(YES); } + + self.state = IGListBatchUpdateStateIdle; } static NSArray *objectsWithDuplicateIdentifiersRemoved(NSArray> *objects) { @@ -112,7 +109,7 @@ - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView { - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView { IGAssertMainThread(); - IGAssert(!self.batchUpdateOrReloadInProgress, @"should not call this when updating"); + 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 if (collectionView == nil) { @@ -132,6 +129,8 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView [self cleanupState]; void (^executeUpdateBlocks)() = ^{ + self.state = IGListBatchUpdateStateExecutingBatchUpdateBlock; + // run the update block so that the adapter can set its items. this makes sure that just before the update is // committed that the data source is updated to the /latest/ "toObjects". this makes the data source in sync // with the items that the updater is transitioning to @@ -145,9 +144,13 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView for (IGListItemUpdateBlock itemUpdateBlock in itemUpdateBlocks) { itemUpdateBlock(); } + + self.state = IGListBatchUpdateStateExecutedBatchUpdateBlock; }; void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) { + self.state = IGListBatchUpdateStateIdle; + for (IGListUpdatingCompletion block in completionBlocks) { block(finished); } @@ -157,12 +160,11 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView // reload data, execute completion blocks, and get outta here const BOOL iOS83OrLater = (NSFoundationVersionNumber >= NSFoundationVersionNumber_iOS_8_3); if (iOS83OrLater && self.allowsBackgroundReloading && collectionView.window == nil) { - [self beginPerformBatchUpdatestoObjects:toObjects]; + [self beginPerformBatchUpdatesToObjects:toObjects]; executeUpdateBlocks(); [self cleanupUpdateBlockState]; [self performBatchUpdatesItemBlockApplied]; [collectionView reloadData]; - [self endPerformBatchUpdates]; executeCompletionBlocks(YES); return; } @@ -183,11 +185,7 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView updateData = [self flushCollectionView:collectionView withDiffResult:result - reloadSections:[self.reloadSections copy] - deleteIndexPaths:[self.deleteIndexPaths copy] - insertIndexPaths:[self.insertIndexPaths copy] - moveIndexPaths:[self.moveIndexPaths copy] - reloadIndexPaths:[self.reloadIndexPaths copy] + batchUpdatesCollector:self.batchUpdatesCollector fromObjects:fromObjects]; [self cleanupUpdateBlockState]; @@ -195,8 +193,6 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView }; void (^completionBlock)(BOOL) = ^(BOOL finished) { - [self endPerformBatchUpdates]; - executeCompletionBlocks(finished); [delegate listAdapterUpdater:self didPerformBatchUpdates:updateData collectionView:collectionView]; @@ -207,7 +203,7 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView }; // disables multiple performBatchUpdates: from happening at the same time - [self beginPerformBatchUpdatestoObjects:toObjects]; + [self beginPerformBatchUpdatesToObjects:toObjects]; @try { [delegate listAdapterUpdater:self willPerformBatchUpdatesWithCollectionView:collectionView]; @@ -255,17 +251,13 @@ void convertReloadToDeleteInsert(NSMutableIndexSet *reloads, - (IGListBatchUpdateData *)flushCollectionView:(UICollectionView *)collectionView withDiffResult:(IGListIndexSetResult *)diffResult - reloadSections:(NSIndexSet *)reloadSections - deleteIndexPaths:(NSSet *)deleteIndexPaths - insertIndexPaths:(NSSet *)insertIndexPaths - moveIndexPaths:(NSSet *)moveFromIndexPaths - reloadIndexPaths:(NSSet *)reloadIndexPaths + batchUpdatesCollector:(IGListBatchUpdates *)batchUpdatesCollector fromObjects:(NSArray > *)fromObjects { NSSet *moves = [[NSSet alloc] initWithArray:diffResult.moves]; // combine section reloads from the diff and manual reloads via reloadItems: NSMutableIndexSet *reloads = [diffResult.updates mutableCopy]; - [reloads addIndexes:reloadSections]; + [reloads addIndexes:batchUpdatesCollector.sectionReloads]; NSMutableIndexSet *inserts = [diffResult.inserts mutableCopy]; NSMutableIndexSet *deletes = [diffResult.deletes mutableCopy]; @@ -284,27 +276,23 @@ - (IGListBatchUpdateData *)flushCollectionView:(UICollectionView *)collectionVie IGListBatchUpdateData *updateData = [[IGListBatchUpdateData alloc] initWithInsertSections:inserts deleteSections:deletes moveSections:moves - insertIndexPaths:insertIndexPaths - deleteIndexPaths:deleteIndexPaths - moveIndexPaths:moveFromIndexPaths - reloadIndexPaths:reloadIndexPaths]; + insertIndexPaths:batchUpdatesCollector.itemInserts + deleteIndexPaths:batchUpdatesCollector.itemDeletes + moveIndexPaths:batchUpdatesCollector.itemMoves + reloadIndexPaths:batchUpdatesCollector.itemReloads]; [collectionView ig_applyBatchUpdateData:updateData]; return updateData; } -- (void)beginPerformBatchUpdatestoObjects:(NSArray *)toObjects { - self.batchUpdateOrReloadInProgress = YES; +- (void)beginPerformBatchUpdatesToObjects:(NSArray *)toObjects { self.pendingTransitionToObjects = toObjects; + self.state = IGListBatchUpdateStateQueuedBatchUpdate; } - (void)performBatchUpdatesItemBlockApplied { self.pendingTransitionToObjects = nil; } -- (void)endPerformBatchUpdates { - self.batchUpdateOrReloadInProgress = NO; -} - - (void)cleanupState { self.queuedUpdateIsAnimated = YES; @@ -325,11 +313,7 @@ - (void)cleanupState { } - (void)cleanupUpdateBlockState { - [self.reloadSections removeAllIndexes]; - [self.deleteIndexPaths removeAllObjects]; - [self.insertIndexPaths removeAllObjects]; - [self.moveIndexPaths removeAllObjects]; - [self.reloadIndexPaths removeAllObjects]; + self.batchUpdatesCollector = [IGListBatchUpdates new]; } - (void)queueUpdateWithCollectionView:(UICollectionView *)collectionView { @@ -342,7 +326,8 @@ - (void)queueUpdateWithCollectionView:(UICollectionView *)collectionView { __weak __typeof__(self) weakSelf = self; dispatch_async(dispatch_get_main_queue(), ^{ - if (weakSelf.batchUpdateOrReloadInProgress || ![weakSelf hasChanges]) { + if (weakSelf.state != IGListBatchUpdateStateIdle + || ![weakSelf hasChanges]) { return; } @@ -441,8 +426,8 @@ - (void)insertItemsIntoCollectionView:(UICollectionView *)collectionView indexPa IGAssertMainThread(); IGParameterAssert(collectionView != nil); IGParameterAssert(indexPaths != nil); - if (self.batchUpdateOrReloadInProgress) { - [self.insertIndexPaths addObjectsFromArray:indexPaths]; + if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { + [self.batchUpdatesCollector.itemInserts addObjectsFromArray:indexPaths]; } else { [self.delegate listAdapterUpdater:self willInsertIndexPaths:indexPaths collectionView:collectionView]; [collectionView insertItemsAtIndexPaths:indexPaths]; @@ -453,8 +438,8 @@ - (void)deleteItemsFromCollectionView:(UICollectionView *)collectionView indexPa IGAssertMainThread(); IGParameterAssert(collectionView != nil); IGParameterAssert(indexPaths != nil); - if (self.batchUpdateOrReloadInProgress) { - [self.deleteIndexPaths addObjectsFromArray:indexPaths]; + if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { + [self.batchUpdatesCollector.itemDeletes addObjectsFromArray:indexPaths]; } else { [self.delegate listAdapterUpdater:self willDeleteIndexPaths:indexPaths collectionView:collectionView]; [collectionView deleteItemsAtIndexPaths:indexPaths]; @@ -464,9 +449,9 @@ - (void)deleteItemsFromCollectionView:(UICollectionView *)collectionView indexPa - (void)moveItemInCollectionView:(UICollectionView *)collectionView fromIndexPath:(NSIndexPath *)fromIndexPath toIndexPath:(NSIndexPath *)toIndexPath { - if (self.batchUpdateOrReloadInProgress) { + if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { IGListMoveIndexPath *move = [[IGListMoveIndexPath alloc] initWithFrom:fromIndexPath to:toIndexPath]; - [self.moveIndexPaths addObject:move]; + [self.batchUpdatesCollector.itemMoves addObject:move]; } else { [self.delegate listAdapterUpdater:self willMoveFromIndexPath:fromIndexPath toIndexPath:toIndexPath collectionView:collectionView]; [collectionView moveItemAtIndexPath:fromIndexPath toIndexPath:toIndexPath]; @@ -477,8 +462,8 @@ - (void)reloadItemsInCollectionView:(UICollectionView *)collectionView indexPath IGAssertMainThread(); IGParameterAssert(collectionView != nil); IGParameterAssert(indexPaths != nil); - if (self.batchUpdateOrReloadInProgress) { - [self.reloadIndexPaths addObjectsFromArray:indexPaths]; + if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { + [self.batchUpdatesCollector.itemReloads addObjectsFromArray:indexPaths]; } else { [self.delegate listAdapterUpdater:self willReloadIndexPaths:indexPaths collectionView:collectionView]; [collectionView reloadItemsAtIndexPaths:indexPaths]; @@ -506,8 +491,8 @@ - (void)reloadCollectionView:(UICollectionView *)collectionView sections:(NSInde IGAssertMainThread(); IGParameterAssert(collectionView != nil); IGParameterAssert(sections != nil); - if (self.batchUpdateOrReloadInProgress) { - [self.reloadSections addIndexes:sections]; + if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { + [self.batchUpdatesCollector.sectionReloads addIndexes:sections]; } else { [self.delegate listAdapterUpdater:self willReloadSections:sections collectionView:collectionView]; [collectionView reloadSections:sections]; diff --git a/Source/Internal/IGListAdapterUpdaterInternal.h b/Source/Internal/IGListAdapterUpdaterInternal.h index 3b1ec0742..b376ea0fd 100644 --- a/Source/Internal/IGListAdapterUpdaterInternal.h +++ b/Source/Internal/IGListAdapterUpdaterInternal.h @@ -13,6 +13,8 @@ #import #import "IGListAdapterUpdater.h" +#import "IGListBatchUpdateState.h" +#import "IGListBatchUpdates.h" NS_ASSUME_NONNULL_BEGIN @@ -32,11 +34,7 @@ FOUNDATION_EXTERN void convertReloadToDeleteInsert(NSMutableIndexSet *reloads, @property (nonatomic, assign) BOOL queuedUpdateIsAnimated; -@property (nonatomic, strong, readonly) NSMutableSet *deleteIndexPaths; -@property (nonatomic, strong, readonly) NSMutableSet *insertIndexPaths; -@property (nonatomic, strong, readonly) NSMutableSet *reloadIndexPaths; -@property (nonatomic, strong, readonly) NSMutableSet *moveIndexPaths; -@property (nonatomic, strong, readonly) NSMutableIndexSet *reloadSections; +@property (nonatomic, strong) IGListBatchUpdates *batchUpdatesCollector; @property (nonatomic, copy, nullable) IGListObjectTransitionBlock objectTransitionBlock; @property (nonatomic, copy, nullable) NSMutableArray *itemUpdateBlocks; @@ -44,7 +42,7 @@ FOUNDATION_EXTERN void convertReloadToDeleteInsert(NSMutableIndexSet *reloads, @property (nonatomic, copy, nullable) IGListReloadUpdateBlock reloadUpdates; @property (nonatomic, assign, getter=hasQueuedReloadData) BOOL queuedReloadData; -@property (nonatomic, assign) BOOL batchUpdateOrReloadInProgress; +@property (nonatomic, assign) IGListBatchUpdateState state; - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView; - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView; diff --git a/Source/Internal/IGListBatchUpdateState.h b/Source/Internal/IGListBatchUpdateState.h new file mode 100644 index 000000000..5b00ed26a --- /dev/null +++ b/Source/Internal/IGListBatchUpdateState.h @@ -0,0 +1,17 @@ +/** + * Copyright (c) 2016-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import + +typedef NS_ENUM (NSInteger, IGListBatchUpdateState) { + IGListBatchUpdateStateIdle, + IGListBatchUpdateStateQueuedBatchUpdate, + IGListBatchUpdateStateExecutingBatchUpdateBlock, + IGListBatchUpdateStateExecutedBatchUpdateBlock, +}; diff --git a/Source/Internal/IGListBatchUpdates.h b/Source/Internal/IGListBatchUpdates.h new file mode 100644 index 000000000..3a6cecf80 --- /dev/null +++ b/Source/Internal/IGListBatchUpdates.h @@ -0,0 +1,25 @@ +/** + * Copyright (c) 2016-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import + +#import + +@class IGListMoveIndexPath; + +IGLK_SUBCLASSING_RESTRICTED +@interface IGListBatchUpdates : NSObject + +@property (nonatomic, strong, readonly) NSMutableIndexSet *sectionReloads; +@property (nonatomic, strong, readonly) NSMutableSet *itemInserts; +@property (nonatomic, strong, readonly) NSMutableSet *itemDeletes; +@property (nonatomic, strong, readonly) NSMutableSet *itemReloads; +@property (nonatomic, strong, readonly) NSMutableSet *itemMoves; + +@end diff --git a/Source/Internal/IGListBatchUpdates.m b/Source/Internal/IGListBatchUpdates.m new file mode 100644 index 000000000..f473cc5e7 --- /dev/null +++ b/Source/Internal/IGListBatchUpdates.m @@ -0,0 +1,25 @@ +/** + * Copyright (c) 2016-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import "IGListBatchUpdates.h" + +@implementation IGListBatchUpdates + +- (instancetype)init { + if (self = [super init]) { + _sectionReloads = [NSMutableIndexSet new]; + _itemInserts = [NSMutableSet new]; + _itemMoves = [NSMutableSet new]; + _itemReloads = [NSMutableSet new]; + _itemDeletes = [NSMutableSet new]; + } + return self; +} + +@end diff --git a/Tests/IGListAdapterE2ETests.m b/Tests/IGListAdapterE2ETests.m index d9e3c5455..40f03d12e 100644 --- a/Tests/IGListAdapterE2ETests.m +++ b/Tests/IGListAdapterE2ETests.m @@ -1436,4 +1436,41 @@ - (void)test_whenAdaptersSwapCollectionViews_ { [self waitForExpectationsWithTimeout:15 handler:nil]; } +- (void)test_whenDidUpdateAsyncReloads_withBatchUpdatesInProgress_thatReloadIsExecuted { + [self setupWithObjects:@[ + genTestObject(@1, @1) + ]]; + + IGTestDelegateController *section = [self.adapter sectionControllerForObject:self.dataSource.objects[0]]; + + XCTestExpectation *expectation1 = genExpectation; + __weak __typeof__(section) weakSection = section; + section.itemUpdateBlock = ^{ + // currently inside -[IGListSectionType didUpdateToObject:], change the item (note: NEVER do this) manually + // so that the data powering numberOfItems changes (1 to 2). dispatch_async the update to skip outside of the + // -[UICollectionView performBatchUpdates:completion:] block execution + dispatch_async(dispatch_get_main_queue(), ^{ + weakSection.item = genTestObject(@1, @2); + [weakSection.collectionContext reloadSectionController:weakSection]; + [expectation1 fulfill]; + XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 2); + }); + }; + + // add an object so that a batch update is triggered (diff result has changes) + self.dataSource.objects = @[ + genTestObject(@1, @1), + genTestObject(@2, @1) + ]; + + XCTestExpectation *expectation2 = genExpectation; + [self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) { + // verify that the section still has 2 items since this completion executes AFTER the reload block above + XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 2); + [expectation2 fulfill]; + }]; + + [self waitForExpectationsWithTimeout:15 handler:nil]; +} + @end diff --git a/Tests/Objects/IGTestDelegateController.h b/Tests/Objects/IGTestDelegateController.h index 2da61bd6d..bcfa3651e 100644 --- a/Tests/Objects/IGTestDelegateController.h +++ b/Tests/Objects/IGTestDelegateController.h @@ -15,7 +15,7 @@ @interface IGTestDelegateController : IGListSectionController -@property (nonatomic, strong, readonly) IGTestObject *item; +@property (nonatomic, strong) IGTestObject *item; @property (nonatomic, assign) CGFloat height;