Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Linking and unlinking channels based on the loaded list of channels" #3465

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -269,7 +269,6 @@ extension ChatChannelListController {
var channelListLinkerBuilder: (
_ query: ChannelListQuery,
_ filter: ((ChatChannel) -> Bool)?,
_ loadedChannels: @escaping () -> StreamCollection<ChatChannel>,
_ clientConfig: ChatClientConfig,
_ databaseContainer: DatabaseContainer,
_ worker: ChannelListUpdater
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
152 changes: 60 additions & 92 deletions Sources/StreamChat/Workers/ChannelListLinker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
}
12 changes: 4 additions & 8 deletions StreamChat.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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>"; };
Expand Down Expand Up @@ -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>";
Expand Down Expand Up @@ -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 */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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)
}
}
Loading
Loading