Revert ListChangeAggregator newIndexPath on Updates

CHANGELOG.md

@@ -17,7 +17,6 @@ _June 06, 2022_
 - Fix not waiting for last batch of events to be processed when connecting as another user [#2016](https://github.com/GetStream/stream-chat-swift/issues/2016)
 - Fix `Date._unconditionallyBridgeFromObjectiveC(NSDate?)` crash [#2027](https://github.com/GetStream/stream-chat-swift/pull/2027)
 - Fix `NSHashTable` count underflow crash [#2032](https://github.com/GetStream/stream-chat-swift/pull/2032)
-- Fix Message List using incorrect indexPath for message updates [#2044](https://github.com/GetStream/stream-chat-swift/pull/2044)
 - Fix crash when participant hard deletes a message [2075](https://github.com/GetStream/stream-chat-swift/pull/2075)
 ### 🔄 Changed
 - Changing the decoding of `role` to `channel_role` as `role` is now deprecated on the backend. This allows for custom roles defined within your V2 permissions [#2028](https://github.com/GetStream/stream-chat-swift/issues/2028)
@@ -29,7 +28,7 @@ _June 06, 2022_
 - Show channel screen as right detail when channel list is embedded by split view controller [#2011](https://github.com/GetStream/stream-chat-swift/pull/2011)
 ### 🐞 Fixed
 - Fix DM Channel with multiple members displaying only 1 user avatar [#2019](https://github.com/GetStream/stream-chat-swift/pull/2019)
-- Improve stability of Message List with Diffing disabled [#2006](https://github.com/GetStream/stream-chat-swift/pull/2006)
+- Improve stability of Message List with Diffing disabled [#2006](https://github.com/GetStream/stream-chat-swift/pull/2006) [#2076](https://github.com/GetStream/stream-chat-swift/pull/2076)
 - Fix quoted message extra spacing jump UI glitch [#2050](https://github.com/GetStream/stream-chat-swift/pull/2050)
 - Fix edge case where cell would be hidden after reacting to it [#2053](https://github.com/GetStream/stream-chat-swift/pull/2053)
 

Sources/StreamChat/Controllers/ListDatabaseObserver.swift

@@ -340,7 +340,7 @@ class ListChangeAggregator<DTO: NSManagedObject, Item>: NSObject, NSFetchedResul
             currentChanges.append(.move(item, fromIndex: fromIndex, toIndex: toIndex))
 
         case .update:
-            guard let index = newIndexPath else {
+            guard let index = indexPath else {
                 log.warning("Skipping the update from DB because `indexPath` is missing for `.update` change.")
                 return
             }

Sources/StreamChat/WebSocketClient/Events/Event.swift

@@ -7,6 +7,12 @@ import Foundation
 /// An `Event` object representing an event in the chat system.
 public protocol Event {}
 
+extension Event {
+    var name: String {
+        String(describing: Self.self).replacingOccurrences(of: "DTO", with: "")
+    }
+}
+
 /// An internal protocol marking the Events carrying the payload. This payload can be then used for additional work,
 /// i.e. for storing the data to the database.
 protocol EventDTO: Event {
@@ -23,10 +29,6 @@ protocol EventDTO: Event {
 
 extension EventDTO {
     func toDomainEvent(session: DatabaseSession) -> Event? { nil }
-
-    var name: String {
-        String(describing: Self.self).replacingOccurrences(of: "DTO", with: "")
-    }
 }
 
 /// A protocol for any `ChannelEvent` where it has a  `channel` payload.

Sources/StreamChat/Workers/EventNotificationCenter.swift

@@ -31,10 +31,10 @@ class EventNotificationCenter: NotificationCenter {
 
     func process(_ events: [Event], postNotifications: Bool = true, completion: (() -> Void)? = nil) {
         let processingEventsDebugMessage: () -> String = {
-            let eventNames = events.compactMap { ($0 as? EventDTO)?.name }
+            let eventNames = events.map(\.name)
             return "Processing Events: \(eventNames)"
         }
-        log.debug(processingEventsDebugMessage, subsystems: .webSocket)
+        log.debug(processingEventsDebugMessage(), subsystems: .webSocket)
 
         var eventsToPost = [Event]()
         database.write({ session in

Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift

@@ -245,22 +245,11 @@ open class ChatMessageListVC: _ViewController,
             // Because we use an inverted table view, we need to avoid animations since they look odd.
             UIView.performWithoutAnimation {
                 listView.updateMessages(with: changes) { [weak self] in
-                    guard let self = self else {
-                        completion?()
-                        return
-                    }
-
                     if let newMessageInserted = changes.first(where: { $0.isInsertion && $0.indexPath.row == 0 })?.item {
-                        if self.dataSource?.numberOfMessages(in: self) ?? 0 > 1 {
-                            let previousMessageIndexPath = IndexPath(row: 1, section: 0)
-                            self.listView.reloadRows(at: [previousMessageIndexPath], with: .none)
-                        }
-
                         if newMessageInserted.isSentByCurrentUser {
-                            self.listView.scrollToMostRecentMessage()
+                            self?.listView.scrollToMostRecentMessage()
                         }
                     }
-
                     completion?()
                 }
             }

Sources/StreamChatUI/Utils/ListChangeUpdater/CollectionViewListChangeUpdater.swift

@@ -22,39 +22,23 @@ final class CollectionViewListChangeUpdater: ListChangeUpdater {
     ///   - changes: The provided changes reported by a list controller.
     ///   - completion: A callback when the changes are fully executed.
     func performUpdate<Item>(with changes: [ListChange<Item>], completion: ((_ finished: Bool) -> Void)? = nil) {
-        guard let indices = listChangeIndexPathResolver.mapToSetsOfIndexPaths(
+        guard let indices = listChangeIndexPathResolver.resolve(
             changes: changes
         ) else {
             collectionView?.reloadData()
             completion?(true)
             return
         }
 
-        if #available(iOS 15, *) {
-            collectionView?.performBatchUpdates({
-                collectionView?.deleteItems(at: Array(indices.remove))
-                collectionView?.insertItems(at: Array(indices.insert))
-                collectionView?.reloadItems(at: Array(indices.update))
-                indices.move.forEach {
-                    collectionView?.moveItem(at: $0.fromIndex, to: $0.toIndex)
-                }
-            }, completion: { finished in
-                completion?(finished)
-            })
-        } else {
-            // To fix a crash on iOS 14 below, we moved the reloads to the completion block.
-            collectionView?.performBatchUpdates({
-                collectionView?.deleteItems(at: Array(indices.remove))
-                collectionView?.insertItems(at: Array(indices.insert))
-                indices.move.forEach {
-                    collectionView?.moveItem(at: $0.fromIndex, to: $0.toIndex)
-                }
-            }, completion: { [weak self] finished in
-                UIView.performWithoutAnimation {
-                    self?.collectionView?.reloadItems(at: Array(indices.update))
-                    completion?(finished)
-                }
-            })
-        }
+        collectionView?.performBatchUpdates({
+            collectionView?.deleteItems(at: Array(indices.remove))
+            collectionView?.insertItems(at: Array(indices.insert))
+            collectionView?.reloadItems(at: Array(indices.update))
+            indices.move.forEach {
+                collectionView?.moveItem(at: $0.fromIndex, to: $0.toIndex)
+            }
+        }, completion: { finished in
+            completion?(finished)
+        })
     }
 }

Sources/StreamChatUI/Utils/ListChangeUpdater/ListChangeIndexPathResolver.swift

@@ -33,52 +33,63 @@ final class ListChangeIndexPathResolver {
     /// - Parameters:
     ///   - changes: changes
     /// - Returns: Returns the indices mapped to sets. Returns nil if there were conflicts.
-    func mapToSetsOfIndexPaths<Item>(
+    func resolve<Item>(
         changes: [ListChange<Item>]
     ) -> Indexes? {
-        var allIndexes = Set<IndexPath>()
+        var moveFromIndexes = Set<IndexPath>()
+        var moveToIndexes = Set<IndexPath>()
         var moveIndexes = Set<IndexPathMove>()
         var insertIndexes = Set<IndexPath>()
         var removeIndexes = Set<IndexPath>()
         var updateIndexes = Set<IndexPath>()
 
-        var hasConflicts = false
-        let verifyConflict = { (indexPath: IndexPath) in
-            let (inserted, _) = allIndexes.insert(indexPath)
-            hasConflicts = !inserted || hasConflicts
-        }
-
         for change in changes {
-            if hasConflicts {
-                break
-            }
-
             switch change {
             case let .insert(_, index):
-                verifyConflict(index)
                 insertIndexes.insert(index)
             case let .move(_, fromIndex, toIndex):
-                verifyConflict(fromIndex)
-                verifyConflict(toIndex)
                 moveIndexes.insert(IndexPathMove(fromIndex, toIndex))
+                moveFromIndexes.insert(fromIndex)
+                moveToIndexes.insert(toIndex)
             case let .remove(_, index):
-                verifyConflict(index)
                 removeIndexes.insert(index)
             case let .update(_, index):
-                verifyConflict(index)
                 updateIndexes.insert(index)
             }
         }
-
-        if hasConflicts {
-            return nil
-        }
-
-        return (
+
+        let indexes = Indexes(
             move: moveIndexes,
             insert: insertIndexes,
             remove: removeIndexes,
             update: updateIndexes
         )
+
+        // Check if there are conflicts between Inserts<->Moves<->Deletes or Updates<->Moves<->Deletes.
+        // We don't check for conflicts between Inserts<->Updates, since it is not a conflict.
+        let movesAndRemoves = [moveFromIndexes, moveToIndexes, removeIndexes]
+        let hasInsertConflicts = insertIndexes.containsDuplicates(between: movesAndRemoves)
+        let hasUpdateConflicts = updateIndexes.containsDuplicates(between: movesAndRemoves)
+        let hasConflicts = hasInsertConflicts || hasUpdateConflicts
+        if hasConflicts {
+            log.warning("ListChange conflicts found: \(indexes)")
+            return nil
+        }
+
+        return indexes
+    }
+}
+
+private extension Set where Element == IndexPath {
+    // If the count of all elements before the sets are merged is different from the count of the elements
+    // after the sets are merged, then it means there are duplicates between the provided sets.
+    //
+    // Example of a conflict:
+    // ["1", "2", "3"] + ["4", "5", "6"] + ["1", "8", "9"] = 9
+    // ["1", "2", "3", "4", "5", "6", "8", "9"] = 8
+    func containsDuplicates(between sets: [Self]) -> Bool {
+        let allElements = sets.reduce(Array(self), +)
+        let mergedElements = Set(allElements)
+        return allElements.count != mergedElements.count
     }
 }

Sources/StreamChatUI/Utils/ListChangeUpdater/TableViewListChangeUpdater.swift

@@ -22,39 +22,23 @@ final class TableViewListChangeUpdater: ListChangeUpdater {
     ///   - changes: The provided changes reported by a list controller.
     ///   - completion: A callback when the changes are fully executed.
     func performUpdate<Item>(with changes: [ListChange<Item>], completion: ((_ finished: Bool) -> Void)? = nil) {
-        guard let indices = listChangeIndexPathResolver.mapToSetsOfIndexPaths(
+        guard let indices = listChangeIndexPathResolver.resolve(
             changes: changes
         ) else {
             tableView?.reloadData()
             completion?(true)
             return
         }
 
-        if #available(iOS 15, *) {
-            tableView?.performBatchUpdates({
-                tableView?.deleteRows(at: Array(indices.remove), with: .none)
-                tableView?.insertRows(at: Array(indices.insert), with: .none)
-                tableView?.reloadRows(at: Array(indices.update), with: .none)
-                indices.move.forEach {
-                    tableView?.moveRow(at: $0.fromIndex, to: $0.toIndex)
-                }
-            }, completion: { finished in
-                completion?(finished)
-            })
-        } else {
-            // To fix a crash on iOS 14 below, we moved the reloads to the completion block.
-            tableView?.performBatchUpdates({
-                tableView?.deleteRows(at: Array(indices.remove), with: .none)
-                tableView?.insertRows(at: Array(indices.insert), with: .none)
-                indices.move.forEach {
-                    tableView?.moveRow(at: $0.fromIndex, to: $0.toIndex)
-                }
-            }, completion: { [weak self] finished in
-                UIView.performWithoutAnimation {
-                    self?.tableView?.reloadRows(at: Array(indices.update), with: .none)
-                    completion?(finished)
-                }
-            })
-        }
+        tableView?.performBatchUpdates({
+            tableView?.deleteRows(at: Array(indices.remove), with: .none)
+            tableView?.insertRows(at: Array(indices.insert), with: .none)
+            tableView?.reloadRows(at: Array(indices.update), with: .none)
+            indices.move.forEach {
+                tableView?.moveRow(at: $0.fromIndex, to: $0.toIndex)
+            }
+        }, completion: { finished in
+            completion?(finished)
+        })
     }
 }

Tests/StreamChatTests/ListChangeAggregator_Tests.swift

@@ -128,17 +128,17 @@ final class ListChangeAggregator_Tests: XCTestCase {
         aggregator.controller(
             fakeController,
             didChange: updatedObject1,
-            at: [0, 0],
+            at: [1, 0],
             for: .update,
-            newIndexPath: [1, 0] // Should use newIndexPath
+            newIndexPath: nil
         )
 
         aggregator.controller(
             fakeController,
             didChange: updatedObject2,
-            at: [3, 0],
+            at: [4, 0],
             for: .update,
-            newIndexPath: [4, 0] // Should use newIndexPath
+            newIndexPath: nil
         )
 
         // Simulate FRC finishes updating
@@ -227,9 +227,9 @@ final class ListChangeAggregator_Tests: XCTestCase {
         aggregator.controller(
             fakeController,
             didChange: updatedObject,
-            at: [1, 0],
+            at: [2, 0],
             for: .update,
-            newIndexPath: [2, 0]
+            newIndexPath: nil
         )
 
         // Simulate FRC finishes updating

Tests/StreamChatUITests/SnapshotTests/ChatChannelList/ChatChannelListVC_Tests.swift

@@ -252,7 +252,7 @@ extension ChatChannelListVC_Tests {
 
         let hasConflictChanges: [ListChange<ChatChannel>] = [
             .update(.mock(cid: .unique), index: .init(row: 1, section: 0)),
-            .update(.mock(cid: .unique), index: .init(row: 2, section: 0)),
+            .remove(.mock(cid: .unique), index: .init(row: 2, section: 0)),
             .insert(.mock(cid: .unique), index: .init(row: 3, section: 0)),
             .insert(.mock(cid: .unique), index: .init(row: 2, section: 0))
         ]
@@ -275,7 +275,7 @@ extension ChatChannelListVC_Tests {
             .update(.mock(cid: .unique), index: .init(row: 1, section: 0)),
             .update(.mock(cid: .unique), index: .init(row: 2, section: 0)),
             .insert(.mock(cid: .unique), index: .init(row: 3, section: 0)),
-            .update(.mock(cid: .unique), index: .init(row: 3, section: 0))
+            .remove(.mock(cid: .unique), index: .init(row: 3, section: 0))
         ]
 
         mockedChannelListController.simulate(