Skip to content

Commit 657f4d1

Browse files
AndyScherzingerAndyScherzinger
committed
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 <info@andy-scherzinger.de>
1 parent 1dfb0f5 commit 657f4d1

4 files changed

Lines changed: 98 additions & 14 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import com.nextcloud.talk.data.database.model.ChatMessageEntity
2121
import com.nextcloud.talk.data.network.NetworkMonitor
2222
import com.nextcloud.talk.data.user.model.User
2323
import com.nextcloud.talk.models.json.chat.ChatMessageJson
24+
import com.nextcloud.talk.utils.CapabilitiesUtil
2425
import com.nextcloud.talk.utils.SpreedFeatures
2526
import kotlinx.coroutines.Dispatchers
2627
import kotlinx.coroutines.delay
@@ -230,7 +231,8 @@ class ChatMessageSyncer @Inject constructor(
230231
*
231232
* Requires the chat-keep-notifications capability: without it, a background fetch would
232233
* dismiss the user's push notifications for the fetched messages, so the catch-up is skipped
233-
* entirely (same guard as on iOS).
234+
* entirely (same guard as on iOS). It is also skipped when the server turns preloading off
235+
* for mobile clients.
234236
*
235237
* Reachability is deliberately not pre-checked. Callers are gated by a WorkManager
236238
* `NetworkType.CONNECTED` constraint, and a request that fails because the device is offline
@@ -246,18 +248,24 @@ class ChatMessageSyncer @Inject constructor(
246248
limit: Int = DEFAULT_MESSAGES_LIMIT,
247249
lastReadMessage: Int? = null,
248250
unreadMessages: Int = 0
249-
): SyncOutcome {
250-
if (!target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value)) {
251-
Log.d(
252-
TAG,
253-
"Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, " +
254-
"skipping catch-up for ${target.internalConversationId}"
255-
)
256-
return NOTHING_SYNCED
257-
}
251+
): SyncOutcome =
252+
when {
253+
!target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value) -> {
254+
Log.d(
255+
TAG,
256+
"Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, " +
257+
"skipping catch-up for ${target.internalConversationId}"
258+
)
259+
NOTHING_SYNCED
260+
}
258261

259-
return coalescedRoomCatchUp(target, limit, lastReadMessage, unreadMessages)
260-
}
262+
!CapabilitiesUtil.isChatPreloadAllowed(target.user.capabilities?.spreedCapability) -> {
263+
Log.d(TAG, "Server turned off preloading, skipping catch-up for ${target.internalConversationId}")
264+
NOTHING_SYNCED
265+
}
266+
267+
else -> coalescedRoomCatchUp(target, limit, lastReadMessage, unreadMessages)
268+
}
261269

262270
/**
263271
* Runs at most one catch-up per room at a time. A request arriving while one is running only

‎app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.nextcloud.talk.extensions.isPowerSaveMode
2626
import com.nextcloud.talk.logger.Logger
2727
import com.nextcloud.talk.models.domain.ConversationModel
2828
import com.nextcloud.talk.utils.ApiUtils
29+
import com.nextcloud.talk.utils.CapabilitiesUtil
2930
import com.nextcloud.talk.utils.CapabilitiesUtil.isUserStatusAvailable
3031
import com.nextcloud.talk.utils.SpreedFeatures
3132
import com.nextcloud.talk.utils.withRetry
@@ -369,6 +370,11 @@ class OfflineFirstConversationsRepository @Inject constructor(
369370
false
370371
}
371372

373+
!CapabilitiesUtil.isChatPreloadAllowed(user.capabilities?.spreedCapability) -> {
374+
Log.d(TAG, "Server turned off preloading, skipping message catch-up")
375+
false
376+
}
377+
372378
context.isPowerSaveMode() -> {
373379
Log.d(TAG, "Battery saver is active, skipping message catch-up")
374380
false

‎app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,17 @@ object CapabilitiesUtil {
301301
return false
302302
}
303303

304+
/**
305+
* Whether the server lets mobile clients preload chat messages before the user opens a
306+
* conversation.
307+
*
308+
* True unless the server says `mobile-preload-chat` is false, so a server that does not know
309+
* the setting, does not report it, or has not been asked for its capabilities yet, allows
310+
* preloading.
311+
*/
312+
fun isChatPreloadAllowed(spreedCapabilities: SpreedCapability?): Boolean =
313+
spreedCapabilities?.config?.get("chat")?.get(MOBILE_PRELOAD_CHAT)?.toString()?.lowercase() != "false"
314+
304315
fun isTranslationsSupported(spreedCapabilities: SpreedCapability): Boolean =
305316
spreedCapabilities.config?.containsKey("chat") == true &&
306317
spreedCapabilities.config!!["chat"] != null &&
@@ -418,4 +429,5 @@ object CapabilitiesUtil {
418429
private const val SERVER_VERSION_MIN_SUPPORTED = 17
419430
private const val SERVER_VERSION_SUPPORT_WARNING = 26
420431
private const val CONVERSATION_DESCRIPTION_LENGTH_FOR_OLD_SERVER = 500
432+
private const val MOBILE_PRELOAD_CHAT = "mobile-preload-chat"
421433
}

