Skip to content

Commit 36d0837

Browse files
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 <info@andy-scherzinger.de>
1 parent f7eb4c3 commit 36d0837

2 files changed

Lines changed: 69 additions & 7 deletions

File tree

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,15 @@ class ChatMessageSyncer @Inject constructor(
132132
limit: Int = DEFAULT_MESSAGES_LIMIT,
133133
threadId: Long? = null,
134134
lastCommonRead: Int? = null,
135-
markNotificationsAsRead: Boolean = true
135+
markNotificationsAsRead: Boolean = true,
136+
prefetch: Boolean = false
136137
): HashMap<String, Int> {
137138
val fieldMap = HashMap<String, Int>()
138139

140+
if (prefetch) {
141+
fieldMap["prefetch"] = 1
142+
}
143+
139144
fieldMap["includeLastKnown"] = if (includeLastKnown) 1 else 0
140145

141146
if (lastKnown != null) {
@@ -350,15 +355,17 @@ class ChatMessageSyncer @Inject constructor(
350355
target = target,
351356
fromMessageId = newestMessageIdFromDb,
352357
limit = limit,
353-
markNotificationsAsRead = false
358+
markNotificationsAsRead = false,
359+
prefetch = true
354360
)
355361
} else {
356362
initialCatchUp(
357363
target = target,
358364
limit = limit,
359365
lastReadMessage = lastReadMessage,
360366
unreadMessages = unreadMessages,
361-
markNotificationsAsRead = false
367+
markNotificationsAsRead = false,
368+
prefetch = true
362369
)
363370
}
364371

@@ -399,6 +406,7 @@ class ChatMessageSyncer @Inject constructor(
399406
unreadMessages: Int = 0,
400407
lastCommonRead: Int? = null,
401408
markNotificationsAsRead: Boolean = true,
409+
prefetch: Boolean = false,
402410
events: Events = NO_EVENTS
403411
): SyncOutcome {
404412
val closableBacklogAnchor = lastReadMessage?.takeIf {
@@ -413,6 +421,7 @@ class ChatMessageSyncer @Inject constructor(
413421
unreadMessages = unreadMessages,
414422
lastCommonRead = lastCommonRead,
415423
markNotificationsAsRead = markNotificationsAsRead,
424+
prefetch = prefetch,
416425
events = events
417426
)
418427
} else {
@@ -426,7 +435,8 @@ class ChatMessageSyncer @Inject constructor(
426435
limit = limit,
427436
threadId = target.threadId,
428437
lastCommonRead = lastCommonRead,
429-
markNotificationsAsRead = markNotificationsAsRead
438+
markNotificationsAsRead = markNotificationsAsRead,
439+
prefetch = prefetch
430440
),
431441
events
432442
)
@@ -445,6 +455,7 @@ class ChatMessageSyncer @Inject constructor(
445455
unreadMessages: Int,
446456
lastCommonRead: Int?,
447457
markNotificationsAsRead: Boolean,
458+
prefetch: Boolean,
448459
events: Events
449460
): SyncOutcome {
450461
Log.d(
@@ -462,7 +473,8 @@ class ChatMessageSyncer @Inject constructor(
462473
limit = limit,
463474
threadId = target.threadId,
464475
lastCommonRead = lastCommonRead,
465-
markNotificationsAsRead = markNotificationsAsRead
476+
markNotificationsAsRead = markNotificationsAsRead,
477+
prefetch = prefetch
466478
),
467479
events
468480
)
@@ -478,6 +490,7 @@ class ChatMessageSyncer @Inject constructor(
478490
limit = limit,
479491
lastCommonRead = lastCommonRead,
480492
markNotificationsAsRead = markNotificationsAsRead,
493+
prefetch = prefetch,
481494
events = events
482495
)
483496
return SyncOutcome(
@@ -510,6 +523,7 @@ class ChatMessageSyncer @Inject constructor(
510523
limit: Int = DEFAULT_MESSAGES_LIMIT,
511524
lastCommonRead: Int? = null,
512525
markNotificationsAsRead: Boolean = true,
526+
prefetch: Boolean = false,
513527
events: Events = NO_EVENTS
514528
): SyncOutcome {
515529
var anchor = fromMessageId
@@ -527,7 +541,8 @@ class ChatMessageSyncer @Inject constructor(
527541
limit = limit,
528542
threadId = target.threadId,
529543
lastCommonRead = lastCommonRead,
530-
markNotificationsAsRead = markNotificationsAsRead
544+
markNotificationsAsRead = markNotificationsAsRead,
545+
prefetch = prefetch
531546
)
532547
val roundOutcome = pullAndPersistMessages(target, fieldMap, events)
533548

@@ -569,7 +584,8 @@ class ChatMessageSyncer @Inject constructor(
569584
limit = limit,
570585
threadId = target.threadId,
571586
lastCommonRead = lastCommonRead,
572-
markNotificationsAsRead = markNotificationsAsRead
587+
markNotificationsAsRead = markNotificationsAsRead,
588+
prefetch = prefetch
573589
),
574590
events
575591
)

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,57 @@ class ChatMessageSyncerTest {
9595
)
9696

9797
assertFalse(fieldMap.containsKey("markNotificationsAsRead"))
98+
assertFalse(fieldMap.containsKey("prefetch"))
9899
assertFalse(fieldMap.containsKey("lastKnownMessageId"))
99100
assertEquals(0, fieldMap["setReadMarker"])
100101
assertEquals(1, fieldMap["includeLastKnown"])
101102
}
102103

104+
@Test
105+
fun `buildFieldMap marks the request as a prefetch when asked`() {
106+
val fieldMap = syncer.buildFieldMap(
107+
lookIntoFuture = true,
108+
timeout = 0,
109+
includeLastKnown = false,
110+
lastKnown = 42,
111+
prefetch = true
112+
)
113+
114+
assertEquals(1, fieldMap["prefetch"])
115+
}
116+
117+
@Test
118+
fun `catchUpRoom marks its request as a prefetch`() =
119+
runTest {
120+
whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null))
121+
.thenReturn(42L)
122+
whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L))
123+
.thenReturn(flowOf(listOf(block(oldest = 10, newest = 42))))
124+
wheneverBlocking { network.pullChatMessages(any(), any(), any()) }
125+
.thenReturn(Response.success(overall(message(43))))
126+
127+
syncer.catchUpRoom(target())
128+
129+
val fieldMapCaptor = argumentCaptor<HashMap<String, Int>>()
130+
verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) }
131+
assertEquals(1, fieldMapCaptor.firstValue["prefetch"])
132+
}
133+
134+
@Test
135+
fun `a fetch for a chat the user is reading is not marked as a prefetch`() =
136+
runTest {
137+
whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L))
138+
.thenReturn(flowOf(listOf(block(oldest = 10, newest = 42))))
139+
wheneverBlocking { network.pullChatMessages(any(), any(), any()) }
140+
.thenReturn(Response.success(overall(message(43))))
141+
142+
syncer.tryCloseBacklog(target(), fromMessageId = 42L)
143+
144+
val fieldMapCaptor = argumentCaptor<HashMap<String, Int>>()
145+
verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) }
146+
assertFalse(fieldMapCaptor.firstValue.containsKey("prefetch"))
147+
}
148+
103149
@Test
104150
fun `catchUpRoom skips without chat-keep-notifications capability`() =
105151
runTest {

0 commit comments

Comments
 (0)