Skip to content

Commit

Permalink
Replace diff result API for batch updates
Browse files Browse the repository at this point in the history 
Summary:
Replacing the move+update API with a batch-updates-safe API on the diff results object. This makes using the diff results w/out the rest of IGListKit infra much easier when working with `UITableView` or `UICollectionView`.

- Added unit tests
- Removed outdated unit tests

Reviewed By: dshahidehpour

Differential Revision: D4065798

fbshipit-source-id: 30da8a7b483d56d5acc497da9320dc07a6d0b7ad
  • Loading branch information
Ryan Nystrom authored and Facebook Github Bot committed Oct 26, 2016
1 parent 920aad6 commit b5aa5e3
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 117 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag

This release closes the [2.0.0 milestone](https://github.com/Instagram/IGListKit/milestone/1?closed=1).

### Breaking Changes

- Diff result method `-resultWithUpdatedMovesAsDeleteInserts` removed and replaced with `-resultForBatchUpdates`

### Enhancements

- Added support for cells created from nibs. [Sven Bacia](https://github.com/svenbacia) [(#56)](https://github.com/Instagram/IGListKit/pull/56)
Expand Down
4 changes: 0 additions & 4 deletions IGListKit.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
88144F0B1D870EDC007C7F66 /* IGListDiffSwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 88144EE61D870EDC007C7F66 /* IGListDiffSwiftTests.swift */; };
88144F0C1D870EDC007C7F66 /* IGListDiffTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88144EE81D870EDC007C7F66 /* IGListDiffTests.m */; };
88144F0D1D870EDC007C7F66 /* IGListDisplayHandlerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88144EE91D870EDC007C7F66 /* IGListDisplayHandlerTests.m */; };
88144F0E1D870EDC007C7F66 /* IGListIndexResultTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88144EEA1D870EDC007C7F66 /* IGListIndexResultTests.m */; };
88144F0F1D870EDC007C7F66 /* IGListObjectMapTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88144EEC1D870EDC007C7F66 /* IGListObjectMapTests.m */; };
88144F101D870EDC007C7F66 /* IGListSingleItemControllerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88144EED1D870EDC007C7F66 /* IGListSingleItemControllerTests.m */; };
88144F111D870EDC007C7F66 /* IGListStackItemControllerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88144EEE1D870EDC007C7F66 /* IGListStackItemControllerTests.m */; };
Expand Down Expand Up @@ -148,7 +147,6 @@
88144EE61D870EDC007C7F66 /* IGListDiffSwiftTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IGListDiffSwiftTests.swift; sourceTree = "<group>"; };
88144EE81D870EDC007C7F66 /* IGListDiffTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListDiffTests.m; sourceTree = "<group>"; };
88144EE91D870EDC007C7F66 /* IGListDisplayHandlerTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListDisplayHandlerTests.m; sourceTree = "<group>"; };
88144EEA1D870EDC007C7F66 /* IGListIndexResultTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListIndexResultTests.m; sourceTree = "<group>"; };
88144EEB1D870EDC007C7F66 /* IGListKitTests-Bridging-Header.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "IGListKitTests-Bridging-Header.h"; sourceTree = "<group>"; };
88144EEC1D870EDC007C7F66 /* IGListObjectMapTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListObjectMapTests.m; sourceTree = "<group>"; };
88144EED1D870EDC007C7F66 /* IGListSingleItemControllerTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListSingleItemControllerTests.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -420,7 +418,6 @@
88144EE61D870EDC007C7F66 /* IGListDiffSwiftTests.swift */,
88144EE81D870EDC007C7F66 /* IGListDiffTests.m */,
88144EE91D870EDC007C7F66 /* IGListDisplayHandlerTests.m */,
88144EEA1D870EDC007C7F66 /* IGListIndexResultTests.m */,
88144EEB1D870EDC007C7F66 /* IGListKitTests-Bridging-Header.h */,
88144EEC1D870EDC007C7F66 /* IGListObjectMapTests.m */,
88144EED1D870EDC007C7F66 /* IGListSingleItemControllerTests.m */,
Expand Down Expand Up @@ -680,7 +677,6 @@
88144F181D870EDC007C7F66 /* IGTestDelegateController.m in Sources */,
88144F0D1D870EDC007C7F66 /* IGListDisplayHandlerTests.m in Sources */,
88144F1B1D870EDC007C7F66 /* IGTestSingleItemDataSource.m in Sources */,
88144F0E1D870EDC007C7F66 /* IGListIndexResultTests.m in Sources */,
88144F171D870EDC007C7F66 /* IGTestCell.m in Sources */,
821BC4C01DB8C9D500172ED0 /* IGListSingleStoryboardItemControllerTests.m in Sources */,
88144F141D870EDC007C7F66 /* IGListTestOffsettingLayout.m in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion Source/IGListDiff.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ static id IGListDiffing(BOOL returnIndexPaths,
} else {
// note that an entry can be updated /and/ moved
if (record.entry->updated) {
addIndexToCollection(mUpdates, toSection, oldIndex);
addIndexToCollection(mUpdates, fromSection, oldIndex);
}

// calculate the offset and determine if there was a move
Expand Down
6 changes: 2 additions & 4 deletions Source/IGListIndexPathResult.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,9 @@ NS_ASSUME_NONNULL_BEGIN
- (nullable NSIndexPath *)newIndexPathForIdentifier:(id<NSObject>)identifier;

/**
Create a new result object transforming index paths that are both moved and updated into delete and inserts.
@discussion This is a convenience method for using a result object to perform UICollectionView and UITableView updates.
Create a new result object with operations safe for use in UITableView and UICollectionView batch updates.
*/
- (IGListIndexPathResult *)resultWithUpdatedMovesAsDeleteInserts;
- (IGListIndexPathResult *)resultForBatchUpdates;

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
Expand Down
14 changes: 12 additions & 2 deletions Source/IGListIndexPathResult.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ - (BOOL)hasChanges {
return self.inserts.count || self.deletes.count || self.updates.count || self.moves.count;
}

