DenTelezhkin / DTCollectionViewManager

Protocol-oriented UICollectionView management, powered by generics and associated types.
MIT License
312 stars 30 forks source link

Function storageDidPerformUpdate Batch update error #23

Closed Genhain closed 6 years ago

Genhain commented 7 years ago

Got this little ditty from the compiler as it threw an runtime exception

Invalid update: invalid number of items in section 0. The number of items contained in an existing section after the update (10) must be equal to the number of items contained in that section before the update (10), plus or minus the number of items inserted or deleted from that section (10 inserted, 0 deleted) and plus or minus the number of items moved into or out of that section (0 moved in, 0 moved out).

Now i recall a year or so back running into the same problem when working with UICollectionViews and i did come up with a solution but it escapes me, luckily i believe someone else documented their struggles and solutions on the same problem.

any change of an update or would you like me to make a pull request? I wont be able to do so myself for a few days at least...Work deadlines and such.

DenTelezhkin commented 7 years ago

Hey!

I've seen a number of workarounds to fix issues with UICollectionView in the past. Latest releases of DTCollectionViewManager are almost completely cleaned from all of them, one thing remaining is number of sections workaround here - https://github.com/DenHeadless/DTCollectionViewManager/blob/master/Source/DTCollectionViewManager.swift#L159-L163.

Before introducing another workaround, we need to have a reproducible case where this crash is easily seen, and have a unit test for it. If unit test is impossible for some reason, it would be great to have a sample project that reproduces the issue.

Let's start with identifying the problem and then try to find solution for it. I don't think issue is super pressing, we use DTCollectionViewManager in lots of projects, and they are stable. I think the issue may be minor and only occur in some uncommon situations. This doesn't mean of course, that we should not fix it =)

Please, take your time, it would be great if you came up with unit test or sample project and then we'll proceed with finding a solution.

RenGate commented 6 years ago

Hello folks. Here is the simple unit test that will help you to reproduce mentioned issue.

import XCTest
import DTCollectionViewManager

class Cell: UICollectionViewCell, ModelTransfer {
    func update(with model: Model) { }
}

class Model { }

class CollectionViewCrashTest: XCTestCase, DTCollectionViewManageable {
    weak var collectionView: UICollectionView?
    private var retainCollectionView: UICollectionView!

    override func setUp() {
        let flow = UICollectionViewFlowLayout()
        retainCollectionView = UICollectionView(frame: CGRect(x: 0, y: 0, width: 100, height: 100), collectionViewLayout: flow)
        collectionView = retainCollectionView

        manager.startManaging(withDelegate: self)
        manager.registerNibless(Cell.self)
    }

    func testThisWillWork() {
        manager.memoryStorage.addItems([Model(), Model(), Model()])
    }

    func testThisWillCrash() {
        manager.memoryStorage.setItems([Model(), Model()])
        manager.memoryStorage.addItems([Model(), Model(), Model()])
    }
}

If you will place a breakpoint at func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int at DTCollectionViewDataSource.swift, you will see that if addItems is called after the setItems, the numberOfItemsInSection method will be called twice and the number of returned items will be the same in both cases, which will lead to crash, just as it was described in mentioned blog post.

Genhain commented 6 years ago

@RenGate Thanks for taking the time, i have switched projects and have been swamped with other work related issues that this fell on the back burner. Much appreciated

DenTelezhkin commented 6 years ago

Hey guys!

I've pushed 6.1.0-beta.1 release, which hopefully resolves this issue 🎉