replace IGListBatchUpdates with IGListItemUpdatesCollector

Summary:
Before adding `transactions`, lets clarify what information is collected before vs during the update. We can split apart `IGListBatchUpdates`.
* Before the update
  * Collect the update blocks in `itemUpdateBlocks` which will be executed later during the update.
  * Collect completion blocks in the same `completionBlocks` list as the other updates.
* During the update
  * Collect item updates in `IGListItemUpdatesCollector`, like inserts and deletes.
  * Collect new completion blocks (in case a section-controller schedules an update in -didUpdateToObject) in a separete list `inUpdateCompletionBlocks`

Reviewed By: patters

Differential Revision: D23145778

fbshipit-source-id: cae2c0689ac1ef0d93e3c04856b0495856e305b6
This commit is contained in:
Maxime Ollivier 2020-09-08 09:06:16 -07:00 committed by Facebook GitHub Bot
parent 3f1a2fb0ac
commit 8c1253246f
8 changed files with 192 additions and 61 deletions

View file

@ -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);

View file

@ -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<IGListAdapterUpdaterDelegate> 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<IGListAdapterUpdaterDelegate> 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];

View file

@ -8,23 +8,28 @@
#import <UIKit/UIKit.h>
@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<id<IGListDiffable>> *fromObjects);
NSMutableIndexSet *deletes,
NSMutableIndexSet *inserts,
IGListIndexSetResult *result,
NSArray<id<IGListDiffable>> *fromObjects);
IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *collectionView,
IGListIndexSetResult *diffResult,
IGListBatchUpdates *batchUpdates,
NSArray<id<IGListDiffable>> *fromObjects,
BOOL sectionMovesAsDeletesInserts,
BOOL preferItemReloadsForSectionReloads);
IGListIndexSetResult *diffResult,
NSMutableIndexSet *sectionReloads,
NSMutableArray<NSIndexPath *> *itemInserts,
NSMutableArray<NSIndexPath *> *itemDeletes,
NSMutableArray<IGListReloadIndexPath *> *itemReloads,
NSMutableArray<IGListMoveIndexPath *> *itemMoves,
NSArray<id<IGListDiffable>> *fromObjects,
BOOL sectionMovesAsDeletesInserts,
BOOL preferItemReloadsForSectionReloads);
NS_ASSUME_NONNULL_END

View file

@ -12,7 +12,6 @@
#import <IGListDiffKit/IGListDiffable.h>
#import <IGListDiffKit/IGListIndexSetResult.h>
#import "IGListBatchUpdates.h"
#import "IGListReloadIndexPath.h"
#import "UICollectionView+IGListBatchUpdateData.h"
@ -55,7 +54,11 @@ static NSArray<NSIndexPath *> *convertSectionReloadToItemUpdates(NSIndexSet *sec
IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *collectionView,
IGListIndexSetResult *diffResult,
IGListBatchUpdates *batchUpdates,
NSMutableIndexSet *sectionReloads,
NSMutableArray<NSIndexPath *> *itemInserts,
NSMutableArray<NSIndexPath *> *itemDeletes,
NSMutableArray<IGListReloadIndexPath *> *itemReloads,
NSMutableArray<IGListMoveIndexPath *> *itemMoves,
NSArray<id<IGListDiffable>> *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<NSIndexPath *> *itemInserts = batchUpdates.itemInserts;
NSMutableArray<NSIndexPath *> *itemDeletes = batchUpdates.itemDeletes;
NSMutableArray<IGListMoveIndexPath *> *itemMoves = batchUpdates.itemMoves;
NSSet<NSIndexPath *> *uniqueDeletes = [NSSet setWithArray:itemDeletes];
NSMutableSet<NSIndexPath *> *reloadDeletePaths = [NSMutableSet new];
NSMutableSet<NSIndexPath *> *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];

View file

@ -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<IGListItemUpdateBlock> *itemUpdateBlocks;
@property (nonatomic, strong) NSMutableArray<IGListUpdatingCompletion> *inUpdateCompletionBlocks;
@property (nonatomic, strong) IGListItemUpdatesCollector *inUpdateItemCollector;
@property (nonatomic, copy, nullable) IGListTransitionDataApplyBlock applyDataBlock;

View file

@ -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 <Foundation/Foundation.h>
#import <IGListDiffKit/IGListMacros.h>
@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<NSIndexPath *> *itemInserts;
@property (nonatomic, strong, readonly) NSMutableArray<NSIndexPath *> *itemDeletes;
@property (nonatomic, strong, readonly) NSMutableArray<IGListReloadIndexPath *> *itemReloads;
@property (nonatomic, strong, readonly) NSMutableArray<IGListMoveIndexPath *> *itemMoves;
- (BOOL)hasChanges;
@end

View file

@ -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

View file

@ -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];