diff --git a/apps/ios/Shared/Views/Chat/ChatItemsLoader.swift b/apps/ios/Shared/Views/Chat/ChatItemsLoader.swift index 9987fb4697..5213b5509b 100644 --- a/apps/ios/Shared/Views/Chat/ChatItemsLoader.swift +++ b/apps/ios/Shared/Views/Chat/ChatItemsLoader.swift @@ -31,11 +31,11 @@ func apiLoadMessages( let chatModel = ChatModel.shared - // For .initial allow the chatItems to be empty as well as chatModel.chatId to not match this chat because these values become set after .initial finishes let paginationIsInitial = switch pagination { case .initial: true; default: false } let paginationIsLast = switch pagination { case .last: true; default: false } - // When openAroundItemId is provided, chatId can be different too - if ((chatModel.chatId != chat.id || chat.chatItems.isEmpty) && !paginationIsInitial && !paginationIsLast && openAroundItemId == nil) || Task.isCancelled { + // For .initial allow the chatItems to be empty, as well as for .last that is used for searching + let allowEmptyItems = paginationIsInitial || paginationIsLast || openAroundItemId != nil + if chatWasSwitched(chat, pagination, openAroundItemId) || (!allowEmptyItems && chat.chatItems.isEmpty) || Task.isCancelled { return } @@ -79,6 +79,7 @@ func apiLoadMessages( newItems.insert(contentsOf: chat.chatItems, at: insertAt) let newReversed: [ChatItem] = newItems.reversed() await MainActor.run { + if chatWasSwitched(chat, pagination, openAroundItemId) { return } im.reversedChatItems = newReversed im.chatState.splits = modifiedSplits.newSplits im.chatState.moveUnreadAfterItem(modifiedSplits.oldUnreadSplitIndex, modifiedSplits.newUnreadSplitIndex, oldItems) @@ -99,6 +100,7 @@ func apiLoadMessages( let new: [ChatItem] = newItems let newReversed: [ChatItem] = newItems.reversed() await MainActor.run { + if chatWasSwitched(chat, pagination, openAroundItemId) { return } im.reversedChatItems = newReversed im.chatState.splits = newSplits im.chatState.moveUnreadAfterItem(im.chatState.splits.first ?? new.last!.id, new) @@ -122,6 +124,7 @@ func apiLoadMessages( let newReversed: [ChatItem] = newItems.reversed() let orderedSplits = newSplits await MainActor.run { + if chatWasSwitched(chat, pagination, openAroundItemId) { return } im.reversedChatItems = newReversed im.chatState.splits = orderedSplits im.chatState.unreadAfterItemId = chat.chatItems.last!.id @@ -147,6 +150,7 @@ func apiLoadMessages( newItems.append(contentsOf: chat.chatItems) let items = newItems await MainActor.run { + if chatWasSwitched(chat, pagination, openAroundItemId) { return } im.reversedChatItems = items.reversed() im.chatState.splits = newSplits if im.secondaryIMFilter == nil { @@ -158,6 +162,14 @@ func apiLoadMessages( } +/// .initial pagination and opening around item set ChatModel.chatId themselves after the items are loaded. +/// In other cases the chat could be switched while the items were loading, and the items of the previously opened chat +/// must not be added to the items of the currently opened one +private func chatWasSwitched(_ chat: Chat, _ pagination: ChatPagination, _ openAroundItemId: ChatItem.ID?) -> Bool { + let paginationIsInitial = switch pagination { case .initial: true; default: false } + return !paginationIsInitial && openAroundItemId == nil && ChatModel.shared.chatId != chat.id +} + private class ModifiedSplits { let oldUnreadSplitIndex: Int let newUnreadSplitIndex: Int diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt index 6562d40cec..fd912a8c9e 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt @@ -33,13 +33,21 @@ suspend fun apiLoadMessages( visibleItemIndexesNonReversed: () -> IntRange = { 0 .. 0 } ) = coroutineScope { val (chat, navInfo) = chatModel.controller.apiGetChat(rhId, chatType, apiId, chatsCtx.groupScopeInfo?.toChatScope(), contentTag ?: chatsCtx.contentTag, pagination, search) ?: return@coroutineScope - // For .initial allow the chatItems to be empty as well as chatModel.chatId to not match this chat because these values become set after .initial finishes - /** When [openAroundItemId] is provided, chatId can be different too */ - if (((chatModel.chatId.value != chat.id || chat.chatItems.isEmpty()) && pagination !is ChatPagination.Initial && pagination !is ChatPagination.Last && openAroundItemId == null) + // For .initial allow the chatItems to be empty, as well as for .last that is used for searching + val allowEmptyItems = pagination is ChatPagination.Initial || pagination is ChatPagination.Last || openAroundItemId != null + if (chatWasSwitched(chat, pagination, openAroundItemId) + || (!allowEmptyItems && chat.chatItems.isEmpty()) || !isActive) return@coroutineScope processLoadedChat(chatsCtx, chat, navInfo, pagination, openAroundItemId, visibleItemIndexesNonReversed) } +/** .initial pagination and opening around item set [ChatModel.chatId] themselves after the items are loaded. + * In other cases the chat could be switched while the items were loading, and the items of the previously opened chat + * must not be added to the items of the currently opened one */ +private fun chatWasSwitched(chat: Chat, pagination: ChatPagination, openAroundItemId: Long?): Boolean = + pagination !is ChatPagination.Initial && openAroundItemId == null && + (chatModel.chatId.value != chat.id || chatModel.remoteHostId() != chat.remoteHostId) + suspend fun processLoadedChat( chatsCtx: ChatModel.ChatsContext, chat: Chat, @@ -94,6 +102,7 @@ suspend fun processLoadedChat( val insertAt = (indexInCurrentItems - (wasSize - newItems.size) + trimmedIds.size).coerceAtLeast(0) newItems.addAll(insertAt, chat.chatItems) withContext(Dispatchers.Main) { + if (chatWasSwitched(chat, pagination, openAroundItemId)) return@withContext chatsCtx.chatItems.replaceAll(newItems) splits.value = newSplits chatState.moveUnreadAfterItem(oldUnreadSplitIndex, newUnreadSplitIndex, oldItems) @@ -113,6 +122,7 @@ suspend fun processLoadedChat( val indexToAddIsLast = indexToAdd == newItems.size newItems.addAll(indexToAdd, chat.chatItems) withContext(Dispatchers.Main) { + if (chatWasSwitched(chat, pagination, openAroundItemId)) return@withContext chatsCtx.chatItems.replaceAll(newItems) splits.value = newSplits chatState.moveUnreadAfterItem(splits.value.firstOrNull() ?: newItems.last().id, newItems) @@ -135,6 +145,7 @@ suspend fun processLoadedChat( newSplits.add(splitIndex, chat.chatItems.last().id) withContext(Dispatchers.Main) { + if (chatWasSwitched(chat, pagination, openAroundItemId)) return@withContext chatsCtx.chatItems.replaceAll(newItems) splits.value = newSplits unreadAfterItemId.value = chat.chatItems.last().id @@ -158,6 +169,7 @@ suspend fun processLoadedChat( removeDuplicates(newItems, chat) newItems.addAll(chat.chatItems) withContext(Dispatchers.Main) { + if (chatWasSwitched(chat, pagination, openAroundItemId)) return@withContext chatsCtx.chatItems.replaceAll(newItems) chatState.splits.value = newSplits unreadAfterNewestLoaded.value = 0 diff --git a/apps/multiplatform/common/src/desktopTest/kotlin/chat/simplex/app/ChatItemsLoaderTest.kt b/apps/multiplatform/common/src/desktopTest/kotlin/chat/simplex/app/ChatItemsLoaderTest.kt new file mode 100644 index 0000000000..3337839ff0 --- /dev/null +++ b/apps/multiplatform/common/src/desktopTest/kotlin/chat/simplex/app/ChatItemsLoaderTest.kt @@ -0,0 +1,65 @@ +package chat.simplex.app + +import chat.simplex.common.model.* +import chat.simplex.common.platform.chatModel +import chat.simplex.common.views.chat.processLoadedChat +import chat.simplex.common.model.replaceAll +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withContext +import kotlin.test.Test +import kotlin.test.assertEquals + +class ChatItemsLoaderTest { + + private fun groupChat(groupId: Long, itemIds: List): Chat = + Chat( + remoteHostId = null, + chatInfo = ChatInfo.Group(GroupInfo.sampleData.copy(groupId = groupId), groupChatScope = null), + chatItems = itemIds.map { ChatItem.getSampleData(it, CIDirection.GroupRcv(GroupMember.sampleData)) } + ) + + private suspend fun openChatWithItems(itemIds: List): Pair { + val chatsCtx = ChatModel.ChatsContext(null) + val opened = groupChat(groupId = 2, itemIds = itemIds) + withContext(Dispatchers.Main) { + chatsCtx.chatItems.replaceAll(opened.chatItems) + chatModel.chatId.value = opened.chatInfo.id + } + return chatsCtx to opened + } + + private fun itemIds(chatsCtx: ChatModel.ChatsContext): List = chatsCtx.chatItems.value.map { it.id } + + @Test + fun lastPageLoadedForAnotherChatIsNotAddedToOpenedChat() = runBlocking { + val (chatsCtx, _) = openChatWithItems(listOf(101, 102)) + val anotherChat = groupChat(groupId = 1, itemIds = listOf(201, 202)) + processLoadedChat(chatsCtx, anotherChat, NavigationInfo(), ChatPagination.Last(2), openAroundItemId = null) + assertEquals(listOf(101L, 102L), itemIds(chatsCtx)) + } + + @Test + fun lastPageLoadedForOpenedChatIsAdded() = runBlocking { + val (chatsCtx, _) = openChatWithItems(listOf(101, 102)) + val sameChat = groupChat(groupId = 2, itemIds = listOf(103, 104)) + processLoadedChat(chatsCtx, sameChat, NavigationInfo(), ChatPagination.Last(2), openAroundItemId = null) + assertEquals(listOf(101L, 102L, 103L, 104L), itemIds(chatsCtx)) + } + + @Test + fun beforePageLoadedForAnotherChatIsNotAddedToOpenedChat() = runBlocking { + val (chatsCtx, _) = openChatWithItems(listOf(101, 102)) + val anotherChat = groupChat(groupId = 1, itemIds = listOf(201, 202)) + processLoadedChat(chatsCtx, anotherChat, NavigationInfo(), ChatPagination.Before(101, 2), openAroundItemId = null) + assertEquals(listOf(101L, 102L), itemIds(chatsCtx)) + } + + @Test + fun aroundPageLoadedForAnotherChatIsNotAddedToOpenedChat() = runBlocking { + val (chatsCtx, _) = openChatWithItems(listOf(101, 102)) + val anotherChat = groupChat(groupId = 1, itemIds = listOf(201, 202)) + processLoadedChat(chatsCtx, anotherChat, NavigationInfo(), ChatPagination.Around(101, 2), openAroundItemId = null) + assertEquals(listOf(101L, 102L), itemIds(chatsCtx)) + } +}