From 7e5f1042f34a26e5aab04a696d5431bc08eaacf4 Mon Sep 17 00:00:00 2001 From: Gian <47775302+gpunto@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:33:52 +0200 Subject: [PATCH 1/3] state: Keep the loader up while a grouped channel list fetches its first page (develop) Port of #6674 to develop. Opening a grouped channel list with nothing cached for that group showed the empty state ("no channels") for the whole duration of the first queryGroupedChannels round trip, instead of the loading indicator. Standard channel lists hold the loader until the network answers. loadOfflineGroupedChannels() cleared the first-page loading flag unconditionally, so a cache miss moved channelsStateData from Loading to OfflineNoResults while the request was still in flight, and nothing raised it again. Raise it in the request listener instead, for each named first-page group, guarded on !hasCompletedAQuery && getChannels().isNullOrEmpty(). Both halves matter: without the marker a settled empty group returns to a spinner on every reconnect, since SyncManager recovers grouped lists through this listener; without the emptiness check a request hides cached channels behind a spinner. A failure does not count as completion, so a retry raises the loader again, matching queryOffline for a standard list. Recovery of a group whose first page failed does differ from standard, which never raises there because queryFirstPage bypasses the listener; grouped showing a spinner while the retry runs is the more accurate of the two. Clear the flag only once there is something to show, and end the load on failure and for a group the response omits. Only the query-channels path is affected. The two channel-logic implementations on this branch, ChannelLogicImpl and ChannelLogicLegacyImpl, are the chat screen and are untouched here; that half is tracked separately in AND-1462. Differences from the v6 version are mechanical: the files live under stream-chat-android-client rather than stream-chat-android-state, ChannelsStateData is imported from client.api.state, and QueryChannelsStateLogic takes an extra isLocalUnreadCountEnabled parameter that the test harness now passes. Co-Authored-By: Claude Opus 5 (1M context) --- .../QueryGroupedChannelsListenerState.kt | 51 +++-- .../internal/QueryChannelsLogic.kt | 58 +++++- .../QueryGroupedChannelsListenerStateTest.kt | 128 ++++++++++-- .../internal/QueryChannelsLogicGroupedTest.kt | 196 +++++++++++++++++- 4 files changed, 405 insertions(+), 28 deletions(-) diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt index 74b47f062607..4057d1a51b7c 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt @@ -44,15 +44,35 @@ internal class QueryGroupedChannelsListenerState( // set — we don't know the keys until the response, so we defer to the result-side capture // (which only fires on success, but in that case there is no failure to recover from). groups?.forEach { (key, groupQuery) -> - logic.queryChannels(QueryChannelsIdentifier.Grouped(key)) - .setGroupedQueryConfig( - GroupedQueryConfig( - limit = limit, - pageSize = groupQuery.limit, - watch = watch, - presence = presence, - ), - ) + val queryLogic = logic.queryChannels(QueryChannelsIdentifier.Grouped(key)) + queryLogic.setGroupedQueryConfig( + GroupedQueryConfig( + limit = limit, + pageSize = groupQuery.limit, + watch = watch, + presence = presence, + ), + ) + // The only point that knows a fetch is starting: grouped state is initialised separately + // from the query, so the offline load cannot tell an in-flight first page from no fetch. + if (groupQuery.isFirstPage()) { + queryLogic.startLoadingFirstPageIfNeverLoaded() + } + } + } + + private fun GroupedChannelsGroupQuery.isFirstPage(): Boolean = next == null && prev == null + + /** Ends the first-page load for every [keys] entry that was requested as a first page. */ + private suspend fun finishFirstPageLoads( + keys: Set, + groups: Map?, + completed: Boolean, + ) { + keys.forEach { key -> + if (groups?.get(key)?.isFirstPage() == true) { + logic.queryChannels(QueryChannelsIdentifier.Grouped(key)).finishFirstPageLoad(completed) + } } } @@ -63,10 +83,14 @@ internal class QueryGroupedChannelsListenerState( watch: Boolean, presence: Boolean, ) { - if (result !is Result.Success) return + if (result !is Result.Success) { + // The success path ends the load while applying the result, so failures must do it here. + finishFirstPageLoads(groups.orEmpty().keys, groups, completed = false) + return + } // Only the first page carries initial unread counts. - val isFirstPageRequest = groups.orEmpty().values.none { it.next != null || it.prev != null } + val isFirstPageRequest = groups.orEmpty().values.all { it.isFirstPage() } if (isFirstPageRequest) { val next = groupedUnreadChannelsUpdater.calculateUpdatedCounts( current = globalState.groupedUnreadChannels.value, @@ -78,11 +102,14 @@ internal class QueryGroupedChannelsListenerState( // Route each returned group's channels into the per-group state. The captured config lets // both ChannelListViewModel.loadMoreGroupedChannels and SyncManager.updateGroupedQueryChannels // reuse the caller's original parameters on paginated and recovery calls respectively. + // A requested group the response omits never reaches applyGroupedResult, so end it here. + finishFirstPageLoads(groups.orEmpty().keys - result.value.groups.keys, groups, completed = true) + result.value.groups.forEach { (key, group) -> // A request without `next`/`prev` cursors for this key (or no per-group query at all) // is a first-page request → replace channels. With a cursor → paginated → append. val perGroupQuery = groups?.get(key) - val isFirstPage = perGroupQuery?.let { it.next == null && it.prev == null } ?: true + val isFirstPage = perGroupQuery?.isFirstPage() ?: true val perGroupLimit = perGroupQuery?.limit val queryLogic = logic.queryChannels(QueryChannelsIdentifier.Grouped(key)) queryLogic.setGroupedQueryConfig( diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt index ac2407f4a36e..eb3d8ded7216 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt @@ -20,6 +20,7 @@ import io.getstream.chat.android.client.ChatClient import io.getstream.chat.android.client.api.event.EventHandlingResult import io.getstream.chat.android.client.api.models.QueryChannelsRequest import io.getstream.chat.android.client.api.models.QueryChannelsResult +import io.getstream.chat.android.client.api.state.ChannelsStateData import io.getstream.chat.android.client.api.state.querychannels.GroupedQueryConfig import io.getstream.chat.android.client.events.ChatEvent import io.getstream.chat.android.client.events.CidEvent @@ -57,6 +58,14 @@ internal class QueryChannelsLogic( */ private val groupedResultMutex = Mutex() + /** + * Whether a grouped query has ever finished for this group, successfully or not. Distinguishes + * a group that has never loaded from one that loaded and is genuinely empty, which state alone + * cannot express: both hold an empty channel map. + */ + @Volatile + private var hasCompletedAQuery = false + /** * Sets the current request and optimistically loads any cached channels for the given * [request] from the local database. The cached channels are added to the in-memory state. @@ -102,12 +111,17 @@ internal class QueryChannelsLogic( val cachedChannels = fetchChannelsFromCache(pagination) groupedResultMutex.withLock { val existing = queryChannelsStateLogic.getChannels() - if (existing.isNullOrEmpty() && !cachedChannels.isNullOrEmpty()) { + val hasCachedChannels = !cachedChannels.isNullOrEmpty() + if (existing.isNullOrEmpty() && hasCachedChannels) { logger.d { "[loadOfflineGroupedChannels] showing ${cachedChannels.size} cached channels" } queryChannelsStateLogic.addChannelsState(cachedChannels) } queryChannelsStateLogic.initializeChannelsIfNeeded() - queryChannelsStateLogic.setLoadingFirstPage(false) + // Only stop loading once there is something to show. On a miss the flag is left as it + // is: raised if a request is in flight, false if none is coming. + if (hasCachedChannels || !existing.isNullOrEmpty()) { + queryChannelsStateLogic.setLoadingFirstPage(false) + } } } @@ -147,6 +161,45 @@ internal class QueryChannelsLogic( } } + /** + * Marks a grouped first page as loading, for a group that has nothing to show and has never had + * a query finish. + * + * Both halves are needed. Without [hasCompletedAQuery] a settled empty group would go back to a + * spinner on every reconnect, since `SyncManager` recovers grouped lists through this listener. + * Without the emptiness check a request would hide cached channels behind a spinner, because + * [ChannelsStateData] reports `Loading` whenever the flag is set, regardless of content. + * + * Shares [groupedResultMutex] so the check cannot read a half-applied update. + */ + internal suspend fun startLoadingFirstPageIfNeverLoaded() { + groupedResultMutex.withLock { + if (!hasCompletedAQuery && queryChannelsStateLogic.getChannels().isNullOrEmpty()) { + queryChannelsStateLogic.setLoadingFirstPage(true) + } + } + } + + /** + * Ends a grouped first-page load that produced no channels of its own: a failed request, or a + * requested group the response left out. + * + * Both steps are needed to leave `Loading`, which [ChannelsStateData] reports while the flag is + * set *or* while channels are still null. + * + * [completed] tells the two apart. A group the response omits has been answered, so it settles + * and a later request leaves it on the empty state. A failure has not, so the group stays + * never-loaded and a retry raises the loader again, which is what `queryOffline` does for a + * standard list after a failed empty first page. + */ + internal suspend fun finishFirstPageLoad(completed: Boolean) { + if (completed) hasCompletedAQuery = true + groupedResultMutex.withLock { + queryChannelsStateLogic.initializeChannelsIfNeeded() + queryChannelsStateLogic.setLoadingFirstPage(false) + } + } + internal fun setCurrentRequest(request: QueryChannelsRequest) { queryChannelsStateLogic.setCurrentRequest(request) } @@ -264,6 +317,7 @@ internal class QueryChannelsLogic( queryChannelsStateLogic.setLoadingFirstPage(false) queryChannelsStateLogic.setLoadingMore(false) queryChannelsStateLogic.setRecoveryNeeded(false) + hasCompletedAQuery = true // Persist queryChannelsDatabaseLogic.insertQueryChannels(queryChannelsStateLogic.getQuerySpecs()) diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.kt index fb02b84a544c..8423da25eda5 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.kt @@ -32,6 +32,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Test import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doNothing import org.mockito.kotlin.doReturn import org.mockito.kotlin.eq @@ -43,9 +44,14 @@ import org.mockito.kotlin.whenever internal class QueryGroupedChannelsListenerStateTest { - private val queryChannelsLogic: QueryChannelsLogic = mock() + // One mock per group key: a single shared mock cannot tell "routed to the right group" from + // "routed anywhere", which is exactly the class of bug the omitted-group handling guards against. + private val groupLogics = mutableMapOf() + private fun logicFor(key: String): QueryChannelsLogic = groupLogics.getOrPut(key) { mock() } private val logic: LogicRegistry = mock { - on(it.queryChannels(any())) doReturn queryChannelsLogic + on(it.queryChannels(any())) doAnswer { invocation -> + logicFor((invocation.arguments[0] as QueryChannelsIdentifier.Grouped).groupKey) + } } private val globalState: MutableGlobalState = mock() private val stateRegistry: StateRegistry = mock() @@ -55,6 +61,100 @@ internal class QueryGroupedChannelsListenerStateTest { ) private val listener = QueryGroupedChannelsListenerState(logic, globalState, groupedUnreadChannelsUpdater) + @Test + fun `first-page request raises the loading flag for each named group`() = runTest { + // when + listener.onQueryGroupedChannelsRequest( + limit = 10, + groups = mapOf( + "support" to GroupedChannelsGroupQuery(limit = 5), + "direct" to GroupedChannelsGroupQuery(limit = 5), + ), + watch = true, + presence = false, + ) + + // then — the offline read cannot tell an in-flight first page from one that never comes, + // so the loader has to be raised here, where we know a fetch is starting. + verify(logicFor("support")).startLoadingFirstPageIfNeverLoaded() + verify(logicFor("direct")).startLoadingFirstPageIfNeverLoaded() + } + + @Test + fun `paginated request does not raise the loading flag`() = runTest { + // when — a cursor means the list already has content and is appending to it. + listener.onQueryGroupedChannelsRequest( + limit = 10, + groups = mapOf("support" to GroupedChannelsGroupQuery(limit = 5, next = "cursor")), + watch = true, + presence = false, + ) + + // then + verify(logicFor("support"), never()).startLoadingFirstPageIfNeverLoaded() + } + + @Test + fun `failed first-page result clears the loading flag`() = runTest { + // when + listener.onQueryGroupedChannelsResult( + result = Result.Failure(Error.GenericError("boom")), + limit = 10, + groups = mapOf("support" to GroupedChannelsGroupQuery(limit = 5)), + watch = true, + presence = false, + ) + + // then — the success path ends it while applying the result, so a failure that skipped + // that path would otherwise leave a permanent spinner. + verify(logicFor("support")).finishFirstPageLoad(completed = false) + } + + @Test + fun `successful result ends the load for a requested group the response left out`() = runTest { + // given — two groups requested, only one echoed back. + whenever(globalState.groupedUnreadChannels) doReturn MutableStateFlow(emptyMap()) + doNothing().`when`(globalState).setGroupedUnreadChannels(any()) + val result = Result.Success( + GroupedChannels( + groups = mapOf( + "support" to GroupedChannelsGroup(groupKey = "support", channels = emptyList(), next = null, prev = null), + ), + ), + ) + + // when + listener.onQueryGroupedChannelsResult( + result = result, + limit = 10, + groups = mapOf( + "support" to GroupedChannelsGroupQuery(limit = 5), + "direct" to GroupedChannelsGroupQuery(limit = 5), + ), + watch = true, + presence = false, + ) + + // then — the omitted group never reaches applyGroupedResult, so its loader is ended here. + verify(logicFor("direct")).finishFirstPageLoad(completed = true) + verify(logicFor("support"), never()).finishFirstPageLoad(any()) + } + + @Test + fun `failed paginated result leaves the first-page flag alone`() = runTest { + // when + listener.onQueryGroupedChannelsResult( + result = Result.Failure(Error.GenericError("boom")), + limit = 10, + groups = mapOf("support" to GroupedChannelsGroupQuery(limit = 5, next = "cursor")), + watch = true, + presence = false, + ) + + // then + verify(logicFor("support"), never()).finishFirstPageLoad(any()) + } + @Test fun `successful first-page result merges returned unread counts into existing global state`() = runTest { // given @@ -244,8 +344,8 @@ internal class QueryGroupedChannelsListenerStateTest { // then verify(logic).queryChannels(eq(QueryChannelsIdentifier.Grouped("direct"))) verify(logic).queryChannels(eq(QueryChannelsIdentifier.Grouped("support"))) - verify(queryChannelsLogic).applyGroupedResult(groupDirect, isFirstPage = true) - verify(queryChannelsLogic).applyGroupedResult(groupSupport, isFirstPage = true) + verify(logicFor("direct")).applyGroupedResult(groupDirect, isFirstPage = true) + verify(logicFor("support")).applyGroupedResult(groupSupport, isFirstPage = true) } @Test @@ -261,10 +361,10 @@ internal class QueryGroupedChannelsListenerStateTest { presence = false, ) // then - per-group override captured for "a", request-level only for "b" - verify(queryChannelsLogic).setGroupedQueryConfig( + verify(logicFor("a")).setGroupedQueryConfig( GroupedQueryConfig(limit = 20, pageSize = 5, watch = true, presence = false), ) - verify(queryChannelsLogic).setGroupedQueryConfig( + verify(logicFor("b")).setGroupedQueryConfig( GroupedQueryConfig(limit = 20, pageSize = null, watch = true, presence = false), ) verify(logic).queryChannels(eq(QueryChannelsIdentifier.Grouped("a"))) @@ -280,8 +380,10 @@ internal class QueryGroupedChannelsListenerStateTest { watch = true, presence = false, ) - // then - no per-group keys to write to; defer to result-side capture - verify(queryChannelsLogic, never()).setGroupedQueryConfig(any()) + // then - no per-group keys to write to; defer to result-side capture. Asserted on the + // registry rather than on a group's logic: with groups = null nothing resolves a logic, so + // logicFor() would mint a fresh mock and never() would pass for free. + verify(logic, never()).queryChannels(any()) } @Test @@ -304,10 +406,10 @@ internal class QueryGroupedChannelsListenerStateTest { presence = false, ) // then - per-group override captured for "a", request-level only for "b" - verify(queryChannelsLogic).setGroupedQueryConfig( + verify(logicFor("a")).setGroupedQueryConfig( GroupedQueryConfig(limit = 20, pageSize = 5, watch = true, presence = false), ) - verify(queryChannelsLogic).setGroupedQueryConfig( + verify(logicFor("b")).setGroupedQueryConfig( GroupedQueryConfig(limit = 20, pageSize = null, watch = true, presence = false), ) } @@ -328,7 +430,7 @@ internal class QueryGroupedChannelsListenerStateTest { presence = true, ) // then - config still written so subsequent pagination knows the watch/presence flags - verify(queryChannelsLogic).setGroupedQueryConfig( + verify(logicFor("direct")).setGroupedQueryConfig( GroupedQueryConfig(limit = null, pageSize = null, watch = true, presence = true), ) } @@ -357,7 +459,7 @@ internal class QueryGroupedChannelsListenerStateTest { presence = false, ) // then - verify(queryChannelsLogic).applyGroupedResult(groupSupport, isFirstPage = false) + verify(logicFor("support")).applyGroupedResult(groupSupport, isFirstPage = false) } @Test @@ -384,6 +486,6 @@ internal class QueryGroupedChannelsListenerStateTest { presence = false, ) // then - verify(queryChannelsLogic).applyGroupedResult(groupSupport, isFirstPage = false) + verify(logicFor("support")).applyGroupedResult(groupSupport, isFirstPage = false) } } diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt index 913985f3ff59..8c105f47340f 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt @@ -17,8 +17,10 @@ package io.getstream.chat.android.client.internal.state.plugin.logic.querychannels.internal import io.getstream.chat.android.client.ChatClient +import io.getstream.chat.android.client.api.state.ChannelsStateData import io.getstream.chat.android.client.api.state.QueryChannelsState import io.getstream.chat.android.client.internal.state.plugin.QueryChannelsIdentifier +import io.getstream.chat.android.client.internal.state.plugin.state.querychannels.internal.QueryChannelsMutableState import io.getstream.chat.android.client.query.QueryChannelsSpec import io.getstream.chat.android.client.query.pagination.AnyChannelPaginationRequest import io.getstream.chat.android.models.Channel @@ -31,6 +33,7 @@ import io.getstream.chat.android.test.TestCoroutineRule import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.Rule +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.mockito.kotlin.any @@ -341,9 +344,116 @@ internal class QueryChannelsLogicGroupedTest { // When logic.loadOfflineGroupedChannels() - // Then — no channels to add, but state is initialized and loading flag reset. + // Then — no channels to add, state is initialized, and the loading flag is left alone so an + // in-flight first page keeps its loader instead of falling through to the empty state. verify(queryChannelsStateLogic, never()).addChannelsState(any()) verify(queryChannelsStateLogic).initializeChannelsIfNeeded() + verify(queryChannelsStateLogic, never()).setLoadingFirstPage(any()) + } + + @Test + fun `loadOfflineGroupedChannels leaves the loading flag alone when the cache is empty`() = runTest { + // Given — a spec exists but its channels are gone, which reads the same as a miss. + whenever(queryChannelsStateLogic.getChannels()) doReturn null + whenever( + queryChannelsDatabaseLogic.fetchChannelsFromCache( + any(), + any(), + ), + ) doReturn CachedQueryChannels(spec = queryChannelsSpec, channels = emptyList()) + + // When + logic.loadOfflineGroupedChannels() + + // Then + verify(queryChannelsStateLogic, never()).addChannelsState(any()) + verify(queryChannelsStateLogic, never()).setLoadingFirstPage(any()) + } + + @Test + fun `startLoadingFirstPageIfNeverLoaded raises the flag for a group that never loaded`() = runTest { + whenever(queryChannelsStateLogic.getChannels()) doReturn null + + logic.startLoadingFirstPageIfNeverLoaded() + + verify(queryChannelsStateLogic).setLoadingFirstPage(true) + } + + @Test + fun `startLoadingFirstPageIfNeverLoaded does not hide an already populated list behind a loader`() = runTest { + whenever(queryChannelsStateLogic.getChannels()) doReturn mapOf("messaging:ch1" to randomChannel(id = "ch1")) + + logic.startLoadingFirstPageIfNeverLoaded() + + verify(queryChannelsStateLogic, never()).setLoadingFirstPage(any()) + } + + @Test + fun `cached channels are not hidden behind a loader before the first query finishes`() = runTest { + // Given — the offline read populated the list; no query has completed yet. + whenever(queryChannelsStateLogic.getChannels()) doReturn mapOf("messaging:ch1" to randomChannel(id = "ch1")) + + // When + logic.startLoadingFirstPageIfNeverLoaded() + + // Then + verify(queryChannelsStateLogic, never()).setLoadingFirstPage(any()) + } + + @Test + fun `a settled empty group is not put back into the loader by a later request`() = runTest { + // Given — a query finished and the group came back empty. This is the shape SyncManager + // recovery hits on every reconnect. + whenever(queryChannelsStateLogic.getChannels()) doReturn emptyMap() + logic.finishFirstPageLoad(completed = true) + + // When + logic.startLoadingFirstPageIfNeverLoaded() + + // Then — no spinner over a tab the user has already seen settle as empty. + verify(queryChannelsStateLogic, never()).setLoadingFirstPage(true) + } + + @Test + fun `a failed first page stays retryable, so a later request raises the loader again`() = runTest { + whenever(queryChannelsStateLogic.getChannels()) doReturn emptyMap() + logic.finishFirstPageLoad(completed = false) + + logic.startLoadingFirstPageIfNeverLoaded() + + // A retry after a failure shows the loader, matching queryOffline for a standard list. + verify(queryChannelsStateLogic).setLoadingFirstPage(true) + } + + @Test + fun `finishFirstPageLoad ends Loading even when the request failed before the offline read`() = runTest { + // Given — nothing has initialised channels yet, so rawChannels is still null. + whenever(queryChannelsStateLogic.getChannels()) doReturn null + + // When + logic.finishFirstPageLoad(completed = false) + + // Then — clearing the flag alone is not enough: ChannelsStateData reports Loading while + // channels are null, so the state has to be initialised too or the spinner never ends. + verify(queryChannelsStateLogic).initializeChannelsIfNeeded() + verify(queryChannelsStateLogic).setLoadingFirstPage(false) + } + + @Test + fun `loadOfflineGroupedChannels stops loading when a concurrent result already populated state`() = runTest { + // Given — applyGroupedResult landed during the DB read; the cache itself has nothing. + whenever(queryChannelsStateLogic.getChannels()) doReturn mapOf("messaging:ch1" to randomChannel(id = "ch1")) + whenever( + queryChannelsDatabaseLogic.fetchChannelsFromCache( + any(), + any(), + ), + ) doReturn null + + // When + logic.loadOfflineGroupedChannels() + + // Then — there are channels on screen, so the loader must not stay up over them. verify(queryChannelsStateLogic).setLoadingFirstPage(false) } @@ -372,6 +482,90 @@ internal class QueryChannelsLogicGroupedTest { // endregion + // region channelsStateData contract + + @Test + fun `an in-flight first page holds Loading on a cache miss and ends on failure`() = runTest { + // Given — real state rather than a mock, so the assertion is the sequence the UI sees + // rather than which setters happened to be called. + val mutableState = QueryChannelsMutableState( + identifier = QueryChannelsIdentifier.Grouped(GROUP_KEY), + scope = testCoroutines.scope, + latestUsers = MutableStateFlow(emptyMap()), + activeLiveLocations = MutableStateFlow(emptyList()), + ) + val stateLogic = QueryChannelsStateLogic( + mutableState = mutableState, + stateRegistry = mock(), + logicRegistry = mock(), + coroutineScope = testCoroutines.scope, + isLocalUnreadCountEnabled = false, + ) + val realLogic = QueryChannelsLogic( + identifier = QueryChannelsIdentifier.Grouped(GROUP_KEY), + client = client, + queryChannelsStateLogic = stateLogic, + queryChannelsDatabaseLogic = queryChannelsDatabaseLogic, + ) + whenever( + queryChannelsDatabaseLogic.fetchChannelsFromCache( + any(), + any(), + ), + ) doReturn null + + // When — the request starts, then the offline read misses + realLogic.startLoadingFirstPageIfNeverLoaded() + realLogic.loadOfflineGroupedChannels() + + // Then — still Loading. Reinstating the unconditional clear in loadOfflineGroupedChannels + // turns this into OfflineNoResults, which is the flash this change removes. + assertEquals(ChannelsStateData.Loading, mutableState.channelsStateData.value) + + // And when the request fails, the load ends rather than leaving the list spinning. + realLogic.finishFirstPageLoad(completed = false) + assertEquals(ChannelsStateData.OfflineNoResults, mutableState.channelsStateData.value) + } + + @Test + fun `a request after the offline read still raises the loader`() = runTest { + // Given — the offline read runs first and misses, so the channel map is already non-null. + val mutableState = QueryChannelsMutableState( + identifier = QueryChannelsIdentifier.Grouped(GROUP_KEY), + scope = testCoroutines.scope, + latestUsers = MutableStateFlow(emptyMap()), + activeLiveLocations = MutableStateFlow(emptyList()), + ) + val realLogic = QueryChannelsLogic( + identifier = QueryChannelsIdentifier.Grouped(GROUP_KEY), + client = client, + queryChannelsStateLogic = QueryChannelsStateLogic( + mutableState = mutableState, + stateRegistry = mock(), + logicRegistry = mock(), + coroutineScope = testCoroutines.scope, + isLocalUnreadCountEnabled = false, + ), + queryChannelsDatabaseLogic = queryChannelsDatabaseLogic, + ) + whenever( + queryChannelsDatabaseLogic.fetchChannelsFromCache( + any(), + any(), + ), + ) doReturn null + realLogic.loadOfflineGroupedChannels() + assertEquals(ChannelsStateData.OfflineNoResults, mutableState.channelsStateData.value) + + // When + realLogic.startLoadingFirstPageIfNeverLoaded() + + // Then — a non-null empty map no longer blocks the raise; only a finished query does. + assertEquals(ChannelsStateData.Loading, mutableState.channelsStateData.value) + } + + // endregion + private companion object { private const val GROUP_KEY = "test-group" } From c43a944690787e1e09e8c7aa297aa5a584831823 Mon Sep 17 00:00:00 2001 From: Gian <47775302+gpunto@users.noreply.github.com> Date: Wed, 2 Sep 2026 09:58:18 +0200 Subject: [PATCH 2/3] state: Pin grouped first-page behaviour when two requests overlap Nothing dedupes concurrent grouped queries, so two first pages for one group can be in flight together. The loading flag is shared, so the earlier completion ends the load while the later request is still running, and the list shows the empty state for that window. That window is the behaviour this branch removes elsewhere, narrowed to the overlap, and it recovers on its own once the later request answers. Gating the clear on an in-flight count would close it, but the count is incremented in doOnStart and decremented in doOnResult, and DoOnResultCall.cancel cancels the scope the consumer runs in. A leaked start would never return to zero, so the loader would stop clearing at all, including on a later success. The test records that trade so a future count cannot land silently. --- .../internal/QueryChannelsLogicGroupedTest.kt | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt index 8c105f47340f..eaa705f266c7 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt @@ -20,6 +20,7 @@ import io.getstream.chat.android.client.ChatClient import io.getstream.chat.android.client.api.state.ChannelsStateData import io.getstream.chat.android.client.api.state.QueryChannelsState import io.getstream.chat.android.client.internal.state.plugin.QueryChannelsIdentifier +import io.getstream.chat.android.client.internal.state.plugin.logic.internal.LogicRegistry import io.getstream.chat.android.client.internal.state.plugin.state.querychannels.internal.QueryChannelsMutableState import io.getstream.chat.android.client.query.QueryChannelsSpec import io.getstream.chat.android.client.query.pagination.AnyChannelPaginationRequest @@ -564,6 +565,68 @@ internal class QueryChannelsLogicGroupedTest { assertEquals(ChannelsStateData.Loading, mutableState.channelsStateData.value) } + @Test + fun `two overlapping first pages, the earlier one failing, end on the later result`() = runTest { + // Given — nothing dedupes concurrent grouped requests, so two first pages for one group can + // be in flight together: an app query and a SyncManager recovery, for instance. + val mutableState = QueryChannelsMutableState( + identifier = QueryChannelsIdentifier.Grouped(GROUP_KEY), + scope = testCoroutines.scope, + latestUsers = MutableStateFlow(emptyMap()), + activeLiveLocations = MutableStateFlow(emptyList()), + ) + // applyGroupedResult writes through to per-channel logic, so the registry needs to answer. + val logicRegistry = mock() + whenever(logicRegistry.channel(any())) doReturn mock() + val realLogic = QueryChannelsLogic( + identifier = QueryChannelsIdentifier.Grouped(GROUP_KEY), + client = client, + queryChannelsStateLogic = QueryChannelsStateLogic( + mutableState = mutableState, + stateRegistry = mock(), + logicRegistry = logicRegistry, + coroutineScope = testCoroutines.scope, + isLocalUnreadCountEnabled = false, + ), + queryChannelsDatabaseLogic = queryChannelsDatabaseLogic, + ) + whenever( + queryChannelsDatabaseLogic.fetchChannelsFromCache( + any(), + any(), + ), + ) doReturn null + + // When — both requests start and the offline read misses + realLogic.startLoadingFirstPageIfNeverLoaded() + realLogic.startLoadingFirstPageIfNeverLoaded() + realLogic.loadOfflineGroupedChannels() + assertEquals(ChannelsStateData.Loading, mutableState.channelsStateData.value) + + // And the first request fails while the second is still running + realLogic.finishFirstPageLoad(completed = false) + + // Then — the loader drops for the rest of the second request. The flag is shared, so the + // earlier completion ends it; this is the pre-fix empty state, narrowed to that window. + // Gating the clear on an in-flight count would avoid it, but a count that leaks a start + // never returns to zero and the loader would never drop again. + assertEquals(ChannelsStateData.OfflineNoResults, mutableState.channelsStateData.value) + + // And when the second request answers, its channels land. The list recovers on its own, + // which is what keeps the window above acceptable rather than a stuck spinner. + val channels = listOf(randomChannel(id = "ch1")) + realLogic.applyGroupedResult( + group = GroupedChannelsGroup( + groupKey = GROUP_KEY, + channels = channels, + next = null, + prev = null, + ), + isFirstPage = true, + ) + assertEquals(ChannelsStateData.Result(channels), mutableState.channelsStateData.value) + } + // endregion private companion object { From 0f37aa3ef8a067660346504d2f4b26fee30677c1 Mon Sep 17 00:00:00 2001 From: Gian <47775302+gpunto@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:11:51 +0200 Subject: [PATCH 3/3] state: Address review on the grouped first-page loader - Name the omitted-group branch as the guard it is. The server answers every requested group, with an empty channel list when the group has nothing, so it is expected to be a no-op; it stays because a group the response left out would never reach applyGroupedResult and its loader would never come down. - Note in KDoc that an explicit Call.cancel between the raise and the result leaves the group on the loader until a later grouped query finishes. Cancelling the calling coroutine or its scope still runs the result listener, and nothing in the SDK cancels this call. - Set hasCompletedAQuery inside groupedResultMutex, matching applyGroupedResult, so both writers hold the lock. - Drop a test that duplicated the stub, call and assertion of the one below it. --- .../internal/QueryGroupedChannelsListenerState.kt | 9 ++++++--- .../querychannels/internal/QueryChannelsLogic.kt | 12 +++++++++--- .../internal/QueryChannelsLogicGroupedTest.kt | 9 --------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt index 4057d1a51b7c..4f5fa9f9947c 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt @@ -99,12 +99,15 @@ internal class QueryGroupedChannelsListenerState( globalState.setGroupedUnreadChannels(next) } + // Defensive: today the server answers every requested group, with an empty channel list + // when the group has nothing, so this is expected to be a no-op. It stays because a group + // the response left out would never reach applyGroupedResult, and the loader it raised + // would never come down. + finishFirstPageLoads(groups.orEmpty().keys - result.value.groups.keys, groups, completed = true) + // Route each returned group's channels into the per-group state. The captured config lets // both ChannelListViewModel.loadMoreGroupedChannels and SyncManager.updateGroupedQueryChannels // reuse the caller's original parameters on paginated and recovery calls respectively. - // A requested group the response omits never reaches applyGroupedResult, so end it here. - finishFirstPageLoads(groups.orEmpty().keys - result.value.groups.keys, groups, completed = true) - result.value.groups.forEach { (key, group) -> // A request without `next`/`prev` cursors for this key (or no per-group query at all) // is a first-page request → replace channels. With a cursor → paginated → append. diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt index eb3d8ded7216..17011c3cc7a6 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt @@ -171,6 +171,11 @@ internal class QueryChannelsLogic( * [ChannelsStateData] reports `Loading` whenever the flag is set, regardless of content. * * Shares [groupedResultMutex] so the check cannot read a half-applied update. + * + * The raise is undone by the matching result, so an explicit `Call.cancel()` between the two + * leaves the group on the loader until a later grouped query finishes. Cancelling the calling + * coroutine or its scope is fine, the result listener still runs, and nothing in the SDK + * cancels this call. */ internal suspend fun startLoadingFirstPageIfNeverLoaded() { groupedResultMutex.withLock { @@ -181,8 +186,8 @@ internal class QueryChannelsLogic( } /** - * Ends a grouped first-page load that produced no channels of its own: a failed request, or a - * requested group the response left out. + * Ends a grouped first-page load that produced no channels of its own: a failed request, or the + * defensive case of a requested group the response left out. * * Both steps are needed to leave `Loading`, which [ChannelsStateData] reports while the flag is * set *or* while channels are still null. @@ -193,8 +198,9 @@ internal class QueryChannelsLogic( * standard list after a failed empty first page. */ internal suspend fun finishFirstPageLoad(completed: Boolean) { - if (completed) hasCompletedAQuery = true groupedResultMutex.withLock { + // Set with the clear, matching applyGroupedResult, so both writers hold the lock. + if (completed) hasCompletedAQuery = true queryChannelsStateLogic.initializeChannelsIfNeeded() queryChannelsStateLogic.setLoadingFirstPage(false) } diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt index eaa705f266c7..a8ce48b99149 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.kt @@ -380,15 +380,6 @@ internal class QueryChannelsLogicGroupedTest { verify(queryChannelsStateLogic).setLoadingFirstPage(true) } - @Test - fun `startLoadingFirstPageIfNeverLoaded does not hide an already populated list behind a loader`() = runTest { - whenever(queryChannelsStateLogic.getChannels()) doReturn mapOf("messaging:ch1" to randomChannel(id = "ch1")) - - logic.startLoadingFirstPageIfNeverLoaded() - - verify(queryChannelsStateLogic, never()).setLoadingFirstPage(any()) - } - @Test fun `cached channels are not hidden behind a loader before the first query finishes`() = runTest { // Given — the offline read populated the list; no query has completed yet.