Dedupe view models in IGListBindingSectionController

Summary: Issue fixed: #700 - [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. - [X] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Closes #916 Reviewed By: manicakes Differential Revision: D5872117 Pulled By: rnystrom fbshipit-source-id: 420190566c0a822834d8a3b64202f6d807e6a487
Instagram · Sep 21, 2017 · 4f970d2 · 4f970d2
1 parent d9a89c9
commit 4f970d2
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,13 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag
 
 - Avoid crash when invalidating the layout while inside `-[UICollectionView performBatchUpdates:completion:]. [Ryan Nystrom](https://github.com/rnystrom) [(#tbd)](https://github.com/Instagram/IGListKit/pull/tbd)
 
+- Duplicate view models in `IGListBindingSectionController` gets filtered out. [Weyert de Boer](https://github.com/weyert) [(#916)](https://github.com/Instagram/IGListKit/pull/916)
+
+### Enhancements
+
+- Added `-[IGListSectionController didHighlightItemAtIndex:]` and `-[IGListSectionController didUnhighlightItemAtIndex:]` APIs to support `UICollectionView` cell highlighting. [Kevin Delannoy](https://github.com/delannoyk) [(#933)](https://github.com/Instagram/IGListKit/pull/933)
+>>>>>>> source: 872ef395b114 arcpatch-D5872117 - facebook-github-bot: [IGList...
+
 3.1.1
 -----
 

diff --git a/IGListKit.xcodeproj/project.pbxproj b/IGListKit.xcodeproj/project.pbxproj
@@ -258,6 +258,9 @@
 		29DA5CA81EA7D37000113926 /* IGListTestCase.m in Sources */ = {isa = PBXBuildFile; fileRef = 29DA5CA61EA7D37000113926 /* IGListTestCase.m */; };
 		29EA6C491DB43A8000957A88 /* IGTestNibCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 294369B01DB1B7AE0025F6E7 /* IGTestNibCell.xib */; };
 		64D8007E592D0292BE4FC21D /* Pods_IGListKitTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 68CC152B514785B3113DDD4A /* Pods_IGListKitTests.framework */; };
+		7BC0C61B1F5C401F00A06ADD /* IGListArrayUtilsInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BC0C61A1F5C401F00A06ADD /* IGListArrayUtilsInternal.h */; };
+		7BC0C61C1F5C402600A06ADD /* IGListArrayUtilsInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BC0C61A1F5C401F00A06ADD /* IGListArrayUtilsInternal.h */; };
+		7BC0C61D1F5C402600A06ADD /* IGListArrayUtilsInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BC0C61A1F5C401F00A06ADD /* IGListArrayUtilsInternal.h */; };
 		821BC4C01DB8C9D500172ED0 /* IGListSingleStoryboardItemControllerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 821BC4BE1DB8C95300172ED0 /* IGListSingleStoryboardItemControllerTests.m */; };
 		821BC4C41DB8CEF800172ED0 /* IGTestStoryboard.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 821BC4C21DB8CAE900172ED0 /* IGTestStoryboard.storyboard */; };
 		821BC4CB1DB8D60100172ED0 /* IGTestStoryboardViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 821BC4C81DB8D5B200172ED0 /* IGTestStoryboardViewController.m */; };
@@ -498,6 +501,7 @@
 		4883B4E449DE18EC9C37773B /* Pods-IGListKitTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-IGListKitTests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-IGListKitTests/Pods-IGListKitTests.debug.xcconfig"; sourceTree = "<group>"; };
 		68CC152B514785B3113DDD4A /* Pods_IGListKitTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_IGListKitTests.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		6DBA851B0E1DFDC2E7BDA472 /* Pods-IGListKitTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-IGListKitTests.release.xcconfig"; path = "Pods/Target Support Files/Pods-IGListKitTests/Pods-IGListKitTests.release.xcconfig"; sourceTree = "<group>"; };
+		7BC0C61A1F5C401F00A06ADD /* IGListArrayUtilsInternal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IGListArrayUtilsInternal.h; sourceTree = "<group>"; };
 		821BC4BE1DB8C95300172ED0 /* IGListSingleStoryboardItemControllerTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IGListSingleStoryboardItemControllerTests.m; sourceTree = "<group>"; };
 		821BC4C21DB8CAE900172ED0 /* IGTestStoryboard.storyboard */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.storyboard; path = IGTestStoryboard.storyboard; sourceTree = "<group>"; };
 		821BC4C71DB8D5B200172ED0 /* IGTestStoryboardViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IGTestStoryboardViewController.h; sourceTree = "<group>"; };
@@ -700,6 +704,7 @@
 				0B3B92931E08D7F5008390ED /* IGListIndexSetResultInternal.h */,
 				0B3B92941E08D7F5008390ED /* IGListMoveIndexInternal.h */,
 				0B3B92951E08D7F5008390ED /* IGListMoveIndexPathInternal.h */,
+				7BC0C61A1F5C401F00A06ADD /* IGListArrayUtilsInternal.h */,
 			);
 			path = Internal;
 			sourceTree = "<group>";
@@ -932,6 +937,7 @@
 				294652BA1EA927750063BDD9 /* IGListDebuggingUtilities.h in Headers */,
 				0B3B93351E08D7F5008390ED /* IGListDisplayHandler.h in Headers */,
 				0B3B92D31E08D7F5008390ED /* IGListIndexPathResult.h in Headers */,
+				7BC0C61C1F5C402600A06ADD /* IGListArrayUtilsInternal.h in Headers */,
 				0B3B93251E08D7F5008390ED /* IGListSupplementaryViewSource.h in Headers */,
 				0B3B92F31E08D7F5008390ED /* NSString+IGListDiffable.h in Headers */,
 				0B3B932D1E08D7F5008390ED /* IGListAdapterInternal.h in Headers */,
@@ -1014,6 +1020,7 @@
 				0B3B92D01E08D7F5008390ED /* IGListExperiments.h in Headers */,
 				296AC95C1EA518D3005137E2 /* IGListReloadIndexPath.h in Headers */,
 				0B3B93321E08D7F5008390ED /* IGListAdapterUpdaterInternal.h in Headers */,
+				7BC0C61B1F5C401F00A06ADD /* IGListArrayUtilsInternal.h in Headers */,
 				DA5F484B1E8E9D7000DAE6DA /* IGListAdapter+UICollectionView.h in Headers */,
 				0B3B93381E08D7F5008390ED /* IGListSectionControllerInternal.h in Headers */,
 				2926586F1E75E0830041B56D /* IGListBindingSectionControllerSelectionDelegate.h in Headers */,
@@ -1058,6 +1065,7 @@
 				0B3B934B1E08D82E008390ED /* IGListMoveIndexPathInternal.h in Headers */,
 				0B3B93521E08D839008390ED /* IGListIndexSetResult.h in Headers */,
 				0B3B934C1E08D839008390ED /* IGListAssert.h in Headers */,
+				7BC0C61D1F5C402600A06ADD /* IGListArrayUtilsInternal.h in Headers */,
 				292658551E7498220041B56D /* IGListKit.h in Headers */,
 				0B3B93561E08D839008390ED /* IGListMoveIndexPath.h in Headers */,
 				0B3B93511E08D839008390ED /* IGListIndexPathResult.h in Headers */,

diff --git a/Source/Common/Internal/IGListArrayUtilsInternal.h b/Source/Common/Internal/IGListArrayUtilsInternal.h
@@ -0,0 +1,33 @@
+/**
+ * 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.
+ */
+
+#ifndef IGListArrayUtilsInternal_h
+#define IGListArrayUtilsInternal_h
+
+static NSArray *objectsWithDuplicateIdentifiersRemoved(NSArray<id<IGListDiffable>> *objects) {
+    if (objects == nil) {
+        return nil;
+    }
+
+    NSMutableSet *identifiers = [NSMutableSet new];
+    NSMutableArray *uniqueObjects = [NSMutableArray new];
+    for (id<IGListDiffable> object in objects) {
+        id diffIdentifier = [object diffIdentifier];
+        if (diffIdentifier != nil
+            && ![identifiers containsObject:diffIdentifier]) {
+            [identifiers addObject:diffIdentifier];
+            [uniqueObjects addObject:object];
+        } else {
+            IGLKLog(@"WARNING: Object %@ already appeared in objects array", object);
+        }
+    }
+    return uniqueObjects;
+}
+
+#endif /* IGListArrayUtilsInternal_h */
diff --git a/Source/IGListAdapterUpdater.m b/Source/IGListAdapterUpdater.m
@@ -18,6 +18,7 @@
 #import "IGListIndexSetResultInternal.h"
 #import "IGListMoveIndexPathInternal.h"
 #import "IGListReloadIndexPath.h"
+#import "IGListArrayUtilsInternal.h"
 
 @implementation IGListAdapterUpdater
 
@@ -34,7 +35,6 @@ - (instancetype)init {
     return self;
 }
 
-
 #pragma mark - Private API
 
 - (BOOL)hasChanges {
@@ -56,7 +56,7 @@ - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView {
     void (^reloadUpdates)() = self.reloadUpdates;
     IGListBatchUpdates *batchUpdates = self.batchUpdates;
     NSMutableArray *completionBlocks = [self.completionBlocks mutableCopy];
-    
+
     [self cleanStateBeforeUpdates];
 
     // item updates must not send mutations to the collection view while we are reloading
@@ -72,11 +72,11 @@ - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView {
     for (IGListItemUpdateBlock itemUpdateBlock in batchUpdates.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];
@@ -94,26 +94,6 @@ - (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView {
     self.state = IGListBatchUpdateStateIdle;
 }
 
-static NSArray *objectsWithDuplicateIdentifiersRemoved(NSArray<id<IGListDiffable>> *objects) {
-    if (objects == nil) {
-        return nil;
-    }
-
-    NSMutableSet *identifiers = [NSMutableSet new];
-    NSMutableArray *uniqueObjects = [NSMutableArray new];
-    for (id<IGListDiffable> object in objects) {
-        id diffIdentifier = [object diffIdentifier];
-        if (diffIdentifier != nil
-            && ![identifiers containsObject:diffIdentifier]) {
-            [identifiers addObject:diffIdentifier];
-            [uniqueObjects addObject:object];
-        } else {
-            IGLKLog(@"WARNING: Object %@ already appeared in objects array", object);
-        }
-    }
-    return uniqueObjects;
-}
-
 - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView {
     IGAssertMainThread();
     IGAssert(self.state == IGListBatchUpdateStateIdle, @"Should not call batch updates when state isn't idle");
@@ -151,7 +131,7 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView
         for (IGListItemUpdateBlock itemUpdateBlock in batchUpdates.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];
@@ -361,7 +341,7 @@ - (void)cleanStateBeforeUpdates {
 
     // remove indexpath/item changes
     self.objectTransitionBlock = nil;
-    
+
     // removes all object completion blocks. done before updates to start collecting completion blocks for coalesced
     // or re-entrant object updates
     [self.completionBlocks removeAllObjects];
@@ -471,7 +451,7 @@ - (void)performUpdateWithCollectionView:(UICollectionView *)collectionView
     IGAssertMainThread();
     IGParameterAssert(collectionView != nil);
     IGParameterAssert(itemUpdates != nil);
-    
+
     IGListBatchUpdates *batchUpdates = self.batchUpdates;
     if (completion != nil) {
         [batchUpdates.itemCompletionBlocks addObject:completion];
@@ -483,11 +463,11 @@ - (void)performUpdateWithCollectionView:(UICollectionView *)collectionView
         itemUpdates();
     } else {
         [batchUpdates.itemUpdateBlocks addObject:itemUpdates];
-        
+
         // disabled animations will always take priority
         // reset to YES in -cleanupState
         self.queuedUpdateIsAnimated = self.queuedUpdateIsAnimated && animated;
-        
+
         [self queueUpdateWithCollectionView:collectionView];
     }
 }
@@ -569,3 +549,4 @@ - (void)reloadCollectionView:(UICollectionView *)collectionView sections:(NSInde
 }
 
 @end
+
diff --git a/Source/IGListBindingSectionController.m b/Source/IGListBindingSectionController.m
@@ -13,6 +13,9 @@
 #import <IGListKit/IGListDiffable.h>
 #import <IGListKit/IGListDiff.h>
 #import <IGListKit/IGListBindable.h>
+#import <IGListKit/IGListAdapterUpdater.h>
+
+#import "IGListArrayUtilsInternal.h"
 
 typedef NS_ENUM(NSInteger, IGListDiffingSectionState) {
     IGListDiffingSectionStateIdle = 0,
@@ -58,7 +61,8 @@ - (void)updateAnimated:(BOOL)animated completion:(void (^)(BOOL))completion {
         id<IGListDiffable> object = self.object;
         IGAssert(object != nil, @"Expected IGListBindingSectionController object to be non-nil before updating.");
 
-        self.viewModels = [self.dataSource sectionController:self viewModelsForObject:object];
+        NSArray *newViewModels = [self.dataSource sectionController:self viewModelsForObject:object];
+        self.viewModels = objectsWithDuplicateIdentifiersRemoved(newViewModels);
         result = IGListDiff(oldViewModels, self.viewModels, IGListDiffEquality);
 
         [result.updates enumerateIndexesUsingBlock:^(NSUInteger oldUpdatedIndex, BOOL *stop) {

diff --git a/Tests/IGListBindingSectionControllerTests.m b/Tests/IGListBindingSectionControllerTests.m
@@ -105,6 +105,41 @@ - (void)test_whenUpdating_withAddedModels_thatCellsCorrectAndConfigured {
     [self waitForExpectationsWithTimeout:30 handler:nil];
 }
 
+- (void)test_whenUpdating_withNotUniqueModels_thatCellsCorrectAndConfigured {
+    [self setupWithObjects:@[
+                             [[IGTestDiffingObject alloc] initWithKey:@1 objects:@[@7, @"seven"]],
+                             ]];
+    [self.adapter reloadObjects:@[[[IGTestDiffingObject alloc] initWithKey:@1 objects:@[@"four", @4, @"seven", @7, @"seven", @10]]]];
+
+    IGTestNumberBindableCell *cell00 = [self cellAtSection:0 item:0];
+    IGTestStringBindableCell *cell01 = [self cellAtSection:0 item:1];
+
+    XCTAssertEqualObjects(cell00.textField.text, @"7");
+    XCTAssertEqualObjects(cell01.label.text, @"seven");
+    XCTAssertNil([self cellAtSection:0 item:2]);
+    XCTAssertNil([self cellAtSection:0 item:3]);
+
+    // "fake" batch updates to make sure that calling reload triggers a diffed batch update
+    XCTestExpectation *expectation = [self expectationWithDescription:NSStringFromSelector(_cmd)];
+    [self.adapter performBatchAnimated:YES updates:^(id<IGListBatchContext> batchContext){} completion:^(BOOL finished) {
+        IGTestStringBindableCell *batchedCell00 = [self cellAtSection:0 item:0];
+        IGTestNumberBindableCell *batchedCell01 = [self cellAtSection:0 item:1];
+        IGTestStringBindableCell *batchedCell02 = [self cellAtSection:0 item:2];
+        IGTestNumberBindableCell *batchedCell03 = [self cellAtSection:0 item:3];
+        IGTestNumberBindableCell *batchedCell04 = [self cellAtSection:0 item:4];
+
+        XCTAssertEqualObjects(batchedCell00.label.text, @"four");
+        XCTAssertEqualObjects(batchedCell01.textField.text, @"4");
+        XCTAssertEqualObjects(batchedCell02.label.text, @"seven");
+        XCTAssertEqualObjects(batchedCell03.textField.text, @"7");
+        XCTAssertEqualObjects(batchedCell04.textField.text, @"10");
+
+        [expectation fulfill];
+    }];
+
+    [self waitForExpectationsWithTimeout:30 handler:nil];
+}
+
 - (void)test_whenSelectingCell_thatCorrectViewModelSelected {
     [self setupWithObjects:@[
                              [[IGTestDiffingObject alloc] initWithKey:@1 objects:@[@7, @"seven"]],
@@ -262,3 +297,4 @@ - (void)test_whenUpdatingManually_with2Updates_thatBothCompletionBlocksCalled {
 }
 
 @end
+