diff --git a/CHANGELOG.md b/CHANGELOG.md index fa16b5fe..7ae471c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag ### Fixes +- Don't crash if you use `IGListSectionController` without a subclass [Maxime Ollivier](https://github.com/maxolls) (tbd) + - Testing crash fix when calling `-[IGListAdapter reloadObjects ...]` during an update [Maxime Ollivier](https://github.com/maxolls) (tbd) - Repaired Swift Package Manager support. [Petro Rovenskyy](https://github.com/3a4oT/) [(#1487)](https://github.com/Instagram/IGListKit/pull/1487) diff --git a/Source/IGListKit/IGListAdapter.m b/Source/IGListKit/IGListAdapter.m index 72db9d0d..21e79524 100644 --- a/Source/IGListKit/IGListAdapter.m +++ b/Source/IGListKit/IGListAdapter.m @@ -709,6 +709,15 @@ typedef struct OffsetRange { dataSource, object); return; } + + if ([sectionController isMemberOfClass:[IGListSectionController class]]) { + // If IGListSectionController is not subclassed, it could be a side effect of a problem. For example, nothing stops + // dataSource from returning a plain IGListSectionController if it doesn't recognize the object type, instead of throwing. + // Why not throw here then? Maybe we should, but in most cases, it feels like an over reaction. If we don't know how to render + // a single item, terminating the entire app might not be necessary. The dataSource should be the one who decides if throwing is appropriate. + IGFailAssert(@"Ignoring IGListSectionController that's not a subclass from data source %@ for object %@", NSStringFromClass([dataSource class]), NSStringFromClass([object class])); + return; + } // in case the section controller was created outside of -listAdapter:sectionControllerForObject: sectionController.collectionContext = self; diff --git a/Tests/IGListAdapterE2ETests.m b/Tests/IGListAdapterE2ETests.m index e5979d50..afe2ca00 100644 --- a/Tests/IGListAdapterE2ETests.m +++ b/Tests/IGListAdapterE2ETests.m @@ -16,6 +16,7 @@ #import "IGListAdapterUpdater.h" #import "IGListAdapterUpdaterInternal.h" #import "IGListTestCase.h" +#import "IGListTestCollectionViewLayout.h" #import "IGListTestHelpers.h" #import "IGListTestOffsettingLayout.h" #import "IGListUpdateTransactionBuilder.h" @@ -2643,4 +2644,20 @@ [self waitForExpectationsWithTimeout:30 handler:nil]; } +- (void)test_whenSectionControllerNotSubclassed_thatDoesNotCrash { + // We need a custom layout that creates attributes for all cells, even the ones with size + // zero, so that the UICollectionView requests all cells. Using `UICollectionViewDelegateFlowLayout` + // doesn't crash, because it doesn't seem to return attributes where the size is zero. + self.collectionView.collectionViewLayout = [IGListTestCollectionViewLayout new]; + + [self setupWithObjects:@[kIGTestDelegateDataSourceNoSectionControllerSubclass]]; + + XCTestExpectation *expectation = genExpectation; + [self.adapter reloadDataWithCompletion:^(BOOL finished) { + [self.collectionView layoutIfNeeded]; + [expectation fulfill]; + }]; + [self waitForExpectationsWithTimeout:30 handler:nil]; +} + @end diff --git a/Tests/Objects/IGListTestCollectionViewLayout.h b/Tests/Objects/IGListTestCollectionViewLayout.h new file mode 100644 index 00000000..28c9c8f0 --- /dev/null +++ b/Tests/Objects/IGListTestCollectionViewLayout.h @@ -0,0 +1,17 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import + +NS_ASSUME_NONNULL_BEGIN + +/// Layout that 1) creates all attributes regardless of size, and 2) positions them with origin (0,0) +@interface IGListTestCollectionViewLayout : UICollectionViewLayout + +@end + +NS_ASSUME_NONNULL_END diff --git a/Tests/Objects/IGListTestCollectionViewLayout.m b/Tests/Objects/IGListTestCollectionViewLayout.m new file mode 100644 index 00000000..f4c3934c --- /dev/null +++ b/Tests/Objects/IGListTestCollectionViewLayout.m @@ -0,0 +1,56 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "IGListTestCollectionViewLayout.h" + +@implementation IGListTestCollectionViewLayout { + NSDictionary *_attributes; +} + +- (void)prepareLayout { + UICollectionView *const collectionView = self.collectionView; + + // Get the UICollectionViewDelegateFlowLayout for sizes + if (![collectionView.delegate conformsToProtocol:@protocol(UICollectionViewDelegateFlowLayout)]) { + _attributes = nil; + return; + } + const id flowDelegate = (id)collectionView.delegate; + + // Create the attributes + NSMutableDictionary *const attributes = [NSMutableDictionary new]; + const NSInteger numberOfSections = collectionView.numberOfSections; + + for (NSInteger section = 0; section < numberOfSections; section++) { + const NSInteger numberOfItems = [collectionView numberOfItemsInSection:section]; + for (NSInteger item = 0; item < numberOfItems; item++) { + NSIndexPath *const indexPath = [NSIndexPath indexPathForItem:item inSection:section]; + UICollectionViewLayoutAttributes *const attribute = [UICollectionViewLayoutAttributes layoutAttributesForCellWithIndexPath:indexPath]; + const CGSize size = [flowDelegate collectionView:collectionView layout:self sizeForItemAtIndexPath:indexPath]; + attribute.frame = CGRectMake(0, 0, size.width, size.height); + attributes[indexPath] = attribute; + } + } + + _attributes = [attributes copy]; +} + +- (nullable NSArray<__kindof UICollectionViewLayoutAttributes *> *)layoutAttributesForElementsInRect:(CGRect)rect { + NSMutableArray *attributes = [NSMutableArray new]; + [_attributes enumerateKeysAndObjectsUsingBlock:^(NSIndexPath *indexPath, UICollectionViewLayoutAttributes *attribute, BOOL* stop) { + if (CGRectIntersectsRect(attribute.frame, rect)) { + [attributes addObject:attribute]; + } + }]; + return [attributes copy]; +} + +- (nullable UICollectionViewLayoutAttributes *)layoutAttributesForItemAtIndexPath:(NSIndexPath *)indexPath { + return _attributes[indexPath]; +} + +@end diff --git a/Tests/Objects/IGTestDelegateDataSource.h b/Tests/Objects/IGTestDelegateDataSource.h index 99e6f94e..ba82eb34 100644 --- a/Tests/Objects/IGTestDelegateDataSource.h +++ b/Tests/Objects/IGTestDelegateDataSource.h @@ -15,6 +15,7 @@ @class IGTestObject; extern NSObject *const kIGTestDelegateDataSourceSkipObject; +extern NSObject *const kIGTestDelegateDataSourceNoSectionControllerSubclass; @interface IGTestDelegateDataSource : NSObject diff --git a/Tests/Objects/IGTestDelegateDataSource.m b/Tests/Objects/IGTestDelegateDataSource.m index 7a05da69..b6b8d360 100644 --- a/Tests/Objects/IGTestDelegateDataSource.m +++ b/Tests/Objects/IGTestDelegateDataSource.m @@ -13,6 +13,7 @@ #import "IGTestObject.h" NSObject *const kIGTestDelegateDataSourceSkipObject = @"kIGTestDelegateDataSourceSkipObject"; +NSObject *const kIGTestDelegateDataSourceNoSectionControllerSubclass = @"kIGTestDelegateDataSourceNoSectionControllerSubclass"; @implementation IGTestDelegateDataSource @@ -23,6 +24,8 @@ NSObject *const kIGTestDelegateDataSourceSkipObject = @"kIGTestDelegateDataSourc - (IGListSectionController *)listAdapter:(IGListAdapter *)listAdapter sectionControllerForObject:(id)object { if ([object isEqual:kIGTestDelegateDataSourceSkipObject]) { return nil; + } else if ([object isEqual:kIGTestDelegateDataSourceNoSectionControllerSubclass]) { + return [IGListSectionController new]; } IGTestDelegateController *sectionController = [[IGTestDelegateController alloc] init]; sectionController.cellConfigureBlock = self.cellConfigureBlock;