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 c4f526753..f1738d7c6 100644 --- a/app/src/main/java/com/bitchat/android/services/AppStateStore.kt +++ b/app/src/main/java/com/bitchat/android/services/AppStateStore.kt @@ -192,7 +192,10 @@ object AppStateStore { if (seenMessageIds.contains(msg.id) || seenPublicMessageKeys.contains(publicKey)) return seenMessageIds.add(msg.id) seenPublicMessageKeys.add(publicKey) - _publicMessages.value = _publicMessages.value + msg + // Insert by source packet timestamp so a store-forwarded/gossip-synced backlog is + // interleaved chronologically instead of appended at the bottom in receive order. + _publicMessages.value = com.bitchat.android.util.MessageOrdering + .withMessageInserted(_publicMessages.value, msg) } } @@ -828,8 +831,8 @@ object AppStateStore { if (seenMessageIds.contains(msg.id)) return seenMessageIds.add(msg.id) val map = _channelMessages.value.toMutableMap() - val list = (map[channel] ?: emptyList()) + msg - map[channel] = list + map[channel] = com.bitchat.android.util.MessageOrdering + .withMessageInserted(map[channel] ?: emptyList(), msg) _channelMessages.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 31e7d1afb..d9fee8984 100644 --- a/app/src/main/java/com/bitchat/android/ui/MessageManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/MessageManager.kt @@ -21,7 +21,9 @@ class MessageManager(private val state: ChatState) { fun addMessage(message: BitchatMessage) { val currentMessages = state.getMessagesValue().toMutableList() - currentMessages.add(message) + // Order by the source packet timestamp so a store-forwarded backlog lands in its + // chronological slot instead of appending at the bottom in receive order. + com.bitchat.android.util.MessageOrdering.insertByTimestamp(currentMessages, message) state.setMessages(currentMessages) // Reflect into process-wide store so snapshot replacements don't drop local outgoing messages try { com.bitchat.android.services.AppStateStore.addPublicMessage(message) } catch (_: Exception) { } @@ -52,7 +54,7 @@ class MessageManager(private val state: ChatState) { } val channelMessageList = currentChannelMessages[channel]?.toMutableList() ?: mutableListOf() - channelMessageList.add(message) + com.bitchat.android.util.MessageOrdering.insertByTimestamp(channelMessageList, message) currentChannelMessages[channel] = channelMessageList state.setChannelMessages(currentChannelMessages) // 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 new file mode 100644 index 000000000..9cb9dbe28 --- /dev/null +++ b/app/src/main/java/com/bitchat/android/util/MessageOrdering.kt @@ -0,0 +1,52 @@ +package com.bitchat.android.util + +import com.bitchat.android.model.BitchatMessage + +/** + * Ordering helpers for chat timelines. + * + * Incoming mesh messages can arrive out of order. When a peer store-forwards or gossip-syncs an + * older backlog after reconnecting, hour-old messages show up after current ones if we simply + * append in receive order. [BitchatMessage.timestamp] carries the source packet time, so inserting + * each message at its timestamp position keeps the visible timeline chronological. + */ +object MessageOrdering { + + /** + * Insert [message] into [messages] so the list stays sorted by [BitchatMessage.timestamp]. + * + * Assumes [messages] is already timestamp-ordered (the timelines it is used on are always built + * 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) { + val mid = (lo + hi) ushr 1 + if (messages[mid].timestamp.time <= ts) lo = mid + 1 else hi = mid + } + messages.add(lo, message) + } + + /** + * Return a new list containing [messages] plus [message], kept sorted by timestamp (stable). + */ + fun withMessageInserted(messages: List, message: BitchatMessage): List { + val result = messages.toMutableList() + insertByTimestamp(result, message) + return result + } +} diff --git a/app/src/test/kotlin/com/bitchat/android/services/MessageOrderingTest.kt b/app/src/test/kotlin/com/bitchat/android/services/MessageOrderingTest.kt new file mode 100644 index 000000000..252914257 --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/services/MessageOrderingTest.kt @@ -0,0 +1,147 @@ +package com.bitchat.android.services + +import com.bitchat.android.model.BitchatMessage +import com.bitchat.android.util.MessageOrdering +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import java.util.Date + +/** + * Regression tests for issue #525 (dups #425/#420/#302): incoming mesh messages were shown in + * receive order instead of by their source packet timestamp, so a store-forwarded/gossip-synced + * backlog appended at the bottom of the timeline interleaved with current messages. + */ +class MessageOrderingTest { + + @Before + fun setUp() { + AppStateStore.clear() + } + + @After + fun tearDown() { + AppStateStore.clear() + } + + private fun msg(id: String, tsMillis: Long, content: String = "m-$id"): BitchatMessage = + BitchatMessage( + id = id, + sender = "alice", + content = content, + timestamp = Date(tsMillis), + senderPeerID = "1122334455667788" + ) + + @Test + fun `public timeline orders a store-forwarded backlog by timestamp`() { + val now = 1_700_000_000_000L + // A current message is already on screen... + val current = msg("current", now, content = "current") + // ...then a peer reconnects and replays an hour-old backlog out of order. + val oldA = msg("old-a", now - 3_600_000L, content = "old-a") + val oldB = msg("old-b", now - 1_800_000L, content = "old-b") + + AppStateStore.addPublicMessage(current) + AppStateStore.addPublicMessage(oldB) + AppStateStore.addPublicMessage(oldA) + + val result = AppStateStore.publicMessages.value + assertEquals(listOf(oldA, oldB, current), result) + // Timestamps must be non-decreasing. + assertEquals(result.sortedBy { it.timestamp.time }, result) + } + + @Test + fun `channel timeline orders messages by timestamp`() { + val now = 1_700_000_000_000L + val c0 = msg("c0", now) + val cOld = msg("c-old", now - 5_000L) + val cMid = msg("c-mid", now - 2_000L) + + AppStateStore.addChannelMessage("#general", c0) + AppStateStore.addChannelMessage("#general", cOld) + AppStateStore.addChannelMessage("#general", cMid) + + assertEquals( + listOf(cOld, cMid, c0), + AppStateStore.channelMessages.value["#general"] + ) + } + + @Test + fun `dedup still drops repeats while keeping timestamp order`() { + val now = 1_700_000_000_000L + val a = msg("a", now) + val b = msg("b", now - 1_000L) + + AppStateStore.addPublicMessage(a) + AppStateStore.addPublicMessage(b) + // Same id arriving again over a second transport path must be ignored. + AppStateStore.addPublicMessage(a.copy(content = "dup-by-id")) + // Same sender/timestamp/content with a fresh android id (request-sync replay) must be ignored. + AppStateStore.addPublicMessage(b.copy(id = "b-replay")) + + assertEquals(listOf(b, a), AppStateStore.publicMessages.value) + } + + @Test + fun `equal timestamps keep insertion order (stable)`() { + val ts = 1_700_000_000_000L + val first = msg("first", ts, content = "first") + val second = msg("second", ts, content = "second") + val third = msg("third", ts, content = "third") + + // Direct helper check: inserting three equal-timestamp messages preserves arrival order. + val list = mutableListOf() + MessageOrdering.insertByTimestamp(list, first) + MessageOrdering.insertByTimestamp(list, second) + MessageOrdering.insertByTimestamp(list, third) + assertEquals(listOf(first, second, third), list) + + // And through the store add-path (distinct ids so dedup does not collapse them). + AppStateStore.addPublicMessage(first) + AppStateStore.addPublicMessage(second) + AppStateStore.addPublicMessage(third) + assertEquals(listOf(first, second, third), AppStateStore.publicMessages.value) + } + + @Test + fun `newer equal-timestamp message sorts after existing ones`() { + val ts = 1_700_000_000_000L + val existing = msg("existing", ts) + val older = msg("older", ts - 10_000L) + + val list = mutableListOf(older, existing) + val incomingSameAsExisting = msg("incoming", ts) + MessageOrdering.insertByTimestamp(list, incomingSameAsExisting) + + assertEquals(listOf(older, existing, incomingSameAsExisting), list) + } + + // Note: private (DM) timelines are intentionally NOT timestamp-ordered here. + // AppStateStore orders them by arrival sequence (PrivateMessageArrivalOrder / + // ContactDirectory.canonicalizePrivateChats), because a peer's clock can't be + // trusted to order a conversation. So this fix covers public and channel + // timelines only; DM ordering is owned by that arrival-order path. + + @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) + } +}