Merge pull request badoo#596 from badoo/reusing-of-item-presenters

Reusing of item presenters
appsailor · Aug 23, 2019 · ade6d67 · ade6d67
2 parents f1da3a3 + be04c3f
commit ade6d67
Show file tree

Hide file tree

Showing 38 changed files with 1,033 additions and 92 deletions.
diff --git a/Chatto/Source/Chat Items/BaseChatItemPresenter.swift b/Chatto/Source/Chat Items/BaseChatItemPresenter.swift
@@ -39,6 +39,14 @@ open class BaseChatItemPresenter<CellT: UICollectionViewCell>: ChatItemPresenter
         assert(false, "Implement in subclass")
     }
 
+    open var isItemUpdateSupported: Bool {
+        fatalError("Implement in subclass")
+    }
+
+    open func update(with chatItem: ChatItemProtocol) {
+        fatalError("Implement in subclass")
+    }
+
     open var canCalculateHeightInBackground: Bool {
         return false
     }

diff --git a/Chatto/Source/Chat Items/ChatItemProtocolDefinitions.swift b/Chatto/Source/Chat Items/ChatItemProtocolDefinitions.swift
@@ -42,6 +42,10 @@ public protocol ChatItemMenuPresenterProtocol {
 
 public protocol ChatItemPresenterProtocol: AnyObject, ChatItemMenuPresenterProtocol {
     static func registerCells(_ collectionView: UICollectionView)
+
+    var isItemUpdateSupported: Bool { get }
+    func update(with chatItem: ChatItemProtocol)
+
     var canCalculateHeightInBackground: Bool { get } // Default is false
     func heightForCell(maximumWidth width: CGFloat, decorationAttributes: ChatItemDecorationAttributesProtocol?) -> CGFloat
     func dequeueCell(collectionView: UICollectionView, indexPath: IndexPath) -> UICollectionViewCell
@@ -64,3 +68,9 @@ public protocol ChatItemPresenterBuilderProtocol {
     func createPresenterWithChatItem(_ chatItem: ChatItemProtocol) -> ChatItemPresenterProtocol
     var presenterType: ChatItemPresenterProtocol.Type { get }
 }
+
+// MARK: - Updatable Chat Items
+
+public protocol ContentEquatableChatItemProtocol: ChatItemProtocol {
+    func hasSameContent(as anotherItem: ChatItemProtocol) -> Bool
+}
diff --git a/Chatto/Source/Chat Items/DummyChatItemPresenter.swift b/Chatto/Source/Chat Items/DummyChatItemPresenter.swift
@@ -24,13 +24,19 @@
 
 import Foundation
 
-// Handles messages that aren't supported so they appear as invisible
+// Handles messages which aren't supported. So, they appear as invisible.
 class DummyChatItemPresenter: ChatItemPresenterProtocol {
 
     class func registerCells(_ collectionView: UICollectionView) {
         collectionView.register(DummyCollectionViewCell.self, forCellWithReuseIdentifier: "cell-id-unhandled-message")
     }
 
+    let isItemUpdateSupported = true
+
+    func update(with chatItem: ChatItemProtocol) {
+        // Does nothing
+    }
+
     var canCalculateHeightInBackground: Bool {
         return true
     }

diff --git a/Chatto/Source/ChatController/BaseChatViewController+Changes.swift b/Chatto/Source/ChatController/BaseChatViewController+Changes.swift
@@ -301,18 +301,24 @@ extension BaseChatViewController {
 
     private func createCompanionCollection(fromChatItems newItems: [DecoratedChatItem], previousCompanionCollection oldItems: ChatItemCompanionCollection) -> ChatItemCompanionCollection {
         return ChatItemCompanionCollection(items: newItems.map { (decoratedChatItem) -> ChatItemCompanion in
-            let chatItem = decoratedChatItem.chatItem
-            var presenter: ChatItemPresenterProtocol!
-            // We assume that a same messageId can't mutate from one cell class to a different one.
-            // If we ever need to support that then generation of changes needs to suppport reloading items.
-            // Oherwise updateVisibleCells may try to update existing cell with a new presenter which is working with a different type of cell
-
-            // Optimization: reuse presenter if it's the same instance.
-            if let oldChatItemCompanion = oldItems[decoratedChatItem.uid], oldChatItemCompanion.chatItem === chatItem {
-                presenter = oldChatItemCompanion.presenter
-            } else {
-                presenter = self.createPresenterForChatItem(decoratedChatItem.chatItem)
-            }
+
+            /*
+             We use an assumption, that message having a specific messageId never changes its type.
+             If such changes has to be supported, then generation of changes has to suppport reloading items.
+             Otherwise, updateVisibleCells may try to update the existing cells with new presenters which aren't able to work with another types.
+             */
+
+            let presenter: ChatItemPresenterProtocol = {
+                guard let oldChatItemCompanion = oldItems[decoratedChatItem.uid] ?? oldItems[decoratedChatItem.chatItem.uid],
+                    oldChatItemCompanion.chatItem.type == decoratedChatItem.chatItem.type,
+                    oldChatItemCompanion.presenter.isItemUpdateSupported else {
+                        return self.createPresenterForChatItem(decoratedChatItem.chatItem)
+                }
+
+                oldChatItemCompanion.presenter.update(with: decoratedChatItem.chatItem)
+                return oldChatItemCompanion.presenter
+            }()
+
             return ChatItemCompanion(uid: decoratedChatItem.uid, chatItem: decoratedChatItem.chatItem, presenter: presenter, decorationAttributes: decoratedChatItem.decorationAttributes)
         })
     }

diff --git a/Chatto/Source/ChatController/Collaborators/ChatDataSourceProtocol.swift b/Chatto/Source/ChatController/Collaborators/ChatDataSourceProtocol.swift
@@ -24,7 +24,7 @@
 
 import Foundation
 
-public enum UpdateType {
+public enum UpdateType: CaseIterable {
     case normal
     case firstLoad
     case pagination

diff --git a/Chatto/Tests/ChatController/BaseChatViewControllerTestHelpers.swift b/Chatto/Tests/ChatController/BaseChatViewControllerTestHelpers.swift
@@ -32,7 +32,7 @@ func createFakeChatItems(count: Int) -> [ChatItemProtocol] {
     return items
 }
 
-class TesteableChatViewController: BaseChatViewController {
+final class TesteableChatViewController: BaseChatViewController {
     let presenterBuilders: [ChatItemType: [ChatItemPresenterBuilderProtocol]]
     let chatInputView = UIView()
     init(presenterBuilders: [ChatItemType: [ChatItemPresenterBuilderProtocol]] = [ChatItemType: [ChatItemPresenterBuilderProtocol]]()) {
@@ -53,7 +53,7 @@ class TesteableChatViewController: BaseChatViewController {
     }
 }
 
-class FakeDataSource: ChatDataSourceProtocol {
+final class FakeDataSource: ChatDataSourceProtocol {
     var hasMoreNext = false
     var hasMorePrevious = false
     var wasRequestedForPrevious = false
@@ -80,43 +80,27 @@ class FakeDataSource: ChatDataSourceProtocol {
     }
 }
 
-class FakeCell: UICollectionViewCell {}
+final class FakeCell: UICollectionViewCell {}
+
+final class FakePresenterBuilder: ChatItemPresenterBuilderProtocol {
+    private(set) var createdPresenters: [ChatItemPresenterProtocol] = []
 
-class FakePresenterBuilder: ChatItemPresenterBuilderProtocol {
-    var presentersCreatedCount: Int = 0
     func canHandleChatItem(_ chatItem: ChatItemProtocol) -> Bool {
         return chatItem.type == "fake-type"
     }
 
     func createPresenterWithChatItem(_ chatItem: ChatItemProtocol) -> ChatItemPresenterProtocol {
-        self.presentersCreatedCount += 1
-        return FakePresenter()
+        let presenter = FakePresenter()
+        presenter._isItemUpdateSupportedResult = false
+        self.createdPresenters.append(presenter)
+        return presenter
     }
 
     var presenterType: ChatItemPresenterProtocol.Type {
         return FakePresenter.self
     }
 }
 
-class FakePresenter: BaseChatItemPresenter<FakeCell> {
-    override class func registerCells(_ collectionView: UICollectionView) {
-        collectionView.register(FakeCell.self, forCellWithReuseIdentifier: "fake-cell")
-    }
-
-    override func heightForCell(maximumWidth width: CGFloat, decorationAttributes: ChatItemDecorationAttributesProtocol?) -> CGFloat {
-        return 10
-    }
-
-    override func dequeueCell(collectionView: UICollectionView, indexPath: IndexPath) -> UICollectionViewCell {
-        return collectionView.dequeueReusableCell(withReuseIdentifier: "fake-cell", for: indexPath as IndexPath)
-    }
-
-    override func configureCell(_ cell: UICollectionViewCell, decorationAttributes: ChatItemDecorationAttributesProtocol?) {
-        let fakeCell = cell as! FakeCell
-        fakeCell.backgroundColor = UIColor.red
-    }
-}
-
 final class FakeChatItem: ChatItemProtocol {
     var uid: String
     var type: ChatItemType
@@ -126,14 +110,6 @@ final class FakeChatItem: ChatItemProtocol {
     }
 }
 
-final class FakeChatItemPresenter: ChatItemPresenterProtocol {
-    init() {}
-    static func registerCells(_ collectionView: UICollectionView) {}
-    func heightForCell(maximumWidth width: CGFloat, decorationAttributes: ChatItemDecorationAttributesProtocol?) -> CGFloat { return 0 }
-    func dequeueCell(collectionView: UICollectionView, indexPath: IndexPath) -> UICollectionViewCell { return UICollectionViewCell() }
-    func configureCell(_ cell: UICollectionViewCell, decorationAttributes: ChatItemDecorationAttributesProtocol?) {}
-}
-
 final class SerialTaskQueueTestHelper: SerialTaskQueueProtocol {
 
     var onAllTasksFinished: (() -> Void)?
@@ -179,3 +155,62 @@ final class SerialTaskQueueTestHelper: SerialTaskQueueProtocol {
         }
     }
 }
+
+// MARK: - Updatable
+
+final class FakeUpdatablePresenterBuilder: ChatItemPresenterBuilderProtocol {
+
+    private(set) var createdPresenters: [ChatItemPresenterProtocol] = []
+
+    var updatedPresentersCount: Int {
+        return self.createdPresenters.reduce(0) { return $0 + ($1 as! FakePresenter)._updateWithChatItemCallsCount }
+    }
+
+    func canHandleChatItem(_ chatItem: ChatItemProtocol) -> Bool {
+        return chatItem.type == "fake-type"
+    }
+
+    func createPresenterWithChatItem(_ chatItem: ChatItemProtocol) -> ChatItemPresenterProtocol {
+        let presenter = FakePresenter()
+        presenter._isItemUpdateSupportedResult = true
+        self.createdPresenters.append(presenter)
+        return presenter
+    }
+
+    var presenterType: ChatItemPresenterProtocol.Type {
+        return FakePresenter.self
+    }
+}
+
+final class FakePresenter: ChatItemPresenterProtocol {
+
+    static func registerCells(_ collectionView: UICollectionView) {
+        collectionView.register(FakeCell.self, forCellWithReuseIdentifier: "fake-cell")
+    }
+
+    var _isItemUpdateSupportedResult: Bool!
+    var isItemUpdateSupported: Bool {
+        return self._isItemUpdateSupportedResult
+    }
+
+    private var _updateWithChatItemCalls: [(ChatItemProtocol)] = []
+    var _updateWithChatItemIsCalled: Bool { return self._updateWithChatItemCallsCount > 0 }
+    var _updateWithChatItemCallsCount: Int { return self._updateWithChatItemCalls.count }
+    var _updateWithChatItemLastCallParams: ChatItemProtocol? { return self._updateWithChatItemCalls.last }
+    func update(with chatItem: ChatItemProtocol) {
+        self._updateWithChatItemCalls.append((chatItem))
+    }
+
+    func heightForCell(maximumWidth width: CGFloat, decorationAttributes: ChatItemDecorationAttributesProtocol?) -> CGFloat {
+        return 10
+    }
+
+    func dequeueCell(collectionView: UICollectionView, indexPath: IndexPath) -> UICollectionViewCell {
+        return collectionView.dequeueReusableCell(withReuseIdentifier: "fake-cell", for: indexPath as IndexPath)
+    }
+
+    func configureCell(_ cell: UICollectionViewCell, decorationAttributes: ChatItemDecorationAttributesProtocol?) {
+        let fakeCell = cell as! FakeCell
+        fakeCell.backgroundColor = UIColor.red
+    }
+}
diff --git a/Chatto/Tests/ChatController/BaseChatViewControllerTests.swift b/Chatto/Tests/ChatController/BaseChatViewControllerTests.swift
@@ -67,7 +67,7 @@ class ChatViewControllerTests: XCTestCase {
         fakeDataSource.chatItems = createFakeChatItems(count: 2)
         controller.chatDataSource = fakeDataSource
         self.fakeDidAppearAndLayout(controller: controller)
-        XCTAssertEqual(2, presenterBuilder.presentersCreatedCount)
+        XCTAssertEqual(2, presenterBuilder.createdPresenters.count)
     }
 
     func testThat_WhenDataSourceChanges_ThenCollectionViewUpdatesAsynchronously() {
@@ -268,10 +268,115 @@ class ChatViewControllerTests: XCTestCase {
 
     // MARK: helpers
 
-    private func fakeDidAppearAndLayout(controller: TesteableChatViewController) {
+    fileprivate func fakeDidAppearAndLayout(controller: TesteableChatViewController) {
         controller.view.frame = CGRect(x: 0, y: 0, width: 400, height: 900)
         controller.viewWillAppear(true)
         controller.viewDidAppear(true)
         controller.view.layoutIfNeeded()
     }
 }
+
+extension ChatViewControllerTests {
+
+    // MARK: Same Items
+
+    func testThat_GivenDataSourceWithNotUpdatableItemPresenters_AndTwoItems_WhenItIsUpdatedWithSameItems_ThenTwoPresentersAreCreated() {
+        let presenterBuilder = FakePresenterBuilder()
+        let controller = TesteableChatViewController(presenterBuilders: ["fake-type": [presenterBuilder]])
+        let fakeDataSource = FakeDataSource()
+        fakeDataSource.chatItems = createFakeChatItems(count: 2)
+        controller.chatDataSource = fakeDataSource
+        self.fakeDidAppearAndLayout(controller: controller)
+        let numberOfCreatedPresentersBeforeUpdate = presenterBuilder.createdPresenters.count
+
+        fakeDataSource.chatItems = createFakeChatItems(count: 2)
+        let asyncExpectation = expectation(description: "update")
+        controller.enqueueModelUpdate(updateType: .normal) {
+            asyncExpectation.fulfill()
+        }
+
+        self.waitForExpectations(timeout: 1) { _ in
+            let numberOfCreatedPresentersAfterUpdate = presenterBuilder.createdPresenters.count
+            let numberOfCreatedPresenters = numberOfCreatedPresentersAfterUpdate - numberOfCreatedPresentersBeforeUpdate
+            XCTAssertEqual(numberOfCreatedPresenters, 2)
+        }
+    }
+
+    func testThat_GivenDataSourceWithUpdatableItemPresenters_AndTwoItems_WhenItIsUpdatedWithSameItems_ThenTwoPresentersAreUpdated() {
+        let presenterBuilder = FakeUpdatablePresenterBuilder()
+        let controller = TesteableChatViewController(presenterBuilders: ["fake-type": [presenterBuilder]])
+        let fakeDataSource = FakeDataSource()
+        fakeDataSource.chatItems = createFakeChatItems(count: 2)
+        controller.chatDataSource = fakeDataSource
+        self.fakeDidAppearAndLayout(controller: controller)
+        let numberOfUpdatedPresentersBeforeUpdate = presenterBuilder.updatedPresentersCount
+        let numberOfCreatedPresentersBeforeUpdate = presenterBuilder.createdPresenters.count
+
+        fakeDataSource.chatItems = createFakeChatItems(count: 2)
+        let asyncExpectation = expectation(description: "update")
+        controller.enqueueModelUpdate(updateType: .normal) {
+            asyncExpectation.fulfill()
+        }
+
+        self.waitForExpectations(timeout: 1) { _ in
+            let numberOfUpdatedPresentersAfterUpdate = presenterBuilder.updatedPresentersCount
+            let numberOfUpdatedPresenters = numberOfUpdatedPresentersAfterUpdate - numberOfUpdatedPresentersBeforeUpdate
+            XCTAssertEqual(numberOfUpdatedPresenters, 2)
+
+            let numberOfCreatedPresentersAfterUpdate = presenterBuilder.createdPresenters.count
+            let numberOfCreatedPresenters = numberOfCreatedPresentersAfterUpdate - numberOfCreatedPresentersBeforeUpdate
+            XCTAssertEqual(numberOfCreatedPresenters, 0)
+        }
+    }
+
+    // MARK: New Items
+
+    func testThat_GivenDataSourceWithNotUpdatableItemPresenters_AndTwoItems_WhenItIsUpdatedWithOneNewItem_ThenThreePresentersAreCreated() {
+        let presenterBuilder = FakePresenterBuilder()
+        let controller = TesteableChatViewController(presenterBuilders: ["fake-type": [presenterBuilder]])
+        let fakeDataSource = FakeDataSource()
+        fakeDataSource.chatItems = createFakeChatItems(count: 2)
+        controller.chatDataSource = fakeDataSource
+        self.fakeDidAppearAndLayout(controller: controller)
+        let numberOfCreatedPresentersBeforeUpdate = presenterBuilder.createdPresenters.count
+
+        fakeDataSource.chatItems = createFakeChatItems(count: 3)
+        let asyncExpectation = expectation(description: "update")
+        controller.enqueueModelUpdate(updateType: .normal) {
+            asyncExpectation.fulfill()
+        }
+
+        self.waitForExpectations(timeout: 1) { _ in
+            let numberOfCreatedPresentersAfterUpdate = presenterBuilder.createdPresenters.count
+            let numberOfCreatedPresenters = numberOfCreatedPresentersAfterUpdate - numberOfCreatedPresentersBeforeUpdate
+            XCTAssertEqual(numberOfCreatedPresenters, 3)
+        }
+    }
+
+    func testThat_GivenDataSourceWithUpdatableItemPresenters_AndTwoItems_WhenItIsUpdatedWithOneNewItem_ThenTwoPresentersAreUpdated_AndOnePresenterIsCreated() {
+        let presenterBuilder = FakeUpdatablePresenterBuilder()
+        let controller = TesteableChatViewController(presenterBuilders: ["fake-type": [presenterBuilder]])
+        let fakeDataSource = FakeDataSource()
+        fakeDataSource.chatItems = createFakeChatItems(count: 2)
+        controller.chatDataSource = fakeDataSource
+        self.fakeDidAppearAndLayout(controller: controller)
+        let numberOfUpdatedPresentersBeforeUpdate = presenterBuilder.updatedPresentersCount
+        let numberOfCreatedPresentersBeforeUpdate = presenterBuilder.createdPresenters.count
+
+        fakeDataSource.chatItems = createFakeChatItems(count: 3)
+        let asyncExpectation = expectation(description: "update")
+        controller.enqueueModelUpdate(updateType: .normal) {
+            asyncExpectation.fulfill()
+        }
+
+        self.waitForExpectations(timeout: 1) { _ in
+            let numberOfUpdatedPresentersAfterUpdate = presenterBuilder.updatedPresentersCount
+            let numberOfUpdatedPresenters = numberOfUpdatedPresentersAfterUpdate - numberOfUpdatedPresentersBeforeUpdate
+            XCTAssertEqual(numberOfUpdatedPresenters, 2)
+
+            let numberOfCreatedPresentersAfterUpdate = presenterBuilder.createdPresenters.count
+            let numberOfCreatedPresenters = numberOfCreatedPresentersAfterUpdate - numberOfCreatedPresentersBeforeUpdate
+            XCTAssertEqual(numberOfCreatedPresenters, 1)
+        }
+    }
+}
diff --git a/Chatto/Tests/ChatItemCompanionCollectionTests.swift b/Chatto/Tests/ChatItemCompanionCollectionTests.swift
@@ -31,7 +31,7 @@ class ChatItemCompanionCollectionTests: XCTestCase {
 
     override func setUp() {
         super.setUp()
-        let fakeChatItemPresenter = FakeChatItemPresenter()
+        let fakeChatItemPresenter = FakePresenter()
         let items = [
             ChatItemCompanion(uid: "3", chatItem: FakeChatItem(uid: "3", type: "type3"), presenter: fakeChatItemPresenter, decorationAttributes: nil),
             ChatItemCompanion(uid: "1", chatItem: FakeChatItem(uid: "1", type: "type1"), presenter: fakeChatItemPresenter, decorationAttributes: nil),