From da4ce063f308bdfebf8a72ad6787f86b148943e6 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Mon, 18 Jan 2021 15:50:31 +0000 Subject: [PATCH 01/12] Add foundation for batched updates --- .../CollectionCoordinator.swift | 214 +++++++------ .../ComposedUI/Common/ChangesReducer.swift | 297 ++++++++++++++++++ Sources/ComposedUI/Common/Changeset.swift | 19 ++ .../ComposedUITests/ChangesReducerTests.swift | 259 +++++++++++++++ .../CollectionCoordinator.swift | 11 - .../CollectionCoordinatorTests.swift | 80 +++++ 6 files changed, 771 insertions(+), 109 deletions(-) create mode 100644 Sources/ComposedUI/Common/ChangesReducer.swift create mode 100644 Sources/ComposedUI/Common/Changeset.swift create mode 100644 Tests/ComposedUITests/ChangesReducerTests.swift delete mode 100644 Tests/ComposedUITests/CollectionCoordinator.swift create mode 100644 Tests/ComposedUITests/CollectionCoordinatorTests.swift diff --git a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift index a04ab4f..22b821c 100644 --- a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift +++ b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift @@ -1,5 +1,6 @@ import UIKit import Composed +import os.log /// Conform to this protocol to receive `CollectionCoordinator` events public protocol CollectionCoordinatorDelegate: class { @@ -38,17 +39,13 @@ open class CollectionCoordinator: NSObject { return mapper.provider } - private var mapper: SectionProviderMapping + internal var changesReducer = ChangesReducer() - private var defersUpdate: Bool = false - private var sectionRemoves: [() -> Void] = [] - private var sectionInserts: [() -> Void] = [] - private var sectionUpdates: [() -> Void] = [] + private var mapper: SectionProviderMapping - private var removes: [() -> Void] = [] - private var inserts: [() -> Void] = [] - private var changes: [() -> Void] = [] - private var moves: [() -> Void] = [] + private var isPerformingBatchedUpdates: Bool { + changesReducer.hasActiveUpdates + } private let collectionView: UICollectionView @@ -159,6 +156,8 @@ open class CollectionCoordinator: NSObject { // Prepares and caches the section to improve performance private func prepareSections() { + debugLog("Preparing sections") + cachedProviders.removeAll() mapper.delegate = self @@ -169,20 +168,8 @@ open class CollectionCoordinator: NSObject { switch section.cell.dequeueMethod { case let .fromNib(type): - // `UINib(nibName:bundle:)` is an expensive call because it reads the NIB from the - // disk, which can have a large impact on performance when this is called multiple times. - // - // Each registration is cached to ensure that the same nib is not read from disk multiple times. - - let nibName = String(describing: type) - let nibBundle = Bundle(for: type) - let nibRegistration = NIBRegistration(nibName: nibName, bundle: nibBundle, reuseIdentifier: section.cell.reuseIdentifier) - - guard !nibRegistrations.contains(nibRegistration) else { break } - - let nib = UINib(nibName: nibName, bundle: nibBundle) + let nib = UINib(nibName: String(describing: type), bundle: Bundle(for: type)) collectionView.register(nib, forCellWithReuseIdentifier: section.cell.reuseIdentifier) - nibRegistrations.insert(nibRegistration) case let .fromClass(type): collectionView.register(type, forCellWithReuseIdentifier: section.cell.reuseIdentifier) case .fromStoryboard: @@ -210,31 +197,30 @@ open class CollectionCoordinator: NSObject { delegate?.coordinatorDidUpdate(self) } + fileprivate func debugLog(_ message: String) { + if #available(iOS 12, *) { + os_log("%@", log: OSLog(subsystem: "ComposedUI", category: "CollectionCoordinator"), type: .debug, message) + } + } } // MARK: - SectionProviderMappingDelegate extension CollectionCoordinator: SectionProviderMappingDelegate { - - private func reset() { - removes.removeAll() - inserts.removeAll() - changes.removeAll() - moves.removeAll() - sectionInserts.removeAll() - sectionRemoves.removeAll() - } - public func mappingDidInvalidate(_ mapping: SectionProviderMapping) { assert(Thread.isMainThread) - reset() + + debugLog(#function) + changesReducer = ChangesReducer() prepareSections() collectionView.reloadData() } public func mappingWillBeginUpdating(_ mapping: SectionProviderMapping) { - reset() - defersUpdate = true + debugLog(#function) + assert(Thread.isMainThread) + + changesReducer.beginUpdating() // This is called here to ensure that the collection view's internal state is in-sync with the state of the // data in hierarchy of sections. If this is not done it can cause various crashes when `performBatchUpdates` is called @@ -246,89 +232,118 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { } public func mappingDidEndUpdating(_ mapping: SectionProviderMapping) { + debugLog(#function) assert(Thread.isMainThread) + + guard let changeset = changesReducer.endUpdating() else { return } + + /** + Deletes are processed before inserts in batch operations. This means the indexes for the deletions are processed relative to the indexes of the collection view’s state before the batch operation, and the indexes for the insertions are processed relative to the indexes of the state after all the deletions in the batch operation. + */ + debugLog("Performing batch updates") collectionView.performBatchUpdates({ - if defersUpdate { - prepareSections() + prepareSections() + + debugLog("Deleting \(changeset.groupsRemoved)") + collectionView.deleteItems(at: Array(changeset.elementsRemoved)) + + debugLog("Inserting \(changeset.groupsInserted)") + collectionView.insertItems(at: Array(changeset.elementsInserted)) + + // TODO: Account for `section.prefersReload` + debugLog("Updating \(changeset.groupsUpdated)") + collectionView.reloadItems(at: Array(changeset.elementsUpdated)) + + changeset.elementsMoved.forEach { move in + debugLog("Moving \(move.from) to \(move.to)") + collectionView.moveItem(at: move.from, to: move.to) } - removes.forEach { $0() } - inserts.forEach { $0() } - changes.forEach { $0() } - moves.forEach { $0() } - sectionRemoves.forEach { $0() } - sectionInserts.forEach { $0() } - sectionUpdates.forEach { $0() } - reset() - defersUpdate = false + debugLog("Deleting \(changeset.groupsRemoved)") + collectionView.deleteSections(IndexSet(changeset.groupsRemoved)) + + debugLog("Inserting \(changeset.groupsInserted)") + collectionView.insertSections(IndexSet(changeset.groupsInserted)) + + debugLog("Updating \(changeset.groupsUpdated)") + collectionView.reloadSections(IndexSet(changeset.groupsUpdated)) + // TODO: Implement Moves }) } public func mapping(_ mapping: SectionProviderMapping, didUpdateSections sections: IndexSet) { assert(Thread.isMainThread) - sectionUpdates.append { [weak self] in - guard let self = self else { return } - if !self.defersUpdate { self.prepareSections() } - self.collectionView.reloadSections(sections) + + guard isPerformingBatchedUpdates else { + prepareSections() + collectionView.reloadSections(sections) + return } - if defersUpdate { return } - mappingDidEndUpdating(mapping) + + changesReducer.updateGroups(sections) } public func mapping(_ mapping: SectionProviderMapping, didInsertSections sections: IndexSet) { assert(Thread.isMainThread) - sectionInserts.append { [weak self] in - guard let self = self else { return } - if !self.defersUpdate { self.prepareSections() } - self.collectionView.insertSections(sections) + + guard isPerformingBatchedUpdates else { + prepareSections() + collectionView.insertSections(sections) + return } - if defersUpdate { return } - mappingDidEndUpdating(mapping) + + changesReducer.insertGroups(sections) } public func mapping(_ mapping: SectionProviderMapping, didRemoveSections sections: IndexSet) { assert(Thread.isMainThread) - sectionRemoves.append { [weak self] in - guard let self = self else { return } - if !self.defersUpdate { self.prepareSections() } - self.collectionView.deleteSections(sections) + + guard isPerformingBatchedUpdates else { + prepareSections() + collectionView.deleteSections(sections) + return } - if defersUpdate { return } - mappingDidEndUpdating(mapping) + + changesReducer.removeGroups(sections) } public func mapping(_ mapping: SectionProviderMapping, didInsertElementsAt indexPaths: [IndexPath]) { assert(Thread.isMainThread) - inserts.append { [weak self] in - guard let self = self else { return } - self.collectionView.insertItems(at: indexPaths) + + guard isPerformingBatchedUpdates else { + prepareSections() + collectionView.insertItems(at: indexPaths) + return } - if defersUpdate { return } - mappingDidEndUpdating(mapping) + + changesReducer.insertElements(at: indexPaths) } public func mapping(_ mapping: SectionProviderMapping, didRemoveElementsAt indexPaths: [IndexPath]) { assert(Thread.isMainThread) - removes.append { [weak self] in - guard let self = self else { return } - self.collectionView.deleteItems(at: indexPaths) + + guard isPerformingBatchedUpdates else { + prepareSections() + collectionView.deleteItems(at: indexPaths) + return } - if defersUpdate { return } - mappingDidEndUpdating(mapping) + + changesReducer.removeElements(at: indexPaths) } public func mapping(_ mapping: SectionProviderMapping, didUpdateElementsAt indexPaths: [IndexPath]) { assert(Thread.isMainThread) - changes.append { [weak self] in - guard let self = self else { return } + + guard isPerformingBatchedUpdates else { + prepareSections() var indexPathsToReload: [IndexPath] = [] for indexPath in indexPaths { guard let section = self.sectionProvider.sections[indexPath.section] as? CollectionUpdateHandler, - !section.prefersReload(forElementAt: indexPath.item), - let cell = self.collectionView.cellForItem(at: indexPath) else { - indexPathsToReload.append(indexPath) - continue + !section.prefersReload(forElementAt: indexPath.item), + let cell = self.collectionView.cellForItem(at: indexPath) else { + indexPathsToReload.append(indexPath) + continue } self.cachedProviders[indexPath.section].cell.configure(cell, indexPath.item, self.mapper.provider.sections[indexPath.section]) @@ -341,19 +356,22 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { self.collectionView.reloadItems(at: indexPathsToReload) CATransaction.setDisableActions(false) CATransaction.commit() + return } - if defersUpdate { return } - mappingDidEndUpdating(mapping) + + changesReducer.updateElements(at: indexPaths) } public func mapping(_ mapping: SectionProviderMapping, didMoveElementsAt moves: [(IndexPath, IndexPath)]) { assert(Thread.isMainThread) - self.moves.append { [weak self] in - guard let self = self else { return } - moves.forEach { self.collectionView.moveItem(at: $0.0, to: $0.1) } + + guard isPerformingBatchedUpdates else { + prepareSections() + moves.forEach { collectionView.moveItem(at: $0.0, to: $0.1) } + return } - if defersUpdate { return } - mappingDidEndUpdating(mapping) + + changesReducer.moveElements(moves) } public func mapping(_ mapping: SectionProviderMapping, selectedIndexesIn section: Int) -> [Int] { @@ -373,7 +391,7 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { } public func mapping(_ mapping: SectionProviderMapping, move sourceIndexPath: IndexPath, to destinationIndexPath: IndexPath) { - collectionView.moveItem(at: sourceIndexPath, to: destinationIndexPath) + self.mapping(mapping, didMoveElementsAt: [(sourceIndexPath, destinationIndexPath)]) } } @@ -508,8 +526,8 @@ extension CollectionCoordinator { public func collectionView(_ collectionView: UICollectionView, contextMenuConfigurationForItemAt indexPath: IndexPath, point: CGPoint) -> UIContextMenuConfiguration? { guard let provider = mapper.provider.sections[indexPath.section] as? CollectionContextMenuHandler, - provider.allowsContextMenu(forElementAt: indexPath.item), - let cell = collectionView.cellForItem(at: indexPath) else { return nil } + provider.allowsContextMenu(forElementAt: indexPath.item), + let cell = collectionView.cellForItem(at: indexPath) else { return nil } let preview = provider.contextMenu(previewForElementAt: indexPath.item, cell: cell) return UIContextMenuConfiguration(identifier: indexPath.string, previewProvider: preview) { suggestedElements in return provider.contextMenu(forElementAt: indexPath.item, cell: cell, suggestedActions: suggestedElements) @@ -519,21 +537,21 @@ extension CollectionCoordinator { public func collectionView(_ collectionView: UICollectionView, previewForHighlightingContextMenuWithConfiguration configuration: UIContextMenuConfiguration) -> UITargetedPreview? { guard let identifier = configuration.identifier as? String, let indexPath = IndexPath(string: identifier) else { return nil } guard let cell = collectionView.cellForItem(at: indexPath), - let provider = mapper.provider.sections[indexPath.section] as? CollectionContextMenuHandler else { return nil } + let provider = mapper.provider.sections[indexPath.section] as? CollectionContextMenuHandler else { return nil } return provider.contextMenu(previewForHighlightingElementAt: indexPath.item, cell: cell) } public func collectionView(_ collectionView: UICollectionView, previewForDismissingContextMenuWithConfiguration configuration: UIContextMenuConfiguration) -> UITargetedPreview? { guard let identifier = configuration.identifier as? String, let indexPath = IndexPath(string: identifier) else { return nil } guard let cell = collectionView.cellForItem(at: indexPath), - let provider = mapper.provider.sections[indexPath.section] as? CollectionContextMenuHandler else { return nil } + let provider = mapper.provider.sections[indexPath.section] as? CollectionContextMenuHandler else { return nil } return provider.contextMenu(previewForDismissingElementAt: indexPath.item, cell: cell) } public func collectionView(_ collectionView: UICollectionView, willPerformPreviewActionForMenuWith configuration: UIContextMenuConfiguration, animator: UIContextMenuInteractionCommitAnimating) { guard let identifier = configuration.identifier as? String, let indexPath = IndexPath(string: identifier) else { return } guard let cell = collectionView.cellForItem(at: indexPath), - let provider = mapper.provider.sections[indexPath.section] as? CollectionContextMenuHandler else { return } + let provider = mapper.provider.sections[indexPath.section] as? CollectionContextMenuHandler else { return } provider.contextMenu(willPerformPreviewActionForElementAt: indexPath.item, cell: cell, animator: animator) } @@ -703,8 +721,8 @@ extension CollectionCoordinator: UICollectionViewDropDelegate { guard !indexPath.isEmpty else { return nil } guard let section = sectionProvider.sections[indexPath.section] as? CollectionDragHandler, - let cell = collectionView.cellForItem(at: indexPath) else { - return originalDragDelegate?.collectionView?(collectionView, dragPreviewParametersForItemAt: indexPath) + let cell = collectionView.cellForItem(at: indexPath) else { + return originalDragDelegate?.collectionView?(collectionView, dragPreviewParametersForItemAt: indexPath) } return section.dragSession(previewParametersForElementAt: indexPath.item, cell: cell) @@ -712,7 +730,7 @@ extension CollectionCoordinator: UICollectionViewDropDelegate { public func collectionView(_ collectionView: UICollectionView, dropPreviewParametersForItemAt indexPath: IndexPath) -> UIDragPreviewParameters? { guard let section = sectionProvider.sections[indexPath.section] as? CollectionDropHandler, - let cell = collectionView.cellForItem(at: indexPath) else { + let cell = collectionView.cellForItem(at: indexPath) else { return originalDropDelegate? .collectionView?(collectionView, dropPreviewParametersForItemAt: indexPath) } @@ -750,8 +768,8 @@ extension CollectionCoordinator: UICollectionViewDropDelegate { let destinationIndexPath = coordinator.destinationIndexPath ?? IndexPath(item: 0, section: 0) guard coordinator.proposal.operation == .move, - let section = sectionProvider.sections[destinationIndexPath.section] as? MoveHandler else { - return + let section = sectionProvider.sections[destinationIndexPath.section] as? MoveHandler else { + return } let item = coordinator.items.lazy diff --git a/Sources/ComposedUI/Common/ChangesReducer.swift b/Sources/ComposedUI/Common/ChangesReducer.swift new file mode 100644 index 0000000..b18c160 --- /dev/null +++ b/Sources/ComposedUI/Common/ChangesReducer.swift @@ -0,0 +1,297 @@ +import Foundation + +/** + A value that collects and reduces changes to allow them to allow multiple changes + to be applied at once. + + The logic of how to reduce the changes is designed to match that of `UICollectionView` + and `UITableView`, allowing for reuse between both. + + `ChangesReducer` uses the generalised terms "group" and "element", which can be mapped directly + to "section" and "row" for `UITableView`s and "section" and "item" for `UICollectionView`. + */ +internal struct ChangesReducer { + internal var hasActiveUpdates: Bool { + return activeUpdates > 0 + } + + private var activeUpdates = 0 + + private var changeset: Changeset = Changeset() + + /// Begin performing updates. This must be called prior to making updates. + /// + /// It is possible to call this function multiple times to build up a batch of changes. + /// + /// All calls to this must be balanced with a call to `endUpdating`. + internal mutating func beginUpdating() { + activeUpdates += 1 + } + + /// End the current collection of updates. + /// + /// - Returns: The completed changeset, if this ends the last update in the batch. + internal mutating func endUpdating() -> Changeset? { + activeUpdates -= 1 + + guard activeUpdates == 0 else { + assert(activeUpdates > 0, "`endUpdating` calls must be balanced with `beginUpdating`") + return nil + } + + let changeset = self.changeset + self.changeset = Changeset() + return changeset + } + + internal mutating func updateGroups(_ groups: IndexSet) { + changeset.groupsUpdated.formUnion(groups) + } + + internal mutating func insertGroups(_ groups: [Int]) { + insertGroups(IndexSet(groups)) + } + + internal mutating func insertGroups(_ groups: IndexSet) { + groups.forEach { insertedGroup in + if changeset.groupsRemoved.remove(insertedGroup) != nil { + changeset.groupsUpdated.insert(insertedGroup) + } else { + changeset.groupsRemoved = Set(changeset.groupsRemoved.map { removedGroup in + if removedGroup > insertedGroup { + return removedGroup + 1 + } else { + return removedGroup + } + }) + changeset.groupsInserted.insert(insertedGroup) + } + + changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in + if insertedGroup > insertedGroup { + return insertedGroup + 1 + } else { + return insertedGroup + } + }) + + changeset.groupsUpdated = Set(changeset.groupsUpdated.map { updatedGroup in + if updatedGroup > insertedGroup { + return updatedGroup + 1 + } else { + return updatedGroup + } + }) + + changeset.elementsRemoved = Set(changeset.elementsRemoved.map { removedIndexPath in + var removedIndexPath = removedIndexPath + + if removedIndexPath.section > insertedGroup { + removedIndexPath.section += 1 + } + + return removedIndexPath + }) + + changeset.elementsInserted = Set(changeset.elementsInserted.map { insertedIndexPath in + var insertedIndexPath = insertedIndexPath + + if insertedIndexPath.section > insertedGroup { + insertedIndexPath.section += 1 + } + + return insertedIndexPath + }) + + changeset.elementsUpdated = Set(changeset.elementsUpdated.map { updatedIndexPath in + var updatedIndexPath = updatedIndexPath + + if updatedIndexPath.section > insertedGroup { + updatedIndexPath.section += 1 + } + + return updatedIndexPath + }) + + changeset.elementsMoved = Set(changeset.elementsMoved.map { move in + var move = move + + if move.from.section > insertedGroup { + move.from.section += 1 + } + + if move.to.section > insertedGroup { + move.to.section += 1 + } + + return move + }) + } + } + + internal mutating func removeGroups(_ groups: [Int]) { + removeGroups(IndexSet(groups)) + } + + internal mutating func removeGroups(_ groups: IndexSet) { + groups.forEach { removedGroup in + if changeset.groupsInserted.remove(removedGroup) == nil { + changeset.groupsRemoved = Set(changeset.groupsRemoved + .sorted(by: <) + .reduce(into: (previous: Int?.none, groupsRemoved: [Int]()), { (result, groupsRemoved) in + if groupsRemoved == removedGroup { + result.groupsRemoved.append(groupsRemoved) + result.groupsRemoved.append(groupsRemoved + 1) + result.previous = groupsRemoved + 1 + } else if let previous = result.previous, groupsRemoved == previous { + result.groupsRemoved.append(groupsRemoved + 1) + result.previous = groupsRemoved + 1 + } else { + result.groupsRemoved.append(groupsRemoved) + result.previous = groupsRemoved + } + }) + .groupsRemoved + ) + + if !changeset.groupsRemoved.contains(removedGroup) { + changeset.groupsRemoved.insert(removedGroup) + } + } + + changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in + if insertedGroup > removedGroup { + return insertedGroup - 1 + } else { + return insertedGroup + } + }) + + changeset.groupsUpdated = Set(changeset.groupsUpdated.map { updatedGroup in + if updatedGroup > removedGroup { + return updatedGroup - 1 + } else { + return updatedGroup + } + }) + + changeset.elementsInserted = Set(changeset.elementsInserted.compactMap { insertedIndexPath in + guard insertedIndexPath.section != removedGroup else { return nil } + + var batchedRowInsert = insertedIndexPath + + if batchedRowInsert.section > removedGroup { + batchedRowInsert.section -= 1 + } + + return batchedRowInsert + }) + + changeset.elementsUpdated = Set(changeset.elementsUpdated.compactMap { updatedIndexPath in + guard updatedIndexPath.section != removedGroup else { return nil } + + var batchedRowUpdate = updatedIndexPath + + if batchedRowUpdate.section > removedGroup { + batchedRowUpdate.section -= 1 + } + + return batchedRowUpdate + }) + + changeset.elementsRemoved = Set(changeset.elementsRemoved.compactMap { removedIndexPath in + guard removedIndexPath.section != removedGroup else { return nil } + + var batchedRowRemoval = removedIndexPath + + if batchedRowRemoval.section > removedGroup { + batchedRowRemoval.section -= 1 + } + + return batchedRowRemoval + }) + + changeset.elementsMoved = Set(changeset.elementsMoved.compactMap { move in + guard move.to.section != removedGroup else { return nil } + + var move = move + + if move.from.section > removedGroup { + move.from.section -= 1 + } + + if move.to.section > removedGroup { + move.to.section -= 1 + } + + return move + }) + } + } + + internal mutating func insertElements(at indexPaths: [IndexPath]) { + changeset.elementsInserted.formUnion(indexPaths) + } + + internal mutating func removeElements(at indexPaths: [IndexPath]) { + indexPaths.forEach { removedIndexPath in + if changeset.elementsUpdated.remove(removedIndexPath) == nil, changeset.elementsInserted.remove(removedIndexPath) == nil { + changeset.elementsRemoved.insert(removedIndexPath) + } + + changeset.elementsUpdated = Set(changeset.elementsUpdated.compactMap { updatedIndexPath in + guard updatedIndexPath.section == removedIndexPath.section else { return updatedIndexPath } + + if updatedIndexPath.item > removedIndexPath.item { + if updatedIndexPath.item == removedIndexPath.item + 1 { + // Triggering an update to row with the same index path as one that's been removed + // will trigger "attempt to delete and reload the same index path" + changeset.elementsRemoved.insert(updatedIndexPath) + changeset.elementsInserted.insert(updatedIndexPath) + return nil + } + + return IndexPath(item: updatedIndexPath.item - 1, section: updatedIndexPath.section) + } else { + return updatedIndexPath + } + }) + + changeset.elementsInserted = Set(changeset.elementsInserted.map { insertedIndexPath in + guard insertedIndexPath.section == removedIndexPath.section else { return insertedIndexPath } + + if insertedIndexPath.item > removedIndexPath.item { + return IndexPath(item: insertedIndexPath.item - 1, section: insertedIndexPath.section) + } else { + return insertedIndexPath + } + }) + + changeset.elementsMoved = Set(changeset.elementsMoved.map { move in + var move = move + + if move.from.section == removedIndexPath.section, move.from.item > removedIndexPath.item { + move.from.item -= 1 + } + + if move.to.section == removedIndexPath.section, move.to.item > removedIndexPath.item { + move.to.item -= 1 + } + + return move + }) + } + } + + internal mutating func updateElements(at indexPaths: [IndexPath]) { + changeset.elementsUpdated.formUnion(indexPaths) + } + + internal mutating func moveElements(_ moves: [Changeset.Move]) { + changeset.elementsMoved.formUnion(moves) + } + + internal mutating func moveElements(_ moves: [(from: IndexPath, to: IndexPath)]) { + changeset.elementsMoved.formUnion(moves.map { Changeset.Move(from: $0.from, to: $0.to) }) + } +} diff --git a/Sources/ComposedUI/Common/Changeset.swift b/Sources/ComposedUI/Common/Changeset.swift new file mode 100644 index 0000000..97aeab3 --- /dev/null +++ b/Sources/ComposedUI/Common/Changeset.swift @@ -0,0 +1,19 @@ +import Foundation + +/** + A collection of changes to be applied in batch. + */ +internal struct Changeset { + internal struct Move: Hashable { + internal var from: IndexPath + internal var to: IndexPath + } + + internal var groupsInserted: Set = [] + internal var groupsRemoved: Set = [] + internal var groupsUpdated: Set = [] + internal var elementsRemoved: Set = [] + internal var elementsInserted: Set = [] + internal var elementsUpdated: Set = [] + internal var elementsMoved: Set = [] +} diff --git a/Tests/ComposedUITests/ChangesReducerTests.swift b/Tests/ComposedUITests/ChangesReducerTests.swift new file mode 100644 index 0000000..2fe854e --- /dev/null +++ b/Tests/ComposedUITests/ChangesReducerTests.swift @@ -0,0 +1,259 @@ +import XCTest +import Composed +@testable import ComposedUI + +final class ChangesReducerTests: XCTestCase { + func testGroupInserts() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + changesReducer.insertGroups(IndexSet([0, 2])) + changesReducer.insertGroups(IndexSet(integer: 1)) + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual(changeset!.groupsInserted, [0, 1, 2]) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsRemoved.isEmpty) + XCTAssertTrue(changeset!.elementsInserted.isEmpty) + XCTAssertTrue(changeset!.elementsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsMoved.isEmpty) + } + + func testGroupRemoves() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + changesReducer.removeGroups(IndexSet([0, 2])) + changesReducer.removeGroups(IndexSet(integer: 1)) + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) + XCTAssertTrue(changeset!.groupsInserted.isEmpty) + XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsRemoved.isEmpty) + XCTAssertTrue(changeset!.elementsInserted.isEmpty) + XCTAssertTrue(changeset!.elementsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsMoved.isEmpty) + } + + func testRemovingAGroupInsertedInTheSameBatch() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + changesReducer.insertGroups([0]) + changesReducer.removeGroups([0]) + changesReducer.insertGroups([0, 1, 2, 3, 4]) + changesReducer.removeGroups([2]) + + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual(changeset!.groupsInserted, [0, 1, 2, 3]) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsRemoved.isEmpty) + XCTAssertTrue(changeset!.elementsInserted.isEmpty) + XCTAssertTrue(changeset!.elementsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsMoved.isEmpty) + } + + func testMoveElement() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) + + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual( + changeset!.elementsMoved, + [ + Changeset.Move( + from: IndexPath(row: 0, section: 0), + to: IndexPath(row: 1, section: 0) + ) + ] + ) + XCTAssertTrue(changeset!.elementsRemoved.isEmpty) + XCTAssertTrue(changeset!.elementsInserted.isEmpty) + XCTAssertTrue(changeset!.elementsUpdated.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + } + + func testMoveElementThenRemoveElementBeforeMovedElement() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + /** + This is testing: + + - A + - B + - C + - D + + # Swap C and D + + - A + - B + - D + - C + + # Delete A + + - B + - D + - C + */ + + changesReducer.moveElements([(from: IndexPath(row: 2, section: 0), to: IndexPath(row: 3, section: 0))]) + changesReducer.removeElements(at: [IndexPath(row: 0, section: 0)]) + + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual( + changeset!.elementsMoved, + [ + Changeset.Move( + from: IndexPath(row: 1, section: 0), + to: IndexPath(row: 2, section: 0) + ) + ] + ) + XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 0, section: 0)]) + XCTAssertTrue(changeset!.elementsInserted.isEmpty) + XCTAssertTrue(changeset!.elementsUpdated.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + } + + func testRemoveAMovedIndex() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + /** + This is testing: + + - A + - B + - C + + # Swap B and C + + - A + - C + - B + + # Delete B + + - A + - C + + `UICollectionView` does not support deleting an index path and moving to the same index path, so this should produce: + + - Delete 0 + - Delete 1 + - Delete 2 + - Insert 0 + - Insert 2 + */ + + changesReducer.moveElements([(from: IndexPath(row: 1, section: 0), to: IndexPath(row: 2, section: 0))]) + changesReducer.removeElements(at: [IndexPath(row: 1, section: 0)]) + + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual( + changeset!.elementsRemoved, + [ + IndexPath(row: 0, section: 0), + IndexPath(row: 1, section: 0), + IndexPath(row: 2, section: 0), + ] + ) + XCTAssertEqual( + changeset!.elementsInserted, + [ + IndexPath(row: 0, section: 0), + IndexPath(row: 1, section: 0), + ] + ) + XCTAssertTrue(changeset!.elementsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsMoved.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + } + + func testMoveElementAtSameIndexAsRemove() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + /** + This is testing: + + - A + - B + - C + + # Delete B + + - A + - C + + # Swap A and C + + - C + - A + + `UICollectionView` does not support deleting an index path and moving to the same index path, so this should produce: + + - Delete 0 + - Delete 1 + - Delete 2 + - Insert 0 + - Insert 2 + */ + + changesReducer.removeElements(at: [IndexPath(row: 1, section: 0)]) + changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) + + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual( + changeset!.elementsRemoved, + [ + IndexPath(row: 0, section: 0), + IndexPath(row: 1, section: 0), + IndexPath(row: 2, section: 0), + ] + ) + XCTAssertEqual( + changeset!.elementsInserted, + [ + IndexPath(row: 0, section: 0), + IndexPath(row: 1, section: 0), + ] + ) + XCTAssertTrue(changeset!.elementsUpdated.isEmpty) + XCTAssertTrue(changeset!.elementsMoved.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + } +} diff --git a/Tests/ComposedUITests/CollectionCoordinator.swift b/Tests/ComposedUITests/CollectionCoordinator.swift deleted file mode 100644 index f02b2eb..0000000 --- a/Tests/ComposedUITests/CollectionCoordinator.swift +++ /dev/null @@ -1,11 +0,0 @@ -import XCTest -import Composed -@testable import ComposedUI - -final class CollectionCoordinatorTests: XCTestCase { - - func testFoo() { - XCTAssert(true) - } - -} diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift new file mode 100644 index 0000000..d4bc4c9 --- /dev/null +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -0,0 +1,80 @@ +import XCTest +import Composed +@testable import ComposedUI + +final class CollectionCoordinatorTests: XCTestCase { + /// A series of updates that are performed in batch. This isn't testing `CollectionCoordinator` as much as + /// it tests `ChangesReducer`. These tests are closer to end-to-end tests, essentially testing that the updates from + /// sections are correctly passed up to the `ChangesReducer`, and that the `ChangesReducer` provides the correct updates + /// to `UICollectionView`. + /// + /// One way for these tests to fail is by `UICollectionView` throwing a `NSInternalInconsistencyException', reason: 'Invalid update...'` + /// error, which would likely indicate an error in `ChangesReducer`. + func testBatchUpdates() { + let collectionView = UICollectionView(frame: .zero, collectionViewLayout: UICollectionViewFlowLayout()) + let rootSectionProvider = ComposedSectionProvider() + let collectionCoordinator = CollectionCoordinator(collectionView: collectionView, sectionProvider: rootSectionProvider) + _ = collectionCoordinator + + let child0 = MockCollectionArraySection(["1", "2", "3"]) + let child1 = MockCollectionArraySection(["1", "2", "3"]) + let child2 = MockCollectionArraySection(["1", "2", "3"]) + let child3 = MockCollectionArraySection(["1", "2", "3"]) + var child4 = MockCollectionArraySection(["1", "2", "3"]) + + rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + + rootSectionProvider.append(child0) + rootSectionProvider.append(child1) + rootSectionProvider.append(child2) + rootSectionProvider.append(child3) + rootSectionProvider.append(child4) + + rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + + rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + + rootSectionProvider.remove(child3) + rootSectionProvider.remove(child0) + rootSectionProvider.remove(child1) + rootSectionProvider.remove(child2) + rootSectionProvider.remove(child4) + + rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + + rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + + rootSectionProvider.append(child0) + rootSectionProvider.remove(child0) + rootSectionProvider.append(child0) + rootSectionProvider.append(child1) + rootSectionProvider.append(child2) + rootSectionProvider.append(child3) + rootSectionProvider.append(child4) + rootSectionProvider.remove(child2) + + rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + + rootSectionProvider.insert(child2, after: child1) + + rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + + rootSectionProvider.remove(child1) + child3.remove(at: 1) + child4.swapAt(0, 2) + rootSectionProvider.remove(child2) + child4.append("4") + rootSectionProvider.remove(child0) + child4.remove(at: 1) + rootSectionProvider.insert(child0, at: 0) + + rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + } +} + +private final class MockCollectionArraySection: ArraySection, CollectionSectionProvider { + func section(with traitCollection: UITraitCollection) -> CollectionSection { + let cell = CollectionCellElement(section: self, dequeueMethod: .fromClass(UICollectionViewCell.self), configure: { _, _, _ in }) + return CollectionSection(section: self, cell: cell) + } +} From 3833d85def32a15e9afb4cd79bf75692a46d70d0 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Sat, 23 Jan 2021 22:03:31 +0000 Subject: [PATCH 02/12] Full in Open Net changes --- .../Core/SectionProviderMapping.swift | 6 - .../CollectionCoordinator.swift | 40 ++-- .../ComposedUI/Common/ChangesReducer.swift | 226 ++++++++---------- Sources/ComposedUI/Common/Changeset.swift | 2 - 4 files changed, 115 insertions(+), 159 deletions(-) diff --git a/Sources/Composed/Core/SectionProviderMapping.swift b/Sources/Composed/Core/SectionProviderMapping.swift index 303fab4..e9c1935 100644 --- a/Sources/Composed/Core/SectionProviderMapping.swift +++ b/Sources/Composed/Core/SectionProviderMapping.swift @@ -39,12 +39,6 @@ public protocol SectionProviderMappingDelegate: class { /// - indexPaths: The element indexPaths func mapping(_ mapping: SectionProviderMapping, didRemoveElementsAt indexPaths: [IndexPath]) - /// Notifies the delegate that the mapping did update sections - /// - Parameters: - /// - mapping: The mapping that provided this update - /// - sections: The section indexes - func mapping(_ mapping: SectionProviderMapping, didUpdateSections sections: IndexSet) - /// Notifies the delegate that the mapping did update elements /// - Parameters: /// - mapping: The mapping that provided this update diff --git a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift index 22b821c..9835246 100644 --- a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift +++ b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift @@ -39,6 +39,9 @@ open class CollectionCoordinator: NSObject { return mapper.provider } + /// If `true` this `CollectionCoordinator` instance will log changes to the system log. + public var enableLogs: Bool = false + internal var changesReducer = ChangesReducer() private var mapper: SectionProviderMapping @@ -198,7 +201,7 @@ open class CollectionCoordinator: NSObject { } fileprivate func debugLog(_ message: String) { - if #available(iOS 12, *) { + if #available(iOS 12, *), enableLogs { os_log("%@", log: OSLog(subsystem: "ComposedUI", category: "CollectionCoordinator"), type: .debug, message) } } @@ -238,51 +241,35 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { guard let changeset = changesReducer.endUpdating() else { return } /** - Deletes are processed before inserts in batch operations. This means the indexes for the deletions are processed relative to the indexes of the collection view’s state before the batch operation, and the indexes for the insertions are processed relative to the indexes of the state after all the deletions in the batch operation. + _Item_ deletes are processed first, with indexes relative to the state at the start of `performBatchUpdates`. + + _Section_ are processed next, with indexes relative to the state at the start of `performBatchUpdates` (since section indexes are not changed by item deletes). + + All other updates are processed relative to the indexes **after** these deletes have occurred. */ debugLog("Performing batch updates") collectionView.performBatchUpdates({ prepareSections() - debugLog("Deleting \(changeset.groupsRemoved)") + debugLog("Deleting items \(changeset.elementsRemoved)") collectionView.deleteItems(at: Array(changeset.elementsRemoved)) - debugLog("Inserting \(changeset.groupsInserted)") + debugLog("Inserting items \(changeset.elementsInserted)") collectionView.insertItems(at: Array(changeset.elementsInserted)) - // TODO: Account for `section.prefersReload` - debugLog("Updating \(changeset.groupsUpdated)") - collectionView.reloadItems(at: Array(changeset.elementsUpdated)) - changeset.elementsMoved.forEach { move in debugLog("Moving \(move.from) to \(move.to)") collectionView.moveItem(at: move.from, to: move.to) } - debugLog("Deleting \(changeset.groupsRemoved)") + debugLog("Deleting sections \(changeset.groupsRemoved)") collectionView.deleteSections(IndexSet(changeset.groupsRemoved)) - debugLog("Inserting \(changeset.groupsInserted)") + debugLog("Inserting sections \(changeset.groupsInserted)") collectionView.insertSections(IndexSet(changeset.groupsInserted)) - - debugLog("Updating \(changeset.groupsUpdated)") - collectionView.reloadSections(IndexSet(changeset.groupsUpdated)) - // TODO: Implement Moves }) } - public func mapping(_ mapping: SectionProviderMapping, didUpdateSections sections: IndexSet) { - assert(Thread.isMainThread) - - guard isPerformingBatchedUpdates else { - prepareSections() - collectionView.reloadSections(sections) - return - } - - changesReducer.updateGroups(sections) - } - public func mapping(_ mapping: SectionProviderMapping, didInsertSections sections: IndexSet) { assert(Thread.isMainThread) @@ -391,6 +378,7 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { } public func mapping(_ mapping: SectionProviderMapping, move sourceIndexPath: IndexPath, to destinationIndexPath: IndexPath) { + // TODO: Check `isPerformingBatchedUpdates` self.mapping(mapping, didMoveElementsAt: [(sourceIndexPath, destinationIndexPath)]) } diff --git a/Sources/ComposedUI/Common/ChangesReducer.swift b/Sources/ComposedUI/Common/ChangesReducer.swift index b18c160..76e1585 100644 --- a/Sources/ComposedUI/Common/ChangesReducer.swift +++ b/Sources/ComposedUI/Common/ChangesReducer.swift @@ -9,6 +9,26 @@ import Foundation `ChangesReducer` uses the generalised terms "group" and "element", which can be mapped directly to "section" and "row" for `UITableView`s and "section" and "item" for `UICollectionView`. + + Final updates are applied in the order: + + - Element removals + - Using original index paths + - Group removals + - Using original index paths + - Element moves + - Decomposed in to delete and insert + - Delete post-element removals, but pre-group removals? + + To confirm: + - Group inserts + - Using index paths after removals + - Element inserts + - Using index paths after removals + - Group reloads + - Using index paths after removals and inserts + - Element reloads + - Using index paths after removals and inserts */ internal struct ChangesReducer { internal var hasActiveUpdates: Bool { @@ -19,6 +39,11 @@ internal struct ChangesReducer { private var changeset: Changeset = Changeset() + /// Clears existing updates, keeping active updates count. + internal mutating func clearUpdates() { + changeset = Changeset() + } + /// Begin performing updates. This must be called prior to making updates. /// /// It is possible to call this function multiple times to build up a batch of changes. @@ -44,75 +69,32 @@ internal struct ChangesReducer { return changeset } - internal mutating func updateGroups(_ groups: IndexSet) { - changeset.groupsUpdated.formUnion(groups) - } - - internal mutating func insertGroups(_ groups: [Int]) { - insertGroups(IndexSet(groups)) - } - internal mutating func insertGroups(_ groups: IndexSet) { groups.forEach { insertedGroup in - if changeset.groupsRemoved.remove(insertedGroup) != nil { - changeset.groupsUpdated.insert(insertedGroup) - } else { - changeset.groupsRemoved = Set(changeset.groupsRemoved.map { removedGroup in - if removedGroup > insertedGroup { - return removedGroup + 1 - } else { - return removedGroup - } - }) - changeset.groupsInserted.insert(insertedGroup) - } - - changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in - if insertedGroup > insertedGroup { - return insertedGroup + 1 - } else { - return insertedGroup + changeset.groupsInserted = Set(changeset.groupsInserted.map { existingInsertedGroup in + if existingInsertedGroup >= insertedGroup { + return existingInsertedGroup + 1 } - }) - changeset.groupsUpdated = Set(changeset.groupsUpdated.map { updatedGroup in - if updatedGroup > insertedGroup { - return updatedGroup + 1 - } else { - return updatedGroup - } + return existingInsertedGroup }) - changeset.elementsRemoved = Set(changeset.elementsRemoved.map { removedIndexPath in - var removedIndexPath = removedIndexPath - - if removedIndexPath.section > insertedGroup { - removedIndexPath.section += 1 - } - - return removedIndexPath - }) + if changeset.groupsRemoved.contains(insertedGroup) { + changeset.groupsInserted.insert(insertedGroup) + } else { + changeset.groupsInserted.insert(insertedGroup) + } changeset.elementsInserted = Set(changeset.elementsInserted.map { insertedIndexPath in var insertedIndexPath = insertedIndexPath - if insertedIndexPath.section > insertedGroup { + if insertedIndexPath.section >= insertedGroup { insertedIndexPath.section += 1 } return insertedIndexPath }) - changeset.elementsUpdated = Set(changeset.elementsUpdated.map { updatedIndexPath in - var updatedIndexPath = updatedIndexPath - - if updatedIndexPath.section > insertedGroup { - updatedIndexPath.section += 1 - } - - return updatedIndexPath - }) - changeset.elementsMoved = Set(changeset.elementsMoved.map { move in var move = move @@ -135,6 +117,8 @@ internal struct ChangesReducer { internal mutating func removeGroups(_ groups: IndexSet) { groups.forEach { removedGroup in + let removedGroup = removedGroup - changeset.groupsInserted.filter { $0 < removedGroup }.count + changeset.groupsRemoved.filter { $0 <= removedGroup }.count + if changeset.groupsInserted.remove(removedGroup) == nil { changeset.groupsRemoved = Set(changeset.groupsRemoved .sorted(by: <) @@ -144,6 +128,7 @@ internal struct ChangesReducer { result.groupsRemoved.append(groupsRemoved + 1) result.previous = groupsRemoved + 1 } else if let previous = result.previous, groupsRemoved == previous { + // TODO: Test this result.groupsRemoved.append(groupsRemoved + 1) result.previous = groupsRemoved + 1 } else { @@ -162,17 +147,9 @@ internal struct ChangesReducer { changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in if insertedGroup > removedGroup { return insertedGroup - 1 - } else { - return insertedGroup } - }) - changeset.groupsUpdated = Set(changeset.groupsUpdated.map { updatedGroup in - if updatedGroup > removedGroup { - return updatedGroup - 1 - } else { - return updatedGroup - } + return insertedGroup }) changeset.elementsInserted = Set(changeset.elementsInserted.compactMap { insertedIndexPath in @@ -187,30 +164,6 @@ internal struct ChangesReducer { return batchedRowInsert }) - changeset.elementsUpdated = Set(changeset.elementsUpdated.compactMap { updatedIndexPath in - guard updatedIndexPath.section != removedGroup else { return nil } - - var batchedRowUpdate = updatedIndexPath - - if batchedRowUpdate.section > removedGroup { - batchedRowUpdate.section -= 1 - } - - return batchedRowUpdate - }) - - changeset.elementsRemoved = Set(changeset.elementsRemoved.compactMap { removedIndexPath in - guard removedIndexPath.section != removedGroup else { return nil } - - var batchedRowRemoval = removedIndexPath - - if batchedRowRemoval.section > removedGroup { - batchedRowRemoval.section -= 1 - } - - return batchedRowRemoval - }) - changeset.elementsMoved = Set(changeset.elementsMoved.compactMap { move in guard move.to.section != removedGroup else { return nil } @@ -230,61 +183,84 @@ internal struct ChangesReducer { } internal mutating func insertElements(at indexPaths: [IndexPath]) { - changeset.elementsInserted.formUnion(indexPaths) + indexPaths.forEach { insertedIndexPath in + changeset.elementsInserted.insert(insertedIndexPath) + } } internal mutating func removeElements(at indexPaths: [IndexPath]) { - indexPaths.forEach { removedIndexPath in - if changeset.elementsUpdated.remove(removedIndexPath) == nil, changeset.elementsInserted.remove(removedIndexPath) == nil { - changeset.elementsRemoved.insert(removedIndexPath) - } + /** + Element removals are handled before all other updates. + */ + indexPaths.sorted(by: { $0.item > $1.item }).forEach { removedIndexPath in + let sectionsRemovedBefore = changeset + .groupsRemoved + .sorted(by: <) + .enumerated() + .map { element in + element.element - element.offset + } + .filter { $0 <= removedIndexPath.section } + .count - changeset.elementsUpdated = Set(changeset.elementsUpdated.compactMap { updatedIndexPath in - guard updatedIndexPath.section == removedIndexPath.section else { return updatedIndexPath } + let sectionsInsertedBefore = changeset.groupsInserted.filter { $0 <= removedIndexPath.section }.count - if updatedIndexPath.item > removedIndexPath.item { - if updatedIndexPath.item == removedIndexPath.item + 1 { - // Triggering an update to row with the same index path as one that's been removed - // will trigger "attempt to delete and reload the same index path" - changeset.elementsRemoved.insert(updatedIndexPath) - changeset.elementsInserted.insert(updatedIndexPath) - return nil + var removedIndexPath = removedIndexPath + removedIndexPath.section += sectionsRemovedBefore + removedIndexPath.section -= sectionsInsertedBefore + + if !changeset.groupsInserted.contains(removedIndexPath.section) { + let itemInsertsInSection = changeset + .elementsInserted + .filter { $0.section == removedIndexPath.section } + .map(\.item) + + changeset.elementsInserted = Set(changeset.elementsInserted.map { existingInsertedIndexPath in + guard existingInsertedIndexPath.section == removedIndexPath.section else { + // Different section; don't modify + return existingInsertedIndexPath } - return IndexPath(item: updatedIndexPath.item - 1, section: updatedIndexPath.section) - } else { - return updatedIndexPath - } - }) + guard !changeset.elementsRemoved.contains(existingInsertedIndexPath) else { + // This insert is really a reload (delete and insert) + return existingInsertedIndexPath + } - changeset.elementsInserted = Set(changeset.elementsInserted.map { insertedIndexPath in - guard insertedIndexPath.section == removedIndexPath.section else { return insertedIndexPath } + var existingInsertedIndexPath = existingInsertedIndexPath - if insertedIndexPath.item > removedIndexPath.item { - return IndexPath(item: insertedIndexPath.item - 1, section: insertedIndexPath.section) - } else { - return insertedIndexPath - } - }) + if existingInsertedIndexPath.item > removedIndexPath.item { + existingInsertedIndexPath.item -= 1 + } else if existingInsertedIndexPath.item == removedIndexPath.item && !changeset.elementsRemoved.contains(existingInsertedIndexPath) { + existingInsertedIndexPath.item -= 1 + } - changeset.elementsMoved = Set(changeset.elementsMoved.map { move in - var move = move + return existingInsertedIndexPath + }) - if move.from.section == removedIndexPath.section, move.from.item > removedIndexPath.item { - move.from.item -= 1 - } + let itemRemovalsInSection = changeset + .elementsRemoved + .filter { $0.section == removedIndexPath.section } + .map(\.item) - if move.to.section == removedIndexPath.section, move.to.item > removedIndexPath.item { - move.to.item -= 1 - } + let availableSpaces = (0.. $1.item }).forEach { updatedElement in + // Reloads are decomposed in to a delete and insert + removeElements(at: [updatedElement]) + insertElements(at: [updatedElement]) + } } internal mutating func moveElements(_ moves: [Changeset.Move]) { @@ -292,6 +268,6 @@ internal struct ChangesReducer { } internal mutating func moveElements(_ moves: [(from: IndexPath, to: IndexPath)]) { - changeset.elementsMoved.formUnion(moves.map { Changeset.Move(from: $0.from, to: $0.to) }) + moveElements(moves.map { Changeset.Move(from: $0.from, to: $0.to) }) } } diff --git a/Sources/ComposedUI/Common/Changeset.swift b/Sources/ComposedUI/Common/Changeset.swift index 97aeab3..d25895b 100644 --- a/Sources/ComposedUI/Common/Changeset.swift +++ b/Sources/ComposedUI/Common/Changeset.swift @@ -11,9 +11,7 @@ internal struct Changeset { internal var groupsInserted: Set = [] internal var groupsRemoved: Set = [] - internal var groupsUpdated: Set = [] internal var elementsRemoved: Set = [] internal var elementsInserted: Set = [] - internal var elementsUpdated: Set = [] internal var elementsMoved: Set = [] } From cc5fdbb3c7c585fc03544eb034ac1f0a64cd68ce Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Sat, 23 Jan 2021 22:05:17 +0000 Subject: [PATCH 03/12] Pull Open Net tests --- .../ComposedUITests/ChangesReducerTests.swift | 1279 ++++++++++++++--- .../CollectionCoordinatorTests.swift | 391 ++++- 2 files changed, 1421 insertions(+), 249 deletions(-) diff --git a/Tests/ComposedUITests/ChangesReducerTests.swift b/Tests/ComposedUITests/ChangesReducerTests.swift index 2fe854e..b782a04 100644 --- a/Tests/ComposedUITests/ChangesReducerTests.swift +++ b/Tests/ComposedUITests/ChangesReducerTests.swift @@ -3,257 +3,1128 @@ import Composed @testable import ComposedUI final class ChangesReducerTests: XCTestCase { - func testGroupInserts() { + func testMultipleElementRemovals() { var changesReducer = ChangesReducer() changesReducer.beginUpdating() - changesReducer.insertGroups(IndexSet([0, 2])) - changesReducer.insertGroups(IndexSet(integer: 1)) - let changeset = changesReducer.endUpdating() - - XCTAssertNotNil(changeset) - - XCTAssertEqual(changeset!.groupsInserted, [0, 1, 2]) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsRemoved.isEmpty) - XCTAssertTrue(changeset!.elementsInserted.isEmpty) - XCTAssertTrue(changeset!.elementsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsMoved.isEmpty) - } - func testGroupRemoves() { - var changesReducer = ChangesReducer() - changesReducer.beginUpdating() - changesReducer.removeGroups(IndexSet([0, 2])) - changesReducer.removeGroups(IndexSet(integer: 1)) - let changeset = changesReducer.endUpdating() - - XCTAssertNotNil(changeset) - - XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) - XCTAssertTrue(changeset!.groupsInserted.isEmpty) - XCTAssertTrue(changeset!.groupsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsRemoved.isEmpty) - XCTAssertTrue(changeset!.elementsInserted.isEmpty) - XCTAssertTrue(changeset!.elementsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsMoved.isEmpty) - } + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements(at: [IndexPath(item: 0, section: 0)]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - func testRemovingAGroupInsertedInTheSameBatch() { - var changesReducer = ChangesReducer() - changesReducer.beginUpdating() + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsRemoved, + [IndexPath(item: 0, section: 0)] + ) + }) - changesReducer.insertGroups([0]) - changesReducer.removeGroups([0]) - changesReducer.insertGroups([0, 1, 2, 3, 4]) - changesReducer.removeGroups([2]) + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements( + at: [ + IndexPath(item: 3, section: 0), + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + IndexPath(item: 2, section: 0), + ] + ) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - let changeset = changesReducer.endUpdating() + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + IndexPath(item: 2, section: 0), + IndexPath(item: 3, section: 0), + IndexPath(item: 4, section: 0), + ] + ) + }) - XCTAssertNotNil(changeset) + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements( + at: [ + IndexPath(item: 8, section: 0), + ] + ) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - XCTAssertEqual(changeset!.groupsInserted, [0, 1, 2, 3]) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsRemoved.isEmpty) - XCTAssertTrue(changeset!.elementsInserted.isEmpty) - XCTAssertTrue(changeset!.elementsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsMoved.isEmpty) - } + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + IndexPath(item: 2, section: 0), + IndexPath(item: 3, section: 0), + IndexPath(item: 4, section: 0), + IndexPath(item: 13, section: 0), + ] + ) + }) - func testMoveElement() { - var changesReducer = ChangesReducer() - changesReducer.beginUpdating() + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements( + at: [ + IndexPath(item: 4, section: 0), + ] + ) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } + + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + IndexPath(item: 2, section: 0), + IndexPath(item: 3, section: 0), + IndexPath(item: 4, section: 0), + IndexPath(item: 9, section: 0), + IndexPath(item: 13, section: 0), + ] + ) + }) + + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements( + at: [ + IndexPath(item: 0, section: 0), + ] + ) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } + + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + IndexPath(item: 2, section: 0), + IndexPath(item: 3, section: 0), + IndexPath(item: 4, section: 0), + IndexPath(item: 5, section: 0), + IndexPath(item: 9, section: 0), + IndexPath(item: 13, section: 0), + ] + ) + }) - changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements( + at: [ + IndexPath(item: 1, section: 0), + ] + ) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - let changeset = changesReducer.endUpdating() + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + IndexPath(item: 2, section: 0), + IndexPath(item: 3, section: 0), + IndexPath(item: 4, section: 0), + IndexPath(item: 5, section: 0), + IndexPath(item: 7, section: 0), + IndexPath(item: 9, section: 0), + IndexPath(item: 13, section: 0), + ] + ) + }) - XCTAssertNotNil(changeset) + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements( + at: [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + ] + ) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - XCTAssertEqual( - changeset!.elementsMoved, - [ - Changeset.Move( - from: IndexPath(row: 0, section: 0), - to: IndexPath(row: 1, section: 0) + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + IndexPath(item: 2, section: 0), + IndexPath(item: 3, section: 0), + IndexPath(item: 4, section: 0), + IndexPath(item: 5, section: 0), + IndexPath(item: 6, section: 0), + IndexPath(item: 7, section: 0), + IndexPath(item: 8, section: 0), + IndexPath(item: 9, section: 0), + IndexPath(item: 13, section: 0), + ] ) - ] - ) - XCTAssertTrue(changeset!.elementsRemoved.isEmpty) - XCTAssertTrue(changeset!.elementsInserted.isEmpty) - XCTAssertTrue(changeset!.elementsUpdated.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsUpdated.isEmpty) + }) } - func testMoveElementThenRemoveElementBeforeMovedElement() { + func testInsertAndRemovalInSameSection() { var changesReducer = ChangesReducer() changesReducer.beginUpdating() /** - This is testing: - - - A - - B - - C - - D - - # Swap C and D + - Element A + - Element B + - Element C + */ - - A - - B - - D - - C + AssertApplyingUpdates( + { changesReducer in + changesReducer.insertElements(at: [IndexPath(item: 3, section: 0)]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - # Delete A + XCTAssertTrue(changeset.elementsRemoved.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsInserted, + [IndexPath(item: 3, section: 0)] + ) + }) - - B - - D - - C + /** + - Element A + - Element B + - Element C + - Element D */ - changesReducer.moveElements([(from: IndexPath(row: 2, section: 0), to: IndexPath(row: 3, section: 0))]) - changesReducer.removeElements(at: [IndexPath(row: 0, section: 0)]) + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements(at: [IndexPath(item: 0, section: 0)]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - let changeset = changesReducer.endUpdating() + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsInserted, + [IndexPath(item: 2, section: 0)] + ) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + ] + ) + }) - XCTAssertNotNil(changeset) + /** + - Element B + - Element C + - Element D + */ - XCTAssertEqual( - changeset!.elementsMoved, - [ - Changeset.Move( - from: IndexPath(row: 1, section: 0), - to: IndexPath(row: 2, section: 0) - ) - ] - ) - XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 0, section: 0)]) - XCTAssertTrue(changeset!.elementsInserted.isEmpty) - XCTAssertTrue(changeset!.elementsUpdated.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsUpdated.isEmpty) - } + AssertApplyingUpdates( + { changesReducer in + changesReducer.insertElements(at: [IndexPath(item: 0, section: 0)]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - func testRemoveAMovedIndex() { - var changesReducer = ChangesReducer() - changesReducer.beginUpdating() + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsInserted, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 2, section: 0), + ] + ) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + ] + ) + }) /** - This is testing: - - - A - - B - - C + - New Element + - Element B + - Element C + - Element D + */ - # Swap B and C + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements(at: [IndexPath(item: 2, section: 0)]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - - A - - C - - B + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsInserted, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 1, section: 0), + ] + ) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 0), + IndexPath(item: 2, section: 0), + ] + ) + }) - # Delete B + /** + - New Element + - Element B + - Element D + */ - - A - - C + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements(at: [IndexPath(item: 0, section: 0)]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - `UICollectionView` does not support deleting an index path and moving to the same index path, so this should produce: + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.elementsInserted, + [ + IndexPath(item: 1, section: 0), + ] + ) + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 2, section: 0), + IndexPath(item: 0, section: 0), + ] + ) + }) - - Delete 0 - - Delete 1 - - Delete 2 - - Insert 0 - - Insert 2 + /** + - Element B + - Element D */ - - changesReducer.moveElements([(from: IndexPath(row: 1, section: 0), to: IndexPath(row: 2, section: 0))]) - changesReducer.removeElements(at: [IndexPath(row: 1, section: 0)]) - - let changeset = changesReducer.endUpdating() - - XCTAssertNotNil(changeset) - - XCTAssertEqual( - changeset!.elementsRemoved, - [ - IndexPath(row: 0, section: 0), - IndexPath(row: 1, section: 0), - IndexPath(row: 2, section: 0), - ] - ) - XCTAssertEqual( - changeset!.elementsInserted, - [ - IndexPath(row: 0, section: 0), - IndexPath(row: 1, section: 0), - ] - ) - XCTAssertTrue(changeset!.elementsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsMoved.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsUpdated.isEmpty) } - func testMoveElementAtSameIndexAsRemove() { - var changesReducer = ChangesReducer() - changesReducer.beginUpdating() +// func testGroupInserts() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// changesReducer.insertGroups(IndexSet([0, 2])) +// changesReducer.insertGroups(IndexSet(integer: 1)) +// let changeset = changesReducer.endUpdating() +// +// XCTAssertNotNil(changeset) +// +// XCTAssertEqual(changeset!.groupsInserted, [0, 1, 2]) +// XCTAssertTrue(changeset!.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) +// XCTAssertTrue(changeset!.elementsRemoved.isEmpty) +// XCTAssertTrue(changeset!.elementsInserted.isEmpty) +// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) +// XCTAssertTrue(changeset!.elementsMoved.isEmpty) +// } +// +// func testGroupRemoves() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// changesReducer.removeGroups(IndexSet([0, 2])) +// changesReducer.removeGroups(IndexSet(integer: 0)) +// let changeset = changesReducer.endUpdating() +// +// XCTAssertNotNil(changeset) +// +// XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) +// XCTAssertTrue(changeset!.groupsInserted.isEmpty) +// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) +// XCTAssertTrue(changeset!.elementsRemoved.isEmpty) +// XCTAssertTrue(changeset!.elementsInserted.isEmpty) +// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) +// XCTAssertTrue(changeset!.elementsMoved.isEmpty) +// } +// +// func testGroupAndElementRemoves() { +// /** +// Because element removals are processed before group removals, any element removals that are +// performed after a group removal should have their section increased. +// */ +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// changesReducer.removeGroups(IndexSet([0, 1])) +// changesReducer.removeElements(at: [IndexPath(item: 1, section: 2)]) +// changesReducer.removeGroups(IndexSet(integer: 0)) +// let changeset = changesReducer.endUpdating() +// +// XCTAssertNotNil(changeset) +// +// XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) +// XCTAssertTrue(changeset!.groupsInserted.isEmpty) +// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) +// XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 1, section: 4)]) +// XCTAssertTrue(changeset!.elementsInserted.isEmpty) +// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) +// XCTAssertTrue(changeset!.elementsMoved.isEmpty) +// } +// +// func testRemoveSectionThenRemoveElementThenRemoveSection() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// changesReducer.removeGroups([1]) +// changesReducer.removeElements(at: [IndexPath(row: 1, section: 2)]) +// changesReducer.removeGroups([1]) +// } +// +// func testMoveElement() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) +// +// let changeset = changesReducer.endUpdating() +// +// XCTAssertNotNil(changeset) +// +// XCTAssertEqual( +// changeset!.elementsMoved, +// [ +// Changeset.Move( +// from: IndexPath(row: 0, section: 0), +// to: IndexPath(row: 1, section: 0) +// ) +// ] +// ) +// XCTAssertTrue(changeset!.elementsRemoved.isEmpty) +// XCTAssertTrue(changeset!.elementsInserted.isEmpty) +// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) +// XCTAssertTrue(changeset!.groupsInserted.isEmpty) +// XCTAssertTrue(changeset!.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) +// } +// +// func testElementRemovalAfterOtherChanges() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// changesReducer.removeGroups([0]) +// changesReducer.insertGroups([2]) +// changesReducer.moveElements([Changeset.Move(from: IndexPath(item: 3, section: 1), to: IndexPath(item: 3, section: 1))]) +// changesReducer.insertElements(at: [IndexPath(item: 5, section: 2)]) +// changesReducer.insertGroups([1]) +// changesReducer.insertElements(at: [IndexPath(item: 6, section: 3)]) +// +// guard let changeset = changesReducer.endUpdating() else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.elementsRemoved.isEmpty) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 5, section: 3), +// IndexPath(row: 6, section: 3), +// ] +// ) +// XCTAssertTrue(changeset.elementsUpdated.isEmpty) +// XCTAssertEqual( +// changeset.groupsInserted, +// [1] +// ) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// } +// +// func testGroupAndElementInserts() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// changesReducer.insertElements(at: [IndexPath(item: 5, section: 2)]) +// changesReducer.insertGroups([1]) +// changesReducer.insertElements(at: [IndexPath(item: 6, section: 3)]) +// +// guard let changeset = changesReducer.endUpdating() else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.elementsRemoved.isEmpty) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 5, section: 3), +// IndexPath(row: 6, section: 3), +// ] +// ) +// XCTAssertTrue(changeset.elementsUpdated.isEmpty) +// XCTAssertEqual( +// changeset.groupsInserted, +// [1] +// ) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// } +// +// func testMoveElementThenRemoveElementBeforeMovedElement() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// /** +// This is testing: +// +// - A +// - B +// - C +// - D +// +// # Swap C and D +// +// - A +// - B +// - D +// - C +// +// # Delete A +// +// - B +// - D +// - C +// */ +// +// changesReducer.moveElements([(from: IndexPath(row: 2, section: 0), to: IndexPath(row: 3, section: 0))]) +// changesReducer.removeElements(at: [IndexPath(row: 0, section: 0)]) +// +// let changeset = changesReducer.endUpdating() +// +// XCTAssertNotNil(changeset) +// +// XCTAssertEqual( +// changeset!.elementsMoved, +// [ +// Changeset.Move( +// from: IndexPath(row: 1, section: 0), +// to: IndexPath(row: 2, section: 0) +// ) +// ] +// ) +// XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 0, section: 0)]) +// XCTAssertTrue(changeset!.elementsInserted.isEmpty) +// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) +// XCTAssertTrue(changeset!.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset!.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) +// } +// +// func testRemoveAnIndexPathWithAMoveTo() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// /** +// This is testing: +// +// - A +// - B +// - C +// +// # Swap B and C +// +// - A +// - C +// - B +// +// # Delete B +// +// - A +// - C +// +// `UICollectionView` does not support deleting an index path and moving to the same index path, so this should produce: +// +// - Delete 1 +// - Delete 2 +// - Insert 1 +// */ +// +// changesReducer.moveElements([(from: IndexPath(row: 1, section: 0), to: IndexPath(row: 2, section: 0))]) +// changesReducer.removeElements(at: [IndexPath(row: 2, section: 0)]) +// +// guard let changeset = changesReducer.endUpdating() else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 1, section: 0), +// IndexPath(row: 2, section: 0), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 1, section: 0), +// ] +// ) +// XCTAssertTrue(changeset.elementsUpdated.isEmpty) +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// } +// +// func testRemoveAnIndexPathWithAMoveFrom() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// /** +// This is testing: +// +// - A +// - B +// - C +// +// # Swap B and C +// +// - A +// - C +// - B +// +// # Delete B +// +// - A +// - C +// +// `UICollectionView` does not support deleting an index path and moving to the same index path, so this should produce: +// +// - Delete 1 +// - Delete 2 +// - Insert 1 +// */ +// +// changesReducer.moveElements([(from: IndexPath(row: 2, section: 0), to: IndexPath(row: 1, section: 0))]) +// changesReducer.removeElements(at: [IndexPath(row: 2, section: 0)]) +// +// guard let changeset = changesReducer.endUpdating() else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 1, section: 0), +// IndexPath(row: 2, section: 0), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 1, section: 0), +// ] +// ) +// XCTAssertTrue(changeset.elementsUpdated.isEmpty) +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// } +// +// func testMoveElementAtSameIndexAsRemove() { +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// /** +// This is testing: +// +// - A +// - B +// - C +// +// # Delete B +// +// - A +// - C +// +// # Swap A and C +// +// - C +// - A +// +// `UICollectionView` does not support deleting an index path and moving to the same index path, so this should produce: +// +// - Update 0 +// - Update 1 +// - Delete 2 +// */ +// +// changesReducer.removeElements(at: [IndexPath(row: 1, section: 0)]) +// changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) +// +// let changeset = changesReducer.endUpdating() +// +// XCTAssertNotNil(changeset) +// +// XCTAssertEqual( +// changeset!.elementsRemoved, +// [ +// IndexPath(row: 2, section: 0), +// ] +// ) +// XCTAssertTrue(changeset!.elementsInserted.isEmpty) +// XCTAssertEqual( +// changeset!.elementsUpdated, +// [ +// IndexPath(row: 0, section: 0), +// IndexPath(row: 1, section: 0), +// ] +// ) +// XCTAssertTrue(changeset!.elementsMoved.isEmpty) +// XCTAssertTrue(changeset!.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset!.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) +// } - /** - This is testing: +// func testBuildingUpComplexChanges() { +// /** +// This test continuously builds upon the same `ChangesReducer` to test +// applying a large number of changes at the same time. +// +// These changes are mirrored by the `CollectionCoordinatorTests.testBatchUpdates`. +// */ +// var changesReducer = ChangesReducer() +// changesReducer.beginUpdating() +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.insertGroups([0]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsRemoved.isEmpty) +// XCTAssertTrue(changeset.elementsInserted.isEmpty) +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// }) +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.insertElements(at: [IndexPath(item: 0, section: 0)]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsRemoved.isEmpty) +// XCTAssertTrue(changeset.elementsInserted.isEmpty) +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0] +// ) +// }) +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.updateElements(at: [IndexPath(item: 1, section: 1), IndexPath(item: 2, section: 1)]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 1, section: 0), +// IndexPath(row: 2, section: 0), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 1, section: 1), +// IndexPath(row: 2, section: 1), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0] +// ) +// }) +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.insertGroups([1]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 1, section: 0), +// IndexPath(row: 2, section: 0), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 1, section: 2), +// IndexPath(row: 2, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0, 1] +// ) +// }) - - A - - B - - C +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.removeElements(at: [IndexPath(item: 2, section: 3)]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 1, section: 0), +// IndexPath(row: 2, section: 0), +// IndexPath(row: 2, section: 1), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 1, section: 2), +// IndexPath(row: 2, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0, 1] +// ) +// }) +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.insertGroups([5, 4]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 1, section: 0), +// IndexPath(row: 2, section: 0), +// IndexPath(row: 2, section: 1), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 1, section: 2), +// IndexPath(row: 2, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0, 1, 4, 5] +// ) +// }) - # Delete B - - A - - C - # Swap A and C + +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.removeGroups([1]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 0, section: 3), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 0, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsUpdated, +// [ +// IndexPath(row: 1, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0] +// ) +// }) +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.removeElements(at: [IndexPath(item: 2, section: 1)]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsRemoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 2, section: 1), +// IndexPath(row: 0, section: 3), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 0, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsUpdated, +// [ +// IndexPath(row: 1, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0] +// ) +// }) +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.removeGroups([2]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 0, section: 3), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 0, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsUpdated, +// [ +// IndexPath(row: 1, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0] +// ) +// XCTAssertEqual( +// changeset.groupsRemoved, +// [1] +// ) +// }) +// +// AssertApplyingUpdates( +// { changesReducer in +// changesReducer.insertElements(at: [IndexPath(item: 2, section: 1), IndexPath(item: 3, section: 1)]) +// }, +// changesReducer: &changesReducer, +// produces: { changeset in +// guard let changeset = changeset else { +// XCTFail("Changeset should not be `nil`") +// return +// } +// +// XCTAssertTrue(changeset.elementsMoved.isEmpty) +// XCTAssertTrue(changeset.groupsUpdated.isEmpty) +// XCTAssertEqual( +// changeset.elementsRemoved, +// [ +// IndexPath(row: 0, section: 3), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsInserted, +// [ +// IndexPath(row: 0, section: 2), +// IndexPath(row: 2, section: 1), +// IndexPath(row: 3, section: 1), +// ] +// ) +// XCTAssertEqual( +// changeset.elementsUpdated, +// [ +// IndexPath(row: 1, section: 2), +// ] +// ) +// XCTAssertEqual( +// changeset.groupsInserted, +// [0] +// ) +// XCTAssertEqual( +// changeset.groupsRemoved, +// [1] +// ) +// }) - - C - - A +// changesReducer.removeElements(at: [IndexPath(item: 2, section: 1)]) +// changesReducer.insertGroups([1, 2, 4]) +// changesReducer.removeGroups([2]) +// } +} - `UICollectionView` does not support deleting an index path and moving to the same index path, so this should produce: +private func AssertApplyingUpdates(_ updates: (inout ChangesReducer) -> Void, changesReducer: inout ChangesReducer, produces resultChecker: (Changeset?) -> Void) { + updates(&changesReducer) - - Delete 0 - - Delete 1 - - Delete 2 - - Insert 0 - - Insert 2 - */ + var changesReducerCopy = changesReducer + let changeset = changesReducerCopy.endUpdating() - changesReducer.removeElements(at: [IndexPath(row: 1, section: 0)]) - changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) - - let changeset = changesReducer.endUpdating() - - XCTAssertNotNil(changeset) - - XCTAssertEqual( - changeset!.elementsRemoved, - [ - IndexPath(row: 0, section: 0), - IndexPath(row: 1, section: 0), - IndexPath(row: 2, section: 0), - ] - ) - XCTAssertEqual( - changeset!.elementsInserted, - [ - IndexPath(row: 0, section: 0), - IndexPath(row: 1, section: 0), - ] - ) - XCTAssertTrue(changeset!.elementsUpdated.isEmpty) - XCTAssertTrue(changeset!.elementsMoved.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsRemoved.isEmpty) - XCTAssertTrue(changeset!.groupsUpdated.isEmpty) - } + resultChecker(changeset) } diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift index d4bc4c9..32e95fe 100644 --- a/Tests/ComposedUITests/CollectionCoordinatorTests.swift +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -10,71 +10,372 @@ final class CollectionCoordinatorTests: XCTestCase { /// /// One way for these tests to fail is by `UICollectionView` throwing a `NSInternalInconsistencyException', reason: 'Invalid update...'` /// error, which would likely indicate an error in `ChangesReducer`. + /// + /// It may also fail without throwing an exception, instead logging: `Invalid update: invalid ... - will perform reloadData`. func testBatchUpdates() { - let collectionView = UICollectionView(frame: .zero, collectionViewLayout: UICollectionViewFlowLayout()) - let rootSectionProvider = ComposedSectionProvider() - let collectionCoordinator = CollectionCoordinator(collectionView: collectionView, sectionProvider: rootSectionProvider) - _ = collectionCoordinator + let tester = Tester() { sections in + sections.rootSectionProvider.append(sections.child0) + sections.rootSectionProvider.append(sections.child1) + sections.rootSectionProvider.append(sections.child2) + sections.rootSectionProvider.append(sections.child3) + } - let child0 = MockCollectionArraySection(["1", "2", "3"]) - let child1 = MockCollectionArraySection(["1", "2", "3"]) - let child2 = MockCollectionArraySection(["1", "2", "3"]) - let child3 = MockCollectionArraySection(["1", "2", "3"]) - var child4 = MockCollectionArraySection(["1", "2", "3"]) + /** + - Child 0 + - Child 1 + - Child 2 + - Child 3 + */ - rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child0) + } - rootSectionProvider.append(child0) - rootSectionProvider.append(child1) - rootSectionProvider.append(child2) - rootSectionProvider.append(child3) - rootSectionProvider.append(child4) + /** + - Child 1 + - Child 2 + - Child 3 + */ - rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) +// tester.applyUpdate { sections in +// sections.child2.swapAt(0, 3) +// } - rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + tester.applyUpdate { sections in + sections.rootSectionProvider.insert(sections.child0, at: 0) + } - rootSectionProvider.remove(child3) - rootSectionProvider.remove(child0) - rootSectionProvider.remove(child1) - rootSectionProvider.remove(child2) - rootSectionProvider.remove(child4) + /** + - Child 0 + - Child 1 + - Child 2 + - Child 3 + */ - rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + tester.applyUpdate { sections in + sections.child0.append("new-0") + } - rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + tester.applyUpdate { sections in + sections.child2[1] = "new-1" + } - rootSectionProvider.append(child0) - rootSectionProvider.remove(child0) - rootSectionProvider.append(child0) - rootSectionProvider.append(child1) - rootSectionProvider.append(child2) - rootSectionProvider.append(child3) - rootSectionProvider.append(child4) - rootSectionProvider.remove(child2) + tester.applyUpdate { sections in + sections.child2[2] = "new-2" + } - rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child3) + } - rootSectionProvider.insert(child2, after: child1) + /** + - Child 0 + - Child 1 + - Child 2 + */ - rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + tester.applyUpdate { sections in + sections.child2.append("appended") + } - rootSectionProvider.remove(child1) - child3.remove(at: 1) - child4.swapAt(0, 2) - rootSectionProvider.remove(child2) - child4.append("4") - rootSectionProvider.remove(child0) - child4.remove(at: 1) - rootSectionProvider.insert(child0, at: 0) + tester.applyUpdate { sections in + sections.rootSectionProvider.insert(sections.child4, at: 1) + } - rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + /** + - Child 0 + - Child 4 + - Child 1 + - Child 2 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.append(sections.child6) + } + + /** + - Child 0 + - Child 4 + - Child 1 + - Child 2 + - Child 6 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.insert(sections.child5, before: sections.child6) + } + + /** + - Child 0 + - Child 4 + - Child 1 + - Child 2 + - Child 5 + - Child 6 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child4) + } + + /** + - Child 0 + - Child 1 + - Child 2 + - Child 5 + - Child 6 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.insert(sections.child4, before: sections.child5) + } + + /** + - Child 0 + - Child 1 + - Child 2 + - Child 4 + - Child 5 + - Child 6 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.insert(sections.child3, before: sections.child4) + } + + /** + - Child 0 + - Child 1 + - Child 2 + - Child 3 + - Child 4 + - Child 5 + - Child 6 + */ + + tester.applyUpdate { sections in + sections.child3.remove(at: 2) + } + + tester.applyUpdate { sections in + sections.child3.insert("new-2", at: 2) + } + + tester.applyUpdate { sections in + sections.child3.insert("new-3", at: 3) + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child2) + } + + /** + - Child 0 + - Child 1 + - Child 3 + - Child 4 + - Child 5 + - Child 6 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child3) + } + + /** + - Child 0 + - Child 1 + - Child 4 + - Child 5 + - Child 6 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.insert(sections.child2, at: 2) + } + + /** + - Child 0 + - Child 1 + - Child 2 + - Child 4 + - Child 5 + - Child 6 + */ + +// tester.applyUpdate { sections in +// sections.child5.swapAt(0, 8) +// } + +// tester.applyUpdate { sections in +// sections.child2.swapAt(0, 3) +// } + } + + func testBatchedSectionRemovals() { + let tester = Tester() { sections in + sections.rootSectionProvider.append(sections.child0) + sections.rootSectionProvider.append(sections.child1) + sections.rootSectionProvider.append(sections.child2) + sections.rootSectionProvider.append(sections.child3) + sections.rootSectionProvider.append(sections.child4) + sections.rootSectionProvider.append(sections.child5) + } + + /** + - Child 0 + - Child 1 + - Child 2 + - Child 3 + - Child 4 + - Child 5 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child0) + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child3) + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child5) + } + + /** + - Child 1 + - Child 2 + - Child 4 + */ + + tester.applyUpdate { sections in + _ = sections.child4.remove(at: 0) + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child4) + } + + tester.applyUpdate { sections in + sections.child4.append("appended") + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child2) + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child1) + } + } + + func testInsertInToSectionAfterInsertion() { + let tester = Tester() { sections in + sections.rootSectionProvider.append(sections.child0) + sections.rootSectionProvider.append(sections.child1) + sections.rootSectionProvider.append(sections.child3) + } + + tester.applyUpdate { sections in + sections.child0.append("new-element") + } + + tester.applyUpdate { sections in + sections.child3.append("new-element") + } + + /** + - Child 0 + - Child 1 + - Child 3 + */ + + tester.applyUpdate { sections in + sections.rootSectionProvider.insert(sections.child2, after: sections.child1) + } + + /** + - Child 0 + - Child 1 + - Child 2 + - Child 3 + */ + + tester.applyUpdate { sections in + sections.child0.append("new-element") + } + + tester.applyUpdate { sections in + sections.child3.append("new-element") + } + } + + func testSwapping() { + let tester = Tester() { sections in + sections.rootSectionProvider.append(sections.child2) + } + + tester.applyUpdate { sections in + sections.child2.swapAt(0, 3) + } } } -private final class MockCollectionArraySection: ArraySection, CollectionSectionProvider { +private final class MockCollectionArraySection: ArraySection, SingleUICollectionViewSection { func section(with traitCollection: UITraitCollection) -> CollectionSection { let cell = CollectionCellElement(section: self, dequeueMethod: .fromClass(UICollectionViewCell.self), configure: { _, _, _ in }) return CollectionSection(section: self, cell: cell) } } + +private final class TestSections { + let rootSectionProvider = ComposedSectionProvider() + + let child0 = MockCollectionArraySection([]) + let child1 = MockCollectionArraySection(["1"]) + var child2 = MockCollectionArraySection(["1", "2", "3", "4"]) + let child3 = MockCollectionArraySection(["1", "2", "3"]) + let child4 = MockCollectionArraySection(["1", "2", "3", "4", "5"]) + var child5 = MockCollectionArraySection(["1", "2", "3", "4", "5", "6", "7", "8", "9"]) + let child6 = MockCollectionArraySection([]) +} + +private final class Tester { + typealias Updater = (TestSections) -> Void + + private var updaters: [Updater] = [] + + private var sections: TestSections + + private let initialState: Updater + + private var collectionViews: [UICollectionView] = [] + + init(initialState: @escaping Updater) { + self.initialState = initialState + + sections = TestSections() + } + + func applyUpdate(_ updater: @escaping Updater) { + updaters.append(updater) + sections = TestSections() + initialState(sections) + + let rootSectionProvider = sections.rootSectionProvider + + let collectionView = UICollectionView(frame: .zero, collectionViewLayout: UICollectionViewFlowLayout()) + collectionViews.append(collectionView) + + let collectionCoordinator = CollectionCoordinator(collectionView: collectionView, sectionProvider: rootSectionProvider) + collectionCoordinator.enableLogs = true + collectionView.reloadData() + + rootSectionProvider.updateDelegate?.willBeginUpdating(rootSectionProvider) + + updaters.forEach { $0(sections) } + + rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + } +} From 236077f385bf2f052f2c8e8668e3515ea55c6f8d Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Sat, 23 Jan 2021 22:11:54 +0000 Subject: [PATCH 04/12] Fix tests compilation --- Tests/ComposedUITests/CollectionCoordinatorTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift index 32e95fe..23ac73c 100644 --- a/Tests/ComposedUITests/CollectionCoordinatorTests.swift +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -322,7 +322,7 @@ final class CollectionCoordinatorTests: XCTestCase { } } -private final class MockCollectionArraySection: ArraySection, SingleUICollectionViewSection { +private final class MockCollectionArraySection: ArraySection, CollectionSectionProvider { func section(with traitCollection: UITraitCollection) -> CollectionSection { let cell = CollectionCellElement(section: self, dequeueMethod: .fromClass(UICollectionViewCell.self), configure: { _, _, _ in }) return CollectionSection(section: self, cell: cell) From 40bbb1efd8f79d25b154ed03f8a1f99296979446 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Sat, 23 Jan 2021 22:12:00 +0000 Subject: [PATCH 05/12] Add tests to ComposedUI scheme --- .../xcshareddata/xcschemes/ComposedUI.xcscheme | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/ComposedUI.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/ComposedUI.xcscheme index 194c762..96ed47b 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/ComposedUI.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/ComposedUI.xcscheme @@ -26,8 +26,19 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - shouldUseLaunchSchemeArgsEnv = "YES"> + shouldUseLaunchSchemeArgsEnv = "YES" + codeCoverageEnabled = "YES"> + + + + Date: Mon, 1 Feb 2021 13:30:24 +0000 Subject: [PATCH 06/12] Fix swaps # Conflicts: # Tests/ComposedUITests/CollectionCoordinatorTests.swift --- .../ComposedUI/Common/ChangesReducer.swift | 6 +- .../ComposedUITests/ChangesReducerTests.swift | 82 +++++++++++++++++++ .../CollectionCoordinatorTests.swift | 34 +++++--- 3 files changed, 108 insertions(+), 14 deletions(-) diff --git a/Sources/ComposedUI/Common/ChangesReducer.swift b/Sources/ComposedUI/Common/ChangesReducer.swift index 76e1585..b9d69cf 100644 --- a/Sources/ComposedUI/Common/ChangesReducer.swift +++ b/Sources/ComposedUI/Common/ChangesReducer.swift @@ -212,9 +212,13 @@ internal struct ChangesReducer { if !changeset.groupsInserted.contains(removedIndexPath.section) { let itemInsertsInSection = changeset .elementsInserted - .filter { $0.section == removedIndexPath.section } + .filter { $0.section == removedIndexPath.section - sectionsRemovedBefore + sectionsInsertedBefore } .map(\.item) + if changeset.elementsRemoved.contains(removedIndexPath), changeset.elementsInserted.remove(removedIndexPath) != nil { + return + } + changeset.elementsInserted = Set(changeset.elementsInserted.map { existingInsertedIndexPath in guard existingInsertedIndexPath.section == removedIndexPath.section else { // Different section; don't modify diff --git a/Tests/ComposedUITests/ChangesReducerTests.swift b/Tests/ComposedUITests/ChangesReducerTests.swift index b782a04..a0776e3 100644 --- a/Tests/ComposedUITests/ChangesReducerTests.swift +++ b/Tests/ComposedUITests/ChangesReducerTests.swift @@ -388,6 +388,88 @@ final class ChangesReducerTests: XCTestCase { */ } + func testRemoveSectionThenSwapElements() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + /** + - Section A + - Section B + - Section C + - Section C-0 + - Section C-1 + - Section C-2 + - Section C-3 + - Section C-4 + */ + + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeGroups([0]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } + + XCTAssertTrue(changeset.elementsRemoved.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.groupsRemoved, + [0] + ) + }) + + /** + - Section B + - Section C + - Section C-0 + - Section C-1 + - Section C-2 + - Section C-3 + - Section C-4 + */ + + AssertApplyingUpdates( + { changesReducer in + // Simulate a swap + changesReducer.updateElements(at: [ + IndexPath(item: 0, section: 1), + ]) + changesReducer.updateElements(at: [ + IndexPath(item: 3, section: 1), + ]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } + + XCTAssertEqual( + changeset.elementsRemoved, + [ + IndexPath(item: 0, section: 2), + IndexPath(item: 3, section: 2), + ] + ) + XCTAssertEqual( + changeset.elementsInserted, + [ + IndexPath(item: 0, section: 1), + IndexPath(item: 3, section: 1), + ] + ) + XCTAssertEqual( + changeset.groupsRemoved, + [0] + ) + }) + } + // func testGroupInserts() { // var changesReducer = ChangesReducer() // changesReducer.beginUpdating() diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift index 23ac73c..ff2e348 100644 --- a/Tests/ComposedUITests/CollectionCoordinatorTests.swift +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -37,9 +37,11 @@ final class CollectionCoordinatorTests: XCTestCase { - Child 3 */ -// tester.applyUpdate { sections in -// sections.child2.swapAt(0, 3) -// } + tester.applyUpdate({ sections in + sections.child2.swapAt(0, 3) + }, postUpdateChecks: { sections in + XCTAssertEqual(sections.child2.requestedCells, [0, 3]) + }) tester.applyUpdate { sections in sections.rootSectionProvider.insert(sections.child0, at: 0) @@ -203,13 +205,13 @@ final class CollectionCoordinatorTests: XCTestCase { - Child 6 */ -// tester.applyUpdate { sections in -// sections.child5.swapAt(0, 8) -// } + tester.applyUpdate { sections in + sections.child5.swapAt(0, 8) + } -// tester.applyUpdate { sections in -// sections.child2.swapAt(0, 3) -// } + tester.applyUpdate { sections in + sections.child2.swapAt(0, 3) + } } func testBatchedSectionRemovals() { @@ -323,8 +325,12 @@ final class CollectionCoordinatorTests: XCTestCase { } private final class MockCollectionArraySection: ArraySection, CollectionSectionProvider { + private(set) var requestedCells: [Int] = [] + func section(with traitCollection: UITraitCollection) -> CollectionSection { - let cell = CollectionCellElement(section: self, dequeueMethod: .fromClass(UICollectionViewCell.self), configure: { _, _, _ in }) + let cell = CollectionCellElement(section: self, dequeueMethod: .fromClass(UICollectionViewCell.self), configure: { [weak self] _, cellIndex, _ in + self?.requestedCells.append(cellIndex) + }) return CollectionSection(section: self, cell: cell) } } @@ -334,7 +340,7 @@ private final class TestSections { let child0 = MockCollectionArraySection([]) let child1 = MockCollectionArraySection(["1"]) - var child2 = MockCollectionArraySection(["1", "2", "3", "4"]) + var child2 = MockCollectionArraySection(["0", "1", "2", "3"]) let child3 = MockCollectionArraySection(["1", "2", "3"]) let child4 = MockCollectionArraySection(["1", "2", "3", "4", "5"]) var child5 = MockCollectionArraySection(["1", "2", "3", "4", "5", "6", "7", "8", "9"]) @@ -354,11 +360,11 @@ private final class Tester { init(initialState: @escaping Updater) { self.initialState = initialState - + sections = TestSections() } - func applyUpdate(_ updater: @escaping Updater) { + func applyUpdate(_ updater: @escaping Updater, postUpdateChecks: ((TestSections) -> Void)? = nil) { updaters.append(updater) sections = TestSections() initialState(sections) @@ -377,5 +383,7 @@ private final class Tester { updaters.forEach { $0(sections) } rootSectionProvider.updateDelegate?.didEndUpdating(rootSectionProvider) + + postUpdateChecks?(sections) } } From 2b78ab5bcd3b032b7a79491a098dd9e12eaa7c20 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Mon, 1 Feb 2021 13:50:38 +0000 Subject: [PATCH 07/12] Fix batch removal from `ArraySection` --- Sources/Composed/Sections/ArraySection.swift | 4 ++-- .../CollectionCoordinatorTests.swift | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Sources/Composed/Sections/ArraySection.swift b/Sources/Composed/Sections/ArraySection.swift index 2c80d63..66588eb 100644 --- a/Sources/Composed/Sections/ArraySection.swift +++ b/Sources/Composed/Sections/ArraySection.swift @@ -132,7 +132,7 @@ extension ArraySection: MutableCollection, RandomAccessCollection, Bidirectional let oldCount = elements.count elements.removeLast(k) let newCount = elements.count - (newCount..).forEach { updateDelegate?.section(self, didRemoveElementAt: $0) } updateDelegate?.didEndUpdating(self) @@ -159,7 +159,7 @@ extension ArraySection: MutableCollection, RandomAccessCollection, Bidirectional public func removeAll() { updateDelegate?.willBeginUpdating(self) let indexes = IndexSet(integersIn: indices) - indexes.forEach { updateDelegate?.section(self, didRemoveElementAt: $0) } + indexes.sorted(by: >).forEach { updateDelegate?.section(self, didRemoveElementAt: $0) } elements.removeAll() updateDelegate?.didEndUpdating(self) } diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift index ff2e348..87096b8 100644 --- a/Tests/ComposedUITests/CollectionCoordinatorTests.swift +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -322,6 +322,26 @@ final class CollectionCoordinatorTests: XCTestCase { sections.child2.swapAt(0, 3) } } + + func testRemoveAll() { + let tester = Tester() { sections in + sections.rootSectionProvider.append(sections.child2) + } + + tester.applyUpdate { sections in + sections.child2.removeAll() + } + } + + func testRemoveLast2() { + let tester = Tester() { sections in + sections.rootSectionProvider.append(sections.child2) + } + + tester.applyUpdate { sections in + sections.child2.removeLast(2) + } + } } private final class MockCollectionArraySection: ArraySection, CollectionSectionProvider { From 53a8b2186b3ec5328a957936b739da5df3d9592c Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Mon, 1 Feb 2021 14:26:27 +0000 Subject: [PATCH 08/12] Ignore requested cells order --- Tests/ComposedUITests/CollectionCoordinatorTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift index 87096b8..9c1a68d 100644 --- a/Tests/ComposedUITests/CollectionCoordinatorTests.swift +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -40,7 +40,7 @@ final class CollectionCoordinatorTests: XCTestCase { tester.applyUpdate({ sections in sections.child2.swapAt(0, 3) }, postUpdateChecks: { sections in - XCTAssertEqual(sections.child2.requestedCells, [0, 3]) + XCTAssertEqual(Set(sections.child2.requestedCells), Set([0, 3])) }) tester.applyUpdate { sections in From de35128dda3d350c4c3ce40245eaacb72c782c2c Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Mon, 1 Feb 2021 14:28:20 +0000 Subject: [PATCH 09/12] Uncomment some tests --- .../ComposedUITests/ChangesReducerTests.swift | 204 ++++++++---------- 1 file changed, 93 insertions(+), 111 deletions(-) diff --git a/Tests/ComposedUITests/ChangesReducerTests.swift b/Tests/ComposedUITests/ChangesReducerTests.swift index a0776e3..9809e88 100644 --- a/Tests/ComposedUITests/ChangesReducerTests.swift +++ b/Tests/ComposedUITests/ChangesReducerTests.swift @@ -470,21 +470,108 @@ final class ChangesReducerTests: XCTestCase { }) } + func testGroupAndElementRemoves() { + /** + Because element removals are processed before group removals, any element removals that are + performed after a group removal should have their section increased. + */ + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + changesReducer.removeGroups(IndexSet([0, 1])) + changesReducer.removeElements(at: [IndexPath(item: 1, section: 2)]) + changesReducer.removeGroups(IndexSet(integer: 0)) + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) + XCTAssertTrue(changeset!.groupsInserted.isEmpty) + XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 1, section: 4)]) + XCTAssertTrue(changeset!.elementsInserted.isEmpty) + XCTAssertTrue(changeset!.elementsMoved.isEmpty) + } + + func testRemoveSectionThenRemoveElementThenRemoveSection() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + changesReducer.removeGroups([1]) + changesReducer.removeElements(at: [IndexPath(row: 1, section: 2)]) + changesReducer.removeGroups([1]) + } + + func testMoveElement() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) + + let changeset = changesReducer.endUpdating() + + XCTAssertNotNil(changeset) + + XCTAssertEqual( + changeset!.elementsMoved, + [ + Changeset.Move( + from: IndexPath(row: 0, section: 0), + to: IndexPath(row: 1, section: 0) + ) + ] + ) + XCTAssertTrue(changeset!.elementsRemoved.isEmpty) + XCTAssertTrue(changeset!.elementsInserted.isEmpty) + XCTAssertTrue(changeset!.groupsInserted.isEmpty) + XCTAssertTrue(changeset!.groupsRemoved.isEmpty) + } + + func testGroupAndElementInserts() { + var changesReducer = ChangesReducer() + changesReducer.beginUpdating() + + changesReducer.insertElements(at: [IndexPath(item: 5, section: 2)]) + changesReducer.insertGroups([1]) + changesReducer.insertElements(at: [IndexPath(item: 6, section: 3)]) + + guard let changeset = changesReducer.endUpdating() else { + XCTFail("Changeset should not be `nil`") + return + } + + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertTrue(changeset.elementsRemoved.isEmpty) + XCTAssertEqual( + changeset.elementsInserted, + [ + IndexPath(row: 5, section: 3), + IndexPath(row: 6, section: 3), + ] + ) + XCTAssertEqual( + changeset.groupsInserted, + [1] + ) + XCTAssertTrue(changeset.groupsRemoved.isEmpty) + } + + // MARK:- Unfinished Tests + // func testGroupInserts() { // var changesReducer = ChangesReducer() // changesReducer.beginUpdating() +// // changesReducer.insertGroups(IndexSet([0, 2])) +// // changesReducer.insertGroups(IndexSet(integer: 1)) +// // let changeset = changesReducer.endUpdating() // // XCTAssertNotNil(changeset) // -// XCTAssertEqual(changeset!.groupsInserted, [0, 1, 2]) +// XCTAssertEqual(changeset!.groupsInserted, [0, 0, 2]) // XCTAssertTrue(changeset!.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) // XCTAssertTrue(changeset!.elementsRemoved.isEmpty) // XCTAssertTrue(changeset!.elementsInserted.isEmpty) -// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) // XCTAssertTrue(changeset!.elementsMoved.isEmpty) // } // @@ -499,72 +586,11 @@ final class ChangesReducerTests: XCTestCase { // // XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) // XCTAssertTrue(changeset!.groupsInserted.isEmpty) -// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) // XCTAssertTrue(changeset!.elementsRemoved.isEmpty) // XCTAssertTrue(changeset!.elementsInserted.isEmpty) -// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) -// XCTAssertTrue(changeset!.elementsMoved.isEmpty) -// } -// -// func testGroupAndElementRemoves() { -// /** -// Because element removals are processed before group removals, any element removals that are -// performed after a group removal should have their section increased. -// */ -// var changesReducer = ChangesReducer() -// changesReducer.beginUpdating() -// changesReducer.removeGroups(IndexSet([0, 1])) -// changesReducer.removeElements(at: [IndexPath(item: 1, section: 2)]) -// changesReducer.removeGroups(IndexSet(integer: 0)) -// let changeset = changesReducer.endUpdating() -// -// XCTAssertNotNil(changeset) -// -// XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) -// XCTAssertTrue(changeset!.groupsInserted.isEmpty) -// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) -// XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 1, section: 4)]) -// XCTAssertTrue(changeset!.elementsInserted.isEmpty) -// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) // XCTAssertTrue(changeset!.elementsMoved.isEmpty) // } // -// func testRemoveSectionThenRemoveElementThenRemoveSection() { -// var changesReducer = ChangesReducer() -// changesReducer.beginUpdating() -// -// changesReducer.removeGroups([1]) -// changesReducer.removeElements(at: [IndexPath(row: 1, section: 2)]) -// changesReducer.removeGroups([1]) -// } -// -// func testMoveElement() { -// var changesReducer = ChangesReducer() -// changesReducer.beginUpdating() -// -// changesReducer.moveElements([(from: IndexPath(row: 0, section: 0), to: IndexPath(row: 1, section: 0))]) -// -// let changeset = changesReducer.endUpdating() -// -// XCTAssertNotNil(changeset) -// -// XCTAssertEqual( -// changeset!.elementsMoved, -// [ -// Changeset.Move( -// from: IndexPath(row: 0, section: 0), -// to: IndexPath(row: 1, section: 0) -// ) -// ] -// ) -// XCTAssertTrue(changeset!.elementsRemoved.isEmpty) -// XCTAssertTrue(changeset!.elementsInserted.isEmpty) -// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) -// XCTAssertTrue(changeset!.groupsInserted.isEmpty) -// XCTAssertTrue(changeset!.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) -// } -// // func testElementRemovalAfterOtherChanges() { // var changesReducer = ChangesReducer() // changesReducer.beginUpdating() @@ -590,44 +616,11 @@ final class ChangesReducerTests: XCTestCase { // IndexPath(row: 6, section: 3), // ] // ) -// XCTAssertTrue(changeset.elementsUpdated.isEmpty) -// XCTAssertEqual( -// changeset.groupsInserted, -// [1] -// ) -// XCTAssertTrue(changeset.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset.groupsUpdated.isEmpty) -// } -// -// func testGroupAndElementInserts() { -// var changesReducer = ChangesReducer() -// changesReducer.beginUpdating() -// -// changesReducer.insertElements(at: [IndexPath(item: 5, section: 2)]) -// changesReducer.insertGroups([1]) -// changesReducer.insertElements(at: [IndexPath(item: 6, section: 3)]) -// -// guard let changeset = changesReducer.endUpdating() else { -// XCTFail("Changeset should not be `nil`") -// return -// } -// -// XCTAssertTrue(changeset.elementsMoved.isEmpty) -// XCTAssertTrue(changeset.elementsRemoved.isEmpty) -// XCTAssertEqual( -// changeset.elementsInserted, -// [ -// IndexPath(row: 5, section: 3), -// IndexPath(row: 6, section: 3), -// ] -// ) -// XCTAssertTrue(changeset.elementsUpdated.isEmpty) // XCTAssertEqual( // changeset.groupsInserted, // [1] // ) // XCTAssertTrue(changeset.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset.groupsUpdated.isEmpty) // } // // func testMoveElementThenRemoveElementBeforeMovedElement() { @@ -674,10 +667,8 @@ final class ChangesReducerTests: XCTestCase { // ) // XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 0, section: 0)]) // XCTAssertTrue(changeset!.elementsInserted.isEmpty) -// XCTAssertTrue(changeset!.elementsUpdated.isEmpty) // XCTAssertTrue(changeset!.groupsRemoved.isEmpty) // XCTAssertTrue(changeset!.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) // } // // func testRemoveAnIndexPathWithAMoveTo() { @@ -730,11 +721,9 @@ final class ChangesReducerTests: XCTestCase { // IndexPath(row: 1, section: 0), // ] // ) -// XCTAssertTrue(changeset.elementsUpdated.isEmpty) // XCTAssertTrue(changeset.elementsMoved.isEmpty) // XCTAssertTrue(changeset.groupsRemoved.isEmpty) // XCTAssertTrue(changeset.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset.groupsUpdated.isEmpty) // } // // func testRemoveAnIndexPathWithAMoveFrom() { @@ -787,11 +776,9 @@ final class ChangesReducerTests: XCTestCase { // IndexPath(row: 1, section: 0), // ] // ) -// XCTAssertTrue(changeset.elementsUpdated.isEmpty) // XCTAssertTrue(changeset.elementsMoved.isEmpty) // XCTAssertTrue(changeset.groupsRemoved.isEmpty) // XCTAssertTrue(changeset.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset.groupsUpdated.isEmpty) // } // // func testMoveElementAtSameIndexAsRemove() { @@ -846,9 +833,8 @@ final class ChangesReducerTests: XCTestCase { // XCTAssertTrue(changeset!.elementsMoved.isEmpty) // XCTAssertTrue(changeset!.groupsRemoved.isEmpty) // XCTAssertTrue(changeset!.groupsRemoved.isEmpty) -// XCTAssertTrue(changeset!.groupsUpdated.isEmpty) // } - +// // func testBuildingUpComplexChanges() { // /** // This test continuously builds upon the same `ChangesReducer` to test @@ -962,7 +948,7 @@ final class ChangesReducerTests: XCTestCase { // [0, 1] // ) // }) - +// // AssertApplyingUpdates( // { changesReducer in // changesReducer.removeElements(at: [IndexPath(item: 2, section: 3)]) @@ -1030,10 +1016,6 @@ final class ChangesReducerTests: XCTestCase { // [0, 1, 4, 5] // ) // }) - - - - // // AssertApplyingUpdates( // { changesReducer in @@ -1195,7 +1177,7 @@ final class ChangesReducerTests: XCTestCase { // [1] // ) // }) - +// // changesReducer.removeElements(at: [IndexPath(item: 2, section: 1)]) // changesReducer.insertGroups([1, 2, 4]) // changesReducer.removeGroups([2]) From 564c40290d253b66b4b1f1fd345ecc716cbe4660 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Tue, 2 Feb 2021 18:31:58 +0000 Subject: [PATCH 10/12] WIP: Reloads, unifying transformations --- .../CollectionCoordinator.swift | 9 +- .../ComposedUI/Common/ChangesReducer.swift | 147 ++++++++++++------ Sources/ComposedUI/Common/Changeset.swift | 1 + .../ComposedUITests/ChangesReducerTests.swift | 73 +++++++-- .../CollectionCoordinatorTests.swift | 39 ++++- 5 files changed, 207 insertions(+), 62 deletions(-) diff --git a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift index 9835246..b210329 100644 --- a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift +++ b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift @@ -251,20 +251,23 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { collectionView.performBatchUpdates({ prepareSections() + debugLog("Deleting sections \(changeset.groupsRemoved)") + collectionView.deleteSections(IndexSet(changeset.groupsRemoved)) + debugLog("Deleting items \(changeset.elementsRemoved)") collectionView.deleteItems(at: Array(changeset.elementsRemoved)) debugLog("Inserting items \(changeset.elementsInserted)") collectionView.insertItems(at: Array(changeset.elementsInserted)) + debugLog("Reloading items \(changeset.elementsUpdated)") + collectionView.reloadItems(at: Array(changeset.elementsUpdated)) + changeset.elementsMoved.forEach { move in debugLog("Moving \(move.from) to \(move.to)") collectionView.moveItem(at: move.from, to: move.to) } - debugLog("Deleting sections \(changeset.groupsRemoved)") - collectionView.deleteSections(IndexSet(changeset.groupsRemoved)) - debugLog("Inserting sections \(changeset.groupsInserted)") collectionView.insertSections(IndexSet(changeset.groupsInserted)) }) diff --git a/Sources/ComposedUI/Common/ChangesReducer.swift b/Sources/ComposedUI/Common/ChangesReducer.swift index b9d69cf..a3eb2dd 100644 --- a/Sources/ComposedUI/Common/ChangesReducer.swift +++ b/Sources/ComposedUI/Common/ChangesReducer.swift @@ -12,6 +12,14 @@ import Foundation Final updates are applied in the order: + | Update | Order | Indexes | + |------------------|-------------|----------| + | Element Removals | High to low | Original | + | Element Reloads | N/A | Original | + | Group removals | High to low | Original | + + https://developer.apple.com/videos/play/wwdc2018/225/ is useful. Page 62 of the slides helps confirm the above table. + - Element removals - Using original index paths - Group removals @@ -116,34 +124,51 @@ internal struct ChangesReducer { } internal mutating func removeGroups(_ groups: IndexSet) { - groups.forEach { removedGroup in - let removedGroup = removedGroup - changeset.groupsInserted.filter { $0 < removedGroup }.count + changeset.groupsRemoved.filter { $0 <= removedGroup }.count - - if changeset.groupsInserted.remove(removedGroup) == nil { - changeset.groupsRemoved = Set(changeset.groupsRemoved - .sorted(by: <) - .reduce(into: (previous: Int?.none, groupsRemoved: [Int]()), { (result, groupsRemoved) in - if groupsRemoved == removedGroup { - result.groupsRemoved.append(groupsRemoved) - result.groupsRemoved.append(groupsRemoved + 1) - result.previous = groupsRemoved + 1 - } else if let previous = result.previous, groupsRemoved == previous { - // TODO: Test this - result.groupsRemoved.append(groupsRemoved + 1) - result.previous = groupsRemoved + 1 - } else { - result.groupsRemoved.append(groupsRemoved) - result.previous = groupsRemoved - } - }) - .groupsRemoved - ) - - if !changeset.groupsRemoved.contains(removedGroup) { - changeset.groupsRemoved.insert(removedGroup) - } + print(#function, groups) + groups.sorted(by: >).forEach { removedGroup in + print("Removing", removedGroup) + print("groupsInserted", changeset.groupsInserted) + + /** + + */ +// changeset.groupsRemoved = Set(changeset.groupsRemoved +// .sorted(by: >) +// .map { existingRemovedGroup in +// existingRemovedGroup +// } +// .reduce(into: (previous: Int?.none, groupsRemoved: [Int]()), { (result, groupsRemoved) in +// if groupsRemoved == removedGroup { +// result.groupsRemoved.append(groupsRemoved) +// result.groupsRemoved.append(groupsRemoved + 1) +// result.previous = groupsRemoved + 1 +// } else if let previous = result.previous, groupsRemoved == previous { +// // TODO: Test this +// result.groupsRemoved.append(groupsRemoved + 1) +// result.previous = groupsRemoved + 1 +// } else { +// result.groupsRemoved.append(groupsRemoved) +// result.previous = groupsRemoved +// } +// }) +// .groupsRemoved +// ) + + var removedGroup = removedGroup + + if changeset.groupsInserted.remove(removedGroup) != nil { + // TODO: Offset future indexes? + removedGroup = transformSection(removedGroup) + } else { + removedGroup = transformSection(removedGroup) + changeset.groupsRemoved.insert(removedGroup) } + +// if !changeset.groupsRemoved.contains(removedGroup) { + +// } + changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in if insertedGroup > removedGroup { return insertedGroup - 1 @@ -152,6 +177,18 @@ internal struct ChangesReducer { return insertedGroup }) +// changeset.elementsUpdated = Set(changeset.elementsUpdated.compactMap { updatedIndexPath in +// guard updatedIndexPath.section != removedGroup else { return nil } +// +// var updatedIndexPath = updatedIndexPath +// +// if updatedIndexPath.section > removedGroup { +// updatedIndexPath.section -= 1 +// } +// +// return updatedIndexPath +// }) + changeset.elementsInserted = Set(changeset.elementsInserted.compactMap { insertedIndexPath in guard insertedIndexPath.section != removedGroup else { return nil } @@ -193,26 +230,12 @@ internal struct ChangesReducer { Element removals are handled before all other updates. */ indexPaths.sorted(by: { $0.item > $1.item }).forEach { removedIndexPath in - let sectionsRemovedBefore = changeset - .groupsRemoved - .sorted(by: <) - .enumerated() - .map { element in - element.element - element.offset - } - .filter { $0 <= removedIndexPath.section } - .count - - let sectionsInsertedBefore = changeset.groupsInserted.filter { $0 <= removedIndexPath.section }.count - - var removedIndexPath = removedIndexPath - removedIndexPath.section += sectionsRemovedBefore - removedIndexPath.section -= sectionsInsertedBefore + var removedIndexPath = transformIndexPath(removedIndexPath, toContext: .original) if !changeset.groupsInserted.contains(removedIndexPath.section) { let itemInsertsInSection = changeset .elementsInserted - .filter { $0.section == removedIndexPath.section - sectionsRemovedBefore + sectionsInsertedBefore } + .filter { $0.section == removedIndexPath.section } .map(\.item) if changeset.elementsRemoved.contains(removedIndexPath), changeset.elementsInserted.remove(removedIndexPath) != nil { @@ -261,9 +284,11 @@ internal struct ChangesReducer { internal mutating func updateElements(at indexPaths: [IndexPath]) { indexPaths.sorted(by: { $0.item > $1.item }).forEach { updatedElement in - // Reloads are decomposed in to a delete and insert - removeElements(at: [updatedElement]) - insertElements(at: [updatedElement]) + let updatedElement = transformIndexPath(updatedElement, toContext: .original) + + if !changeset.groupsInserted.contains(updatedElement.section) { + changeset.elementsUpdated.insert(updatedElement) + } } } @@ -274,4 +299,36 @@ internal struct ChangesReducer { internal mutating func moveElements(_ moves: [(from: IndexPath, to: IndexPath)]) { moveElements(moves.map { Changeset.Move(from: $0.from, to: $0.to) }) } + + private enum IndexPathContext { + /// Start of updates. + case original + + /// After deletes and reloads + case afterUpdates + } + + private func transformIndexPath(_ indexPath: IndexPath, toContext context: IndexPathContext) -> IndexPath { + var indexPath = indexPath + + switch context { + case .original: + indexPath.section = transformSection(indexPath.section) + case .afterUpdates: + break + } + + return indexPath + } + + private func transformSection(_ section: Int) -> Int { + let groupsRemoved = changeset.groupsRemoved + let groupsInserted = changeset.groupsInserted + let availableSpaces = (0.. = [] internal var elementsInserted: Set = [] internal var elementsMoved: Set = [] + internal var elementsUpdated: Set = [] } diff --git a/Tests/ComposedUITests/ChangesReducerTests.swift b/Tests/ComposedUITests/ChangesReducerTests.swift index 9809e88..ce271c9 100644 --- a/Tests/ComposedUITests/ChangesReducerTests.swift +++ b/Tests/ComposedUITests/ChangesReducerTests.swift @@ -477,18 +477,71 @@ final class ChangesReducerTests: XCTestCase { */ var changesReducer = ChangesReducer() changesReducer.beginUpdating() - changesReducer.removeGroups(IndexSet([0, 1])) - changesReducer.removeElements(at: [IndexPath(item: 1, section: 2)]) - changesReducer.removeGroups(IndexSet(integer: 0)) - let changeset = changesReducer.endUpdating() - XCTAssertNotNil(changeset) + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeGroups(IndexSet([0, 1])) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } - XCTAssertEqual(changeset!.groupsRemoved, [0, 1, 2]) - XCTAssertTrue(changeset!.groupsInserted.isEmpty) - XCTAssertEqual(changeset!.elementsRemoved, [IndexPath(row: 1, section: 4)]) - XCTAssertTrue(changeset!.elementsInserted.isEmpty) - XCTAssertTrue(changeset!.elementsMoved.isEmpty) + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.groupsRemoved, + [0, 1] + ) + }) + + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeElements(at: [IndexPath(item: 1, section: 1)]) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } + + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.groupsRemoved, + [0, 1] + ) + XCTAssertEqual( + changeset.elementsRemoved, + [IndexPath(row: 1, section: 3)] + ) + }) + + AssertApplyingUpdates( + { changesReducer in + changesReducer.removeGroups(IndexSet(integer: 0)) + }, + changesReducer: &changesReducer, + produces: { changeset in + guard let changeset = changeset else { + XCTFail("Changeset should not be `nil`") + return + } + + XCTAssertTrue(changeset.elementsInserted.isEmpty) + XCTAssertTrue(changeset.elementsMoved.isEmpty) + XCTAssertEqual( + changeset.groupsRemoved, + [0, 1, 2] + ) + XCTAssertEqual( + changeset.elementsRemoved, + [IndexPath(row: 1, section: 3)] + ) + }) } func testRemoveSectionThenRemoveElementThenRemoveSection() { diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift index 9c1a68d..ee1b2ae 100644 --- a/Tests/ComposedUITests/CollectionCoordinatorTests.swift +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -58,13 +58,17 @@ final class CollectionCoordinatorTests: XCTestCase { sections.child0.append("new-0") } - tester.applyUpdate { sections in + tester.applyUpdate({ sections in sections.child2[1] = "new-1" - } + }, postUpdateChecks: { sections in + XCTAssertEqual(Set(sections.child2.requestedCells), Set([0, 1, 3])) + }) - tester.applyUpdate { sections in + tester.applyUpdate({ sections in sections.child2[2] = "new-2" - } + }, postUpdateChecks: { sections in + XCTAssertEqual(Set(sections.child2.requestedCells), Set([0, 1, 2, 3])) + }) tester.applyUpdate { sections in sections.rootSectionProvider.remove(sections.child3) @@ -272,6 +276,33 @@ final class CollectionCoordinatorTests: XCTestCase { } } + func testGroupAndElementRemoves() { + // Mirror of `ChangesReducerTests.testGroupAndElementRemoves` + let tester = Tester() { sections in + sections.child0.removeAll() + sections.child1.removeAll() + sections.child2.removeAll() + + sections.rootSectionProvider.append(sections.child0) + sections.rootSectionProvider.append(sections.child1) + sections.rootSectionProvider.append(sections.child2) + sections.rootSectionProvider.append(sections.child3) + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child1) + sections.rootSectionProvider.remove(sections.child0) + } + + tester.applyUpdate { sections in + sections.child3.remove(at: 1) + } + + tester.applyUpdate { sections in + sections.rootSectionProvider.remove(sections.child2) + } + } + func testInsertInToSectionAfterInsertion() { let tester = Tester() { sections in sections.rootSectionProvider.append(sections.child0) From 2ed210387a7179ba5dc0da5f9d4020de7d7caa82 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Tue, 2 Feb 2021 21:06:06 +0000 Subject: [PATCH 11/12] Fixes for `removeGroups(_:)` --- .../CollectionCoordinator.swift | 10 +-- .../ComposedUI/Common/ChangesReducer.swift | 69 ++++--------------- .../ComposedUITests/ChangesReducerTests.swift | 9 +-- .../CollectionCoordinatorTests.swift | 8 +-- 4 files changed, 25 insertions(+), 71 deletions(-) diff --git a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift index b210329..967ddef 100644 --- a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift +++ b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift @@ -251,16 +251,16 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { collectionView.performBatchUpdates({ prepareSections() - debugLog("Deleting sections \(changeset.groupsRemoved)") + debugLog("Deleting sections \(changeset.groupsRemoved.sorted(by: >))") collectionView.deleteSections(IndexSet(changeset.groupsRemoved)) - debugLog("Deleting items \(changeset.elementsRemoved)") + debugLog("Deleting items \(changeset.elementsRemoved.sorted(by: >))") collectionView.deleteItems(at: Array(changeset.elementsRemoved)) - debugLog("Inserting items \(changeset.elementsInserted)") + debugLog("Inserting items \(changeset.elementsInserted.sorted(by: <))") collectionView.insertItems(at: Array(changeset.elementsInserted)) - debugLog("Reloading items \(changeset.elementsUpdated)") + debugLog("Reloading items \(changeset.elementsUpdated.sorted(by: <))") collectionView.reloadItems(at: Array(changeset.elementsUpdated)) changeset.elementsMoved.forEach { move in @@ -268,7 +268,7 @@ extension CollectionCoordinator: SectionProviderMappingDelegate { collectionView.moveItem(at: move.from, to: move.to) } - debugLog("Inserting sections \(changeset.groupsInserted)") + debugLog("Inserting sections \(changeset.groupsInserted.sorted(by: >))") collectionView.insertSections(IndexSet(changeset.groupsInserted)) }) } diff --git a/Sources/ComposedUI/Common/ChangesReducer.swift b/Sources/ComposedUI/Common/ChangesReducer.swift index a3eb2dd..1cb63a3 100644 --- a/Sources/ComposedUI/Common/ChangesReducer.swift +++ b/Sources/ComposedUI/Common/ChangesReducer.swift @@ -124,70 +124,31 @@ internal struct ChangesReducer { } internal mutating func removeGroups(_ groups: IndexSet) { - print(#function, groups) groups.sorted(by: >).forEach { removedGroup in - print("Removing", removedGroup) - print("groupsInserted", changeset.groupsInserted) - - /** - - */ -// changeset.groupsRemoved = Set(changeset.groupsRemoved -// .sorted(by: >) -// .map { existingRemovedGroup in -// existingRemovedGroup -// } -// .reduce(into: (previous: Int?.none, groupsRemoved: [Int]()), { (result, groupsRemoved) in -// if groupsRemoved == removedGroup { -// result.groupsRemoved.append(groupsRemoved) -// result.groupsRemoved.append(groupsRemoved + 1) -// result.previous = groupsRemoved + 1 -// } else if let previous = result.previous, groupsRemoved == previous { -// // TODO: Test this -// result.groupsRemoved.append(groupsRemoved + 1) -// result.previous = groupsRemoved + 1 -// } else { -// result.groupsRemoved.append(groupsRemoved) -// result.previous = groupsRemoved -// } -// }) -// .groupsRemoved -// ) - var removedGroup = removedGroup if changeset.groupsInserted.remove(removedGroup) != nil { - // TODO: Offset future indexes? + changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in + if insertedGroup > removedGroup { + return insertedGroup - 1 + } + + return insertedGroup + }) removedGroup = transformSection(removedGroup) } else { + changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in + if insertedGroup > removedGroup { + return insertedGroup - 1 + } + + return insertedGroup + }) removedGroup = transformSection(removedGroup) changeset.groupsRemoved.insert(removedGroup) } - -// if !changeset.groupsRemoved.contains(removedGroup) { - -// } - - changeset.groupsInserted = Set(changeset.groupsInserted.map { insertedGroup in - if insertedGroup > removedGroup { - return insertedGroup - 1 - } - - return insertedGroup - }) - -// changeset.elementsUpdated = Set(changeset.elementsUpdated.compactMap { updatedIndexPath in -// guard updatedIndexPath.section != removedGroup else { return nil } -// -// var updatedIndexPath = updatedIndexPath -// -// if updatedIndexPath.section > removedGroup { -// updatedIndexPath.section -= 1 -// } -// -// return updatedIndexPath -// }) + changeset.elementsUpdated = Set(changeset.elementsUpdated.filter { $0.section != removedGroup }) changeset.elementsInserted = Set(changeset.elementsInserted.compactMap { insertedIndexPath in guard insertedIndexPath.section != removedGroup else { return nil } diff --git a/Tests/ComposedUITests/ChangesReducerTests.swift b/Tests/ComposedUITests/ChangesReducerTests.swift index ce271c9..db8ebc2 100644 --- a/Tests/ComposedUITests/ChangesReducerTests.swift +++ b/Tests/ComposedUITests/ChangesReducerTests.swift @@ -450,19 +450,12 @@ final class ChangesReducerTests: XCTestCase { } XCTAssertEqual( - changeset.elementsRemoved, + changeset.elementsUpdated, [ IndexPath(item: 0, section: 2), IndexPath(item: 3, section: 2), ] ) - XCTAssertEqual( - changeset.elementsInserted, - [ - IndexPath(item: 0, section: 1), - IndexPath(item: 3, section: 1), - ] - ) XCTAssertEqual( changeset.groupsRemoved, [0] diff --git a/Tests/ComposedUITests/CollectionCoordinatorTests.swift b/Tests/ComposedUITests/CollectionCoordinatorTests.swift index ee1b2ae..65151e8 100644 --- a/Tests/ComposedUITests/CollectionCoordinatorTests.swift +++ b/Tests/ComposedUITests/CollectionCoordinatorTests.swift @@ -390,11 +390,11 @@ private final class TestSections { let rootSectionProvider = ComposedSectionProvider() let child0 = MockCollectionArraySection([]) - let child1 = MockCollectionArraySection(["1"]) + let child1 = MockCollectionArraySection([]) var child2 = MockCollectionArraySection(["0", "1", "2", "3"]) - let child3 = MockCollectionArraySection(["1", "2", "3"]) - let child4 = MockCollectionArraySection(["1", "2", "3", "4", "5"]) - var child5 = MockCollectionArraySection(["1", "2", "3", "4", "5", "6", "7", "8", "9"]) + let child3 = MockCollectionArraySection(["0", "1", "2"]) + let child4 = MockCollectionArraySection(["0"]) + var child5 = MockCollectionArraySection(["0", "1", "2", "3", "4", "5", "6", "7", "8"]) let child6 = MockCollectionArraySection([]) } From a7dc1cb0d0d719626d4b4c1b0ead5006f9918d92 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Mon, 8 Feb 2021 18:06:10 +0000 Subject: [PATCH 12/12] Update assertion message to ease debugging --- Sources/ComposedUI/CollectionView/CollectionCoordinator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift index 967ddef..db76d9d 100644 --- a/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift +++ b/Sources/ComposedUI/CollectionView/CollectionCoordinator.swift @@ -474,7 +474,7 @@ extension CollectionCoordinator: UICollectionViewDataSource { } else { guard let view = originalDataSource?.collectionView?(collectionView, viewForSupplementaryElementOfKind: kind, at: indexPath) else { // when in production its better to return 'something' to prevent crashing - assertionFailure("Unsupported supplementary kind: \(kind) at indexPath: \(indexPath). Did you forget to register your header or footer?") + assertionFailure("Unsupported supplementary kind: \(kind) at indexPath: \(indexPath). Check if your layout it returning attributes for the supplementary element at \(indexPath)") return collectionView.dequeue(supplementary: PlaceholderSupplementaryView.self, ofKind: PlaceholderSupplementaryView.kind, for: indexPath) }