Keep the loader up while a grouped channel list fetches its first page

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<String>,
+        groups: Map<String, GroupedChannelsGroupQuery>?,
+        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,
@@ -75,14 +99,20 @@ 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.
         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(

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,51 @@ 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.
+     *
+     * 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 {
+            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 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.
+     *
+     * [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) {
+        groupedResultMutex.withLock {
+            // Set with the clear, matching applyGroupedResult, so both writers hold the lock.
+            if (completed) hasCompletedAQuery = true
+            queryChannelsStateLogic.initializeChannelsIfNeeded()
+            queryChannelsStateLogic.setLoadingFirstPage(false)
+        }
+    }
+
     internal fun setCurrentRequest(request: QueryChannelsRequest) {
         queryChannelsStateLogic.setCurrentRequest(request)
     }
@@ -264,6 +323,7 @@ internal class QueryChannelsLogic(
             queryChannelsStateLogic.setLoadingFirstPage(false)
             queryChannelsStateLogic.setLoadingMore(false)
             queryChannelsStateLogic.setRecoveryNeeded(false)
+            hasCompletedAQuery = true
 
             // Persist
             queryChannelsDatabaseLogic.insertQueryChannels(queryChannelsStateLogic.getQuerySpecs())

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<String, QueryChannelsLogic>()
+    private fun logicFor(key: String): QueryChannelsLogic = groupLogics.getOrPut(key) { mock() }
     private val logic: LogicRegistry = mock {
-        on(it.queryChannels(any<QueryChannelsIdentifier>())) doReturn queryChannelsLogic
+        on(it.queryChannels(any<QueryChannelsIdentifier>())) 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<QueryChannelsIdentifier>())
     }
 
     @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)
     }
 }