diff --git a/Source/IGListKit/IGListAdapterUpdater.m b/Source/IGListKit/IGListAdapterUpdater.m index 2b94169c..301f784f 100644 --- a/Source/IGListKit/IGListAdapterUpdater.m +++ b/Source/IGListKit/IGListAdapterUpdater.m @@ -231,8 +231,12 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); moveIndexPaths:@[]]; } else { self.applyingUpdateData = IGListApplyUpdatesToCollectionView(collectionView, - result, - self.batchUpdates, + result, + self.batchUpdates.sectionReloads, + self.batchUpdates.itemInserts, + self.batchUpdates.itemDeletes, + self.batchUpdates.itemReloads, + self.batchUpdates.itemMoves, fromObjects, self.sectionMovesAsDeletesInserts, self.preferItemReloadsForSectionReloads); diff --git a/Source/IGListKit/IGListExperimentalAdapterUpdater.m b/Source/IGListKit/IGListExperimentalAdapterUpdater.m index cab933b3..27c29cc3 100644 --- a/Source/IGListKit/IGListExperimentalAdapterUpdater.m +++ b/Source/IGListKit/IGListExperimentalAdapterUpdater.m @@ -38,7 +38,7 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); // the default is to use animations unless NO is passed _queuedUpdateIsAnimated = YES; _completionBlocks = [NSMutableArray new]; - _batchUpdates = [IGListBatchUpdates new]; + _itemUpdateBlocks = [NSMutableArray new]; _allowsBackgroundReloading = YES; _allowsReloadingOnTooManyUpdates = YES; } @@ -49,7 +49,7 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); - (BOOL)hasChanges { return self.hasQueuedReloadData - || [self.batchUpdates hasChanges] + || self.itemUpdateBlocks.count > 0 || self.dataBlock != nil; } @@ -58,8 +58,8 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); id delegate = self.delegate; void (^reloadUpdates)(void) = self.reloadUpdates; - IGListBatchUpdates *batchUpdates = self.batchUpdates; - NSMutableArray *completionBlocks = [self.completionBlocks mutableCopy]; + NSArray *itemUpdateBlocks = [self.itemUpdateBlocks copy]; + NSArray *completionBlocks = [self.completionBlocks copy]; [self cleanStateBeforeUpdates]; @@ -68,13 +68,20 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); block(finished); } + // Execute any completion blocks from item updates. Added after item blocks are executed in order to capture any + // re-entrant updates. + NSArray *inUpdateCompletionBlocks = [self.inUpdateCompletionBlocks copy]; + for (IGListUpdatingCompletion block in inUpdateCompletionBlocks) { + block(finished); + } + + [self _cleanStateAfterUpdates]; 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); [_delegate listAdapterUpdater:self didFinishWithoutUpdatesWithCollectionView:collectionView]; return; @@ -90,18 +97,12 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); // execute all stored item update blocks even if we are just calling reloadData. the actual collection view // mutations will be discarded, but clients are encouraged to put their actual /data/ mutations inside the // update block as well, so if we don't execute the block the changes will never happen - for (IGListItemUpdateBlock itemUpdateBlock in batchUpdates.itemUpdateBlocks) { + for (IGListItemUpdateBlock itemUpdateBlock in itemUpdateBlocks) { itemUpdateBlock(); } - // add any completion blocks from item updates. added after item blocks are executed in order to capture any - // re-entrant updates - [completionBlocks addObjectsFromArray:batchUpdates.itemCompletionBlocks]; - self.state = IGListBatchUpdateStateExecutedBatchUpdateBlock; - [self _cleanStateAfterUpdates]; - [delegate listAdapterUpdater:self willReloadDataWithCollectionView:collectionView isFallbackReload:NO]; [collectionView reloadData]; [collectionView.collectionViewLayout invalidateLayout]; @@ -119,28 +120,35 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); id delegate = self.delegate; IGListTransitionDataBlock dataBlock = [self.dataBlock copy]; IGListTransitionDataApplyBlock applyDataBlock = [self.applyDataBlock copy]; - NSMutableArray *completionBlocks = [self.completionBlocks mutableCopy]; + NSArray *completionBlocks = [self.completionBlocks copy]; const BOOL animated = self.queuedUpdateIsAnimated; const BOOL allowsReloadingOnTooManyUpdates = self.allowsReloadingOnTooManyUpdates; const IGListExperiment experiments = self.experiments; - IGListBatchUpdates *batchUpdates = self.batchUpdates; + NSArray *itemUpdateBlocks = [self.itemUpdateBlocks copy]; // 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); } + + // Execute any completion blocks from item updates. Added after item blocks are executed in order to capture any + // re-entrant updates. + NSArray *inUpdateCompletionBlocks = [self.inUpdateCompletionBlocks copy]; + for (IGListUpdatingCompletion block in inUpdateCompletionBlocks) { + block(finished); + } + + [self _cleanStateAfterUpdates]; + self.applyingUpdateData = nil; + 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); [_delegate listAdapterUpdater:self didFinishWithoutUpdatesWithCollectionView:collectionView]; return; @@ -177,21 +185,16 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); // execute each item update block which should make calls like insert, delete, and reload for index paths // we collect all mutations in corresponding sets on self, then filter based on UICollectionView shortcomings // call after the objectTransitionBlock so section level mutations happen before any items - for (IGListItemUpdateBlock itemUpdateBlock in batchUpdates.itemUpdateBlocks) { + for (IGListItemUpdateBlock itemUpdateBlock in itemUpdateBlocks) { itemUpdateBlock(); } - // add any completion blocks from item updates. added after item blocks are executed in order to capture any - // re-entrant updates - [completionBlocks addObjectsFromArray:batchUpdates.itemCompletionBlocks]; - self.state = IGListBatchUpdateStateExecutedBatchUpdateBlock; }; void (^reloadDataFallback)(void) = ^{ [delegate listAdapterUpdater:self willReloadDataWithCollectionView:collectionView isFallbackReload:YES]; executeUpdateBlocks(); - [self _cleanStateAfterUpdates]; [collectionView reloadData]; [collectionView layoutIfNeeded]; executeCompletionBlocks(YES); @@ -234,13 +237,15 @@ typedef void (^IGListAdapterUpdaterCompletionBlock)(BOOL); } else { self.applyingUpdateData = IGListApplyUpdatesToCollectionView(collectionView, result, - self.batchUpdates, + self.inUpdateItemCollector.sectionReloads, + self.inUpdateItemCollector.itemInserts, + self.inUpdateItemCollector.itemDeletes, + self.inUpdateItemCollector.itemReloads, + self.inUpdateItemCollector.itemMoves, fromObjects, self.sectionMovesAsDeletesInserts, self.preferItemReloadsForSectionReloads); } - - [self _cleanStateAfterUpdates]; }; // block used as the second param of -[UICollectionView performBatchUpdates:completion:] @@ -329,10 +334,15 @@ willPerformBatchUpdatesWithCollectionView:collectionView // removes all object completion blocks. done before updates to start collecting completion blocks for coalesced // or re-entrant object updates [self.completionBlocks removeAllObjects]; + + [self.itemUpdateBlocks removeAllObjects]; + self.inUpdateCompletionBlocks = [NSMutableArray new]; + self.inUpdateItemCollector = [IGListItemUpdatesCollector new]; } - (void)_cleanStateAfterUpdates { - self.batchUpdates = [IGListBatchUpdates new]; + self.inUpdateCompletionBlocks = nil; + self.inUpdateItemCollector = nil; } - (void)_queueUpdateWithCollectionViewBlock:(IGListCollectionViewBlock)collectionViewBlock { @@ -427,17 +437,19 @@ static NSUInteger IGListIdentifierHash(const void *item, NSUInteger (*size)(cons IGParameterAssert(collectionViewBlock != nil); IGParameterAssert(itemUpdates != nil); - IGListBatchUpdates *batchUpdates = self.batchUpdates; - if (completion != nil) { - [batchUpdates.itemCompletionBlocks addObject:completion]; - } - // if already inside the execution of the update block, immediately unload the itemUpdates block. // the completion blocks are executed later in the lifecycle, so that still needs to be added to the batch if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { + if (completion != nil) { + [self.inUpdateCompletionBlocks addObject:completion]; + } itemUpdates(); } else { - [batchUpdates.itemUpdateBlocks addObject:itemUpdates]; + if (completion != nil) { + [self.completionBlocks addObject:completion]; + } + + [self.itemUpdateBlocks addObject:itemUpdates]; // disabled animations will always take priority // reset to YES in -cleanupState @@ -452,7 +464,7 @@ static NSUInteger IGListIdentifierHash(const void *item, NSUInteger (*size)(cons IGParameterAssert(collectionView != nil); IGParameterAssert(indexPaths != nil); if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { - [self.batchUpdates.itemInserts addObjectsFromArray:indexPaths]; + [self.inUpdateItemCollector.itemInserts addObjectsFromArray:indexPaths]; } else { [self.delegate listAdapterUpdater:self willInsertIndexPaths:indexPaths collectionView:collectionView]; [collectionView insertItemsAtIndexPaths:indexPaths]; @@ -464,7 +476,7 @@ static NSUInteger IGListIdentifierHash(const void *item, NSUInteger (*size)(cons IGParameterAssert(collectionView != nil); IGParameterAssert(indexPaths != nil); if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { - [self.batchUpdates.itemDeletes addObjectsFromArray:indexPaths]; + [self.inUpdateItemCollector.itemDeletes addObjectsFromArray:indexPaths]; } else { [self.delegate listAdapterUpdater:self willDeleteIndexPaths:indexPaths collectionView:collectionView]; [collectionView deleteItemsAtIndexPaths:indexPaths]; @@ -476,7 +488,7 @@ static NSUInteger IGListIdentifierHash(const void *item, NSUInteger (*size)(cons toIndexPath:(NSIndexPath *)toIndexPath { if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { IGListMoveIndexPath *move = [[IGListMoveIndexPath alloc] initWithFrom:fromIndexPath to:toIndexPath]; - [self.batchUpdates.itemMoves addObject:move]; + [self.inUpdateItemCollector.itemMoves addObject:move]; } else { [self.delegate listAdapterUpdater:self willMoveFromIndexPath:fromIndexPath toIndexPath:toIndexPath collectionView:collectionView]; [collectionView moveItemAtIndexPath:fromIndexPath toIndexPath:toIndexPath]; @@ -488,7 +500,7 @@ static NSUInteger IGListIdentifierHash(const void *item, NSUInteger (*size)(cons toIndexPath:(NSIndexPath *)toIndexPath { if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { IGListReloadIndexPath *reload = [[IGListReloadIndexPath alloc] initWithFromIndexPath:fromIndexPath toIndexPath:toIndexPath]; - [self.batchUpdates.itemReloads addObject:reload]; + [self.inUpdateItemCollector.itemReloads addObject:reload]; } else { [self.delegate listAdapterUpdater:self willReloadIndexPaths:@[fromIndexPath] collectionView:collectionView]; [collectionView reloadItemsAtIndexPaths:@[fromIndexPath]]; @@ -557,7 +569,7 @@ static NSUInteger IGListIdentifierHash(const void *item, NSUInteger (*size)(cons IGParameterAssert(collectionView != nil); IGParameterAssert(sections != nil); if (self.state == IGListBatchUpdateStateExecutingBatchUpdateBlock) { - [self.batchUpdates.sectionReloads addIndexes:sections]; + [self.inUpdateItemCollector.sectionReloads addIndexes:sections]; } else { [self.delegate listAdapterUpdater:self willReloadSections:sections collectionView:collectionView]; [collectionView reloadSections:sections]; diff --git a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h index f1ef9964..66862ab7 100644 --- a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h +++ b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h @@ -8,23 +8,28 @@ #import @class IGListBatchUpdateData; -@class IGListBatchUpdates; @class IGListIndexSetResult; +@class IGListReloadIndexPath; +@class IGListMoveIndexPath; @protocol IGListDiffable; NS_ASSUME_NONNULL_BEGIN void IGListConvertReloadToDeleteInsert(NSMutableIndexSet *reloads, - NSMutableIndexSet *deletes, - NSMutableIndexSet *inserts, - IGListIndexSetResult *result, - NSArray> *fromObjects); + NSMutableIndexSet *deletes, + NSMutableIndexSet *inserts, + IGListIndexSetResult *result, + NSArray> *fromObjects); IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *collectionView, - IGListIndexSetResult *diffResult, - IGListBatchUpdates *batchUpdates, - NSArray> *fromObjects, - BOOL sectionMovesAsDeletesInserts, - BOOL preferItemReloadsForSectionReloads); + IGListIndexSetResult *diffResult, + NSMutableIndexSet *sectionReloads, + NSMutableArray *itemInserts, + NSMutableArray *itemDeletes, + NSMutableArray *itemReloads, + NSMutableArray *itemMoves, + NSArray> *fromObjects, + BOOL sectionMovesAsDeletesInserts, + BOOL preferItemReloadsForSectionReloads); NS_ASSUME_NONNULL_END diff --git a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m index ef1bb665..1c51b38b 100644 --- a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m +++ b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m @@ -12,7 +12,6 @@ #import #import -#import "IGListBatchUpdates.h" #import "IGListReloadIndexPath.h" #import "UICollectionView+IGListBatchUpdateData.h" @@ -55,7 +54,11 @@ static NSArray *convertSectionReloadToItemUpdates(NSIndexSet *sec IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *collectionView, IGListIndexSetResult *diffResult, - IGListBatchUpdates *batchUpdates, + NSMutableIndexSet *sectionReloads, + NSMutableArray *itemInserts, + NSMutableArray *itemDeletes, + NSMutableArray *itemReloads, + NSMutableArray *itemMoves, NSArray> *fromObjects, BOOL sectionMovesAsDeletesInserts, BOOL preferItemReloadsForSectionReloads) { @@ -63,7 +66,7 @@ IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *coll // combine section reloads from the diff and manual reloads via reloadItems: NSMutableIndexSet *reloads = [diffResult.updates mutableCopy]; - [reloads addIndexes:batchUpdates.sectionReloads]; + [reloads addIndexes:sectionReloads]; NSMutableIndexSet *inserts = [diffResult.inserts mutableCopy]; NSMutableIndexSet *deletes = [diffResult.deletes mutableCopy]; @@ -97,14 +100,10 @@ IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *coll IGListConvertReloadToDeleteInsert(reloads, deletes, inserts, diffResult, fromObjects); } - NSMutableArray *itemInserts = batchUpdates.itemInserts; - NSMutableArray *itemDeletes = batchUpdates.itemDeletes; - NSMutableArray *itemMoves = batchUpdates.itemMoves; - NSSet *uniqueDeletes = [NSSet setWithArray:itemDeletes]; NSMutableSet *reloadDeletePaths = [NSMutableSet new]; NSMutableSet *reloadInsertPaths = [NSMutableSet new]; - for (IGListReloadIndexPath *reload in batchUpdates.itemReloads) { + for (IGListReloadIndexPath *reload in itemReloads) { if (![uniqueDeletes containsObject:reload.fromIndexPath]) { [reloadDeletePaths addObject:reload.fromIndexPath]; [reloadInsertPaths addObject:reload.toIndexPath]; diff --git a/Source/IGListKit/Internal/IGListExperimentalAdapterUpdaterInternal.h b/Source/IGListKit/Internal/IGListExperimentalAdapterUpdaterInternal.h index 7fae5fd4..76e63d71 100644 --- a/Source/IGListKit/Internal/IGListExperimentalAdapterUpdaterInternal.h +++ b/Source/IGListKit/Internal/IGListExperimentalAdapterUpdaterInternal.h @@ -13,7 +13,7 @@ #import "IGListExperimentalAdapterUpdater.h" #import "IGListBatchUpdateState.h" -#import "IGListBatchUpdates.h" +#import "IGListItemUpdatesCollector.h" NS_ASSUME_NONNULL_BEGIN @@ -24,7 +24,10 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign) BOOL queuedUpdateIsAnimated; -@property (nonatomic, strong) IGListBatchUpdates *batchUpdates; +@property (nonatomic, strong) NSMutableArray *itemUpdateBlocks; + +@property (nonatomic, strong) NSMutableArray *inUpdateCompletionBlocks; +@property (nonatomic, strong) IGListItemUpdatesCollector *inUpdateItemCollector; @property (nonatomic, copy, nullable) IGListTransitionDataApplyBlock applyDataBlock; diff --git a/Source/IGListKit/Internal/IGListItemUpdatesCollector.h b/Source/IGListKit/Internal/IGListItemUpdatesCollector.h new file mode 100644 index 00000000..d2910ee6 --- /dev/null +++ b/Source/IGListKit/Internal/IGListItemUpdatesCollector.h @@ -0,0 +1,27 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import + +#import + +@class IGListMoveIndexPath; +@class IGListReloadIndexPath; + +/// Object to collect item updates. Will replace `IGListBatchUpdates`. +IGLK_SUBCLASSING_RESTRICTED +@interface IGListItemUpdatesCollector : NSObject + +@property (nonatomic, strong, readonly) NSMutableIndexSet *sectionReloads; +@property (nonatomic, strong, readonly) NSMutableArray *itemInserts; +@property (nonatomic, strong, readonly) NSMutableArray *itemDeletes; +@property (nonatomic, strong, readonly) NSMutableArray *itemReloads; +@property (nonatomic, strong, readonly) NSMutableArray *itemMoves; + +- (BOOL)hasChanges; + +@end diff --git a/Source/IGListKit/Internal/IGListItemUpdatesCollector.m b/Source/IGListKit/Internal/IGListItemUpdatesCollector.m new file mode 100644 index 00000000..c452ccca --- /dev/null +++ b/Source/IGListKit/Internal/IGListItemUpdatesCollector.m @@ -0,0 +1,31 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "IGListItemUpdatesCollector.h" + +@implementation IGListItemUpdatesCollector + +- (instancetype)init { + if (self = [super init]) { + _sectionReloads = [NSMutableIndexSet new]; + _itemInserts = [NSMutableArray new]; + _itemMoves = [NSMutableArray new]; + _itemDeletes = [NSMutableArray new]; + _itemReloads = [NSMutableArray new]; + } + return self; +} + +- (BOOL)hasChanges { + return [self.sectionReloads count] > 0 + || [self.itemInserts count] > 0 + || [self.itemMoves count] > 0 + || [self.itemReloads count] > 0 + || [self.itemDeletes count] > 0; +} + +@end diff --git a/Tests/IGListExperimentalAdapterUpdaterTests.m b/Tests/IGListExperimentalAdapterUpdaterTests.m index 2e9d26e4..8aa77548 100644 --- a/Tests/IGListExperimentalAdapterUpdaterTests.m +++ b/Tests/IGListExperimentalAdapterUpdaterTests.m @@ -634,6 +634,56 @@ [self waitForExpectationsWithTimeout:30 handler:nil]; } +- (void)test_whenPerformingItemUpdateInMiddleOfReload_thatCompletionBlockStillExecuted { + IGSectionObject *object = [IGSectionObject sectionWithObjects:@[@0, @1, @2]]; + self.dataSource.sections = @[object]; + + XCTestExpectation *expectation = genExpectation; + + // Section-controllers can schedule item updates in -didUpdateToObject, so lets make sure the completion block works. + IGListReloadUpdateBlock reloadUpdateBlock = ^{ + [self.updater performUpdateWithCollectionViewBlock:[self collectionViewBlock] + animated:YES + itemUpdates:^{} + completion:^(BOOL finished) { + [expectation fulfill]; + }]; + }; + + [self.updater reloadDataWithCollectionViewBlock:[self collectionViewBlock] + reloadUpdateBlock:reloadUpdateBlock + completion:nil]; + + [self waitForExpectationsWithTimeout:30 handler:nil]; +} + +- (void)test_whenPerformingItemUpdateInMiddleOfUpdate_thatCompletionBlockStillExecuted { + IGSectionObject *object = [IGSectionObject sectionWithObjects:@[@0, @1, @2]]; + self.dataSource.sections = @[object]; + + XCTestExpectation *expectation = genExpectation; + + // Section-controllers can schedule item updates in -didUpdateToObject, so lets make sure the completion block works. + void (^updateItemBlock)(void) = ^{ + [self.updater performUpdateWithCollectionViewBlock:[self collectionViewBlock] + animated:YES + itemUpdates:^{} + completion:^(BOOL finished) { + [expectation fulfill]; + }]; + }; + + [self.updater performExperimentalUpdateAnimated:NO + collectionViewBlock:[self collectionViewBlock] + dataBlock:[self dataBlockFromObjects:self.dataSource.sections toObjects:self.dataSource.sections] + applyDataBlock:^(IGListTransitionData * _Nonnull data) { + updateItemBlock(); + } + completion:nil]; + + [self waitForExpectationsWithTimeout:30 handler:nil]; +} + - (void)test_whenNotInViewHierarchy_thatUpdatesStillExecuteBlocks { [self.collectionView removeFromSuperview];