Skip to content

Commit

Permalink
clean up ig_ios_listkit_fix_net_item_count
Browse files Browse the repository at this point in the history 
Summary: This was a killswitch to be safe, so lets clean it up.

Differential Revision: D67072925

fbshipit-source-id: ac54972030e7d53e7abd95025e45773aa3c5efc6
  • Loading branch information
@facebook-github-bot
Maxime Ollivier authored and facebook-github-bot committed Dec 12, 2024
1 parent 86dcc6e commit 7b78c95
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 109 deletions.
5 changes: 2 additions & 3 deletions Source/IGListDiffKit/IGListBatchUpdateData.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ NS_SWIFT_NAME(ListBatchUpdateData)
@param deleteIndexPaths Item index paths to delete.
@param updateIndexPaths Item index paths to update.
@param moveIndexPaths Item index paths to move.
@param enableNetItemCountFix Fix item count balance issue when removing duplicate deletes
@return A new batch update object.
*/
Expand All @@ -77,8 +77,7 @@ NS_SWIFT_NAME(ListBatchUpdateData)
insertIndexPaths:(NSArray<NSIndexPath *> *)insertIndexPaths
deleteIndexPaths:(NSArray<NSIndexPath *> *)deleteIndexPaths
updateIndexPaths:(NSArray<NSIndexPath *> *)updateIndexPaths
moveIndexPaths:(NSArray<IGListMoveIndexPath *> *)moveIndexPaths
enableNetItemCountFix:(BOOL)enableNetItemCountFix NS_DESIGNATED_INITIALIZER;
moveIndexPaths:(NSArray<IGListMoveIndexPath *> *)moveIndexPaths NS_DESIGNATED_INITIALIZER;

