From 120dd81a35c0812d49e7a7b39880e308b47f3d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bahad=C4=B1r=20=C3=96ncel?= Date: Wed, 11 Aug 2021 11:16:37 +0200 Subject: [PATCH] Fix ChannelListController.synchronize not calling completion in offline mode (#1353) --- CHANGELOG.md | 3 ++ .../ChannelListController.swift | 42 ++++++++++-------- .../ChannelListController_Tests.swift | 43 ++++++++++++++++++- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cace11f7a0..9490310b74b 100644 --- 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 index f25d014fa81..268195ab5f4 100644 --- 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 index 92484991f17..67a3894a9a6 100644 --- 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)