Mark message prefetch requests and let the server turn preloading off

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
@@ -132,10 +133,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<String, Int> {
         val fieldMap = HashMap<String, Int>()
 
+        if (prefetch) {
+            fieldMap["prefetch"] = 1
+        }
+
         fieldMap["includeLastKnown"] = if (includeLastKnown) 1 else 0
 
         if (lastKnown != null) {
@@ -225,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
@@ -241,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
@@ -350,15 +363,17 @@ class ChatMessageSyncer @Inject constructor(
                 target = target,
                 fromMessageId = newestMessageIdFromDb,
                 limit = limit,
-                markNotificationsAsRead = false
+                markNotificationsAsRead = false,
+                prefetch = true
             )
         } else {
             initialCatchUp(
                 target = target,
                 limit = limit,
                 lastReadMessage = lastReadMessage,
                 unreadMessages = unreadMessages,
-                markNotificationsAsRead = false
+                markNotificationsAsRead = false,
+                prefetch = true
             )
         }
 
@@ -399,6 +414,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 +429,7 @@ class ChatMessageSyncer @Inject constructor(
                 unreadMessages = unreadMessages,
                 lastCommonRead = lastCommonRead,
                 markNotificationsAsRead = markNotificationsAsRead,
+                prefetch = prefetch,
                 events = events
             )
         } else {
@@ -426,7 +443,8 @@ class ChatMessageSyncer @Inject constructor(
                     limit = limit,
                     threadId = target.threadId,
                     lastCommonRead = lastCommonRead,
-                    markNotificationsAsRead = markNotificationsAsRead
+                    markNotificationsAsRead = markNotificationsAsRead,
+                    prefetch = prefetch
                 ),
                 events
             )
@@ -445,6 +463,7 @@ class ChatMessageSyncer @Inject constructor(
         unreadMessages: Int,
         lastCommonRead: Int?,
         markNotificationsAsRead: Boolean,
+        prefetch: Boolean,
         events: Events
     ): SyncOutcome {
         Log.d(
@@ -462,7 +481,8 @@ class ChatMessageSyncer @Inject constructor(
                 limit = limit,
                 threadId = target.threadId,
                 lastCommonRead = lastCommonRead,
-                markNotificationsAsRead = markNotificationsAsRead
+                markNotificationsAsRead = markNotificationsAsRead,
+                prefetch = prefetch
             ),
             events
         )
@@ -478,6 +498,7 @@ class ChatMessageSyncer @Inject constructor(
             limit = limit,
             lastCommonRead = lastCommonRead,
             markNotificationsAsRead = markNotificationsAsRead,
+            prefetch = prefetch,
             events = events
         )
         return SyncOutcome(
@@ -510,6 +531,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 +549,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 +592,8 @@ class ChatMessageSyncer @Inject constructor(
                 limit = limit,
                 threadId = target.threadId,
                 lastCommonRead = lastCommonRead,
-                markNotificationsAsRead = markNotificationsAsRead
+                markNotificationsAsRead = markNotificationsAsRead,
+                prefetch = prefetch
             ),
             events
         )

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

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"
 }

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 {
@@ -95,11 +96,110 @@ 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<HashMap<String, Int>>()
+            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<HashMap<String, Int>>()
+            verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) }
+            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 {
@@ -778,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<String, Any>("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) }
+                }
             }
         )
     }