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 10 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
9 changes: 9 additions & 0 deletions Chatto/Source/Chat Items/BaseChatItemPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ open class BaseChatItemPresenter<CellT: UICollectionViewCell>: ChatItemPresenter

public init() {}

open var isItemUpdateSupported: Bool {
assertionFailure("Implement in subclass")
return false
}

open func update(with chatItem: ChatItemProtocol) {
assertionFailure("Implement in subclass")
}

open class func registerCells(_ collectionView: UICollectionView) {
assert(false, "Implement in subclass")
}
Expand Down
5 changes: 5 additions & 0 deletions Chatto/Source/Chat Items/ChatItemProtocolDefinitions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public typealias ChatItemType = String

public protocol ChatItemProtocol: AnyObject, UniqueIdentificable {
var type: ChatItemType { get }
func hasSameContent(as anotherItem: ChatItemProtocol) -> Bool
}

public protocol ChatItemDecorationAttributesProtocol {
Expand All @@ -42,6 +43,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 Down
10 changes: 9 additions & 1 deletion Chatto/Source/Chat Items/DummyChatItemPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,21 @@

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")
}

var isItemUpdateSupported: Bool {
return true
}

func update(with chatItem: ChatItemProtocol) {
// Does nothing
}

var canCalculateHeightInBackground: Bool {
return true
}
Expand Down
36 changes: 24 additions & 12 deletions Chatto/Source/ChatController/BaseChatViewController+Changes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -301,18 +301,30 @@ 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] else {
return self.createPresenterForChatItem(decoratedChatItem.chatItem)
}

guard oldChatItemCompanion.chatItem.type == decoratedChatItem.chatItem.type else {
return self.createPresenterForChatItem(decoratedChatItem.chatItem)
}