‎app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt‎

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import com.nextcloud.talk.data.network.NetworkMonitor
1818
import com.nextcloud.talk.data.user.model.User
1919
import com.nextcloud.talk.models.json.capabilities.Capabilities
2020
import com.nextcloud.talk.models.json.capabilities.SpreedCapability
21+
import com.nextcloud.talk.utils.CapabilitiesUtil
2122
import com.nextcloud.talk.models.json.chat.ChatMessageJson
2223
import com.nextcloud.talk.models.json.chat.ChatOCS
2324
import com.nextcloud.talk.models.json.chat.ChatOverall
@@ -146,6 +147,59 @@ class ChatMessageSyncerTest {
146147
assertFalse(fieldMapCaptor.firstValue.containsKey("prefetch"))
147148
}
148149

150+
@Test
151+
fun `catchUpRoom skips when the server turned preloading off`() =
152+
runTest {
153+
val outcome = syncer.catchUpRoom(target(user(preloadChat = false)))
154+
155+
assertFalse(outcome.persistedNewMessages)
156+
verifyNoInteractions(network)
157+
}
158+
159+
@Test
160+
fun `catchUpRoom preloads when the server does not report the setting`() =
161+
runTest {
162+
whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null))
163+
.thenReturn(42L)
164+
whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L))
165+
.thenReturn(flowOf(listOf(block(oldest = 10, newest = 42))))
166+
wheneverBlocking { network.pullChatMessages(any(), any(), any()) }
167+
.thenReturn(Response.success(overall(message(43))))
168+
169+
val outcome = syncer.catchUpRoom(target(user(preloadChat = null)))
170+
171+
assertTrue(outcome.persistedNewMessages)
172+
}
173+
174+
@Test
175+
fun `catchUpRoom preloads for a user whose capabilities are not known yet`() =
176+
runTest {
177+
val userWithoutCapabilities = user().copy(capabilities = null)
178+
whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null))
179+
.thenReturn(42L)
180+
whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L))
181+
.thenReturn(flowOf(listOf(block(oldest = 10, newest = 42))))
182+
wheneverBlocking { network.pullChatMessages(any(), any(), any()) }
183+
.thenReturn(Response.success(overall(message(43))))
184+
185+
assertTrue(CapabilitiesUtil.isChatPreloadAllowed(userWithoutCapabilities.capabilities?.spreedCapability))
186+
}
187+
188+
@Test
189+
fun `catchUpRoom preloads when the server allows it explicitly`() =
190+
runTest {
191+
whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null))
192+
.thenReturn(42L)
193+
whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L))
194+
.thenReturn(flowOf(listOf(block(oldest = 10, newest = 42))))
195+
wheneverBlocking { network.pullChatMessages(any(), any(), any()) }
196+
.thenReturn(Response.success(overall(message(43))))
197+
198+
val outcome = syncer.catchUpRoom(target(user(preloadChat = true)))
199+
200+
assertTrue(outcome.persistedNewMessages)
201+
}
202+
149203
@Test
150204
fun `catchUpRoom skips without chat-keep-notifications capability`() =
151205
runTest {
@@ -824,19 +878,23 @@ class ChatMessageSyncerTest {
824878
verifyNoInteractions(chatBlocksDao)
825879
}
826880

827-
private fun user(withKeepNotificationsCapability: Boolean = true): User {
881+
private fun user(withKeepNotificationsCapability: Boolean = true, preloadChat: Boolean? = null): User {
828882
val features = if (withKeepNotificationsCapability) {
829883
listOf("chat-keep-notifications")
830884
} else {
831885
emptyList()
832886
}
887+
val chatConfig = preloadChat?.let { hashMapOf<String, Any>("mobile-preload-chat" to it) }
833888
return User(
834889
id = ACCOUNT_ID,
835890
userId = "me",
836891
username = "me",
837892
baseUrl = "https://server.example.com",
838893
capabilities = Capabilities().apply {
839-
spreedCapability = SpreedCapability().apply { this.features = features }
894+
spreedCapability = SpreedCapability().apply {
895+
this.features = features
896+
this.config = chatConfig?.let { hashMapOf("chat" to it) }
897+
}
840898
}
841899
)
842900
}

0 commit comments

Comments
 (0)