diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt index d9d2d3757c79..8490b9162265 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt @@ -96,7 +96,7 @@ public interface ChannelState { /** If the channel is currently muted. */ public val muted: StateFlow - /** If we are currently loading. */ + /** If the channel is loading for the first time, before any data from the local database or the API arrived. */ public val loading: StateFlow /** If we are currently loading older messages. */ diff --git a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt index 36632cde5735..f8db1e25c62e 100644 --- a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt +++ b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt @@ -36,7 +36,6 @@ import io.getstream.chat.android.state.model.querychannels.pagination.internal.Q import io.getstream.chat.android.state.model.querychannels.pagination.internal.toAnyChannelPaginationRequest import io.getstream.chat.android.state.plugin.state.channel.internal.ChannelMutableState import io.getstream.log.taggedLogger -import io.getstream.result.Error import io.getstream.result.Result import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -101,15 +100,6 @@ internal class ChannelLogicImpl( override suspend fun watch(limit: Int, userPresence: Boolean): Result { logger.i { "[watch] messagesLimit: $limit, userPresence: $userPresence" } - // Otherwise it's too easy for devs to create UI bugs which DDOS our API - if (mutableState.loading.value) { - logger.i { "Another request to watch this channel is in progress. Ignoring this request." } - return Result.Failure( - Error.GenericError( - "Another request to watch this channel is in progress. Ignoring this request.", - ), - ) - } channelStateLogic.loadingNewestMessages() return runChannelQuery( "watch", diff --git a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt index a79f2efc6a6a..b8d462c554ec 100644 --- a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt +++ b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt @@ -664,6 +664,7 @@ internal class ChannelStateLogic( mutableState.setChannelConfig(channel.config) + mutableState.setLoading(false) mutableState.setLoadingOlderMessages(false) mutableState.setLoadingNewerMessages(false) } @@ -796,6 +797,7 @@ internal class ChannelStateLogic( } mutableState.recoveryNeeded = true } + mutableState.setLoading(false) mutableState.setLoadingOlderMessages(false) mutableState.setLoadingNewerMessages(false) } diff --git a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt index 84c85b2f836d..99fab8ce28a1 100644 --- a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt +++ b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt @@ -152,7 +152,9 @@ internal class ChannelMutableState( messagesTransformation(pinnedMessagesList) { it.isPinned(now) } override val messagesState: StateFlow = - combineStates(loading, sortedVisibleMessages) { loading: Boolean, messages: List -> + combineStates(loading, sortedVisibleMessages) { loading: Boolean, collected: List -> + // A collected copy of the messages can lag behind loading, so re-read before reporting no results. + val messages = if (!loading && collected.isEmpty()) sortedVisibleMessages.value else collected when { loading -> MessagesState.Loading messages.isEmpty() -> MessagesState.OfflineNoResults @@ -338,6 +340,14 @@ internal class ChannelMutableState( _loading?.value = isLoading } + /** + * Marks the first load of the channel as in progress. Does nothing once the channel has data, so refreshing a + * loaded channel never replaces its content with a loading state. + */ + fun setLoadingIfEmpty() { + if (_channelData?.value == null) setLoading(true) + } + /** * Sets hidden. * diff --git a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCalls.kt b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCalls.kt index 4a9f125f422e..5e6bc055e384 100644 --- a/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCalls.kt +++ b/stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCalls.kt @@ -99,10 +99,12 @@ internal class ChatClientStateCalls( request: QueryChannelRequest, ): ChannelState { logger.v { "[queryChannel] cid: $channelType:$channelId, request: $request" } - chatClient.queryChannel(channelType, channelId, request).launch(scope) - return deferredState + val state = deferredState .await() - .channel(channelType, channelId) + .mutableChannel(channelType, channelId) + .apply { setLoadingIfEmpty() } + chatClient.queryChannel(channelType, channelId, request).launch(scope) + return state } /** Reference request of the watch channel query. */ diff --git a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/extensions/ChatClientExtensionTests.kt b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/extensions/ChatClientExtensionTests.kt index e73fb1ff983e..c83e854f6450 100644 --- a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/extensions/ChatClientExtensionTests.kt +++ b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/extensions/ChatClientExtensionTests.kt @@ -32,6 +32,7 @@ import io.getstream.chat.android.state.plugin.config.StatePluginConfig import io.getstream.chat.android.state.plugin.factory.StreamStatePluginFactory import io.getstream.chat.android.state.plugin.internal.StatePlugin import io.getstream.chat.android.state.plugin.state.StateRegistry +import io.getstream.chat.android.state.plugin.state.channel.internal.ChannelMutableState import io.getstream.chat.android.state.plugin.state.global.GlobalState import io.getstream.chat.android.test.TestCoroutineExtension import io.getstream.chat.android.test.asCall @@ -79,13 +80,13 @@ internal class ChatClientExtensionTests { val statePluginConfig = StatePluginConfig() val globalState: GlobalState = mock() { } - val channelState: ChannelState = mock { + val channelState: ChannelMutableState = mock { on(it.cid) doReturn channel.cid on(it.channelType) doReturn channel.type on(it.channelId) doReturn channel.id } val stateRegistry: StateRegistry = mock { stateRegistry -> - on(stateRegistry.channel(channel.type, channel.id)) doReturn channelState + on(stateRegistry.mutableChannel(channel.type, channel.id)) doReturn channelState } val request = QueryChannelPaginationRequest(MESSAGE_LIMIT) diff --git a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt index b5607b7e0220..82f7b98d1457 100644 --- a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt +++ b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt @@ -66,6 +66,7 @@ import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doReturn import org.mockito.kotlin.eq +import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.spy @@ -247,6 +248,26 @@ internal class ChannelStateLogicTest { verify(mutableState, times(0)).upsertMessages(any()) } + @Test + fun `given channel data arrives, loading should end once the data is in the state`() { + val channel = randomChannel() + + channelStateLogic.updateDataForChannel(channel, messageLimit = 30) + + inOrder(mutableState) { + verify(mutableState).setChannelData(any()) + verify(mutableState).upsertMembers(channel.members) + verify(mutableState).setLoading(false) + } + } + + @Test + fun `given the query fails, loading should end`() { + channelStateLogic.propagateQueryError(Error.GenericError(randomString())) + + verify(mutableState).setLoading(false) + } + @Test fun `given inside search should not upsert messages when messages are not coming from scroll update`() { _insideSearch.value = true diff --git a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableStateTests.kt b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableStateTests.kt index 987563f09a49..abb30fe51501 100644 --- a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableStateTests.kt +++ b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableStateTests.kt @@ -16,7 +16,9 @@ package io.getstream.chat.android.state.plugin.state.channel.internal +import io.getstream.chat.android.models.ChannelData import io.getstream.chat.android.models.Message +import io.getstream.chat.android.models.MessagesState import io.getstream.chat.android.models.User import io.getstream.chat.android.randomChannelUserRead import io.getstream.chat.android.randomConfig @@ -26,7 +28,10 @@ import io.getstream.chat.android.randomUser import io.getstream.chat.android.test.TestCoroutineExtension import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.amshove.kluent.`should be equal to` import org.junit.jupiter.api.Assertions.assertEquals @@ -744,6 +749,41 @@ internal class ChannelMutableStateTests { assertEquals(delivered, userRead) } + @Test + fun `setLoadingIfEmpty on a channel without data should show the loading state`() = runTest { + channelState.setLoadingIfEmpty() + + channelState.loading.value `should be equal to` true + channelState.messagesState.value `should be equal to` MessagesState.Loading + } + + @Test + fun `setLoadingIfEmpty on a channel with data should not show the loading state`() = runTest { + channelState.setChannelData(ChannelData(type = CHANNEL_TYPE, id = CHANNEL_ID)) + + channelState.setLoadingIfEmpty() + + channelState.loading.value `should be equal to` false + channelState.messagesState.value `should be equal to` MessagesState.OfflineNoResults + } + + @Test + fun `messagesState should not report no results while the loaded messages are still propagating`() = + runTest(StandardTestDispatcher(testCoroutines.dispatcher.scheduler)) { + val emissions = mutableListOf() + backgroundScope.launch { channelState.messagesState.collect { emissions += it } } + channelState.setLoadingIfEmpty() + runCurrent() + + channelState.setMessages(createMessages(3)) + channelState.setLoading(false) + runCurrent() + + val afterLoading = emissions.dropWhile { it != MessagesState.Loading } + afterLoading.contains(MessagesState.OfflineNoResults) `should be equal to` false + (afterLoading.last() is MessagesState.Result) `should be equal to` true + } + private fun ChannelMutableState.assertPinnedMessagesSizeEqualsTo(size: Int) { require(pinnedMessages.value.size == size) { "pinnedMessages should have $size items, but was ${pinnedMessages.value.size}" diff --git a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCallsTest.kt b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCallsTest.kt index c3a3643038bf..b86c2523d58f 100644 --- a/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCallsTest.kt +++ b/stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/state/internal/ChatClientStateCallsTest.kt @@ -21,13 +21,16 @@ import io.getstream.chat.android.client.internal.state.plugin.QueryChannelsIdent import io.getstream.chat.android.client.setup.state.ClientState import io.getstream.chat.android.models.InitializationState import io.getstream.chat.android.models.User +import io.getstream.chat.android.randomChannel import io.getstream.chat.android.state.event.handler.chat.factory.ChatEventHandlerFactory import io.getstream.chat.android.state.plugin.internal.StatePlugin import io.getstream.chat.android.state.plugin.logic.internal.LogicRegistry import io.getstream.chat.android.state.plugin.logic.querychannels.internal.QueryChannelsLogic import io.getstream.chat.android.state.plugin.state.StateRegistry +import io.getstream.chat.android.state.plugin.state.channel.internal.ChannelMutableState import io.getstream.chat.android.state.plugin.state.querychannels.QueryChannelsState import io.getstream.chat.android.test.TestCoroutineRule +import io.getstream.chat.android.test.asCall import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch @@ -41,9 +44,11 @@ import org.junit.jupiter.api.Test import org.mockito.kotlin.any import org.mockito.kotlin.doReturn import org.mockito.kotlin.eq +import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever @OptIn(ExperimentalCoroutinesApi::class) internal class ChatClientStateCallsTest { @@ -142,4 +147,23 @@ internal class ChatClientStateCallsTest { // Then - stateRegistry.queryChannels should be called with the identifier verify(stateRegistry).queryChannels(identifier) } + + @Test + fun `watchChannel marks the channel as loading before querying it`() = runTest { + // Given + userFlow.value = User(id = "test-user") + val channelState: ChannelMutableState = mock() + whenever(stateRegistry.mutableChannel("messaging", "123")) doReturn channelState + whenever(chatClient.queryChannel(any(), any(), any(), any())) doReturn randomChannel().asCall() + + // When + val result = chatClientStateCalls.watchChannel("messaging:123", messageLimit = 30, userPresence = true) + + // Then + assertEquals(channelState, result) + inOrder(channelState, chatClient) { + verify(channelState).setLoadingIfEmpty() + verify(chatClient).queryChannel(eq("messaging"), eq("123"), any(), any()) + } + } }