Skip to content

Commit 9877751

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 36d0837 commit 9877751

4 files changed

Lines changed: 102 additions & 18 deletions

File tree

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

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ 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.ChatMessageDto
24+
import com.nextcloud.talk.utils.CapabilitiesUtil
2425
import com.nextcloud.talk.utils.SpreedFeatures
26+
import java.util.concurrent.ConcurrentHashMap
27+
import java.util.concurrent.atomic.AtomicBoolean
28+
import javax.inject.Inject
2529
import kotlinx.coroutines.Dispatchers
2630
import kotlinx.coroutines.delay
2731
import kotlinx.coroutines.flow.Flow
@@ -30,9 +34,6 @@ import kotlinx.coroutines.flow.flow
3034
import kotlinx.coroutines.flow.flowOn
3135
import kotlinx.coroutines.sync.Mutex
3236
import retrofit2.HttpException
33-
import java.util.concurrent.ConcurrentHashMap
34-
import java.util.concurrent.atomic.AtomicBoolean
35-
import javax.inject.Inject
3637

3738
/**
3839
* The chat message fetch-and-persist core, shared between the chat screen
@@ -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
@@ -375,6 +376,11 @@ class OfflineFirstConversationsRepository @Inject constructor(
375376
false
376377
}
377378

379+
!CapabilitiesUtil.isChatPreloadAllowed(user.capabilities?.spreedCapability) -> {
380+
Log.d(TAG, "Server turned off preloading, skipping message catch-up")
381+
false
382+
}
383+
378384
context.isPowerSaveMode() -> {
379385
Log.d(TAG, "Battery saver is active, skipping message catch-up")
380386
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: SpreedCapabilityDto?): Boolean =
313+
spreedCapabilities?.config?.get("chat")?.get(MOBILE_PRELOAD_CHAT)?.toString()?.lowercase() != "false"
314+
304315
fun isTranslationsSupported(spreedCapabilities: SpreedCapabilityDto): 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: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import com.nextcloud.talk.models.json.capabilities.SpreedCapabilityDto
2121
import com.nextcloud.talk.models.json.chat.ChatMessageDto
2222
import com.nextcloud.talk.models.json.chat.ChatOCS
2323
import com.nextcloud.talk.models.json.chat.ChatOverall
24+
import com.nextcloud.talk.utils.CapabilitiesUtil
25+
import java.io.IOException
2426
import kotlinx.coroutines.CompletableDeferred
2527
import kotlinx.coroutines.flow.MutableStateFlow
2628
import kotlinx.coroutines.flow.flowOf
@@ -48,7 +50,6 @@ import org.mockito.kotlin.verifyNoInteractions
4850
import org.mockito.kotlin.whenever
4951
import org.mockito.kotlin.wheneverBlocking
5052
import retrofit2.Response
51-
import java.io.IOException
5253

5354
@Suppress("TooManyFunctions", "LargeClass")
5455
class ChatMessageSyncerTest {
@@ -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 = CapabilitiesDto().apply {
839-
spreedCapability = SpreedCapabilityDto().apply { this.features = features }
894+
spreedCapability = SpreedCapabilityDto().apply {
895+
this.features = features
896+
this.config = chatConfig?.let { hashMapOf("chat" to it) }
897+
}
840898
}
841899
)
842900
}

0 commit comments

Comments
 (0)