Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reusing of item presenters #596

Merged
merged 19 commits into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
415c121
ChatItemProtocol is extended with "func isEqual(to otherItem: ChatIte…
MikhailGasanov Jul 29, 2019
4b7dc7f
BaseChatViewController test items for equality with "isEqual(to:)" to…
MikhailGasanov Jul 29, 2019
e4de8db
Test example in ChattoApp for items reloading and presenters reusage
MikhailGasanov Jul 29, 2019
cdbc86b
BaseChatViewController calls update for reused presenters
MikhailGasanov Aug 1, 2019
19808ed
Text, photo and compound bubble presenters support updating
MikhailGasanov Aug 1, 2019
158b8c7
Test page for updated messages
MikhailGasanov Aug 1, 2019
2179ea8
UpdateType is made CaseIterable
MikhailGasanov Aug 2, 2019
8ab0b79
There is no unbind/bind calls if the cell stays the same
MikhailGasanov Aug 2, 2019
d2f0f6f
Content is updated only when it's needed
MikhailGasanov Aug 2, 2019
b5b063c
Switching from isEqual to hasSameContent
MikhailGasanov Aug 6, 2019
4acecc5
UpdatableChatItemPresenterProtocol & ContentEquatableChatItemProtocol
MikhailGasanov Aug 8, 2019
0a92e7e
guards are squeezed
MikhailGasanov Aug 8, 2019
4bd7502
Redundant hasSameContent is removed
MikhailGasanov Aug 8, 2019
8d6e119
CompoundMessagePresenter.Model conforms to ContentEquatableChatItemPr…
MikhailGasanov Aug 8, 2019
3f7daf5
Support of the message update if uid is changed
MikhailGasanov Aug 12, 2019
8d3b23c
Message model update logic is fixed
MikhailGasanov Aug 14, 2019
1c68374
A default implementation for a message update is moved to BaseMessage…
MikhailGasanov Aug 15, 2019
e9d4a3c
Rollback to the solution with "isItemUpdateSupported"
MikhailGasanov Aug 19, 2019
be04c3f
Merge branch 'master' into reusing-of-item-presenters
MikhailGasanov Aug 23, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Chatto/Source/Chat Items/BaseChatItemPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 10 additions & 0 deletions Chatto/Source/Chat Items/ChatItemProtocolDefinitions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
8 changes: 7 additions & 1 deletion Chatto/Source/Chat Items/DummyChatItemPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
30 changes: 18 additions & 12 deletions Chatto/Source/ChatController/BaseChatViewController+Changes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

import Foundation

public enum UpdateType {
public enum UpdateType: CaseIterable {
case normal
case firstLoad
case pagination
Expand Down
103 changes: 69 additions & 34 deletions Chatto/Tests/ChatController/BaseChatViewControllerTestHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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]]()) {
Expand All @@ -53,7 +53,7 @@ class TesteableChatViewController: BaseChatViewController {
}
}

class FakeDataSource: ChatDataSourceProtocol {
final class FakeDataSource: ChatDataSourceProtocol {
var hasMoreNext = false
var hasMorePrevious = false
var wasRequestedForPrevious = false
Expand All @@ -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
Expand All @@ -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)?
Expand Down Expand Up @@ -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
}
}
109 changes: 107 additions & 2 deletions Chatto/Tests/ChatController/BaseChatViewControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
}
}
}
2 changes: 1 addition & 1 deletion Chatto/Tests/ChatItemCompanionCollectionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Loading