From 45d5d6b71dc0766c120d33b7e2a2c47ed7340b59 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 23 Sep 2026 13:58:37 +0200 Subject: [PATCH 1/2] feat(chat): mark message prefetch requests as such The client fetches a room's messages before the user asks for them, from a push notification and after a conversation list sync, and on the wire those requests are indistinguishable from the ones a user waiting on a chat screen is making. A server operator looking at request volume cannot tell what was speculative and what someone was waiting for. Send prefetch=1 on the chat requests the catch-up makes. The server ignores the parameter - the response is byte for byte the one it sends without it - so this only makes the traffic legible in an access log, where a query parameter needs no configuration to be recorded. The flag travels the same route as markNotificationsAsRead, which already separates these two cases, and defaults to false: the helpers that build the request are shared with the chat the user has open, and labelling those as prefetched would invert the very distinction this is for. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 30 +++++++++--- .../data/network/ChatMessageSyncerTest.kt | 46 +++++++++++++++++++ 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index bb3bc03ad6..e57e3a227b 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -132,10 +132,15 @@ class ChatMessageSyncer @Inject constructor( limit: Int = DEFAULT_MESSAGES_LIMIT, threadId: Long? = null, lastCommonRead: Int? = null, - markNotificationsAsRead: Boolean = true + markNotificationsAsRead: Boolean = true, + prefetch: Boolean = false ): HashMap { val fieldMap = HashMap() + if (prefetch) { + fieldMap["prefetch"] = 1 + } + fieldMap["includeLastKnown"] = if (includeLastKnown) 1 else 0 if (lastKnown != null) { @@ -350,7 +355,8 @@ class ChatMessageSyncer @Inject constructor( target = target, fromMessageId = newestMessageIdFromDb, limit = limit, - markNotificationsAsRead = false + markNotificationsAsRead = false, + prefetch = true ) } else { initialCatchUp( @@ -358,7 +364,8 @@ class ChatMessageSyncer @Inject constructor( limit = limit, lastReadMessage = lastReadMessage, unreadMessages = unreadMessages, - markNotificationsAsRead = false + markNotificationsAsRead = false, + prefetch = true ) } @@ -399,6 +406,7 @@ class ChatMessageSyncer @Inject constructor( unreadMessages: Int = 0, lastCommonRead: Int? = null, markNotificationsAsRead: Boolean = true, + prefetch: Boolean = false, events: Events = NO_EVENTS ): SyncOutcome { val closableBacklogAnchor = lastReadMessage?.takeIf { @@ -413,6 +421,7 @@ class ChatMessageSyncer @Inject constructor( unreadMessages = unreadMessages, lastCommonRead = lastCommonRead, markNotificationsAsRead = markNotificationsAsRead, + prefetch = prefetch, events = events ) } else { @@ -426,7 +435,8 @@ class ChatMessageSyncer @Inject constructor( limit = limit, threadId = target.threadId, lastCommonRead = lastCommonRead, - markNotificationsAsRead = markNotificationsAsRead + markNotificationsAsRead = markNotificationsAsRead, + prefetch = prefetch ), events ) @@ -445,6 +455,7 @@ class ChatMessageSyncer @Inject constructor( unreadMessages: Int, lastCommonRead: Int?, markNotificationsAsRead: Boolean, + prefetch: Boolean, events: Events ): SyncOutcome { Log.d( @@ -462,7 +473,8 @@ class ChatMessageSyncer @Inject constructor( limit = limit, threadId = target.threadId, lastCommonRead = lastCommonRead, - markNotificationsAsRead = markNotificationsAsRead + markNotificationsAsRead = markNotificationsAsRead, + prefetch = prefetch ), events ) @@ -478,6 +490,7 @@ class ChatMessageSyncer @Inject constructor( limit = limit, lastCommonRead = lastCommonRead, markNotificationsAsRead = markNotificationsAsRead, + prefetch = prefetch, events = events ) return SyncOutcome( @@ -510,6 +523,7 @@ class ChatMessageSyncer @Inject constructor( limit: Int = DEFAULT_MESSAGES_LIMIT, lastCommonRead: Int? = null, markNotificationsAsRead: Boolean = true, + prefetch: Boolean = false, events: Events = NO_EVENTS ): SyncOutcome { var anchor = fromMessageId @@ -527,7 +541,8 @@ class ChatMessageSyncer @Inject constructor( limit = limit, threadId = target.threadId, lastCommonRead = lastCommonRead, - markNotificationsAsRead = markNotificationsAsRead + markNotificationsAsRead = markNotificationsAsRead, + prefetch = prefetch ) val roundOutcome = pullAndPersistMessages(target, fieldMap, events) @@ -569,7 +584,8 @@ class ChatMessageSyncer @Inject constructor( limit = limit, threadId = target.threadId, lastCommonRead = lastCommonRead, - markNotificationsAsRead = markNotificationsAsRead + markNotificationsAsRead = markNotificationsAsRead, + prefetch = prefetch ), events ) diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 4503d582f9..5df2da2b6c 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -95,11 +95,57 @@ class ChatMessageSyncerTest { ) assertFalse(fieldMap.containsKey("markNotificationsAsRead")) + assertFalse(fieldMap.containsKey("prefetch")) assertFalse(fieldMap.containsKey("lastKnownMessageId")) assertEquals(0, fieldMap["setReadMarker"]) assertEquals(1, fieldMap["includeLastKnown"]) } + @Test + fun `buildFieldMap marks the request as a prefetch when asked`() { + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = 42, + prefetch = true + ) + + assertEquals(1, fieldMap["prefetch"]) + } + + @Test + fun `catchUpRoom marks its request as a prefetch`() = + runTest { + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(42L) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(block(oldest = 10, newest = 42)))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43)))) + + syncer.catchUpRoom(target()) + + val fieldMapCaptor = argumentCaptor>() + verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) } + assertEquals(1, fieldMapCaptor.firstValue["prefetch"]) + } + + @Test + fun `a fetch for a chat the user is reading is not marked as a prefetch`() = + runTest { + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(block(oldest = 10, newest = 42)))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43)))) + + syncer.tryCloseBacklog(target(), fromMessageId = 42L) + + val fieldMapCaptor = argumentCaptor>() + verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) } + assertFalse(fieldMapCaptor.firstValue.containsKey("prefetch")) + } + @Test fun `catchUpRoom skips without chat-keep-notifications capability`() = runTest { From f85500a5353095a4024afb017873f8485816ae6f Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 23 Sep 2026 14:04:08 +0200 Subject: [PATCH 2/2] feat(chat): let the server turn message preloading off Preloading a conversation's messages before the user opens it spends the server's bandwidth on traffic nobody asked for yet. An operator who would rather not pay that can now say so: the mobile-preload-chat setting turns it off for mobile clients, and the server reports it in the chat config capabilities. Skip the catch-up when the server reports it as false. Anything else preloads - a server that reports it true, and a server too old to know the setting at all, which cannot mean the operator declined something they were never offered. Both entry points check it. The conversation list skips the whole catch-up rather than asking room by room, and the syncer checks it too, because push notifications reach it without passing the list. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 38 ++++++----- .../OfflineFirstConversationsRepository.kt | 6 ++ .../nextcloud/talk/utils/CapabilitiesUtil.kt | 12 ++++ .../data/network/ChatMessageSyncerTest.kt | 64 ++++++++++++++++++- 4 files changed, 102 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index e57e3a227b..5a9b74eeb4 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -21,7 +21,11 @@ import com.nextcloud.talk.data.database.model.ChatMessageEntity import com.nextcloud.talk.data.network.NetworkMonitor import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.models.json.chat.ChatMessageDto +import com.nextcloud.talk.utils.CapabilitiesUtil import com.nextcloud.talk.utils.SpreedFeatures +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicBoolean +import javax.inject.Inject import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -30,9 +34,6 @@ import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.sync.Mutex import retrofit2.HttpException -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.atomic.AtomicBoolean -import javax.inject.Inject /** * The chat message fetch-and-persist core, shared between the chat screen @@ -230,7 +231,8 @@ class ChatMessageSyncer @Inject constructor( * * Requires the chat-keep-notifications capability: without it, a background fetch would * dismiss the user's push notifications for the fetched messages, so the catch-up is skipped - * entirely (same guard as on iOS). + * entirely (same guard as on iOS). It is also skipped when the server turns preloading off + * for mobile clients. * * Reachability is deliberately not pre-checked. Callers are gated by a WorkManager * `NetworkType.CONNECTED` constraint, and a request that fails because the device is offline @@ -246,18 +248,24 @@ class ChatMessageSyncer @Inject constructor( limit: Int = DEFAULT_MESSAGES_LIMIT, lastReadMessage: Int? = null, unreadMessages: Int = 0 - ): SyncOutcome { - if (!target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value)) { - Log.d( - TAG, - "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, " + - "skipping catch-up for ${target.internalConversationId}" - ) - return NOTHING_SYNCED - } + ): SyncOutcome = + when { + !target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value) -> { + Log.d( + TAG, + "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, " + + "skipping catch-up for ${target.internalConversationId}" + ) + NOTHING_SYNCED + } - return coalescedRoomCatchUp(target, limit, lastReadMessage, unreadMessages) - } + !CapabilitiesUtil.isChatPreloadAllowed(target.user.capabilities?.spreedCapability) -> { + Log.d(TAG, "Server turned off preloading, skipping catch-up for ${target.internalConversationId}") + NOTHING_SYNCED + } + + else -> coalescedRoomCatchUp(target, limit, lastReadMessage, unreadMessages) + } /** * Runs at most one catch-up per room at a time. A request arriving while one is running only diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index 47e6b105e3..4aed881b16 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -26,6 +26,7 @@ import com.nextcloud.talk.extensions.isPowerSaveMode import com.nextcloud.talk.logger.Logger import com.nextcloud.talk.models.domain.ConversationModel import com.nextcloud.talk.utils.ApiUtils +import com.nextcloud.talk.utils.CapabilitiesUtil import com.nextcloud.talk.utils.CapabilitiesUtil.isUserStatusAvailable import com.nextcloud.talk.utils.SpreedFeatures import com.nextcloud.talk.utils.withRetry @@ -372,6 +373,11 @@ class OfflineFirstConversationsRepository @Inject constructor( false } + !CapabilitiesUtil.isChatPreloadAllowed(user.capabilities?.spreedCapability) -> { + Log.d(TAG, "Server turned off preloading, skipping message catch-up") + false + } + context.isPowerSaveMode() -> { Log.d(TAG, "Battery saver is active, skipping message catch-up") false diff --git a/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt b/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt index e013e828d2..628e4fa940 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt @@ -301,6 +301,17 @@ object CapabilitiesUtil { return false } + /** + * Whether the server lets mobile clients preload chat messages before the user opens a + * conversation. + * + * True unless the server says `mobile-preload-chat` is false, so a server that does not know + * the setting, does not report it, or has not been asked for its capabilities yet, allows + * preloading. + */ + fun isChatPreloadAllowed(spreedCapabilities: SpreedCapabilityDto?): Boolean = + spreedCapabilities?.config?.get("chat")?.get(MOBILE_PRELOAD_CHAT)?.toString()?.lowercase() != "false" + fun isTranslationsSupported(spreedCapabilities: SpreedCapabilityDto): Boolean = spreedCapabilities.config?.containsKey("chat") == true && spreedCapabilities.config!!["chat"] != null && @@ -418,4 +429,5 @@ object CapabilitiesUtil { private const val SERVER_VERSION_MIN_SUPPORTED = 17 private const val SERVER_VERSION_SUPPORT_WARNING = 26 private const val CONVERSATION_DESCRIPTION_LENGTH_FOR_OLD_SERVER = 500 + private const val MOBILE_PRELOAD_CHAT = "mobile-preload-chat" } diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 5df2da2b6c..90457ed32b 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -21,6 +21,8 @@ import com.nextcloud.talk.models.json.capabilities.SpreedCapabilityDto import com.nextcloud.talk.models.json.chat.ChatMessageDto import com.nextcloud.talk.models.json.chat.ChatOCS import com.nextcloud.talk.models.json.chat.ChatOverall +import com.nextcloud.talk.utils.CapabilitiesUtil +import java.io.IOException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf @@ -48,7 +50,6 @@ import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever import org.mockito.kotlin.wheneverBlocking import retrofit2.Response -import java.io.IOException @Suppress("TooManyFunctions", "LargeClass") class ChatMessageSyncerTest { @@ -146,6 +147,59 @@ class ChatMessageSyncerTest { assertFalse(fieldMapCaptor.firstValue.containsKey("prefetch")) } + @Test + fun `catchUpRoom skips when the server turned preloading off`() = + runTest { + val outcome = syncer.catchUpRoom(target(user(preloadChat = false))) + + assertFalse(outcome.persistedNewMessages) + verifyNoInteractions(network) + } + + @Test + fun `catchUpRoom preloads when the server does not report the setting`() = + runTest { + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(42L) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(block(oldest = 10, newest = 42)))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43)))) + + val outcome = syncer.catchUpRoom(target(user(preloadChat = null))) + + assertTrue(outcome.persistedNewMessages) + } + + @Test + fun `catchUpRoom preloads for a user whose capabilities are not known yet`() = + runTest { + val userWithoutCapabilities = user().copy(capabilities = null) + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(42L) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(block(oldest = 10, newest = 42)))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43)))) + + assertTrue(CapabilitiesUtil.isChatPreloadAllowed(userWithoutCapabilities.capabilities?.spreedCapability)) + } + + @Test + fun `catchUpRoom preloads when the server allows it explicitly`() = + runTest { + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(42L) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(block(oldest = 10, newest = 42)))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43)))) + + val outcome = syncer.catchUpRoom(target(user(preloadChat = true))) + + assertTrue(outcome.persistedNewMessages) + } + @Test fun `catchUpRoom skips without chat-keep-notifications capability`() = runTest { @@ -824,19 +878,23 @@ class ChatMessageSyncerTest { verifyNoInteractions(chatBlocksDao) } - private fun user(withKeepNotificationsCapability: Boolean = true): User { + private fun user(withKeepNotificationsCapability: Boolean = true, preloadChat: Boolean? = null): User { val features = if (withKeepNotificationsCapability) { listOf("chat-keep-notifications") } else { emptyList() } + val chatConfig = preloadChat?.let { hashMapOf("mobile-preload-chat" to it) } return User( id = ACCOUNT_ID, userId = "me", username = "me", baseUrl = "https://server.example.com", capabilities = CapabilitiesDto().apply { - spreedCapability = SpreedCapabilityDto().apply { this.features = features } + spreedCapability = SpreedCapabilityDto().apply { + this.features = features + this.config = chatConfig?.let { hashMapOf("chat" to it) } + } } ) }