Fix ChannelListController.synchronize not calling completion in offli…

…ne mode (#1353)
GetStream · Aug 11, 2021 · 120dd81 · 120dd81
1 parent 1be74e0
commit 120dd81
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 # Upcoming
 
+### 🐞 Fixed
+- Fix `ChannelListController.synchronize` completion closure not being called when the client is connected [#1353](https://github.com/GetStream/stream-chat-swift/issues/1353)
+
 ### 🔄 Changed
 - Fixed race condition on `ChatMessageListVC` and `ChatThreadVC` that caused `UITableView` crashes [#1347](https://github.com/GetStream/stream-chat-swift/pull/1347)
 - Added `GalleryAttachmentViewInjector.galleryViewAspectRatio` to control the aspect ratio of a gallery inside a message cell [#1300](https://github.com/GetStream/stream-chat-swift/pull/1300)

diff --git a/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift b/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift
@@ -107,27 +107,35 @@ public class ChatChannelListController: DataController, DelegateCallable, DataSt
 
     private func setupEventObserversIfNeeded(completion: ((_ error: Error?) -> Void)? = nil) {
         guard !client.config.isLocalStorageEnabled else {
-            return updateChannels(trumpExistingChannels: false, completion)
+            return updateChannelList(trumpExistingChannels: false, completion)
         }
-        connectionObserver = nil
-        // We can't setup event observers in connectionless mode
-        guard let webSocketClient = client.webSocketClient else { return }
-        let center = webSocketClient.eventNotificationCenter
-        connectionObserver = EventObserver(
-            notificationCenter: center,
-            transform: { $0 as? ConnectionStatusUpdated },
-            callback: { [unowned self] in
-                switch $0.webSocketConnectionState {
-                case .connected:
-                    self.updateChannels(trumpExistingChannels: channels.count > requestedChannelsLimit)
-                default:
-                    break
+
+        updateChannelList(trumpExistingChannels: channels.count > requestedChannelsLimit) { [weak self] error in
+            completion?(error)
+
+            guard let self = self else { return }
+            self.connectionObserver = nil
+            // We can't setup event observers in connectionless mode
+            guard let webSocketClient = self.client.webSocketClient else { return }
+            let center = webSocketClient.eventNotificationCenter
+            // We setup a `Connected` Event observer so every time we're connected,
+            // we refresh the channel list
+            self.connectionObserver = EventObserver(
+                notificationCenter: center,
+                transform: { $0 as? ConnectionStatusUpdated },
+                callback: { [unowned self] in
+                    switch $0.webSocketConnectionState {
+                    case .connected:
+                        self.updateChannelList(trumpExistingChannels: self.channels.count > self.requestedChannelsLimit)
+                    default:
+                        break
+                    }
                 }
-            }
-        )
+            )
+        }
     }
 
-    private func updateChannels(
+    private func updateChannelList(
         trumpExistingChannels: Bool,
         _ completion: ((_ error: Error?) -> Void)? = nil
     ) {

diff --git a/Sources/StreamChat/Controllers/ChannelListController/ChannelListController_Tests.swift b/Sources/StreamChat/Controllers/ChannelListController/ChannelListController_Tests.swift
@@ -22,8 +22,12 @@ class ChannelListController_Tests: StressTestCase {
     override func setUp() {
         super.setUp()
 
+        setUp(isLocalStorageEnabled: true)
+    }
+
+    private func setUp(isLocalStorageEnabled: Bool) {
         env = TestEnvironment()
-        client = ChatClient.mock(isLocalStorageEnabled: true)
+        client = ChatClient.mock(isLocalStorageEnabled: isLocalStorageEnabled)
         query = .init(filter: .in(.members, values: [.unique]))
         controller = ChatChannelListController(query: query, client: client, environment: env.environment)
         controllerCallbackQueueID = UUID()
@@ -160,6 +164,43 @@ class ChannelListController_Tests: StressTestCase {
         AssertAsync.canBeReleased(&weakController)
     }
 
+    func test_synchronize_callsChannelQueryUpdater_inOfflineMode() {
+        setUp(isLocalStorageEnabled: false)
+
+        let queueId = UUID()
+        controller.callbackQueue = .testQueue(withId: queueId)
+
+        // Simulate `synchronize` calls and catch the completion
+        var completionCalled = false
+        controller.synchronize { error in
+            XCTAssertNil(error)
+            AssertTestQueue(withId: queueId)
+            completionCalled = true
+        }
+
+        // Keep a weak ref so we can check if it's actually deallocated
+        weak var weakController = controller
+
+        // (Try to) deallocate the controller
+        // by not keeping any references to it
+        controller = nil
+
+        // Assert the updater is called with the query
+        XCTAssertEqual(env.channelListUpdater?.update_queries.first?.filter.filterHash, query.filter.filterHash)
+        // Completion shouldn't be called yet
+        XCTAssertFalse(completionCalled)
+
+        // Simulate successful update
+        env.channelListUpdater!.update_completion?(nil)
+        // Release reference of completion so we can deallocate stuff
+        env.channelListUpdater!.update_completion = nil
+
+        // Completion should be called
+        AssertAsync.willBeTrue(completionCalled)
+        // `weakController` should be deallocated too
+        AssertAsync.canBeReleased(&weakController)
+    }
+
     func test_synchronize_propagatesErrorFromUpdater() {
         let queueId = UUID()
         controller.callbackQueue = .testQueue(withId: queueId)