/**
:nodoc:
Expand Down
51 changes: 23 additions & 28 deletions Source/IGListDiffKit/IGListBatchUpdateData.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ - (instancetype)initWithInsertSections:(nonnull NSIndexSet *)insertSections
insertIndexPaths:(nonnull NSArray<NSIndexPath *> *)insertIndexPaths
deleteIndexPaths:(nonnull NSArray<NSIndexPath *> *)deleteIndexPaths
updateIndexPaths:(nonnull NSArray<NSIndexPath *> *)updateIndexPaths
moveIndexPaths:(nonnull NSArray<IGListMoveIndexPath *> *)moveIndexPaths
enableNetItemCountFix:(BOOL)enableNetItemCountFix {
moveIndexPaths:(nonnull NSArray<IGListMoveIndexPath *> *)moveIndexPaths {
IGParameterAssert(insertSections != nil);
IGParameterAssert(deleteSections != nil);
IGParameterAssert(moveSections != nil);
Expand Down Expand Up @@ -96,41 +95,37 @@ - (instancetype)initWithInsertSections:(nonnull NSIndexSet *)insertSections
toMap[to] = move;
}
}

NSMutableArray<NSIndexPath *> *mDeleteIndexPaths;
NSMutableArray<NSIndexPath *> *mInsertIndexPaths;

// Avoid a flaky UICollectionView bug when deleting from the same index path twice
// exposes a possible data source inconsistency issue
if (enableNetItemCountFix) {
NSMutableDictionary<NSIndexPath *, NSNumber *> *const deleteCounts = [NSMutableDictionary new];

// If we need to remove a duplicate delete, we also need to remove an insert to balance the count.
// Lets build the delete counts for each index, which we can use to skip corresponding inserts.
for (NSIndexPath *deleteIndexPath in deleteIndexPaths) {
const NSInteger deleteCount = deleteCounts[deleteIndexPath].integerValue;
deleteCounts[deleteIndexPath] = @(deleteCount + 1);
}
NSMutableDictionary<NSIndexPath *, NSNumber *> *const deleteCounts = [NSMutableDictionary new];

// Skip inserts that have an associated skipped delete
NSMutableArray<NSIndexPath *> *const trimmedInsertIndexPath = [NSMutableArray new];
for (NSIndexPath *insertIndexPath in insertIndexPaths) {
const NSInteger deleteCount = deleteCounts[insertIndexPath].integerValue;
if (deleteCount > 1) {
// Skip!
deleteCounts[insertIndexPath] = @(deleteCount - 1);
} else {
[trimmedInsertIndexPath addObject:insertIndexPath];
}
}
// If we need to remove a duplicate delete, we also need to remove an insert to balance the count.
// Lets build the delete counts for each index, which we can use to skip corresponding inserts.
for (NSIndexPath *deleteIndexPath in deleteIndexPaths) {
const NSInteger deleteCount = deleteCounts[deleteIndexPath].integerValue;
deleteCounts[deleteIndexPath] = @(deleteCount + 1);
}

mDeleteIndexPaths = [[deleteCounts allKeys] mutableCopy];
mInsertIndexPaths = trimmedInsertIndexPath;
} else {
mDeleteIndexPaths = [[[NSSet setWithArray:deleteIndexPaths] allObjects] mutableCopy];
mInsertIndexPaths = [insertIndexPaths mutableCopy];
// Skip inserts that have an associated skipped delete
NSMutableArray<NSIndexPath *> *const trimmedInsertIndexPath = [NSMutableArray new];
for (NSIndexPath *insertIndexPath in insertIndexPaths) {
const NSInteger deleteCount = deleteCounts[insertIndexPath].integerValue;
if (deleteCount > 1) {
// Skip!
deleteCounts[insertIndexPath] = @(deleteCount - 1);
} else {
[trimmedInsertIndexPath addObject:insertIndexPath];
}
}

mDeleteIndexPaths = [[deleteCounts allKeys] mutableCopy];
mInsertIndexPaths = trimmedInsertIndexPath;


// avoids a bug where a cell is animated twice and one of the snapshot cells is never removed from the hierarchy
[IGListBatchUpdateData _cleanIndexPathsWithMap:fromMap moves:mMoveSections indexPaths:mDeleteIndexPaths deletes:mDeleteSections inserts:mInsertSections];

Expand Down
2 changes: 0 additions & 2 deletions Source/IGListDiffKit/IGListExperiments.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) {
IGListExperimentThrowOnInconsistencyException = 1 << 3,
/// Remove the early exit so multiple updates can't happen at once
IGListExperimentRemoveDataSourceChangeEarlyExit = 1 << 4,
/// Fix item count balance issue when removing duplicate deletes
IGListExperimentEnableNetItemCountFix = 1 << 5,
};

/**
Expand Down
3 changes: 1 addition & 2 deletions Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *coll
NSMutableArray<IGListMoveIndexPath *> *itemMoves,
NSArray<id<IGListDiffable>> *fromObjects,
BOOL sectionMovesAsDeletesInserts,
BOOL preferItemReloadsForSectionReloads,
BOOL enableNetItemCountFix);
BOOL preferItemReloadsForSectionReloads);

NSIndexSet *IGListSectionIndexFromIndexPaths(NSArray<NSIndexPath *> *indexPaths);

Expand Down
6 changes: 2 additions & 4 deletions Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ void IGListConvertReloadToDeleteInsert(NSMutableIndexSet *reloads,
NSMutableArray<IGListMoveIndexPath *> *itemMoves,
NSArray<id<IGListDiffable>> *fromObjects,
BOOL sectionMovesAsDeletesInserts,
BOOL preferItemReloadsForSectionReloads,
BOOL enableNetItemCountFix) {
BOOL preferItemReloadsForSectionReloads) {
NSSet *moves = [[NSSet alloc] initWithArray:diffResult.moves];

// combine section reloads from the diff and manual reloads via reloadItems:
Expand Down Expand Up @@ -126,8 +125,7 @@ void IGListConvertReloadToDeleteInsert(NSMutableIndexSet *reloads,
insertIndexPaths:itemInserts
deleteIndexPaths:itemDeletes
updateIndexPaths:itemUpdates
moveIndexPaths:itemMoves
enableNetItemCountFix:enableNetItemCountFix];
moveIndexPaths:itemMoves];
[collectionView ig_applyBatchUpdateData:updateData];
return updateData;
}
Expand Down
7 changes: 2 additions & 5 deletions Source/IGListKit/Internal/IGListBatchUpdateTransaction.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ - (void)_applyDataUpdates {
}

- (void)_applyCollectioViewUpdates:(IGListIndexSetResult *)diffResult {
const BOOL enableNetItemCountFix = IGListExperimentEnabled(self.config.experiments, IGListExperimentEnableNetItemCountFix);
if (self.config.singleItemSectionUpdates) {
[self.collectionView deleteSections:diffResult.deletes];
[self.collectionView insertSections:diffResult.inserts];
Expand All @@ -257,8 +256,7 @@ - (void)_applyCollectioViewUpdates:(IGListIndexSetResult *)diffResult {
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]
enableNetItemCountFix:enableNetItemCountFix];
moveIndexPaths:@[]];
} else {
self.actualCollectionViewUpdates = IGListApplyUpdatesToCollectionView(self.collectionView,
diffResult,
Expand All @@ -269,8 +267,7 @@ - (void)_applyCollectioViewUpdates:(IGListIndexSetResult *)diffResult {
self.inUpdateItemCollector.itemMoves,
self.sectionData.fromObjects ?: @[],
self.config.sectionMovesAsDeletesInserts,
self.config.preferItemReloadsForSectionReloads,
enableNetItemCountFix);
self.config.preferItemReloadsForSectionReloads);
}
}

Expand Down
21 changes: 7 additions & 14 deletions Tests/IGListAdapterUpdaterTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,8 +779,7 @@ - (void)test_whenReloadIsCalledWithSameItemCount_deleteInsertSectionHappen {
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]
enableNetItemCountFix:NO];
moveIndexPaths:@[]];
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id"]];
NSArray<IGSectionObject *> *to = @[[IGSectionObject sectionWithObjects:@[@2] identifier:@"id"]];
self.dataSource.sections = from;
Expand Down Expand Up @@ -997,8 +996,7 @@ - (void)test_whenReloadIsCalledWithSameItemCount_andPreferItemReload_updateIndex
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[[NSIndexPath indexPathForItem:0 inSection:0]]
moveIndexPaths:@[]
enableNetItemCountFix:NO];
moveIndexPaths:@[]];
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id"]];
// Update the items
NSArray<IGSectionObject *> *to = @[[IGSectionObject sectionWithObjects:@[@2] identifier:@"id"]];
Expand Down Expand Up @@ -1033,8 +1031,7 @@ - (void)test_whenReloadIsCalledWithDifferentItemCount_andPreferItemReload_delete
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]
enableNetItemCountFix:NO];
moveIndexPaths:@[]];
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id"]];
// more items in the section
NSArray<IGSectionObject *> *to = @[[IGSectionObject sectionWithObjects:@[@1, @2] identifier:@"id"]];
Expand Down Expand Up @@ -1070,8 +1067,7 @@ - (void)test_whenReloadIsCalledWithSectionMoveAndUpdate_andPreferItemReload_dele
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]
enableNetItemCountFix:NO];
moveIndexPaths:@[]];
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@2] identifier:@"id2"]];
// move section, and also update the item for "id2"
Expand Down Expand Up @@ -1113,8 +1109,7 @@ - (void)test_whenReloadIsCalledWithSectionMoveAndUpdate_withDifferentSectionLeng
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]
enableNetItemCountFix:NO];
moveIndexPaths:@[]];
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1, @2, @3] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@2] identifier:@"id2"]];
// move section, and also update the item for "id2"
Expand Down Expand Up @@ -1157,8 +1152,7 @@ - (void)test_whenReloadIsCalledWithSectionMoveAndUpdate_withThreeSections_delete
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]
enableNetItemCountFix:NO];
moveIndexPaths:@[]];
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@2] identifier:@"id2"],
[IGSectionObject sectionWithObjects:@[@3] identifier:@"id3"]];
Expand Down Expand Up @@ -1201,8 +1195,7 @@ - (void)test_whenReloadIsCalledWithSectionInsertAndUpdate_andPreferItemReload_no
insertIndexPaths:@[]
deleteIndexPaths:@[]
updateIndexPaths:@[]
moveIndexPaths:@[]
enableNetItemCountFix:NO];
moveIndexPaths:@[]];
NSArray<IGSectionObject *> *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"]];
NSArray<IGSectionObject *> *to = @[[IGSectionObject sectionWithObjects:@[@2] identifier:@"id1"],
[IGSectionObject sectionWithObjects:@[@22] identifier:@"id2"]];
Expand Down
Loading

0 comments on commit 7b78c95

Please sign in to comment.