[CIS-807] Add controllerWillChangeChannels delegate callback

CHANGELOG.md

@@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 # Upcoming
 
+### ✅ Added
+- `ChatChannelListControllerDelegate` now has the `controllerWillChangeChannels` method [#1024](https://github.com/GetStream/stream-chat-swift/pull/1024)
+
 ### 🔄 Changed
 - Split `UIConfig` into `Appearance` and `Components` to improve clarity [#1014](https://github.com/GetStream/stream-chat-swift/pull/1014)
 

Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift

@@ -88,7 +88,13 @@ public class _ChatChannelListController<ExtraData: ExtraDataTypes>: DataControll
                 $0.controller(self, didChangeChannels: changes)
             }
         }
-
+
+        observer.onWillChange = { [unowned self] in
+            self.delegateCallback {
+                $0.controllerWillChangeChannels(self)
+            }
+        }
+
         return observer
     }()
 
@@ -220,6 +226,12 @@ extension _ChatChannelListController where ExtraData == NoExtraData {
 /// please use `_ChatChannelListControllerDelegate` instead.
 ///
 public protocol ChatChannelListControllerDelegate: DataControllerStateDelegate {
+    /// The controller will update the list of observed channels.
+    ///
+    /// - Parameter controller: The controller emitting the change callback.
+    ///
+    func controllerWillChangeChannels(_ controller: ChatChannelListController)
+
     /// The controller changed the list of observed channels.
     ///
     /// - Parameters:
@@ -233,6 +245,8 @@ public protocol ChatChannelListControllerDelegate: DataControllerStateDelegate {
 }
 
 public extension ChatChannelListControllerDelegate {
+    func controllerWillChangeChannels(_ controller: ChatChannelListController) {}
+
     func controller(
         _ controller: _ChatChannelListController<NoExtraData>,
         didChangeChannels changes: [ListChange<ChatChannel>]
@@ -246,7 +260,13 @@ public extension ChatChannelListControllerDelegate {
 ///
 public protocol _ChatChannelListControllerDelegate: DataControllerStateDelegate {
     associatedtype ExtraData: ExtraDataTypes
-
+
+    /// The controller will update the list of observed channels.
+    ///
+    /// - Parameter controller: The controller emitting the change callback.
+    ///
+    func controllerWillChangeChannels(_ controller: _ChatChannelListController<ExtraData>)
+
     /// The controller changed the list of observed channels.
     ///
     /// - Parameters:
@@ -260,6 +280,8 @@ public protocol _ChatChannelListControllerDelegate: DataControllerStateDelegate
 }
 
 public extension _ChatChannelListControllerDelegate {
+    func controllerWillChangeChannels(_ controller: _ChatChannelListController<ExtraData>) {}
+
     func controller(
         _ controller: _ChatChannelListController<ExtraData>,
         didChangeChannels changes: [ListChange<_ChatChannel<ExtraData>>]
@@ -275,6 +297,7 @@ extension ClientError {
 // MARK: - Delegate type eraser
 
 class AnyChannelListControllerDelegate<ExtraData: ExtraDataTypes>: _ChatChannelListControllerDelegate {
+    private var _controllerWillChangeChannels: (_ChatChannelListController<ExtraData>) -> Void
     private var _controllerDidChangeChannels: (_ChatChannelListController<ExtraData>, [ListChange<_ChatChannel<ExtraData>>])
         -> Void
     private var _controllerDidChangeState: (DataController, DataController.State) -> Void
@@ -284,18 +307,24 @@ class AnyChannelListControllerDelegate<ExtraData: ExtraDataTypes>: _ChatChannelL
     init(
         wrappedDelegate: AnyObject?,
         controllerDidChangeState: @escaping (DataController, DataController.State) -> Void,
+        controllerWillChangeChannels: @escaping (_ChatChannelListController<ExtraData>) -> Void,
         controllerDidChangeChannels: @escaping (_ChatChannelListController<ExtraData>, [ListChange<_ChatChannel<ExtraData>>])
             -> Void
     ) {
         self.wrappedDelegate = wrappedDelegate
         _controllerDidChangeState = controllerDidChangeState
+        _controllerWillChangeChannels = controllerWillChangeChannels
         _controllerDidChangeChannels = controllerDidChangeChannels
     }
 
     func controller(_ controller: DataController, didChangeState state: DataController.State) {
         _controllerDidChangeState(controller, state)
     }
 
+    func controllerWillChangeChannels(_ controller: _ChatChannelListController<ExtraData>) {
+        _controllerWillChangeChannels(controller)
+    }
+
     func controller(
         _ controller: _ChatChannelListController<ExtraData>,
         didChangeChannels changes: [ListChange<_ChatChannel<ExtraData>>]
@@ -309,6 +338,7 @@ extension AnyChannelListControllerDelegate {
         self.init(
             wrappedDelegate: delegate,
             controllerDidChangeState: { [weak delegate] in delegate?.controller($0, didChangeState: $1) },
+            controllerWillChangeChannels: { [weak delegate] in delegate?.controllerWillChangeChannels($0) },
             controllerDidChangeChannels: { [weak delegate] in delegate?.controller($0, didChangeChannels: $1) }
         )
     }
@@ -319,6 +349,7 @@ extension AnyChannelListControllerDelegate where ExtraData == NoExtraData {
         self.init(
             wrappedDelegate: delegate,
             controllerDidChangeState: { [weak delegate] in delegate?.controller($0, didChangeState: $1) },
+            controllerWillChangeChannels: { [weak delegate] in delegate?.controllerWillChangeChannels($0) },
             controllerDidChangeChannels: { [weak delegate] in delegate?.controller($0, didChangeChannels: $1) }
         )
     }

Sources/StreamChat/Controllers/ChannelListController/ChannelListController_Tests.swift

@@ -283,7 +283,10 @@ class ChannelListController_Tests: StressTestCase {
 
         let channel: ChatChannel = client.databaseContainer.viewContext.channel(cid: cid)!.asModel()
 
-        AssertAsync.willBeEqual(delegate.didChangeChannels_changes, [.insert(channel, index: [0, 0])])
+        AssertAsync {
+            Assert.willBeTrue(delegate.willChangeChannels_called)
+            Assert.willBeEqual(delegate.didChangeChannels_changes, [.insert(channel, index: [0, 0])])
+        }
     }
 
     func test_genericDelegateMethodsAreCalled() throws {
@@ -298,8 +301,58 @@ class ChannelListController_Tests: StressTestCase {
         }
 
         let channel: ChatChannel = client.databaseContainer.viewContext.channel(cid: cid)!.asModel()
-
-        AssertAsync.willBeEqual(delegate.didChangeChannels_changes, [.insert(channel, index: [0, 0])])
+
+        AssertAsync {
+            Assert.willBeTrue(delegate.willChangeChannels_called)
+            Assert.willBeEqual(delegate.didChangeChannels_changes, [.insert(channel, index: [0, 0])])
+        }
+    }
+
+    func test_willAndDidCallbacks_areCalledInCorrectOrder() throws {
+        class Delegate: ChatChannelListControllerDelegate {
+            let cid: ChannelId
+
+            var willChangeCallbackCalled = false
+            var didChangeCallbackCalled = false
+
+            init(cid: ChannelId) {
+                self.cid = cid
+            }
+
+            func controllerWillChangeChannels(_ controller: ChatChannelListController) {
+                // Check the new channel is NOT in reported channels yet
+                XCTAssertFalse(controller.channels.contains { $0.cid == cid })
+                // Assert the "did" callback hasn't been called yet
+                XCTAssertFalse(didChangeCallbackCalled)
+                willChangeCallbackCalled = true
+            }
+
+            func controller(
+                _ controller: ChatChannelListController,
+                didChangeChannels changes: [ListChange<ChatChannel>]
+            ) {
+                // Check the new channel is in reported channels
+                XCTAssertTrue(controller.channels.contains { $0.cid == cid })
+                // Assert the "will" callback has been called
+                XCTAssertTrue(willChangeCallbackCalled)
+                didChangeCallbackCalled = true
+            }
+        }
+
+        let cid: ChannelId = .unique
+        let delegate = Delegate(cid: cid)
+
+        controller.callbackQueue = .main
+        controller.delegate = delegate
+
+        client.databaseContainer.write { session in
+            try session.saveChannel(payload: self.dummyPayload(with: cid), query: self.query)
+        }
+
+        AssertAsync {
+            Assert.willBeTrue(delegate.willChangeCallbackCalled)
+            Assert.willBeTrue(delegate.didChangeCallbackCalled)
+        }
     }
 
     // MARK: - Channels pagination
@@ -420,13 +473,19 @@ private class TestEnvironment {
 // A concrete `ChannelListControllerDelegate` implementation allowing capturing the delegate calls
 private class TestDelegate: QueueAwareDelegate, ChatChannelListControllerDelegate {
     @Atomic var state: DataController.State?
+    @Atomic var willChangeChannels_called = false
     @Atomic var didChangeChannels_changes: [ListChange<ChatChannel>]?
-    
+
     func controller(_ controller: DataController, didChangeState state: DataController.State) {
         self.state = state
         validateQueue()
     }
-
+
+    func controllerWillChangeChannels(_ controller: ChatChannelListController) {
+        willChangeChannels_called = true
+        validateQueue()
+    }
+
     func controller(
         _ controller: _ChatChannelListController<NoExtraData>,
         didChangeChannels changes: [ListChange<ChatChannel>]
@@ -439,13 +498,19 @@ private class TestDelegate: QueueAwareDelegate, ChatChannelListControllerDelegat
 // A concrete `_ChatChannelListControllerDelegate` implementation allowing capturing the delegate calls.
 private class TestDelegateGeneric: QueueAwareDelegate, _ChatChannelListControllerDelegate {
     @Atomic var state: DataController.State?
+    @Atomic var willChangeChannels_called = false
     @Atomic var didChangeChannels_changes: [ListChange<ChatChannel>]?
-    
+
     func controller(_ controller: DataController, didChangeState state: DataController.State) {
         self.state = state
         validateQueue()
     }
-
+
+    func controllerWillChangeChannels(_ controller: _ChatChannelListController<NoExtraData>) {
+        willChangeChannels_called = true
+        validateQueue()
+    }
+
     func controller(
         _ controller: _ChatChannelListController<NoExtraData>,
         didChangeChannels changes: [ListChange<ChatChannel>]

Sources/StreamChat/Controllers/Controller.swift

@@ -16,11 +16,20 @@ public protocol Controller {
 extension Controller {
     /// A helper function to ensure the callback is performed on the callback queue.
     func callback(_ action: @escaping () -> Void) {
-        if callbackQueue == .main, Thread.current.isMainThread {
-            // Perform the action on the main queue synchronously
-            action()
+        if Thread.current.isMainThread {
+            if callbackQueue == .main {
+                // We perform the callback synchronously
+                action()
+            } else {
+                // We dispatch from the main queue, we must perform
+                // the callback must be performed asynchronously
+                callbackQueue.async {
+                    action()
+                }
+            }
         } else {
-            callbackQueue.async {
+            // Dispatching from a background queue, the callback can be performed synchronously
+            callbackQueue.sync {
                 action()
             }
         }

Sources/StreamChat/Controllers/EntityDatabaseObserver.swift

@@ -261,7 +261,7 @@ extension EntityDatabaseObserver {
 
 private extension ListChangeAggregator {
     func onChange(do action: @escaping ([ListChange<Item>]) -> Void) -> ListChangeAggregator {
-        onChange = action
+        onDidChange = action
         return self
     }
 }

Sources/StreamChat/Controllers/ListDatabaseObserver.swift

@@ -76,12 +76,25 @@ class ListDatabaseObserver<Item, DTO: NSManagedObject> {
     /// The current collection of items matching the provided fetch request. To receive granular updates to this collection,
     /// you can use the `onChange` callback.
     @Cached var items: LazyCachedMapCollection<Item>
-
+
+    /// Called with the aggregated changes after the internal `NSFetchResultsController` calls `controllerWillChangeContent`
+    /// on its delegate.
+    var onWillChange: (() -> Void)? {
+        didSet {
+            changeAggregator.onWillChange = { [weak self] in
+                // Ideally, this should rather be `unowned`, however, `deinit` is not always called on the same thread as this
+                // callback which can cause a race condition when the object is already being deinited on a different thread.
+                guard let self = self else { return }
+                self.onWillChange?()
+            }
+        }
+    }
+
     /// Called with the aggregated changes after the internal `NSFetchResultsController` calls `controllerDidChangeContent`
     /// on its delegate.
     var onChange: (([ListChange<Item>]) -> Void)? {
         didSet {
-            changeAggregator.onChange = { [weak self] in
+            changeAggregator.onDidChange = { [weak self] in
                 // Ideally, this should rather be `unowned`, however, `deinit` is not always called on the same thread as this
                 // callback which can cause a race condition when the object is already being deinited on a different thread.
                 guard let self = self else { return }
@@ -156,7 +169,7 @@ class ListDatabaseObserver<Item, DTO: NSManagedObject> {
         _items.reset()
 
         // This is a workaround for the situation when someone wants to observe only the `items` array without
-        // listening to changes. We just need to make sure the `didSet` callback of `onChange` is executed at least once.
+        // listening to changes. We just need to make sure the `didSet` callback of `onDidChange` is executed at least once.
         if onChange == nil {
             onChange = nil
         }
@@ -221,15 +234,19 @@ class ListDatabaseObserver<Item, DTO: NSManagedObject> {
 }
 
 /// When this object is set as `NSFetchedResultsControllerDelegate`, it aggregates the callbacks from the fetched results
-/// controller and forwards them in the way of `[Change<Item>]`. You can set the `onChange` callback to receive these updates.
+/// controller and forwards them in the way of `[Change<Item>]`. You can set the `onDidChange` callback to receive these updates.
 class ListChangeAggregator<DTO: NSManagedObject, Item>: NSObject, NSFetchedResultsControllerDelegate {
     // TODO: Extend this to also provide `CollectionDifference` and `NSDiffableDataSourceSnapshot`
 
     /// Used for converting the `DTO`s provided by `FetchResultsController` to the resulting `Item`.
     let itemCreator: (DTO) -> Item?
-
+
+    /// Called when the aggregator is about to change the current content. It gets called when the `FetchedResultsController`
+    /// calls `controllerWillChangeContent` on its delegate.
+    var onWillChange: (() -> Void)?
+
     /// Called with the aggregated changes after `FetchResultsController` calls controllerDidChangeContent` on its delegate.
-    var onChange: (([ListChange<Item>]) -> Void)?
+    var onDidChange: (([ListChange<Item>]) -> Void)?
 
     /// An array of changes in the current update. It gets reset every time `controllerWillChangeContent` is called, and
     /// published to the observer when `controllerDidChangeContent` is called.
@@ -248,6 +265,7 @@ class ListChangeAggregator<DTO: NSManagedObject, Item>: NSObject, NSFetchedResul
     // This should ideally be in the extensions but it's not possible to implement @objc methods in extensions of generic types.
 
     func controllerWillChangeContent(_ controller: NSFetchedResultsController<NSFetchRequestResult>) {
+        onWillChange?()
         currentChanges = []
     }
 
@@ -315,7 +333,7 @@ class ListChangeAggregator<DTO: NSManagedObject, Item>: NSObject, NSFetchedResul
             return true
         }
 
-        onChange?(currentChanges)
+        onDidChange?(currentChanges)
     }
 }