- (IGListIndexPathResult *)resultWithUpdatedMovesAsDeleteInserts {
- (IGListIndexPathResult *)resultForBatchUpdates {
NSMutableSet<NSIndexPath *> *deletes = [self.deletes mutableCopy];
NSMutableSet<NSIndexPath *> *inserts = [self.inserts mutableCopy];
NSMutableSet<NSIndexPath *> *filteredUpdates = [self.updates mutableCopy];

NSArray<IGListMoveIndexPath *> *moves = self.moves;
NSMutableArray<IGListMoveIndexPath *> *filteredMoves = [moves mutableCopy];

// convert move+update to delete+insert, respecting the from/to of the move
const NSUInteger moveCount = moves.count;
for (NSInteger i = moveCount - 1; i >= 0; i--) {
IGListMoveIndexPath *move = moves[i];
Expand All @@ -55,9 +56,18 @@ - (IGListIndexPathResult *)resultWithUpdatedMovesAsDeleteInserts {
}
}

// iterate all new identifiers. if its index is updated, delete from the old index and insert the new index
for (id<NSObject> key in [_oldIndexPathMap keyEnumerator]) {
NSIndexPath *indexPath = [_oldIndexPathMap objectForKey:key];
if ([filteredUpdates containsObject:indexPath]) {
[deletes addObject:indexPath];
[inserts addObject:(id)[_newIndexPathMap objectForKey:key]];
}
}

return [[IGListIndexPathResult alloc] initWithInserts:[inserts allObjects]
deletes:[deletes allObjects]
updates:[filteredUpdates allObjects]
updates:[NSArray new]
moves:filteredMoves
oldIndexPathMap:_oldIndexPathMap
newIndexPathMap:_newIndexPathMap];
Expand Down
6 changes: 2 additions & 4 deletions Source/IGListIndexSetResult.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,9 @@ NS_ASSUME_NONNULL_BEGIN
- (NSUInteger)newIndexForIdentifier:(id<NSObject>)identifier;

/**
Create a new result object transforming indexes that are both moved and updated into delete and inserts.
@discussion This is a convenience method for using a result object to perform UICollectionView and UITableView updates.
Create a new result object with operations safe for use in UITableView and UICollectionView batch updates.
*/
- (IGListIndexSetResult *)resultWithUpdatedMovesAsDeleteInserts;
- (IGListIndexSetResult *)resultForBatchUpdates;

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
Expand Down
14 changes: 12 additions & 2 deletions Source/IGListIndexSetResult.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@ - (BOOL)hasChanges {
return self.inserts.count || self.deletes.count || self.updates.count || self.moves.count;
}

- (IGListIndexSetResult *)resultWithUpdatedMovesAsDeleteInserts {
- (IGListIndexSetResult *)resultForBatchUpdates {
NSMutableIndexSet *deletes = [self.deletes mutableCopy];
NSMutableIndexSet *inserts = [self.inserts mutableCopy];
NSMutableIndexSet *filteredUpdates = [self.updates mutableCopy];

NSArray<IGListMoveIndex *> *moves = self.moves;
NSMutableArray<IGListMoveIndex *> *filteredMoves = [moves mutableCopy];

// convert all update+move to delete+insert
const NSUInteger moveCount = moves.count;
for (NSInteger i = moveCount - 1; i >= 0; i--) {
IGListMoveIndex *move = moves[i];
Expand All @@ -57,9 +58,18 @@ - (IGListIndexSetResult *)resultWithUpdatedMovesAsDeleteInserts {
}
}

// iterate all new identifiers. if its index is updated, delete from the old index and insert the new index
for (id<NSObject> key in [_oldIndexMap keyEnumerator]) {
const NSInteger index = [[_oldIndexMap objectForKey:key] integerValue];
if ([filteredUpdates containsIndex:index]) {
[deletes addIndex:index];
[inserts addIndex:[[_newIndexMap objectForKey:key] integerValue]];
}
}

return [[IGListIndexSetResult alloc] initWithInserts:inserts
deletes:deletes
updates:filteredUpdates
updates:[NSIndexSet new]
moves:filteredMoves
oldIndexMap:_oldIndexMap
newIndexMap:_newIndexMap];
Expand Down
62 changes: 62 additions & 0 deletions Tests/IGListDiffTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,4 +366,66 @@ - (void)test_whenDiffing_thatNewIndexPathsMatch {
XCTAssertEqualObjects([result newIndexPathForIdentifier:@9], [NSIndexPath indexPathForItem:1 inSection:1]);
}

- (void)test_whenDiffing_withBatchUpdateResult_thatIndexesMatch {
NSArray *o = @[
genTestObject(@1, @1),
genTestObject(@2, @1),
genTestObject(@3, @1),
genTestObject(@4, @1),
genTestObject(@5, @1),
genTestObject(@6, @1),
];
NSArray *n = @[
// deleted
genTestObject(@2, @2), // updated
genTestObject(@5, @1), // moved
genTestObject(@4, @1),
genTestObject(@7, @1), // inserted
genTestObject(@6, @2), // updated
genTestObject(@3, @2), // moved+updated
];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:4 to:1] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet indexSetWithIndex:0];
[expectedDeletes addIndex:1];
[expectedDeletes addIndex:2];
[expectedDeletes addIndex:5];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSMutableIndexSet *expectedInserts = [NSMutableIndexSet indexSetWithIndex:0];
[expectedInserts addIndex:3];
[expectedInserts addIndex:4];
[expectedInserts addIndex:5];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenDiffing_withBatchUpdateResult_thatIndexPathsMatch {
NSArray *o = @[
genTestObject(@1, @1),
genTestObject(@2, @1),
genTestObject(@3, @1),
genTestObject(@4, @1),
genTestObject(@5, @1),
genTestObject(@6, @1),
];
NSArray *n = @[
// deleted
genTestObject(@2, @2), // updated
genTestObject(@5, @1), // moved
genTestObject(@4, @1),
genTestObject(@7, @1), // inserted
genTestObject(@6, @2), // updated
genTestObject(@3, @2), // moved+updated
];
IGListIndexPathResult *result = [IGListDiffPaths(0, 1, o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndexPath alloc] initWithFrom:genIndexPath(4, 0) to:genIndexPath(1, 1)] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSArray *expectedDeletes = @[genIndexPath(0, 0), genIndexPath(1, 0), genIndexPath(2, 0), genIndexPath(5, 0)];
XCTAssertEqualObjects(sorted(result.deletes), expectedDeletes);
NSArray *expectedInserts = @[genIndexPath(0, 1), genIndexPath(3, 1), genIndexPath(4, 1), genIndexPath(5, 1)];
XCTAssertEqualObjects(sorted(result.inserts), expectedInserts);
}

@end
100 changes: 0 additions & 100 deletions Tests/IGListIndexResultTests.m
View file

This file was deleted.

1 comment on commit b5aa5e3

@pm-dev
Copy link
Contributor

@pm-dev pm-dev commented on b5aa5e3 Apr 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnystrom What's the point of replacing updates & moves with inserts & deletes? UICollectionView has always supported reloadSections and moveSection

Please sign in to comment.