Revert "Linking and unlinking channels based on the loaded list of ch…

…annels" (#3465) * Revert "Linking and unlinking channels based on the loaded list of channels (#3430)" This reverts commit f9cc737. * Update CHANGELOG.md
GetStream · Oct 17, 2024 · e2acb9d · e2acb9d
1 parent e92ffeb
commit e2acb9d
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 354 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Fix rare crash in `WebSocketPingController.connectionStateDidChange` [#3451](https://github.com/GetStream/stream-chat-swift/pull/3451)
 - Improve reliability and performance of resetting ephemeral values [#3439](https://github.com/GetStream/stream-chat-swift/pull/3439)
 - Reduce channel list updates when updating the local state [#3450](https://github.com/GetStream/stream-chat-swift/pull/3450)
+### 🔄 Changed
+- Reverts "Fix old channel updates not being added to the channel list automatically" [#3465](https://github.com/GetStream/stream-chat-swift/pull/3465)
+  - This was causing some issues on the SwiftUI SDK, so we are temporarily reverting this.
 
 ## StreamChatUI
 ### 🐞 Fixed

diff --git a/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift b/Sources/StreamChat/Controllers/ChannelListController/ChannelListController.swift
@@ -121,7 +121,7 @@ public class ChatChannelListController: DataController, DelegateCallable, DataSt
     private let environment: Environment
     private lazy var channelListLinker: ChannelListLinker = self.environment
         .channelListLinkerBuilder(
-            query, filter, { [weak self] in StreamCollection(self?.channels ?? []) }, client.config, client.databaseContainer, worker
+            query, filter, client.config, client.databaseContainer, worker
         )
 
     /// Creates a new `ChannelListController`.
@@ -269,7 +269,6 @@ extension ChatChannelListController {
         var channelListLinkerBuilder: (
             _ query: ChannelListQuery,
             _ filter: ((ChatChannel) -> Bool)?,
-            _ loadedChannels: @escaping () -> StreamCollection<ChatChannel>,
             _ clientConfig: ChatClientConfig,
             _ databaseContainer: DatabaseContainer,
             _ worker: ChannelListUpdater

diff --git a/Sources/StreamChat/StateLayer/ChannelListState+Observer.swift b/Sources/StreamChat/StateLayer/ChannelListState+Observer.swift
@@ -43,7 +43,6 @@ extension ChannelListState {
             channelListLinker = ChannelListLinker(
                 query: query,
                 filter: dynamicFilter,
-                loadedChannels: { [weak channelListObserver] in channelListObserver?.items ?? StreamCollection([]) },
                 clientConfig: clientConfig,
                 databaseContainer: database,
                 worker: channelListUpdater

diff --git a/Sources/StreamChat/Workers/ChannelListLinker.swift b/Sources/StreamChat/Workers/ChannelListLinker.swift
@@ -4,33 +4,31 @@
 
 import Foundation
 
-/// Inserts or removes channels from the currently loaded channel list based on web-socket events.
-///
-/// Requires either `filter` or `isChannelAutomaticFilteringEnabled` to be set.
-/// - Channels are inserted (linked) only when they would end up on the currently loaded pages.
-/// - Channels are removed (unlinked) when not on the currently loaded pages. For example, event changes
-/// extra data which makes it not to match with the current filter closure anymore.
+/// When we receive events, we need to check if a channel should be added or removed from
+/// the current query depending on the following events:
+/// - Channel created: We analyse if the channel should be added to the current query.
+/// - New message sent: This means the channel will reorder and appear on first position,
+///   so we also analyse if it should be added to the current query.
+/// - Channel is updated: We only check if we should remove it from the current query.
+///   We don't try to add it to the current query to not mess with pagination.
 final class ChannelListLinker {
     private let clientConfig: ChatClientConfig
     private let databaseContainer: DatabaseContainer
     private var eventObservers = [EventObserver]()
     private let filter: ((ChatChannel) -> Bool)?
-    private let loadedChannels: () -> StreamCollection<ChatChannel>
+    private let query: ChannelListQuery
     private let worker: ChannelListUpdater
-    let query: ChannelListQuery
 
     init(
         query: ChannelListQuery,
         filter: ((ChatChannel) -> Bool)?,
-        loadedChannels: @escaping () -> StreamCollection<ChatChannel>,
         clientConfig: ChatClientConfig,
         databaseContainer: DatabaseContainer,
         worker: ChannelListUpdater
     ) {
         self.clientConfig = clientConfig
         self.databaseContainer = databaseContainer
         self.filter = filter
-        self.loadedChannels = loadedChannels
         self.query = query
         self.worker = worker
     }
@@ -40,121 +38,91 @@ final class ChannelListLinker {
         eventObservers = [
             EventObserver(
                 notificationCenter: nc,
-                transform: { $0 as? NotificationAddedToChannelEvent },
-                callback: { [weak self] event in self?.handleChannel(event.channel) }
-            ),
+                transform: { $0 as? NotificationAddedToChannelEvent }
+            ) { [weak self] event in self?.linkChannelIfNeeded(event.channel) },
             EventObserver(
                 notificationCenter: nc,
                 transform: { $0 as? MessageNewEvent },
-                callback: { [weak self] event in self?.handleChannel(event.channel) }
+                callback: { [weak self] event in self?.linkChannelIfNeeded(event.channel) }
             ),
             EventObserver(
                 notificationCenter: nc,
                 transform: { $0 as? NotificationMessageNewEvent },
-                callback: { [weak self] event in self?.handleChannel(event.channel) }
+                callback: { [weak self] event in self?.linkChannelIfNeeded(event.channel) }
             ),
             EventObserver(
                 notificationCenter: nc,
                 transform: { $0 as? ChannelUpdatedEvent },
-                callback: { [weak self] event in self?.handleChannel(event.channel) }
-            ),
-            EventObserver(
-                notificationCenter: nc,
-                transform: { $0 as? ChannelHiddenEvent },
-                callback: { [weak self] event in self?.handleChannelId(event.cid) }
+                callback: { [weak self] event in self?.unlinkChannelIfNeeded(event.channel) }
             ),
             EventObserver(
                 notificationCenter: nc,
                 transform: { $0 as? ChannelVisibleEvent },
-                callback: { [weak self] event in self?.handleChannelId(event.cid) }
+                callback: { [weak self, databaseContainer] event in
+                    let context = databaseContainer.backgroundReadOnlyContext
+                    context.perform {
+                        guard let channel = try? context.channel(cid: event.cid)?.asModel() else { return }
+                        self?.linkChannelIfNeeded(channel)
+                    }
+                }
             )
         ]
     }
-
-    var didHandleChannel: ((ChannelId, LinkingAction) -> Void)?
-
-    enum LinkingAction {
-        case link, unlink, none
-    }
-
-    private func handleChannelId(_ cid: ChannelId) {
-        databaseContainer.read { session in
-            guard let dto = session.channel(cid: cid) else {
-                throw ClientError.ChannelDoesNotExist(cid: cid)
-            }
-            return try dto.asModel()
-        } completion: { [weak self] result in
-            switch result {
-            case .success(let channel):
-                self?.handleChannel(channel)
-            case .failure:
-                self?.didHandleChannel?(cid, .none)
+
+    private func isInChannelList(_ channel: ChatChannel, completion: @escaping (Bool) -> Void) {
+        let context = databaseContainer.backgroundReadOnlyContext
+        context.performAndWait { [weak self] in
+            guard let self else { return }
+            if let (channelDTO, queryDTO) = context.getChannelWithQuery(cid: channel.cid, query: self.query) {
+                let isPresent = queryDTO.channels.contains(channelDTO)
+                completion(isPresent)
+            } else {
+                completion(false)
             }
         }
     }
 
-    private func handleChannel(_ channel: ChatChannel) {
-        let action = linkingActionForChannel(channel)
-        switch action {
-        case .link:
-            worker.link(channel: channel, with: query) { [worker, didHandleChannel] error in
+    /// Handles if a channel should be linked to the current query or not.
+    private func linkChannelIfNeeded(_ channel: ChatChannel) {
+        guard shouldChannelBelongToCurrentQuery(channel) else { return }
+        isInChannelList(channel) { [worker, query] exists in
+            guard !exists else { return }
+            worker.link(channel: channel, with: query) { error in
                 if let error = error {
                     log.error(error)
-                    didHandleChannel?(channel.cid, action)
                     return
                 }
                 worker.startWatchingChannels(withIds: [channel.cid]) { error in
-                    if let error {
-                        log.warning(
-                            "Failed to start watching linked channel: \(channel.cid), error: \(error.localizedDescription)"
-                        )
-                    }
-                    didHandleChannel?(channel.cid, action)
+                    guard let error = error else { return }
+                    log.warning(
+                        "Failed to start watching linked channel: \(channel.cid), error: \(error.localizedDescription)"
+                    )
                 }
             }
-        case .unlink:
-            worker.unlink(channel: channel, with: query) { [didHandleChannel] error in
-                if let error {
-                    log.error(error)
-                }
-                didHandleChannel?(channel.cid, action)
-            }
-        case .none:
-            didHandleChannel?(channel.cid, action)
         }
     }
-
-    private func linkingActionForChannel(_ channel: ChatChannel) -> LinkingAction {
-        // Linking/unlinking can only happen when either runtime filter is set or `isChannelAutomaticFilteringEnabled` is true
-        // In other cases the channel list should not be changed.
-        guard filter != nil || clientConfig.isChannelAutomaticFilteringEnabled else { return .none }
-        let belongsToQuery: Bool = {
-            if let filter = filter {
-                return filter(channel)
-            }
+
+    /// Handles if a channel should be unlinked from the current query or not.
+    private func unlinkChannelIfNeeded(_ channel: ChatChannel) {
+        guard !shouldChannelBelongToCurrentQuery(channel) else { return }
+        isInChannelList(channel) { [worker, query] exists in
+            guard exists else { return }
+            worker.unlink(channel: channel, with: query)
+        }
+    }
+
+    /// Checks if the given channel should belong to the current query or not.
+    private func shouldChannelBelongToCurrentQuery(_ channel: ChatChannel) -> Bool {
+        if let filter = filter {
+            return filter(channel)
+        }
+
+        if clientConfig.isChannelAutomaticFilteringEnabled {
             // When auto-filtering is enabled the channel will appear or not automatically if the
             // query matches the DB Predicate. So here we default to saying it always belong to the current query.
-            return clientConfig.isChannelAutomaticFilteringEnabled
-        }()
-
-        let loadedSortedChannels = loadedChannels()
-        if loadedSortedChannels.contains(where: { $0.cid == channel.cid }) {
-            return belongsToQuery ? .none : .unlink
-        } else {
-            // If the channel would be appended, consider it to be part of an old page.
-            if let last = loadedSortedChannels.last {
-                let sort: [Sorting<ChannelListSortingKey>] = query.sort.isEmpty ? [Sorting(key: ChannelListSortingKey.default)] : query.sort
-                let preceedsLastLoaded = [last, channel]
-                    .sorted(using: sort.compactMap(\.sortValue))
-                    .first?.cid == channel.cid
-                if preceedsLastLoaded || loadedSortedChannels.count < query.pagination.pageSize {
-                    return belongsToQuery ? .link : .none
-                } else {
-                    return .none
-                }
-            } else {
-                return belongsToQuery ? .link : .none
-            }
+            return true
         }
+
+        return false
     }
 }
diff --git a/StreamChat.xcodeproj/project.pbxproj b/StreamChat.xcodeproj/project.pbxproj
@@ -272,7 +272,6 @@
 		4F4562F72C240FD200675C7F /* DatabaseItemConverter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F4562F52C240FD200675C7F /* DatabaseItemConverter.swift */; };
 		4F45802E2BEE0B4B0099F540 /* ChannelListLinker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F45802D2BEE0B4B0099F540 /* ChannelListLinker.swift */; };
 		4F45802F2BEE0B4B0099F540 /* ChannelListLinker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F45802D2BEE0B4B0099F540 /* ChannelListLinker.swift */; };
-		4F4817C92CA553EE00BE4A3C /* ChannelListLinker_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F4817C82CA553EE00BE4A3C /* ChannelListLinker_Tests.swift */; };
 		4F5151962BC3DEA1001B7152 /* UserSearch_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F5151952BC3DEA1001B7152 /* UserSearch_Tests.swift */; };
 		4F5151982BC407ED001B7152 /* UserList_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F5151972BC407ED001B7152 /* UserList_Tests.swift */; };
 		4F51519A2BC57C40001B7152 /* MessageState_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F5151992BC57C40001B7152 /* MessageState_Tests.swift */; };
@@ -3197,7 +3196,6 @@
 		4F427F6B2BA2F53200D92238 /* ConnectedUserState+Observer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ConnectedUserState+Observer.swift"; sourceTree = "<group>"; };
 		4F4562F52C240FD200675C7F /* DatabaseItemConverter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatabaseItemConverter.swift; sourceTree = "<group>"; };
 		4F45802D2BEE0B4B0099F540 /* ChannelListLinker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChannelListLinker.swift; sourceTree = "<group>"; };
-		4F4817C82CA553EE00BE4A3C /* ChannelListLinker_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChannelListLinker_Tests.swift; sourceTree = "<group>"; };
 		4F5151952BC3DEA1001B7152 /* UserSearch_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserSearch_Tests.swift; sourceTree = "<group>"; };
 		4F5151972BC407ED001B7152 /* UserList_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserList_Tests.swift; sourceTree = "<group>"; };
 		4F5151992BC57C40001B7152 /* MessageState_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageState_Tests.swift; sourceTree = "<group>"; };
@@ -7084,22 +7082,21 @@
 		A364D09627D0C56C0029857A /* Workers */ = {
 			isa = PBXGroup;
 			children = (
-				A364D09927D0C5D80029857A /* Background */,
-				A364D09827D0C5C20029857A /* EventObservers */,
-				4F4817C82CA553EE00BE4A3C /* ChannelListLinker_Tests.swift */,
+				AD9490582BF5701D00E69224 /* ThreadsRepository_Tests.swift */,
 				792921C424C0479700116BBB /* ChannelListUpdater_Tests.swift */,
 				882C5765252C7F7000E60C44 /* ChannelMemberListUpdater_Tests.swift */,
 				88F6DF93252C8866009A8AF0 /* ChannelMemberUpdater_Tests.swift */,
 				7952B3B424D45D9400AC53D4 /* ChannelUpdater_Tests.swift */,
 				AD90D18425D56196001D03BB /* CurrentUserUpdater_Tests.swift */,
 				F69C4BC324F664A700A3D740 /* EventNotificationCenter_Tests.swift */,
 				84A1D2E926AAFB1D00014712 /* EventSender_Tests.swift */,
-				F61D7C3424FFA6FD00188A0E /* MessageUpdater_Tests.swift */,
 				AD0CC0252BDBF9D1005E2C66 /* ReactionListUpdater_Tests.swift */,
-				AD9490582BF5701D00E69224 /* ThreadsRepository_Tests.swift */,
+				F61D7C3424FFA6FD00188A0E /* MessageUpdater_Tests.swift */,
 				8A0175F325013B6400570345 /* TypingEventSender_Tests.swift */,
 				DA8407322526003D005A0F62 /* UserListUpdater_Tests.swift */,
 				8819DFE1252628CA00FD1A50 /* UserUpdater_Tests.swift */,
+				A364D09927D0C5D80029857A /* Background */,
+				A364D09827D0C5C20029857A /* EventObservers */,
 			);
 			path = Workers;
 			sourceTree = "<group>";
@@ -11913,7 +11910,6 @@
 				7952B3B324D4560E00AC53D4 /* ChannelController_Tests.swift in Sources */,
 				8A0CC9EB24C601F600705CF9 /* MemberEvents_Tests.swift in Sources */,
 				C1A25D6029E70DEB00DAE933 /* FetchCache_Tests.swift in Sources */,
-				4F4817C92CA553EE00BE4A3C /* ChannelListLinker_Tests.swift in Sources */,
 				A32D55142860B40B00E66AF9 /* ChatMessageLinkAttachment_Tests.swift in Sources */,
 				ADA9DB8B2BCF2B1F00C4AE3B /* ThreadParticipantDTO_Tests.swift in Sources */,
 				4F51519A2BC57C40001B7152 /* MessageState_Tests.swift in Sources */,

diff --git a/TestTools/StreamChatTestTools/SpyPattern/Spy/ChannelListUpdater_Spy.swift b/TestTools/StreamChatTestTools/SpyPattern/Spy/ChannelListUpdater_Spy.swift
@@ -23,14 +23,11 @@ final class ChannelListUpdater_Spy: ChannelListUpdater, Spy {
     var startWatchingChannels_callCount = 0
     @Atomic var startWatchingChannels_cids: [ChannelId] = []
     @Atomic var startWatchingChannels_completion: ((Error?) -> Void)?
-    @Atomic var startWatchingChannels_completion_result: Result<Void, Error>?
-
+
     var link_callCount = 0
     var link_completion: ((Error?) -> Void)?
-    @Atomic var link_completion_result: Result<Void, Error>?
-
+
     var unlink_callCount = 0
-    @Atomic var unlink_completion_result: Result<Void, Error>?
 
     func cleanUp() {
         update_queries.removeAll()
@@ -40,15 +37,10 @@ final class ChannelListUpdater_Spy: ChannelListUpdater, Spy {
         fetch_queries.removeAll()
         fetch_completion = nil
 
-        link_completion_result = nil
-
         markAllRead_completion = nil
 
         startWatchingChannels_cids.removeAll()
         startWatchingChannels_completion = nil
-        startWatchingChannels_completion_result = nil
-
-        unlink_completion_result = nil
     }
 
     override func update(
@@ -90,7 +82,6 @@ final class ChannelListUpdater_Spy: ChannelListUpdater, Spy {
     ) {
         link_callCount += 1
         link_completion = completion
-        link_completion_result?.invoke(with: completion)
     }
 
     override func unlink(
@@ -99,13 +90,11 @@ final class ChannelListUpdater_Spy: ChannelListUpdater, Spy {
         completion: ((Error?) -> Void)? = nil
     ) {
         unlink_callCount += 1
-        unlink_completion_result?.invoke(with: completion)
     }
 
     override func startWatchingChannels(withIds ids: [ChannelId], completion: ((Error?) -> Void)?) {
         startWatchingChannels_callCount += 1
         startWatchingChannels_cids = ids
         startWatchingChannels_completion = completion
-        startWatchingChannels_completion_result?.invoke(with: completion)
     }
 }