From de7835baec62e580376fc1e1ee6f9a37758ebea4 Mon Sep 17 00:00:00 2001 From: gunjanjaswal Date: Sun, 16 Aug 2026 11:25:10 +0530 Subject: [PATCH] Order DMs too, and add an O(1) in-order fast path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two refinements from review, matching how iOS handles this in ConversationStore: - Private chats now insert by source packet timestamp as well. iOS routes .direct through the same insert path; leaving DMs on append order kept the #525 bug for private chats, and a store-forwarded DM backlog (the courier path's whole purpose) is the likelier case. Wired through both MessageManager.addPrivateMessage/…NoUnread and AppStateStore.addPrivateMessage. - insertByTimestamp appends in O(1) when the message is at or after the tail, and only binary-searches when it predates the tail. In-order arrival is the common case, so the hot path stays what it was; mirrors iOS's fast path. Tests: a store-forwarded DM backlog orders by timestamp, and a mixed sequence exercises both the fast-path append and the binary-search branch. --- .../bitchat/android/services/AppStateStore.kt | 8 +++- .../com/bitchat/android/ui/MessageManager.kt | 4 +- .../bitchat/android/util/MessageOrdering.kt | 10 +++++ .../android/services/MessageOrderingTest.kt | 37 +++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/services/AppStateStore.kt b/app/src/main/java/com/bitchat/android/services/AppStateStore.kt index 5ca4fd79..ca228792 100644 --- a/app/src/main/java/com/bitchat/android/services/AppStateStore.kt +++ b/app/src/main/java/com/bitchat/android/services/AppStateStore.kt @@ -103,8 +103,12 @@ object AppStateStore { if (seenMessageIds.contains(msg.id)) return seenMessageIds.add(msg.id) val map = _privateMessages.value.toMutableMap() - val list = (map[peerID] ?: emptyList()) + msg - map[peerID] = list + // Order DMs by source packet timestamp too — a store-forwarded private + // backlog is exactly what the courier path delivers, so appending would + // misorder it the way it did public timelines (#525). Matches iOS, which + // routes .direct through the same ConversationStore.insert as everything else. + map[peerID] = com.bitchat.android.util.MessageOrdering + .withMessageInserted(map[peerID] ?: emptyList(), msg) _privateMessages.value = map } } diff --git a/app/src/main/java/com/bitchat/android/ui/MessageManager.kt b/app/src/main/java/com/bitchat/android/ui/MessageManager.kt index 51dbc3c1..bee32a7d 100644 --- a/app/src/main/java/com/bitchat/android/ui/MessageManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/MessageManager.kt @@ -108,7 +108,7 @@ class MessageManager(private val state: ChatState) { } val chatMessages = currentPrivateChats[peerID]?.toMutableList() ?: mutableListOf() - chatMessages.add(message) + com.bitchat.android.util.MessageOrdering.insertByTimestamp(chatMessages, message) currentPrivateChats[peerID] = chatMessages state.setPrivateChats(currentPrivateChats) // Reflect into process-wide store @@ -129,7 +129,7 @@ class MessageManager(private val state: ChatState) { currentPrivateChats[peerID] = mutableListOf() } val chatMessages = currentPrivateChats[peerID]?.toMutableList() ?: mutableListOf() - chatMessages.add(message) + com.bitchat.android.util.MessageOrdering.insertByTimestamp(chatMessages, message) currentPrivateChats[peerID] = chatMessages state.setPrivateChats(currentPrivateChats) // Reflect into process-wide store diff --git a/app/src/main/java/com/bitchat/android/util/MessageOrdering.kt b/app/src/main/java/com/bitchat/android/util/MessageOrdering.kt index fb5d6b03..9cb9dbe2 100644 --- a/app/src/main/java/com/bitchat/android/util/MessageOrdering.kt +++ b/app/src/main/java/com/bitchat/android/util/MessageOrdering.kt @@ -19,9 +19,19 @@ object MessageOrdering { * through this path), so it can binary-search the insertion point and stay cheap on long lists. * Ordering is stable: a message whose timestamp equals existing ones is placed after them, so * equal timestamps keep insertion order. + * + * In-order arrival is overwhelmingly the common case, so a message at or after the current tail + * is appended in O(1) and skips the search; only an out-of-order message (older than the tail — + * e.g. a store-forwarded backlog) pays for the binary search. Mirrors iOS + * `ConversationStore.insert(_:)`. */ fun insertByTimestamp(messages: MutableList, message: BitchatMessage) { val ts = message.timestamp.time + val last = messages.lastOrNull() + if (last == null || ts >= last.timestamp.time) { + messages.add(message) + return + } var lo = 0 var hi = messages.size while (lo < hi) { diff --git a/app/src/test/kotlin/com/bitchat/android/services/MessageOrderingTest.kt b/app/src/test/kotlin/com/bitchat/android/services/MessageOrderingTest.kt index 812a04f8..d22bf641 100644 --- a/app/src/test/kotlin/com/bitchat/android/services/MessageOrderingTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/services/MessageOrderingTest.kt @@ -119,4 +119,41 @@ class MessageOrderingTest { assertEquals(listOf(older, existing, incomingSameAsExisting), list) } + + @Test + fun `private timeline orders a store-forwarded DM backlog by timestamp`() { + val now = 1_700_000_000_000L + val peer = "aabbccddeeff0011" + // Same shape as the public case: a current DM is on screen, then the courier + // path replays an hour-old private backlog out of order. iOS orders DMs the + // same way; before this they appended in receive order (#525). + val current = msg("dm-current", now, content = "dm-current") + val oldA = msg("dm-old-a", now - 3_600_000L, content = "dm-old-a") + val oldB = msg("dm-old-b", now - 1_800_000L, content = "dm-old-b") + + AppStateStore.addPrivateMessage(peer, current) + AppStateStore.addPrivateMessage(peer, oldB) + AppStateStore.addPrivateMessage(peer, oldA) + + assertEquals(listOf(oldA, oldB, current), AppStateStore.privateMessages.value[peer]) + } + + @Test + fun `in-order arrivals append while an older-than-tail message still inserts`() { + // Exercises both branches: the in-order fast path (append) and the + // binary-search path taken only when a message predates the tail. + val base = 1_700_000_000_000L + val m1 = msg("m1", base) + val m2 = msg("m2", base + 1_000L) // in order -> fast-path append + val m3 = msg("m3", base + 2_000L) // in order -> fast-path append + val gap = msg("gap", base + 500L) // older than tail -> binary search + + val list = mutableListOf() + MessageOrdering.insertByTimestamp(list, m1) + MessageOrdering.insertByTimestamp(list, m2) + MessageOrdering.insertByTimestamp(list, m3) + MessageOrdering.insertByTimestamp(list, gap) + + assertEquals(listOf(m1, gap, m2, m3), list) + } }