From 290d72f2b61e7a757261e4f9d37ad72ef355a157 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:03:17 +0200 Subject: [PATCH] fix: make read receipts reliable --- .../android/mesh/BluetoothMeshService.kt | 49 +++++-- .../java/com/bitchat/android/mesh/MeshCore.kt | 34 ++++- .../mesh/RetryingControlPacketSender.kt | 82 ++++++++++++ .../nostr/NostrDirectMessageHandler.kt | 5 +- .../android/services/SeenMessageStore.kt | 59 +++++++-- .../com/bitchat/android/ui/ChatViewModel.kt | 27 +++- .../bitchat/android/ui/MeshDelegateHandler.kt | 77 +++++------ .../bitchat/android/ui/PrivateChatManager.kt | 5 +- .../mesh/RetryingControlPacketSenderTest.kt | 121 ++++++++++++++++++ .../nostr/NostrDirectMessageHandlerTest.kt | 2 +- .../android/services/AppStateStoreTest.kt | 35 +++++ .../MeshDelegateHandlerStateContractTest.kt | 40 +++++- .../android/ui/PrivateChatManagerTest.kt | 49 +++++++ 13 files changed, 512 insertions(+), 73 deletions(-) create mode 100644 app/src/main/java/com/bitchat/android/mesh/RetryingControlPacketSender.kt create mode 100644 app/src/test/kotlin/com/bitchat/android/mesh/RetryingControlPacketSenderTest.kt diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt index 2e0507ab..76ff2dbe 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt @@ -53,6 +53,7 @@ class BluetoothMeshService(private val context: Context) : TransportBridgeServic private val peerManager = PeerManager() private val fragmentManager = FragmentManager() private val serviceScope = CoroutineScope(Dispatchers.IO + SupervisorJob()) + private val readReceiptRetrySender = RetryingControlPacketSender(serviceScope) private val authenticatedPeerStateStore = SecureAuthenticatedPeerStateStore(context) private val authenticatedPeerState by lazy { AuthenticatedPeerStateCoordinator( @@ -500,10 +501,24 @@ class BluetoothMeshService(private val context: Context) : TransportBridgeServic } override fun onDeliveryAckReceived(messageID: String, peerID: String) { + // Status events can arrive while MainActivity has detached the UI delegate. + // Persist first so the next UI collector observes the advancement. + try { + com.bitchat.android.services.AppStateStore.updatePrivateMessageStatus( + messageID, + com.bitchat.android.model.DeliveryStatus.Delivered(peerID, Date()) + ) + } catch (_: Exception) { } delegate?.didReceiveDeliveryAck(messageID, peerID) } override fun onReadReceiptReceived(messageID: String, peerID: String) { + try { + com.bitchat.android.services.AppStateStore.updatePrivateMessageStatus( + messageID, + com.bitchat.android.model.DeliveryStatus.Read(peerID, Date()) + ) + } catch (_: Exception) { } delegate?.didReceiveReadReceipt(messageID, peerID) } @@ -1043,12 +1058,6 @@ class BluetoothMeshService(private val context: Context) : TransportBridgeServic } try { - // Avoid duplicate read receipts: check persistent store first - val seenStore = try { com.bitchat.android.services.SeenMessageStore.getInstance(context.applicationContext) } catch (_: Exception) { null } - if (seenStore?.hasRead(messageID) == true) { - return@launch - } - // Create read receipt payload using NoisePayloadType exactly like iOS val readReceiptPayload = com.bitchat.android.model.NoisePayload( type = com.bitchat.android.model.NoisePayloadType.READ_RECEIPT, @@ -1072,10 +1081,30 @@ class BluetoothMeshService(private val context: Context) : TransportBridgeServic // Sign the packet before broadcasting val signedPacket = signPacketBeforeBroadcast(packet) - broadcastRoutedPacket(RoutedPacket(signedPacket)) - - // Persist as read after successful send - try { seenStore?.markRead(messageID) } catch (_: Exception) { } + val retryKey = "$recipientPeerID:$messageID" + readReceiptRetrySender.enqueue( + key = retryKey, + sendAttempt = { attempt -> + // Keep the addressed packet on the normal broadcaster actor so receipt + // attempts are ordered with other BLE traffic and can use mesh routing. + val accepted = broadcastRoutedPacket(RoutedPacket(signedPacket)) + Log.d( + TAG, + "Read receipt attempt $attempt accepted=$accepted " + + "peer=${recipientPeerID.take(8)} message=${messageID.take(8)}" + ) + accepted + }, + onComplete = { accepted -> + if (accepted) { + try { + com.bitchat.android.services.SeenMessageStore + .getInstance(context.applicationContext) + .markReadReceiptSent(messageID) + } catch (_: Exception) { } + } + } + ) } catch (e: Exception) { Log.e(TAG, "Failed to send read receipt to $recipientPeerID: ${e.message}") diff --git a/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt b/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt index 2387e939..d1185428 100644 --- a/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt +++ b/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt @@ -52,6 +52,7 @@ class MeshCore( private val peerManager = PeerManager() val fragmentManager = FragmentManager() + private val readReceiptRetrySender = RetryingControlPacketSender(scope) private val authenticatedPeerStateStore = SecureAuthenticatedPeerStateStore(context) private val authenticatedPeerState by lazy { AuthenticatedPeerStateCoordinator( @@ -398,10 +399,22 @@ class MeshCore( } override fun onDeliveryAckReceived(messageID: String, peerID: String) { + try { + com.bitchat.android.services.AppStateStore.updatePrivateMessageStatus( + messageID, + com.bitchat.android.model.DeliveryStatus.Delivered(peerID, java.util.Date()) + ) + } catch (_: Exception) { } delegate?.didReceiveDeliveryAck(messageID, peerID) } override fun onReadReceiptReceived(messageID: String, peerID: String) { + try { + com.bitchat.android.services.AppStateStore.updatePrivateMessageStatus( + messageID, + com.bitchat.android.model.DeliveryStatus.Read(peerID, java.util.Date()) + ) + } catch (_: Exception) { } delegate?.didReceiveReadReceipt(messageID, peerID) } @@ -691,8 +704,25 @@ class MeshCore( signature = null, ttl = maxTtl ) - dispatchGlobal(RoutedPacket(signPacketBeforeBroadcast(packet))) - hooks.onReadReceiptSent?.invoke(messageID) + val signedPacket = signPacketBeforeBroadcast(packet) + val retryKey = "$recipientPeerID:$messageID" + readReceiptRetrySender.enqueue( + key = retryKey, + sendAttempt = { + dispatchGlobal(RoutedPacket(signedPacket)) + true + }, + onComplete = { accepted -> + if (accepted) { + try { + com.bitchat.android.services.SeenMessageStore + .getInstance(context.applicationContext) + .markReadReceiptSent(messageID) + } catch (_: Exception) { } + hooks.onReadReceiptSent?.invoke(messageID) + } + } + ) } catch (e: Exception) { Log.e("MeshCore", "Failed to send read receipt: ${e.message}") } diff --git a/app/src/main/java/com/bitchat/android/mesh/RetryingControlPacketSender.kt b/app/src/main/java/com/bitchat/android/mesh/RetryingControlPacketSender.kt new file mode 100644 index 00000000..ba81800c --- /dev/null +++ b/app/src/main/java/com/bitchat/android/mesh/RetryingControlPacketSender.kt @@ -0,0 +1,82 @@ +package com.bitchat.android.mesh + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock + +/** + * Sends small idempotent control packets redundantly while serializing the attempts submitted + * through this sender. Android BLE only reports that a GATT write/notification was accepted; + * it does not prove that the remote application processed the packet. Reusing the exact encoded + * packet makes retries safe: the receiver's packet/replay protection drops copies it already saw. + */ +internal class RetryingControlPacketSender( + private val scope: CoroutineScope, + private val maxAttempts: Int = 3, + private val retryDelayMs: Long = 750L, + private val interSendDelayMs: Long = 75L +) { + private val sendMutex = Mutex() + private val jobsLock = Any() + private val activeJobs = mutableMapOf() + + init { + require(maxAttempts > 0) + require(retryDelayMs >= 0) + require(interSendDelayMs >= 0) + } + + /** + * Coalesces concurrent requests for the same logical packet. Once the retry window finishes, + * a later user action may enqueue the packet again; duplicate receipt processing is idempotent. + */ + fun enqueue( + key: String, + sendAttempt: suspend (attempt: Int) -> Boolean, + onComplete: (acceptedAtLeastOnce: Boolean) -> Unit = {} + ) { + val job = synchronized(jobsLock) { + if (activeJobs[key]?.isActive == true) return + + scope.launch(start = CoroutineStart.LAZY) { + var acceptedAtLeastOnce = false + var completed = false + try { + repeat(maxAttempts) { index -> + if (!isActive) return@launch + val attempt = index + 1 + sendMutex.withLock { + try { + val accepted = try { + sendAttempt(attempt) + } catch (_: Exception) { + false + } + acceptedAtLeastOnce = accepted || acceptedAtLeastOnce + } finally { + if (interSendDelayMs > 0) delay(interSendDelayMs) + } + } + if (attempt < maxAttempts && retryDelayMs > 0) { + delay(retryDelayMs) + } + } + completed = true + } finally { + synchronized(jobsLock) { + activeJobs.remove(key) + } + if (completed) { + try { onComplete(acceptedAtLeastOnce) } catch (_: Exception) { } + } + } + }.also { activeJobs[key] = it } + } + job.start() + } +} diff --git a/app/src/main/java/com/bitchat/android/nostr/NostrDirectMessageHandler.kt b/app/src/main/java/com/bitchat/android/nostr/NostrDirectMessageHandler.kt index 469972c8..fce95c2d 100644 --- a/app/src/main/java/com/bitchat/android/nostr/NostrDirectMessageHandler.kt +++ b/app/src/main/java/com/bitchat/android/nostr/NostrDirectMessageHandler.kt @@ -156,7 +156,7 @@ class NostrDirectMessageHandler( ) val isViewing = state.getSelectedPrivateChatPeerValue() == conversationID - val suppressUnread = seenStore.hasRead(pm.messageID) + val suppressUnread = seenStore.hasBeenReadLocally(pm.messageID) withContext(Dispatchers.Main) { privateChatManager.handleIncomingPrivateMessage( @@ -175,7 +175,8 @@ class NostrDirectMessageHandler( if (isViewing && !suppressUnread) { val nostrTransport = NostrTransport.getInstance(application) nostrTransport.sendReadReceiptGeohash(pm.messageID, senderPubkey, recipientIdentity) - seenStore.markRead(pm.messageID) + seenStore.markReadLocally(pm.messageID) + seenStore.markReadReceiptSent(pm.messageID) } } NoisePayloadType.DELIVERED -> { diff --git a/app/src/main/java/com/bitchat/android/services/SeenMessageStore.kt b/app/src/main/java/com/bitchat/android/services/SeenMessageStore.kt index 15e1c7ca..3b531846 100644 --- a/app/src/main/java/com/bitchat/android/services/SeenMessageStore.kt +++ b/app/src/main/java/com/bitchat/android/services/SeenMessageStore.kt @@ -4,9 +4,14 @@ import android.content.Context import android.util.Log import com.bitchat.android.identity.SecureIdentityStateManager import com.google.gson.Gson +import com.google.gson.annotations.SerializedName /** - * Persistent store for message IDs we've already acknowledged (DELIVERED) or READ. + * Persistent store for message IDs we've already acknowledged as delivered, read locally, or + * admitted to a completed read-receipt send window. + * + * Local read state must not be used as proof that a read-receipt packet reached the sender. + * Transport delivery is best-effort and retryable, while local read state drives unread UI. * Limits to last MAX_IDS entries per set to avoid memory bloat. */ class SeenMessageStore private constructor(private val context: Context) { @@ -27,12 +32,14 @@ class SeenMessageStore private constructor(private val context: Context) { private val secure = SecureIdentityStateManager(context) private val delivered = LinkedHashSet(MAX_IDS) - private val read = LinkedHashSet(MAX_IDS) + private val locallyRead = LinkedHashSet(MAX_IDS) + private val readReceiptsSent = LinkedHashSet(MAX_IDS) init { load() } @Synchronized fun hasDelivered(id: String) = delivered.contains(id) - @Synchronized fun hasRead(id: String) = read.contains(id) + @Synchronized fun hasBeenReadLocally(id: String) = locallyRead.contains(id) + @Synchronized fun hasReadReceiptBeenSent(id: String) = readReceiptsSent.contains(id) @Synchronized fun markDelivered(id: String) { if (delivered.remove(id)) delivered.add(id) else { @@ -42,17 +49,26 @@ class SeenMessageStore private constructor(private val context: Context) { persist() } - @Synchronized fun markRead(id: String) { - if (read.remove(id)) read.add(id) else { - read.add(id) - trim(read) + @Synchronized fun markReadLocally(id: String) { + if (locallyRead.remove(id)) locallyRead.add(id) else { + locallyRead.add(id) + trim(locallyRead) + } + persist() + } + + @Synchronized fun markReadReceiptSent(id: String) { + if (readReceiptsSent.remove(id)) readReceiptsSent.add(id) else { + readReceiptsSent.add(id) + trim(readReceiptsSent) } persist() } @Synchronized fun clear() { delivered.clear() - read.clear() + locallyRead.clear() + readReceiptsSent.clear() persist() } @@ -68,10 +84,19 @@ class SeenMessageStore private constructor(private val context: Context) { try { val json = secure.getSecureValue(STORAGE_KEY) ?: return val data = gson.fromJson(json, StorePayload::class.java) ?: return - delivered.clear(); read.clear() + delivered.clear(); locallyRead.clear(); readReceiptsSent.clear() data.delivered.takeLast(MAX_IDS).forEach { delivered.add(it) } - data.read.takeLast(MAX_IDS).forEach { read.add(it) } - Log.d(TAG, "Loaded delivered=${delivered.size}, read=${read.size}") + data.locallyRead.takeLast(MAX_IDS).forEach { locallyRead.add(it) } + // Older payloads used the local-read set to suppress receipt sends. Seed the new + // explicit set once during migration to avoid replaying an entire chat history. + (data.readReceiptsSent ?: data.locallyRead) + .takeLast(MAX_IDS) + .forEach { readReceiptsSent.add(it) } + Log.d( + TAG, + "Loaded delivered=${delivered.size}, locallyRead=${locallyRead.size}, " + + "readReceiptsSent=${readReceiptsSent.size}" + ) } catch (e: Exception) { Log.e(TAG, "Failed to load SeenMessageStore: ${e.message}") } @@ -79,7 +104,11 @@ class SeenMessageStore private constructor(private val context: Context) { @Synchronized private fun persist() { try { - val payload = StorePayload(delivered.toList(), read.toList()) + val payload = StorePayload( + delivered = delivered.toList(), + locallyRead = locallyRead.toList(), + readReceiptsSent = readReceiptsSent.toList() + ) val json = gson.toJson(payload) secure.storeSecureValue(STORAGE_KEY, json) } catch (e: Exception) { @@ -89,6 +118,10 @@ class SeenMessageStore private constructor(private val context: Context) { private data class StorePayload( val delivered: List = emptyList(), - val read: List = emptyList() + // Keep the existing JSON field name for backward-compatible secure-store migration. + @SerializedName("read") + val locallyRead: List = emptyList(), + @SerializedName("read_receipts_sent") + val readReceiptsSent: List? = null ) } diff --git a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt index eb9e8ee5..c2ef7ae8 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt @@ -99,6 +99,9 @@ class ChatViewModel( // Specialized managers private val dataManager = DataManager(application.applicationContext) private val identityManager by lazy { SecureIdentityStateManager(getApplication()) } + private val seenMessageStore by lazy { + com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) + } private val messageManager = MessageManager(state) private val channelManager = ChannelManager(state, messageManager, dataManager, viewModelScope) @@ -109,7 +112,13 @@ class ChatViewModel( override fun getMyPeerID(): String = mesh.myPeerID } - val privateChatManager = PrivateChatManager(state, messageManager, dataManager, noiseSessionDelegate) + val privateChatManager = PrivateChatManager( + state, + messageManager, + dataManager, + noiseSessionDelegate, + hasReadReceiptBeenSent = seenMessageStore::hasReadReceiptBeenSent + ) private val commandProcessor = CommandProcessor(state, messageManager, channelManager, privateChatManager) private val notificationManager = NotificationManager( application.applicationContext, @@ -146,7 +155,8 @@ class ChatViewModel( coroutineScope = viewModelScope, onHapticFeedback = { ChatViewModelUtils.triggerHapticFeedback(application.applicationContext) }, getMyPeerID = { mesh.myPeerID }, - getMeshService = { mesh } + getMeshService = { mesh }, + markMessageReadLocally = seenMessageStore::markReadLocally ) // New Geohash architecture ViewModel (replaces God object service usage in UI path) @@ -240,11 +250,17 @@ class ChatViewModel( state.setPrivateChats(canonicalChats) // Recompute unread set using SeenMessageStore for robustness across Activity recreation try { - val seen = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) val myNick = state.getNicknameValue() ?: mesh.myPeerID val unread = mutableSetOf() canonicalChats.forEach { (peer, list) -> - if (list.any { msg -> msg.sender != myNick && msg.sender != "system" && !seen.hasRead(msg.id) }) unread.add(peer) + if (list.any { msg -> + msg.sender != myNick && + msg.sender != "system" && + !seenMessageStore.hasBeenReadLocally(msg.id) + } + ) { + unread.add(peer) + } } state.setUnreadPrivateMessages(unread) } catch (_: Exception) { } @@ -405,11 +421,10 @@ class ChatViewModel( // Persistently mark all messages in this conversation as read so Nostr fetches // after app restarts won't re-mark them as unread. try { - val seen = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) val chats = state.getPrivateChatsValue() val messages = chats[conversationID] ?: emptyList() messages.forEach { msg -> - try { seen.markRead(msg.id) } catch (_: Exception) { } + try { seenMessageStore.markReadLocally(msg.id) } catch (_: Exception) { } } } catch (_: Exception) { } } diff --git a/app/src/main/java/com/bitchat/android/ui/MeshDelegateHandler.kt b/app/src/main/java/com/bitchat/android/ui/MeshDelegateHandler.kt index f98c5c87..6b371710 100644 --- a/app/src/main/java/com/bitchat/android/ui/MeshDelegateHandler.kt +++ b/app/src/main/java/com/bitchat/android/ui/MeshDelegateHandler.kt @@ -22,7 +22,8 @@ class MeshDelegateHandler( private val coroutineScope: CoroutineScope, private val onHapticFeedback: () -> Unit, private val getMyPeerID: () -> String, - private val getMeshService: () -> MeshService + private val getMeshService: () -> MeshService, + private val markMessageReadLocally: (messageID: String) -> Unit = {} ) : BluetoothMeshDelegate { override fun didReceiveMessage(message: BitchatMessage) { @@ -247,46 +248,50 @@ class MeshDelegateHandler( val shouldSendReadReceipt = !isAppInBackground && senderConversationID != null && focusedConversationID == senderConversationID - - if (shouldSendReadReceipt) { - android.util.Log.d( - "MeshDelegateHandler", - "Sending reactive read receipt for focused chat with $senderConversationID (message=${message.id})" - ) - val nickname = state.getNicknameValue() ?: "unknown" - val mesh = getMeshService() - val sent = try { - val meshPeerID = senderConversationID - ?.let { ContactDirectory.resolve(it).meshPeerID } - ?: senderPeerID?.takeIf { - com.bitchat.android.services.ContactIdentityResolver.isMeshPeerId(it) - } - if (meshPeerID != null && - mesh.getPeerInfo(meshPeerID)?.isConnected == true && - mesh.hasEstablishedSession(meshPeerID) - ) { - mesh.sendReadReceipt(message.id, meshPeerID, nickname) - true - } else { - false + + if (shouldSendReadReceipt) { + android.util.Log.d( + "MeshDelegateHandler", + "Sending reactive read receipt for focused chat with $senderConversationID (message=${message.id})" + ) + val nickname = state.getNicknameValue().ifBlank { "unknown" } + val mesh = getMeshService() + val sent = try { + val meshPeerID = ContactDirectory.resolve(senderConversationID).meshPeerID + ?: senderPeerID.takeIf { + com.bitchat.android.services.ContactIdentityResolver.isMeshPeerId(it) } - } catch (_: Exception) { + if (meshPeerID != null && + mesh.getPeerInfo(meshPeerID)?.isConnected == true && + mesh.hasEstablishedSession(meshPeerID) + ) { + mesh.sendReadReceipt(message.id, meshPeerID, nickname) + true + } else { false } - if (sent) { - // Ensure unread badge is cleared for this peer immediately - try { - val current = state.getUnreadPrivateMessagesValue().toMutableSet() - val changed = current.remove(senderPeerID) or current.remove(senderConversationID) - if (changed) { - state.setUnreadPrivateMessages(current) - } - } catch (_: Exception) { } - } - } else { - android.util.Log.d("MeshDelegateHandler", "Skipping read receipt - chat not focused (background: $isAppInBackground, current peer: $currentPrivateChatPeer, sender: $senderPeerID)") + } catch (_: Exception) { + false } + if (sent) { + // Receipt scheduling and local-read persistence are separate facts. Record + // the UI state only after the retryable receipt has been admitted. + try { markMessageReadLocally(message.id) } catch (_: Exception) { } + + // Ensure unread badge is cleared for this peer immediately + try { + val current = state.getUnreadPrivateMessagesValue().toMutableSet() + val changed = current.remove(senderPeerID) or current.remove(senderConversationID) + if (changed) { + state.setUnreadPrivateMessages(current) + } + } catch (_: Exception) { } + } + } else { + android.util.Log.d("MeshDelegateHandler", "Skipping read receipt - chat not focused (background: $isAppInBackground, current peer: $currentPrivateChatPeer, sender: $senderPeerID)") } + } + /** * Expose mesh peer info for components that need to resolve identities (e.g., Nostr mapping) */ diff --git a/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt b/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt index 1af48a04..ff182cf8 100644 --- a/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt @@ -33,7 +33,8 @@ class PrivateChatManager( private val state: ChatState, private val messageManager: MessageManager, private val dataManager: DataManager, - private val noiseSessionDelegate: NoiseSessionDelegate + private val noiseSessionDelegate: NoiseSessionDelegate, + private val hasReadReceiptBeenSent: (messageID: String) -> Boolean = { false } ) { companion object { @@ -398,7 +399,7 @@ class PrivateChatManager( senderPeerID == meshPeerID || ContactDirectory.canonicalConversationId(senderPeerID) == canonicalConversationID ) - if (isFromTarget && meshPeerID != null) { + if (isFromTarget && meshPeerID != null && !hasReadReceiptBeenSent(msg.id)) { try { if (hasMesh) { meshService.sendReadReceipt(msg.id, meshPeerID, myNickname) diff --git a/app/src/test/kotlin/com/bitchat/android/mesh/RetryingControlPacketSenderTest.kt b/app/src/test/kotlin/com/bitchat/android/mesh/RetryingControlPacketSenderTest.kt new file mode 100644 index 00000000..7503b6bb --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/mesh/RetryingControlPacketSenderTest.kt @@ -0,0 +1,121 @@ +package com.bitchat.android.mesh + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class RetryingControlPacketSenderTest { + @Test + fun `control packet is sent for the full redundant retry window`() = runTest { + val attempts = mutableListOf() + val sender = RetryingControlPacketSender( + scope = this, + maxAttempts = 3, + retryDelayMs = 10, + interSendDelayMs = 1 + ) + + sender.enqueue( + key = "peer:message", + sendAttempt = { attempt -> + attempts += attempt + true + } + ) + advanceUntilIdle() + + assertEquals(listOf(1, 2, 3), attempts) + } + + @Test + fun `duplicate enqueue is coalesced while receipt retry is active`() = runTest { + var firstRequestAttempts = 0 + var duplicateRequestAttempts = 0 + val sender = RetryingControlPacketSender( + scope = this, + maxAttempts = 3, + retryDelayMs = 10, + interSendDelayMs = 1 + ) + + sender.enqueue( + key = "peer:message", + sendAttempt = { + firstRequestAttempts += 1 + true + } + ) + sender.enqueue( + key = "peer:message", + sendAttempt = { + duplicateRequestAttempts += 1 + true + } + ) + advanceUntilIdle() + + assertEquals(3, firstRequestAttempts) + assertEquals(0, duplicateRequestAttempts) + } + + @Test + fun `transport writes for different receipts are serialized`() = runTest { + var activeWrites = 0 + var maximumActiveWrites = 0 + val sender = RetryingControlPacketSender( + scope = this, + maxAttempts = 1, + retryDelayMs = 0, + interSendDelayMs = 0 + ) + + fun enqueue(key: String) { + sender.enqueue( + key = key, + sendAttempt = { + activeWrites += 1 + maximumActiveWrites = maxOf(maximumActiveWrites, activeWrites) + delay(10) + activeWrites -= 1 + true + } + ) + } + + enqueue("peer:first") + enqueue("peer:second") + advanceUntilIdle() + + assertEquals(1, maximumActiveWrites) + } + + @Test + fun `completion reports whether transport accepted any attempt`() = runTest { + val completions = mutableListOf() + val sender = RetryingControlPacketSender( + scope = this, + maxAttempts = 3, + retryDelayMs = 1, + interSendDelayMs = 0 + ) + + sender.enqueue( + key = "peer:rejected", + sendAttempt = { false }, + onComplete = completions::add + ) + sender.enqueue( + key = "peer:eventually-accepted", + sendAttempt = { attempt -> attempt == 2 }, + onComplete = completions::add + ) + advanceUntilIdle() + + assertEquals(2, completions.size) + assertEquals(setOf(false, true), completions.toSet()) + } +} diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/NostrDirectMessageHandlerTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/NostrDirectMessageHandlerTest.kt index 738b3231..a9ded74d 100644 --- a/app/src/test/kotlin/com/bitchat/android/nostr/NostrDirectMessageHandlerTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/nostr/NostrDirectMessageHandlerTest.kt @@ -67,7 +67,7 @@ class NostrDirectMessageHandlerTest { ) val seenStore = mock() whenever(seenStore.hasDelivered(any())).thenReturn(true) - whenever(seenStore.hasRead(any())).thenReturn(false) + whenever(seenStore.hasBeenReadLocally(any())).thenReturn(false) val handler = NostrDirectMessageHandler( application = application, state = state, diff --git a/app/src/test/kotlin/com/bitchat/android/services/AppStateStoreTest.kt b/app/src/test/kotlin/com/bitchat/android/services/AppStateStoreTest.kt index 577823ac..a70d4fa9 100644 --- a/app/src/test/kotlin/com/bitchat/android/services/AppStateStoreTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/services/AppStateStoreTest.kt @@ -1,8 +1,10 @@ package com.bitchat.android.services import com.bitchat.android.model.BitchatMessage +import com.bitchat.android.model.DeliveryStatus import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import java.util.Date @@ -137,4 +139,37 @@ class AppStateStoreTest { assertEquals(listOf(earlier, later), AppStateStore.privateMessages.value[contactID]) } + + @Test + fun `background receipt status persists and cannot be downgraded`() { + val message = BitchatMessage( + id = "outgoing-message", + sender = "bob", + content = "hello", + timestamp = Date(1), + isPrivate = true, + deliveryStatus = DeliveryStatus.Sending + ) + AppStateStore.addPrivateMessage("peer-a", message) + + AppStateStore.updatePrivateMessageStatus( + message.id, + DeliveryStatus.Delivered("peer-a", Date(2)) + ) + AppStateStore.updatePrivateMessageStatus( + message.id, + DeliveryStatus.Read("peer-a", Date(3)) + ) + AppStateStore.updatePrivateMessageStatus( + message.id, + DeliveryStatus.Delivered("peer-a", Date(4)) + ) + + val status = AppStateStore.privateMessages.value + .values + .flatten() + .single() + .deliveryStatus + assertTrue(status is DeliveryStatus.Read) + } } diff --git a/app/src/test/kotlin/com/bitchat/android/ui/MeshDelegateHandlerStateContractTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/MeshDelegateHandlerStateContractTest.kt index 63ba28ef..64d37810 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/MeshDelegateHandlerStateContractTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/MeshDelegateHandlerStateContractTest.kt @@ -1,6 +1,7 @@ package com.bitchat.android.ui import com.bitchat.android.mesh.MeshService +import com.bitchat.android.mesh.PeerInfo import com.bitchat.android.model.BitchatMessage import com.bitchat.android.model.DeliveryStatus import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -16,6 +17,7 @@ import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.times import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import java.util.Date import java.util.concurrent.atomic.AtomicInteger @@ -29,6 +31,7 @@ class MeshDelegateHandlerStateContractTest { private lateinit var mesh: MeshService private lateinit var handler: MeshDelegateHandler private lateinit var haptics: AtomicInteger + private lateinit var locallyReadMessageIDs: MutableList @Before fun setUp() { @@ -41,6 +44,7 @@ class MeshDelegateHandlerStateContractTest { notifications = mock() mesh = mock() haptics = AtomicInteger() + locallyReadMessageIDs = mutableListOf() handler = MeshDelegateHandler( state = state, messageManager = messages, @@ -50,7 +54,8 @@ class MeshDelegateHandlerStateContractTest { coroutineScope = scope, onHapticFeedback = { haptics.incrementAndGet() }, getMyPeerID = { "self" }, - getMeshService = { mesh } + getMeshService = { mesh }, + markMessageReadLocally = locallyReadMessageIDs::add ) } @@ -89,6 +94,39 @@ class MeshDelegateHandlerStateContractTest { assertTrue(state.messages.value.single().deliveryStatus is DeliveryStatus.Read) } + @Test + fun `focused private message schedules receipt before recording local read`() { + val peerID = "1122334455667788" + val incoming = BitchatMessage( + id = "focused-private-message", + sender = "alice", + content = "hello", + timestamp = Date(1), + isPrivate = true, + senderPeerID = peerID + ) + whenever(notifications.getAppBackgroundState()).thenReturn(false) + whenever(notifications.getCurrentPrivateChatPeer()).thenReturn(peerID) + whenever(mesh.getPeerInfo(peerID)).thenReturn( + PeerInfo( + id = peerID, + nickname = "alice", + isConnected = true, + isDirectConnection = true, + noisePublicKey = ByteArray(32) { 1 }, + signingPublicKey = null, + isVerifiedNickname = false, + lastSeen = System.currentTimeMillis() + ) + ) + whenever(mesh.hasEstablishedSession(peerID)).thenReturn(true) + + handler.didReceiveMessage(incoming) + + verify(mesh).sendReadReceipt(incoming.id, peerID, "Résumé") + assertEquals(listOf(incoming.id), locallyReadMessageIDs) + } + @Test fun `unicode mention notifies once and duplicate transport delivery is suppressed`() { val incoming = message( diff --git a/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt index 8e590949..dde48484 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt @@ -15,6 +15,7 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @@ -106,6 +107,54 @@ class PrivateChatManagerTest { verify(meshService).sendReadReceipt(message.id, meshPeerID, "bob") } + @Test + fun `opening chat skips messages whose receipt send already completed`() { + val noiseKey = ByteArray(32) { 8 } + val meshPeerID = ContactIdentityResolver.peerIdForNoiseKey(noiseKey) + val conversationID = ContactIdentityResolver.contactConversationIdForNoiseKey(noiseKey) + val oldMessage = BitchatMessage( + id = "already-read", + sender = "alice", + content = "old", + timestamp = Date(1), + isPrivate = true, + senderPeerID = meshPeerID + ) + val unreadMessage = oldMessage.copy( + id = "still-unread", + content = "new", + timestamp = Date(2) + ) + val meshService = mock() + manager = PrivateChatManager( + state = state, + messageManager = MessageManager(state), + dataManager = DataManager(RuntimeEnvironment.getApplication()), + noiseSessionDelegate = mock(), + hasReadReceiptBeenSent = { it == oldMessage.id } + ) + state.setNickname("bob") + state.setPrivateChats(mapOf(conversationID to listOf(oldMessage, unreadMessage))) + whenever(meshService.getPeerInfo(meshPeerID)).thenReturn( + PeerInfo( + id = meshPeerID, + nickname = "alice", + isConnected = true, + isDirectConnection = true, + noisePublicKey = noiseKey, + signingPublicKey = null, + isVerifiedNickname = false, + lastSeen = System.currentTimeMillis() + ) + ) + whenever(meshService.hasEstablishedSession(meshPeerID)).thenReturn(true) + + manager.sendReadReceiptsForPeer(conversationID, meshPeerID, meshService) + + verify(meshService, never()).sendReadReceipt(oldMessage.id, meshPeerID, "bob") + verify(meshService).sendReadReceipt(unreadMessage.id, meshPeerID, "bob") + } + @Test fun `canonical conversation send does not require resolved nickname`() { val conversationID =