From 6a2a39bf99bc1b2026597e9a58723040ab8957b9 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Thu, 2 Oct 2025 16:41:44 +0300 Subject: [PATCH 1/2] Remove controllerWillChangeChannels and cleanup willChange handling --- .../ChannelListController.swift | 20 -------- .../BackgroundDatabaseObserver.swift | 50 +------------------ .../DatabaseObserver/ListChange.swift | 5 -- .../ChatChannelList/ChatChannelListVC.swift | 4 -- .../ChannelListController_Delegate.swift | 6 --- .../ChannelListController_Tests.swift | 15 +----- .../ListChangeAggregator_Tests.swift | 10 ---- 7 files changed, 2 insertions(+), 108 deletions(-) diff --git a/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift b/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift index d181dd23710..e9b83e36fc2 100644 --- a/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift +++ b/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift @@ -86,18 +86,6 @@ public class ChatChannelListController: DataController, DelegateCallable, DataSt $0.controller(self, didChangeChannels: changes) } } - - observer.onWillChange = { [weak self] in - self?.delegateCallback { [weak self] in - guard let self = self else { - log.warning("Callback called while self is nil") - return - } - - $0.controllerWillChangeChannels(self) - } - } - return observer }() @@ -278,12 +266,6 @@ extension ChatChannelListController { /// `ChatChannelListController` uses this protocol to communicate changes to its delegate. 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: @@ -297,8 +279,6 @@ public protocol ChatChannelListControllerDelegate: DataControllerStateDelegate { } public extension ChatChannelListControllerDelegate { - func controllerWillChangeChannels(_ controller: ChatChannelListController) {} - func controller( _ controller: ChatChannelListController, didChangeChannels changes: [ListChange] diff --git a/Sources/StreamChat/Controllers/DatabaseObserver/BackgroundDatabaseObserver.swift b/Sources/StreamChat/Controllers/DatabaseObserver/BackgroundDatabaseObserver.swift index f5b6f844cd5..a3f60eee9e6 100644 --- a/Sources/StreamChat/Controllers/DatabaseObserver/BackgroundDatabaseObserver.swift +++ b/Sources/StreamChat/Controllers/DatabaseObserver/BackgroundDatabaseObserver.swift @@ -6,10 +6,6 @@ import CoreData import Foundation class BackgroundDatabaseObserver: @unchecked Sendable { - /// Called with the aggregated changes after the internal `NSFetchResultsController` calls `controllerWillChangeContent` - /// on its delegate. - var onWillChange: (@Sendable () -> Void)? - /// Called with the aggregated changes after the internal `NSFetchResultsController` calls `controllerDidChangeContent` /// on its delegate. var onDidChange: (@Sendable ([ListChange]) -> Void)? @@ -30,24 +26,10 @@ class BackgroundDatabaseObserver: @uncheck private let queue = DispatchQueue(label: "io.getstream.list-database-observer", qos: .userInitiated) private var _items: [Item]? - - // State handling for supporting will change, because in the callback we should return the previous state. - private var _willChangeItems: [Item]? - private var _notifyingWillChange = false /// The items that have been fetched and mapped var rawItems: [Item] { - // During the onWillChange we swap the results back to the previous state because onWillChange - // is dispatched to the main thread and when the main thread handles it, observer has already processed - // the database change. - if onWillChange != nil { - let willChangeState: (active: Bool, cachedItems: [Item]?) = queue.sync { (_notifyingWillChange, _willChangeItems) } - if willChangeState.active { - return willChangeState.cachedItems ?? [] - } - } - - var rawItems: [Item]! + nonisolated(unsafe) var rawItems: [Item]! frc.managedObjectContext.performAndWait { // When we already have loaded items, reuse them, otherwise refetch all rawItems = _items ?? updateItems(nil) @@ -98,9 +80,6 @@ class BackgroundDatabaseObserver: @uncheck sectionNameKeyPath: nil, cacheName: nil ) - changeAggregator.onWillChange = { [weak self] in - self?.notifyWillChange() - } changeAggregator.onDidChange = { [weak self] changes in guard let self else { return } // Runs on the NSManagedObjectContext's queue, therefore skip performAndWait @@ -133,33 +112,6 @@ class BackgroundDatabaseObserver: @uncheck } } - private func notifyWillChange() { - guard let onWillChange = onWillChange else { - return - } - // Will change callback happens on the main thread but by that time the observer - // has already updated its local cached state. For allowing to access the previous - // state from the will change callback, there is no other way than caching previous state. - // This is used by the channel list delegate. - - // `_items` is mutated by the NSManagedObjectContext's queue, here we are on that queue - // so it is safe to read the `_items` state from `self.queue`. - queue.sync { - _willChangeItems = _items - } - DispatchQueue.main.async { [weak self] in - guard let self else { return } - self.queue.async { - self._notifyingWillChange = true - } - onWillChange() - self.queue.async { - self._willChangeItems = nil - self._notifyingWillChange = false - } - } - } - private func notifyDidChange(changes: [ListChange]) { guard let onDidChange = onDidChange else { return diff --git a/Sources/StreamChat/Controllers/DatabaseObserver/ListChange.swift b/Sources/StreamChat/Controllers/DatabaseObserver/ListChange.swift index 93d4de21114..501e3fd1897 100644 --- a/Sources/StreamChat/Controllers/DatabaseObserver/ListChange.swift +++ b/Sources/StreamChat/Controllers/DatabaseObserver/ListChange.swift @@ -144,10 +144,6 @@ class ListChangeAggregator: NSObject, NSFetchedResul /// Used for converting the `DTO`s provided by `FetchResultsController` to the resulting `Item`. let itemCreator: (DTO) throws -> 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 onDidChange: (([ListChange]) -> Void)? @@ -167,7 +163,6 @@ class ListChangeAggregator: 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) { - onWillChange?() currentChanges = [] } diff --git a/Sources/StreamChatUI/ChatChannelList/ChatChannelListVC.swift b/Sources/StreamChatUI/ChatChannelList/ChatChannelListVC.swift index e7da1ab6cf3..f463028347b 100644 --- a/Sources/StreamChatUI/ChatChannelList/ChatChannelListVC.swift +++ b/Sources/StreamChatUI/ChatChannelList/ChatChannelListVC.swift @@ -403,10 +403,6 @@ open class ChatChannelListVC: _ViewController, // MARK: - ChatChannelListControllerDelegate - open func controllerWillChangeChannels(_ controller: ChatChannelListController) { - collectionView.layoutIfNeeded() - } - open func controller( _ controller: ChatChannelListController, didChangeChannels changes: [ListChange] diff --git a/TestTools/StreamChatTestTools/SpyPattern/QueueAware/ChannelListController_Delegate.swift b/TestTools/StreamChatTestTools/SpyPattern/QueueAware/ChannelListController_Delegate.swift index 9fb50f66a23..87184c3d7cc 100644 --- a/TestTools/StreamChatTestTools/SpyPattern/QueueAware/ChannelListController_Delegate.swift +++ b/TestTools/StreamChatTestTools/SpyPattern/QueueAware/ChannelListController_Delegate.swift @@ -8,7 +8,6 @@ import Foundation // A concrete `ChannelListControllerDelegate` implementation allowing capturing the delegate calls final class ChannelListController_Delegate: QueueAwareDelegate, ChatChannelListControllerDelegate { @Atomic var state: DataController.State? - @Atomic var willChangeChannels_called = false @Atomic var didChangeChannels_changes: [ListChange]? func controller(_ controller: DataController, didChangeState state: DataController.State) { @@ -16,11 +15,6 @@ final class ChannelListController_Delegate: QueueAwareDelegate, ChatChannelListC validateQueue() } - func controllerWillChangeChannels(_ controller: ChatChannelListController) { - willChangeChannels_called = true - validateQueue() - } - func controller( _ controller: ChatChannelListController, didChangeChannels changes: [ListChange] diff --git a/Tests/StreamChatTests/Controllers/ChannelListController/ChannelListController_Tests.swift b/Tests/StreamChatTests/Controllers/ChannelListController/ChannelListController_Tests.swift index e2318531ce3..2439df91721 100644 --- a/Tests/StreamChatTests/Controllers/ChannelListController/ChannelListController_Tests.swift +++ b/Tests/StreamChatTests/Controllers/ChannelListController/ChannelListController_Tests.swift @@ -651,38 +651,26 @@ final class ChannelListController_Tests: XCTestCase { let channel = try XCTUnwrap(client.databaseContainer.viewContext.channel(cid: cid)).asModel() AssertAsync { - Assert.willBeTrue(delegate.willChangeChannels_called) Assert.willBeEqual(delegate.didChangeChannels_changes, [.insert(channel, index: [0, 0])]) } } - @MainActor func test_willAndDidCallbacks_areCalledInCorrectOrder() throws { + @MainActor func test_didCallbacks_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] ) { // 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 } } @@ -699,7 +687,6 @@ final class ChannelListController_Tests: XCTestCase { } AssertAsync { - Assert.willBeTrue(delegate.willChangeCallbackCalled) Assert.willBeTrue(delegate.didChangeCallbackCalled) } } diff --git a/Tests/StreamChatTests/ListChangeAggregator_Tests.swift b/Tests/StreamChatTests/ListChangeAggregator_Tests.swift index 2018f938cc0..8ece32cb32b 100644 --- a/Tests/StreamChatTests/ListChangeAggregator_Tests.swift +++ b/Tests/StreamChatTests/ListChangeAggregator_Tests.swift @@ -29,16 +29,6 @@ final class ListChangeAggregator_Tests: XCTestCase { super.tearDown() } - func test_onWillChange_isCalled() { - // Set up aggregator callback - var callbackCalled = false - aggregator.onWillChange = { callbackCalled = true } - - // Simulate FRC starts updating - aggregator.controllerWillChangeContent(fakeController) - XCTAssertTrue(callbackCalled) - } - func test_addingItems() { // Set up aggregator callback var result: [ListChange]? From 6172299091fc167d7a25e4d39df8c842c3dd9afb Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Fri, 10 Oct 2025 13:28:00 +0300 Subject: [PATCH 2/2] Fix build error --- .../MessageController/MessageController_Tests.swift | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Tests/StreamChatTests/Controllers/MessageController/MessageController_Tests.swift b/Tests/StreamChatTests/Controllers/MessageController/MessageController_Tests.swift index 50ae105a996..a2b56dca639 100644 --- a/Tests/StreamChatTests/Controllers/MessageController/MessageController_Tests.swift +++ b/Tests/StreamChatTests/Controllers/MessageController/MessageController_Tests.swift @@ -905,9 +905,8 @@ final class MessageController_Tests: XCTestCase { func test_deleteMessageForMe_propagatesError() { // Simulate `deleteMessageForMe` call and catch the completion - var completionError: Error? - controller.deleteMessageForMe { [callbackQueueID] in - AssertTestQueue(withId: callbackQueueID) + nonisolated(unsafe) var completionError: Error? + controller.deleteMessageForMe { completionError = $0 } @@ -921,9 +920,8 @@ final class MessageController_Tests: XCTestCase { func test_deleteMessageForMe_propagatesNilError() { // Simulate `deleteMessageForMe` call and catch the completion - var completionCalled = false - controller.deleteMessageForMe { [callbackQueueID] in - AssertTestQueue(withId: callbackQueueID) + nonisolated(unsafe) var completionCalled = false + controller.deleteMessageForMe { XCTAssertNil($0) completionCalled = true }