Section Controller Incorrect Index

[amonshiz]

New issue checklist

• I have reviewed the README and documentation
• I have searched existing issues and this is not a duplicate

General information

• IGListKit version: <3.0.0

Debug information

This required only maintaining a reference to a section controller beyond the time when its owning list adapter was deallocated. In this situation the section controller would be retained but the list adapter not (weak property) and so a [sc.collectionContext sectionForSectionController:sc] would return 0 due to nil messaging.

Thankfully #671 addressed part of this issue. Now, because the value of the index is stored in the sectionIndex property that value will at least represent the last known correct value. In situations where the section controller was never added to the list adapter's section map then the value will be NSNotFound.

However, that does not really address the overall issue of a section controller having an index after the list adapter, and thus all list context, being deallocated. In those situations I propose that the sectionIndex for all existing section controllers be reset to NSNotFound in the dealloc method of the IGListSectionMap object.

[rnystrom]

@amonshiz you had a unit test that failed for this, right? Can you paste the test in here?

[jessesquires]

what's the status here? anyone working on this?

[amonshiz]

yeah sorry, the current implementation at least maintains the last known section index correctly.

[amonshiz]

@rnystrom here is a link to the test that will demonstrate the issue: https://github.com/Instagram/IGListKit/blob/master/Tests/IGListAdapterTests.m#L1190-L1215

and here is a version of that test converted to work for code before a4e5ad8:

- (void)test_withStrongRefToSectionController_thatAdapterSectionIndexIsZero_thatSectionControllerIndexDoesNotChange {
  IGListSectionController *sc = nil;

  // hold a weak reference to simulate what would happen to the collectionContext object on a section controller
  // if the section controller were held strongly by an async block and the rest of the infra was deallocated
  __weak IGListAdapter *wAdapter = nil;

  @autoreleasepool {
    IGListTestAdapterDataSource *dataSource = [IGListTestAdapterDataSource new];
    IGListReloadDataUpdater *updater = [IGListReloadDataUpdater new];
    IGListAdapter *adapter = [[IGListAdapter alloc] initWithUpdater:updater
                                                     viewController:nil];
    adapter.dataSource = dataSource;
    adapter.collectionView = self.collectionView;
    wAdapter = adapter;

    dataSource.objects = @[@0, @1, @2];
    [adapter performUpdatesAnimated:NO completion:nil];

    sc = [adapter sectionControllerForSection:1];
    XCTAssertEqual([adapter sectionForSectionController:sc], 1);
  }

  XCTAssertEqual([wAdapter sectionForSectionController:sc], 1);
  XCTAssertEqual([wAdapter sectionForSectionController:sc], 0);

With the current implementation (using the property) the second to last assert will not fire because the property does not update when a list adapter is deallocated. prior to that change that assert would fail because the wAdapter would have deallocated and so would be nil messaging and return 0.