guard 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
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,16 @@ class FakeDataSource: ChatDataSourceProtocol {
class FakeCell: UICollectionViewCell {}

class FakePresenterBuilder: ChatItemPresenterBuilderProtocol {
var presentersCreatedCount: Int = 0
private(set) var createdPresenters: [ChatItemPresenterProtocol] = []

func canHandleChatItem(_ chatItem: ChatItemProtocol) -> Bool {
return chatItem.type == "fake-type"
}

func createPresenterWithChatItem(_ chatItem: ChatItemProtocol) -> ChatItemPresenterProtocol {
self.presentersCreatedCount += 1
return FakePresenter()
let presenter = FakePresenter()
self.createdPresenters.append(presenter)
return presenter
}

var presenterType: ChatItemPresenterProtocol.Type {
Expand All @@ -99,6 +101,20 @@ class FakePresenterBuilder: ChatItemPresenterBuilderProtocol {
}

class FakePresenter: BaseChatItemPresenter<FakeCell> {

var _isItemUpdateSupportedReturnValue: Bool = false
override var isItemUpdateSupported: Bool {
return self._isItemUpdateSupportedReturnValue
}

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 }
override func update(with chatItem: ChatItemProtocol) {
self._updateWithChatItemCalls.append((chatItem))
}

override class func registerCells(_ collectionView: UICollectionView) {
collectionView.register(FakeCell.self, forCellWithReuseIdentifier: "fake-cell")
}
Expand All @@ -124,10 +140,15 @@ final class FakeChatItem: ChatItemProtocol {
self.uid = uid
self.type = type
}
func hasSameContent(as anotherItem: ChatItemProtocol) -> Bool {
return self.type == anotherItem.type
}
}

final class FakeChatItemPresenter: ChatItemPresenterProtocol {
init() {}
var isItemUpdateSupported: Bool { return false }
func update(with chatItem: ChatItemProtocol) {}
static func registerCells(_ collectionView: UICollectionView) {}
func heightForCell(maximumWidth width: CGFloat, decorationAttributes: ChatItemDecorationAttributesProtocol?) -> CGFloat { return 0 }
func dequeueCell(collectionView: UICollectionView, indexPath: IndexPath) -> UICollectionViewCell { return UICollectionViewCell() }
Expand Down
78 changes: 76 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,84 @@ 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 {

func testThat_WhenDataSourceIsUpdatedWithOneNewItem_ThenOneNewItemPresenterIsCreated() {
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)
presenterBuilder.createdPresenters.forEach { ($0 as! FakePresenter)._isItemUpdateSupportedReturnValue = true }
let numberOfPresentersBeforeUpdate = 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 numberOfPresenterAfterUpdate = presenterBuilder.createdPresenters.count
XCTAssertEqual(numberOfPresenterAfterUpdate - numberOfPresentersBeforeUpdate, 1)
}
}

func testThat_GivenPresenterSupportsUpdates_WhenDataSourceIsUpdatedWithTheSameItem_ThendNewPresenterIsNotCreated_AndPresenterIsUpdatedOnce() {
let presenterBuilder = FakePresenterBuilder()
let controller = TesteableChatViewController(presenterBuilders: ["fake-type": [presenterBuilder]])
let fakeDataSource = FakeDataSource()
fakeDataSource.chatItems = createFakeChatItems(count: 1)
controller.chatDataSource = fakeDataSource
self.fakeDidAppearAndLayout(controller: controller)
let presenter = presenterBuilder.createdPresenters.last! as! FakePresenter
presenter._isItemUpdateSupportedReturnValue = true
XCTAssertEqual(presenter._updateWithChatItemCallsCount, 0)
XCTAssertEqual(presenterBuilder.createdPresenters.count, 1)

fakeDataSource.chatItems = createFakeChatItems(count: 1)
let asyncExpectation = expectation(description: "update")
controller.enqueueModelUpdate(updateType: .normal) {
asyncExpectation.fulfill()
}

self.waitForExpectations(timeout: 1) { _ in
XCTAssertEqual(presenterBuilder.createdPresenters.count, 1)
XCTAssertEqual(presenter._updateWithChatItemCallsCount, 1)
XCTAssert(presenter._updateWithChatItemLastCallParams! === fakeDataSource.chatItems.first!)
}
}

func testThat_GivenPresenterDoesntSupportUpdates_WhenDataSourceIsUpdatedWithTheSameItem_ThenPresenterIsNotUpdated_AndNewPresenterIsCreated() {
let presenterBuilder = FakePresenterBuilder()
let controller = TesteableChatViewController(presenterBuilders: ["fake-type": [presenterBuilder]])
let fakeDataSource = FakeDataSource()
fakeDataSource.chatItems = createFakeChatItems(count: 1)
controller.chatDataSource = fakeDataSource
self.fakeDidAppearAndLayout(controller: controller)
let presenter = presenterBuilder.createdPresenters.last! as! FakePresenter
presenter._isItemUpdateSupportedReturnValue = false
XCTAssertEqual(presenter._updateWithChatItemCallsCount, 0)
XCTAssertEqual(presenterBuilder.createdPresenters.count, 1)

fakeDataSource.chatItems = createFakeChatItems(count: 1)
let asyncExpectation = expectation(description: "update")
controller.enqueueModelUpdate(updateType: .normal) {
asyncExpectation.fulfill()
}

self.waitForExpectations(timeout: 1) { _ in
XCTAssertFalse(presenter._updateWithChatItemIsCalled)
XCTAssertEqual(presenterBuilder.createdPresenters.count, 2)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,9 @@ open class MessageModel: MessageModelProtocol {
self.date = date
self.status = status
}

public func hasSameContent(as anotherItem: ChatItemProtocol) -> Bool {
assertionFailure("Should be implemented in a subclass.")
return false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ open class BaseMessagePresenter<BubbleViewT, ViewModelBuilderT, InteractionHandl
self.interactionHandler = interactionHandler
}

public let messageModel: ModelT
public internal(set) var messageModel: ModelT {
didSet { self.messageViewModel = self.createViewModel() }
}
public let sizingCell: BaseMessageCollectionViewCell<BubbleViewT>
public let viewModelBuilder: ViewModelBuilderT
public let interactionHandler: InteractionHandlerT?
public let cellStyle: BaseMessageCollectionViewCellStyleProtocol

public private(set) final lazy var messageViewModel: ViewModelT = {
return self.createViewModel()
}()
public private(set) final lazy var messageViewModel: ViewModelT = self.createViewModel()

open func createViewModel() -> ViewModelT {
let viewModel = self.viewModelBuilder.createViewModel(self.messageModel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ open class CompoundMessagePresenter<ViewModelBuilderT, InteractionHandlerT>
public typealias ViewModelT = ViewModelBuilderT.ViewModelT

public let compoundCellStyle: CompoundBubbleViewStyleProtocol
private let contentFactories: [AnyMessageContentFactory<ModelT>]
private let compoundCellDimensions: CompoundBubbleLayoutProvider.Dimensions

private let compoundCellDimensions: CompoundBubbleLayoutProvider.Dimensions
private let cache: Cache<CompoundBubbleLayoutProvider.Configuration, CompoundBubbleLayoutProvider>
private let accessibilityIdentifier: String?
private let menuPresenter: ChatItemMenuPresenterProtocol?

private var contentPresenters: [MessageContentPresenterProtocol] = []
private let initialContentFactories: [AnyMessageContentFactory<ModelT>]
private var contentFactories: [AnyMessageContentFactory<ModelT>]!
private var contentPresenters: [MessageContentPresenterProtocol]!
private var menuPresenter: ChatItemMenuPresenterProtocol?

public init(
messageModel: ModelT,
Expand All @@ -58,22 +59,17 @@ open class CompoundMessagePresenter<ViewModelBuilderT, InteractionHandlerT>
) {
self.compoundCellStyle = compoundCellStyle
self.compoundCellDimensions = compoundCellDimensions
self.contentFactories = contentFactories.filter { $0.canCreateMessageContent(forModel: messageModel) }
self.initialContentFactories = contentFactories
self.cache = cache
self.accessibilityIdentifier = accessibilityIdentifier
self.menuPresenter = self.contentFactories.lazy.compactMap { $0.createMenuPresenter(forModel: messageModel) }.first
super.init(
messageModel: messageModel,
viewModelBuilder: viewModelBuilder,
interactionHandler: interactionHandler,
sizingCell: sizingCell,
cellStyle: baseCellStyle
)
self.contentPresenters = self.contentFactories.map { factory in
var presenter = factory.createContentPresenter(forModel: self.messageModel)
presenter.delegate = self
return presenter
}
self.updateContent()
}

open override var canCalculateHeightInBackground: Bool {
Expand All @@ -84,6 +80,29 @@ open class CompoundMessagePresenter<ViewModelBuilderT, InteractionHandlerT>
// Cell registration is happening lazily, right before the moment when a cell is dequeued.
}

open override var isItemUpdateSupported: Bool {
return true
}

open override func update(with chatItem: ChatItemProtocol) {
guard let newMessageModel = chatItem as? ModelT else { assertionFailure("Unexpected type of the message: \(type(of: chatItem))."); return }
let isUpdateNeeded = !self.messageModel.hasSameContent(as: newMessageModel)
self.messageModel = newMessageModel
if isUpdateNeeded { self.updateContent() }
}

private func updateContent() {
self.contentFactories = self.initialContentFactories.filter { $0.canCreateMessageContent(forModel: self.messageModel) }

self.contentPresenters = self.contentFactories.compactMap {
var presenter = $0.createContentPresenter(forModel: self.messageModel)
presenter.delegate = self
return presenter
}

self.menuPresenter = self.contentFactories.lazy.compactMap { $0.createMenuPresenter(forModel: self.messageModel) }.first
}

open override func dequeueCell(collectionView: UICollectionView, indexPath: IndexPath) -> UICollectionViewCell {
let cellReuseIdentifier = self.compoundCellReuseId
collectionView.register(CompoundMessageCollectionViewCell.self, forCellWithReuseIdentifier: cellReuseIdentifier)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/

import UIKit
import Chatto

public protocol PhotoMessageModelProtocol: DecoratedMessageModelProtocol {
var image: UIImage { get }
Expand All @@ -33,12 +34,17 @@ open class PhotoMessageModel<MessageModelT: MessageModelProtocol>: PhotoMessageM
public var messageModel: MessageModelProtocol {
return self._messageModel
}
public let _messageModel: MessageModelT // Can't make messasgeModel: MessageModelT: https://gist.github.com/diegosanchezr/5a66c7af862e1117b556
public let _messageModel: MessageModelT // Can't make messageModel: MessageModelT: https://gist.github.com/diegosanchezr/5a66c7af862e1117b556
public let image: UIImage
public let imageSize: CGSize
public init(messageModel: MessageModelT, imageSize: CGSize, image: UIImage) {
self._messageModel = messageModel
self.imageSize = imageSize
self.image = image
}
public func hasSameContent(as anotherItem: ChatItemProtocol) -> Bool {
guard let anotherMessageModel = anotherItem as? PhotoMessageModel else { return false }
return self.image == anotherMessageModel.image
&& self.imageSize == anotherMessageModel.imageSize
}
}
Loading