From 879d8cba127575ca2c359ae25bb9b2f54a625752 Mon Sep 17 00:00:00 2001 From: jack Date: Thu, 11 Jun 2026 12:19:11 +0200 Subject: [PATCH] Cut private message path over to ConversationStore All private-message mutations now flow through store intents: coordinators, PrivateChatManager (its @Published dicts deleted - now read-only views over the store), outbound sends, delivery status, and chat migration. The O(1) store dedup replaces the full-scan duplicate check; insertion order is maintained by the store so sanitizeChat's re-sort is a documented no-op. Both bootstrapper Combine bridges and the Task.yield store synchronization are deleted. ChatViewModel.privateChats/unreadPrivateMessages become get-only derived views (measured: naive rebuild equals a change-invalidated cache within noise, so the simpler form stays). Feature models still read the legacy store, fed by a coalescing LegacyConversationStoreBridge (one mirror per burst, marked for step-5 deletion). pipeline.privateIngest: 9.6k -> 14.7k msg/s (+53%). Co-Authored-By: Claude Fable 5 --- bitchat/App/AppArchitecture.swift | 26 ++ bitchat/App/AppRuntime.swift | 8 + bitchat/App/ConversationStore.swift | 93 ++++++++ .../App/LegacyConversationStoreBridge.swift | 146 ++++++++++++ bitchat/Services/CommandProcessor.swift | 6 +- bitchat/Services/PrivateChatManager.swift | 221 ++++++++--------- .../ViewModels/ChatDeliveryCoordinator.swift | 22 +- .../ViewModels/ChatLifecycleCoordinator.swift | 24 +- .../ChatMediaTransferCoordinator.swift | 9 +- .../ChatPeerIdentityCoordinator.swift | 42 +--- .../ViewModels/ChatPeerListCoordinator.swift | 6 +- .../ChatPrivateConversationCoordinator.swift | 225 +++++++++--------- .../ChatPublicConversationCoordinator.swift | 36 +-- .../ChatTransportEventCoordinator.swift | 34 +-- bitchat/ViewModels/ChatViewModel.swift | 183 +++++++++++--- .../ChatViewModelBootstrapper.swift | 27 +-- bitchatTests/AppArchitectureTests.swift | 2 +- ...ChatLifecycleCoordinatorContextTests.swift | 18 +- ...MediaTransferCoordinatorContextTests.swift | 10 + ...tPeerIdentityCoordinatorContextTests.swift | 27 ++- .../ChatPeerListCoordinatorContextTests.swift | 4 + ...eConversationCoordinatorContextTests.swift | 99 +++++++- ...cConversationCoordinatorContextTests.swift | 28 ++- ...ransportEventCoordinatorContextTests.swift | 24 ++ .../ChatViewModelDeliveryStatusTests.swift | 43 +++- .../ChatViewModelExtensionsTests.swift | 18 +- bitchatTests/ChatViewModelTests.swift | 125 +++++----- bitchatTests/CommandProcessorTests.swift | 6 + .../LegacyConversationStoreBridgeTests.swift | 177 ++++++++++++++ .../PerformanceBaselineTests.swift | 11 + .../Services/PrivateChatManagerTests.swift | 114 ++++----- bitchatTests/TestUtilities/TestHelpers.swift | 22 ++ bitchatTests/ViewSmokeTests.swift | 2 +- 33 files changed, 1293 insertions(+), 545 deletions(-) create mode 100644 bitchat/App/LegacyConversationStoreBridge.swift create mode 100644 bitchatTests/LegacyConversationStoreBridgeTests.swift diff --git a/bitchat/App/AppArchitecture.swift b/bitchat/App/AppArchitecture.swift index 6d2c33e0..65fcaebf 100644 --- a/bitchat/App/AppArchitecture.swift +++ b/bitchat/App/AppArchitecture.swift @@ -356,6 +356,32 @@ final class LegacyConversationStore: ObservableObject { markRead(directConversationID(for: peerID, identityResolver: identityResolver)) } + // MARK: Migration step 2 bridge entry points (DELETE IN STEP 5) + // Used only by `LegacyConversationStoreBridge` to mirror single + // conversations out of the new `ConversationStore` without the + // full-dictionary `synchronizePrivateChats` pass. + + func replaceDirectMessages( + _ messages: [BitchatMessage], + for peerID: PeerID, + identityResolver: IdentityResolver + ) { + let handle = identityResolver.canonicalHandle(for: peerID, displayName: messages.last?.sender) + let conversationID = ConversationID.direct(handle) + directHandlesByConversation[conversationID] = handle + replaceMessages(messages, for: conversationID) + } + + func markUnread( + peerID: PeerID, + identityResolver: IdentityResolver + ) { + let conversationID = directConversationID(for: peerID, identityResolver: identityResolver) + if !unreadConversations.contains(conversationID) { + unreadConversations.insert(conversationID) + } + } + private func normalized(_ messages: [BitchatMessage]) -> [BitchatMessage] { var uniqueMessages: [String: BitchatMessage] = [:] diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 777e00e6..db861132 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -15,6 +15,11 @@ final class AppRuntime: ObservableObject { let chatViewModel: ChatViewModel let events = AppEventStream() let conversationStore: LegacyConversationStore + /// Single source of truth for conversation message state + /// (docs/CONVERSATION-STORE-DESIGN.md). The legacy store above keeps + /// feeding the feature models until step 5, mirrored from this one by + /// `LegacyConversationStoreBridge`. + let conversations: ConversationStore let peerIdentityStore: PeerIdentityStore let locationPresenceStore: LocationPresenceStore let publicChatModel: PublicChatModel @@ -44,10 +49,12 @@ final class AppRuntime: ObservableObject { self.idBridge = idBridge let identityResolver = IdentityResolver() let conversationStore = LegacyConversationStore() + let conversations = ConversationStore() let peerIdentityStore = PeerIdentityStore() let locationPresenceStore = LocationPresenceStore() let locationManager = LocationChannelManager.shared self.conversationStore = conversationStore + self.conversations = conversations self.peerIdentityStore = peerIdentityStore self.locationPresenceStore = locationPresenceStore self.chatViewModel = ChatViewModel( @@ -55,6 +62,7 @@ final class AppRuntime: ObservableObject { idBridge: idBridge, identityManager: SecureIdentityStateManager(keychain), conversationStore: conversationStore, + conversations: conversations, identityResolver: identityResolver, peerIdentityStore: peerIdentityStore, locationPresenceStore: locationPresenceStore, diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index 417574b8..1952b4d8 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -128,6 +128,16 @@ final class Conversation: ObservableObject, Identifiable { return true } + /// Removes a single message by ID. Returns the removed message, or + /// `nil` when no message with that ID exists. + fileprivate func remove(messageID: String) -> BitchatMessage? { + guard let index = indexByMessageID[messageID] else { return nil } + let removed = messages.remove(at: index) + indexByMessageID.removeValue(forKey: messageID) + reindex(from: index) + return removed + } + fileprivate func clearMessages() { messages.removeAll() indexByMessageID.removeAll() @@ -191,6 +201,7 @@ enum ConversationChange { case appended(ConversationID, BitchatMessage) case updated(ConversationID, messageID: String) case statusChanged(ConversationID, messageID: String, DeliveryStatus) + case messageRemoved(ConversationID, messageID: String) case cleared(ConversationID) case removed(ConversationID) case migrated(from: ConversationID, to: ConversationID) @@ -322,6 +333,19 @@ final class ConversationStore: ObservableObject { changes.send(.migrated(from: source, to: destination)) } + /// Removes a single message by ID from a conversation. Returns the + /// removed message, or `nil` (emitting nothing) when the conversation or + /// message is unknown. + @discardableResult + func removeMessage(withID messageID: String, from id: ConversationID) -> BitchatMessage? { + guard let conversation = conversationsByID[id], + let removed = conversation.remove(messageID: messageID) else { + return nil + } + changes.send(.messageRemoved(id, messageID: messageID)) + return removed + } + /// Empties a conversation's timeline but keeps the conversation (and /// its unread/selection state) alive. func clear(_ id: ConversationID) { @@ -371,3 +395,72 @@ final class ConversationStore: ObservableObject { } } } + +// MARK: - Migration step 2 compatibility (raw per-peer keying + derived views) + +extension ConversationID { + /// Direct-conversation ID keyed by the *raw* routing peer ID. + /// + /// Migration step 2 keeps one conversation per `PeerID` — exactly the + /// buckets the legacy `privateChats` dictionary had — so the + /// ephemeral/stable mirroring and consolidation coordinators keep their + /// current semantics. Step 5 canonicalizes direct conversations through + /// `IdentityResolver` and this helper goes away. + static func directPeer(_ peerID: PeerID) -> ConversationID { + .direct(PeerHandle( + id: "peer:\(peerID.id)", + routingPeerID: peerID, + displayName: nil, + noisePublicKeyHex: nil, + nostrPublicKey: nil + )) + } +} + +extension ConversationStore { + /// All direct conversations' messages keyed by routing peer ID — the + /// compat shape of the legacy `privateChats` dictionary. Values are the + /// conversations' backing arrays (COW), so building this is + /// O(#conversations), not O(#messages). + func directMessagesByRoutingPeerID() -> [PeerID: [BitchatMessage]] { + var messagesByPeerID: [PeerID: [BitchatMessage]] = [:] + messagesByPeerID.reserveCapacity(conversationsByID.count) + for (id, conversation) in conversationsByID { + guard case .direct(let handle) = id else { continue } + messagesByPeerID[handle.routingPeerID] = conversation.messages + } + return messagesByPeerID + } + + /// Unread direct conversations as routing peer IDs — the compat shape of + /// the legacy `unreadPrivateMessages` set. + func unreadDirectRoutingPeerIDs() -> Set { + var peerIDs = Set() + for id in unreadConversations { + guard case .direct(let handle) = id else { continue } + peerIDs.insert(handle.routingPeerID) + } + return peerIDs + } + + /// `true` when any direct conversation contains a message with `messageID` + /// (O(1) per conversation via the incremental ID index). + func directConversationsContainMessage(withID messageID: String) -> Bool { + for (id, conversation) in conversationsByID { + guard case .direct = id else { continue } + if conversation.containsMessage(withID: messageID) { return true } + } + return false + } + + /// Removes every direct conversation (panic clear). + func removeAllDirectConversations() { + let directIDs = conversationIDs.filter { id in + if case .direct = id { return true } + return false + } + for id in directIDs { + removeConversation(id) + } + } +} diff --git a/bitchat/App/LegacyConversationStoreBridge.swift b/bitchat/App/LegacyConversationStoreBridge.swift new file mode 100644 index 00000000..7ed57f17 --- /dev/null +++ b/bitchat/App/LegacyConversationStoreBridge.swift @@ -0,0 +1,146 @@ +// +// LegacyConversationStoreBridge.swift +// bitchat +// +// Migration step 2 adapter (DELETE IN STEP 5, see +// docs/CONVERSATION-STORE-DESIGN.md §4). +// +// The new `ConversationStore` is the single writer for private (direct) +// message state; the feature models (`PrivateInboxModel`, +// `PrivateConversationModel`, `ConversationUIModel`, `PeerListModel`) still +// read the replace-based `LegacyConversationStore` until step 5. This bridge +// keeps Legacy fed from the new store's `changes` subject: per-message +// changes mark the affected conversation dirty and a `Task.yield`-coalesced +// flush mirrors only the dirty conversations — a burst of N appends costs +// ONE Legacy replace (like the old debounced sync) without the full-dict +// pass. Structural changes (migration/removal) resynchronize immediately. +// Legacy is therefore eventually consistent within one run-loop tick — the +// same visibility the old `$privateChats` sink provided — while the new +// store stays synchronously authoritative. +// +// This is free and unencumbered software released into the public domain. +// For more information, see +// + +import BitFoundation +import Combine +import Foundation + +@MainActor +final class LegacyConversationStoreBridge { + private let store: ConversationStore + private let legacyStore: LegacyConversationStore + private let identityResolver: IdentityResolver + private var cancellable: AnyCancellable? + + private var dirtyConversations: Set = [] + private var pendingFlushTask: Task? + + init( + store: ConversationStore, + legacyStore: LegacyConversationStore, + identityResolver: IdentityResolver + ) { + self.store = store + self.legacyStore = legacyStore + self.identityResolver = identityResolver + + cancellable = store.changes.sink { [weak self] change in + self?.apply(change) + } + } + + /// Full resynchronization of every direct conversation into Legacy. + /// + /// Needed when `IdentityResolver` learns new peer associations (the + /// canonical handle for an existing conversation can change, re-keying + /// it in Legacy) and after structural store changes. This is the old + /// `synchronizePrivateChats` full pass — acceptable because it only runs + /// on peer-list changes and rare migrations, never per message. + func resynchronizeAll() { + // The full pass covers every conversation; pending per-conversation + // work is redundant. + dirtyConversations.removeAll() + legacyStore.synchronizePrivateChats( + store.directMessagesByRoutingPeerID(), + unreadPeerIDs: store.unreadDirectRoutingPeerIDs(), + identityResolver: identityResolver + ) + } +} + +private extension LegacyConversationStoreBridge { + func apply(_ change: ConversationChange) { + switch change { + case .appended(let id, _), + .updated(let id, _), + .statusChanged(let id, _, _), + .messageRemoved(let id, _), + .cleared(let id): + markDirty(id) + + case .unreadChanged(let id, let isUnread): + guard case .direct(let handle) = id else { return } + if isUnread { + legacyStore.markUnread(peerID: handle.routingPeerID, identityResolver: identityResolver) + } else { + legacyStore.markRead(peerID: handle.routingPeerID, identityResolver: identityResolver) + } + + case .migrated(let source, let destination): + guard isDirect(source) || isDirect(destination) else { return } + resynchronizeAll() + + case .removed(let id): + guard isDirect(id) else { return } + resynchronizeAll() + } + } + + func markDirty(_ id: ConversationID) { + guard isDirect(id) else { return } + dirtyConversations.insert(id) + scheduleFlush() + } + + /// One pending flush at a time, exactly like the old + /// `schedulePrivateConversationStoreSynchronization` debounce: a + /// synchronous burst of mutations coalesces into a single flush on the + /// next main-actor turn. + func scheduleFlush() { + guard pendingFlushTask == nil else { return } + pendingFlushTask = Task { @MainActor [weak self] in + await Task.yield() + guard let self else { return } + self.pendingFlushTask = nil + self.flushDirtyConversations() + } + } + + func flushDirtyConversations() { + guard !dirtyConversations.isEmpty else { return } + let dirty = dirtyConversations + dirtyConversations.removeAll() + for id in dirty { + mirrorConversation(id) + } + } + + func mirrorConversation(_ id: ConversationID) { + guard case .direct(let handle) = id, + let conversation = store.conversationsByID[id] else { + // Removed while dirty; the removal already resynchronized. + return + } + legacyStore.replaceDirectMessages( + conversation.messages, + for: handle.routingPeerID, + identityResolver: identityResolver + ) + } + + func isDirect(_ id: ConversationID) -> Bool { + if case .direct = id { return true } + return false + } +} diff --git a/bitchat/Services/CommandProcessor.swift b/bitchat/Services/CommandProcessor.swift index 99b47288..f435d8e6 100644 --- a/bitchat/Services/CommandProcessor.swift +++ b/bitchat/Services/CommandProcessor.swift @@ -31,7 +31,7 @@ protocol CommandContextProvider: AnyObject { var activeChannel: ChannelID { get } var selectedPrivateChatPeer: PeerID? { get } var blockedUsers: Set { get } - var privateChats: [PeerID: [BitchatMessage]] { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } var idBridge: NostrIdentityBridge { get } // MARK: - Peer Lookup @@ -43,6 +43,8 @@ protocol CommandContextProvider: AnyObject { func startPrivateChat(with peerID: PeerID) func sendPrivateMessage(_ content: String, to peerID: PeerID) func clearCurrentPublicTimeline() + /// Empties the peer's chat (single-writer store intent for `/clear`). + func clearPrivateChat(_ peerID: PeerID) func sendPublicRaw(_ content: String) // MARK: - System Messages @@ -160,7 +162,7 @@ final class CommandProcessor { private func handleClear() -> CommandResult { if let peerID = contextProvider?.selectedPrivateChatPeer { - contextProvider?.privateChats[peerID]?.removeAll() + contextProvider?.clearPrivateChat(peerID) } else { contextProvider?.clearCurrentPublicTimeline() } diff --git a/bitchat/Services/PrivateChatManager.swift b/bitchat/Services/PrivateChatManager.swift index 3e920e12..62a905d2 100644 --- a/bitchat/Services/PrivateChatManager.swift +++ b/bitchat/Services/PrivateChatManager.swift @@ -11,11 +11,14 @@ import BitFoundation import Foundation import SwiftUI -/// Manages all private chat functionality +/// Manages private chat session policy (selection, read receipts, +/// consolidation). Message storage lives in the single-writer +/// `ConversationStore` (docs/CONVERSATION-STORE-DESIGN.md); the +/// `privateChats` / `unreadMessages` properties below are read-only compat +/// views derived from it (migration step 2 — the manager shrinks to +/// read-receipt policy in step 5). final class PrivateChatManager: ObservableObject { - @Published var privateChats: [PeerID: [BitchatMessage]] = [:] @Published var selectedPeer: PeerID? = nil - @Published var unreadMessages: Set = [] private var selectedPeerFingerprint: String? = nil var sentReadReceipts: Set = [] // Made accessible for ChatViewModel @@ -25,13 +28,34 @@ final class PrivateChatManager: ObservableObject { weak var messageRouter: MessageRouter? // Peer service for looking up peer info during consolidation weak var unifiedPeerService: UnifiedPeerService? + /// Single source of truth for message state; injected by the + /// bootstrapper (`wireServiceGraph`). + var conversationStore: ConversationStore? - init(meshService: Transport? = nil) { + init(meshService: Transport? = nil, conversationStore: ConversationStore? = nil) { self.meshService = meshService + self.conversationStore = conversationStore } - // Cap for messages stored per private chat - private let privateChatCap = TransportConfig.privateChatCap + // MARK: - Derived message state (read-only compat views) + + /// All private chats keyed by routing peer ID, derived from the store. + /// Mutations go through the store's intent API only. + @MainActor + var privateChats: [PeerID: [BitchatMessage]] { + conversationStore?.directMessagesByRoutingPeerID() ?? [:] + } + + /// Unread chats, derived from the store's unread state. + @MainActor + var unreadMessages: Set { + conversationStore?.unreadDirectRoutingPeerIDs() ?? [] + } + + @MainActor + private func messages(for peerID: PeerID) -> [BitchatMessage] { + conversationStore?.conversationsByID[.directPeer(peerID)]?.messages ?? [] + } // MARK: - Message Consolidation @@ -44,57 +68,51 @@ final class PrivateChatManager: ObservableObject { /// - Returns: True if any unread messages were found during consolidation @MainActor func consolidateMessages(for peerID: PeerID, peerNickname: String, persistedReadReceipts: Set) -> Bool { - guard let meshService = meshService else { return false } + guard let meshService = meshService, let store = conversationStore else { return false } var hasUnreadMessages = false // 1. Consolidate from stable Noise key (64-char hex) if let peer = unifiedPeerService?.getPeer(by: peerID) { let noiseKeyHex = PeerID(hexData: peer.noisePublicKey) + let nostrMessages = messages(for: noiseKeyHex) - if noiseKeyHex != peerID, let nostrMessages = privateChats[noiseKeyHex], !nostrMessages.isEmpty { - if privateChats[peerID] == nil { - privateChats[peerID] = [] - } - - let existingMessageIds = Set(privateChats[peerID]?.map { $0.id } ?? []) + if noiseKeyHex != peerID, !nostrMessages.isEmpty { for message in nostrMessages { - if !existingMessageIds.contains(message.id) { - // Update senderPeerID for correct read receipts - let updatedMessage = BitchatMessage( - id: message.id, - sender: message.sender, - content: message.content, - timestamp: message.timestamp, - isRelay: message.isRelay, - originalSender: message.originalSender, - isPrivate: message.isPrivate, - recipientNickname: message.recipientNickname, - senderPeerID: message.senderPeerID == meshService.myPeerID ? meshService.myPeerID : peerID, - mentions: message.mentions, - deliveryStatus: message.deliveryStatus - ) - privateChats[peerID]?.append(updatedMessage) + // Update senderPeerID for correct read receipts + let updatedMessage = BitchatMessage( + id: message.id, + sender: message.sender, + content: message.content, + timestamp: message.timestamp, + isRelay: message.isRelay, + originalSender: message.originalSender, + isPrivate: message.isPrivate, + recipientNickname: message.recipientNickname, + senderPeerID: message.senderPeerID == meshService.myPeerID ? meshService.myPeerID : peerID, + mentions: message.mentions, + deliveryStatus: message.deliveryStatus + ) + // Store append dedups by message ID (skips ones the + // target chat already has). + guard store.append(updatedMessage, to: .directPeer(peerID)) else { continue } - // Check for recent unread messages (< 60s, not sent by us, not already read) - // Use persistedReadReceipts to correctly identify already-read messages after app restart - if message.senderPeerID != meshService.myPeerID { - let messageAge = Date().timeIntervalSince(message.timestamp) - if messageAge < 60 && !persistedReadReceipts.contains(message.id) { - hasUnreadMessages = true - } + // Check for recent unread messages (< 60s, not sent by us, not already read) + // Use persistedReadReceipts to correctly identify already-read messages after app restart + if message.senderPeerID != meshService.myPeerID { + let messageAge = Date().timeIntervalSince(message.timestamp) + if messageAge < 60 && !persistedReadReceipts.contains(message.id) { + hasUnreadMessages = true } } } - privateChats[peerID]?.sort { $0.timestamp < $1.timestamp } - if hasUnreadMessages { - unreadMessages.insert(peerID) - } else if unreadMessages.contains(noiseKeyHex) { - unreadMessages.remove(noiseKeyHex) + store.markUnread(.directPeer(peerID)) + } else { + store.markRead(.directPeer(noiseKeyHex)) } - privateChats.removeValue(forKey: noiseKeyHex) + store.removeConversation(.directPeer(noiseKeyHex)) } } @@ -112,52 +130,43 @@ final class PrivateChatManager: ObservableObject { } if !tempPeerIDsToConsolidate.isEmpty { - if privateChats[peerID] == nil { - privateChats[peerID] = [] - } - - let existingMessageIds = Set(privateChats[peerID]?.map { $0.id } ?? []) var consolidatedCount = 0 var hadUnreadTemp = false + let unreadPeerIDs = unreadMessages for tempPeerID in tempPeerIDsToConsolidate { - if unreadMessages.contains(tempPeerID) { + if unreadPeerIDs.contains(tempPeerID) { hadUnreadTemp = true } - if let tempMessages = privateChats[tempPeerID] { - for message in tempMessages { - if !existingMessageIds.contains(message.id) { - let updatedMessage = BitchatMessage( - id: message.id, - sender: message.sender, - content: message.content, - timestamp: message.timestamp, - isRelay: message.isRelay, - originalSender: message.originalSender, - isPrivate: message.isPrivate, - recipientNickname: message.recipientNickname, - senderPeerID: peerID, - mentions: message.mentions, - deliveryStatus: message.deliveryStatus - ) - privateChats[peerID]?.append(updatedMessage) - consolidatedCount += 1 - } + for message in messages(for: tempPeerID) { + let updatedMessage = BitchatMessage( + id: message.id, + sender: message.sender, + content: message.content, + timestamp: message.timestamp, + isRelay: message.isRelay, + originalSender: message.originalSender, + isPrivate: message.isPrivate, + recipientNickname: message.recipientNickname, + senderPeerID: peerID, + mentions: message.mentions, + deliveryStatus: message.deliveryStatus + ) + if store.append(updatedMessage, to: .directPeer(peerID)) { + consolidatedCount += 1 } - privateChats.removeValue(forKey: tempPeerID) - unreadMessages.remove(tempPeerID) } + store.removeConversation(.directPeer(tempPeerID)) } if hadUnreadTemp { - unreadMessages.insert(peerID) + store.markUnread(.directPeer(peerID)) hasUnreadMessages = true SecureLogger.debug("📬 Transferred unread status from temp peer IDs to \(peerID)", category: .session) } if consolidatedCount > 0 { - privateChats[peerID]?.sort { $0.timestamp < $1.timestamp } SecureLogger.info("📥 Consolidated \(consolidatedCount) Nostr messages from temporary peer IDs to \(peerNickname)", category: .session) } } @@ -168,9 +177,7 @@ final class PrivateChatManager: ObservableObject { /// Syncs the read receipt tracking between manager and view model for sent messages @MainActor func syncReadReceiptsForSentMessages(peerID: PeerID, nickname: String, externalReceipts: inout Set) { - guard let messages = privateChats[peerID] else { return } - - for message in messages { + for message in messages(for: peerID) { if message.sender == nickname { if let status = message.deliveryStatus { switch status { @@ -184,86 +191,66 @@ final class PrivateChatManager: ObservableObject { } } } - + /// Start a private chat with a peer + @MainActor func startChat(with peerID: PeerID) { selectedPeer = peerID - + // Store fingerprint for persistence across reconnections if let fingerprint = meshService?.getFingerprint(for: peerID) { selectedPeerFingerprint = fingerprint } - + // Mark messages as read markAsRead(from: peerID) - + // Initialize chat if needed - if privateChats[peerID] == nil { - privateChats[peerID] = [] - } + conversationStore?.conversation(for: .directPeer(peerID)) } - + /// End the current private chat func endChat() { selectedPeer = nil selectedPeerFingerprint = nil } - /// Remove duplicate messages by ID and keep chronological order - func sanitizeChat(for peerID: PeerID) { - guard let arr = privateChats[peerID] else { return } - if arr.count <= 1 { - return - } + /// No-op since the `ConversationStore` cutover: the store maintains + /// chronological order and dedups by message ID on every insert, so the + /// per-append re-sort/dedup sweep this performed is no longer needed. + /// Kept only for API compatibility until step 5 removes the callers. + func sanitizeChat(for peerID: PeerID) {} - var indexByID: [String: Int] = [:] - indexByID.reserveCapacity(arr.count) - var deduped: [BitchatMessage] = [] - deduped.reserveCapacity(arr.count) - - for msg in arr.sorted(by: { $0.timestamp < $1.timestamp }) { - if let existing = indexByID[msg.id] { - deduped[existing] = msg - } else { - indexByID[msg.id] = deduped.count - deduped.append(msg) - } - } - - privateChats[peerID] = deduped - } - /// Mark messages from a peer as read + @MainActor func markAsRead(from peerID: PeerID) { - unreadMessages.remove(peerID) - + conversationStore?.markRead(.directPeer(peerID)) + // Send read receipts for unread messages that haven't been sent yet - if let messages = privateChats[peerID] { - for message in messages { - if message.senderPeerID == peerID && !message.isRelay && !sentReadReceipts.contains(message.id) { - sendReadReceipt(for: message) - } + for message in messages(for: peerID) { + if message.senderPeerID == peerID && !message.isRelay && !sentReadReceipts.contains(message.id) { + sendReadReceipt(for: message) } } } - + // MARK: - Private Methods - + private func sendReadReceipt(for message: BitchatMessage) { guard !sentReadReceipts.contains(message.id), let senderPeerID = message.senderPeerID else { return } - + sentReadReceipts.insert(message.id) - + // Create read receipt using the simplified method let receipt = ReadReceipt( originalMessageID: message.id, readerID: meshService?.myPeerID ?? PeerID(str: ""), readerNickname: meshService?.myNickname ?? "" ) - + // Route via MessageRouter to avoid handshakeRequired spam when session isn't established if let router = messageRouter { SecureLogger.debug("PrivateChatManager: sending READ ack for \(message.id.prefix(8))… to \(senderPeerID.id.prefix(8))… via router", category: .session) diff --git a/bitchat/ViewModels/ChatDeliveryCoordinator.swift b/bitchat/ViewModels/ChatDeliveryCoordinator.swift index 61a4c8e3..e94a5a73 100644 --- a/bitchat/ViewModels/ChatDeliveryCoordinator.swift +++ b/bitchat/ViewModels/ChatDeliveryCoordinator.swift @@ -13,8 +13,13 @@ import Foundation @MainActor protocol ChatDeliveryContext: AnyObject { var messages: [BitchatMessage] { get set } - var privateChats: [PeerID: [BitchatMessage]] { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } var isStartupPhase: Bool { get } + /// Applies a delivery status to a private message by ID (single-writer + /// store intent; full delivery migration is step 4). Returns `false` + /// when the message is unknown or the update would downgrade the status. + @discardableResult + func setPrivateDeliveryStatus(_ status: DeliveryStatus, forMessageID messageID: String, peerID: PeerID) -> Bool /// Drops every recorded read receipt whose message ID is not in `validMessageIDs`. /// Returns the number of receipts removed. (Single mutation path for the /// owner's `sentReadReceipts`; this coordinator never reads the raw set.) @@ -98,7 +103,6 @@ final class ChatDeliveryCoordinator { } var didUpdateStatus = false - var didUpdatePrivateStatus = false let locations = withValidLocations(for: messageID) { $0 } guard !locations.isEmpty else { return false } @@ -116,10 +120,9 @@ final class ChatDeliveryCoordinator { } } - var privateChats = context.privateChats for location in locations { guard case .privateChat(let peerID, let index) = location, - let chatMessages = privateChats[peerID], + let chatMessages = context.privateChats[peerID], index < chatMessages.count, chatMessages[index].id == messageID else { continue @@ -128,14 +131,9 @@ final class ChatDeliveryCoordinator { let currentStatus = chatMessages[index].deliveryStatus guard !shouldSkipUpdate(currentStatus: currentStatus, newStatus: status) else { continue } - chatMessages[index].deliveryStatus = status - privateChats[peerID] = chatMessages - didUpdateStatus = true - didUpdatePrivateStatus = true - } - - if didUpdatePrivateStatus { - context.privateChats = privateChats + if context.setPrivateDeliveryStatus(status, forMessageID: messageID, peerID: peerID) { + didUpdateStatus = true + } } if didUpdateStatus { diff --git a/bitchat/ViewModels/ChatLifecycleCoordinator.swift b/bitchat/ViewModels/ChatLifecycleCoordinator.swift index 43d29459..f13f057d 100644 --- a/bitchat/ViewModels/ChatLifecycleCoordinator.swift +++ b/bitchat/ViewModels/ChatLifecycleCoordinator.swift @@ -13,9 +13,14 @@ import Foundation protocol ChatLifecycleContext: AnyObject { // MARK: Chat & receipt state var messages: [BitchatMessage] { get } - var privateChats: [PeerID: [BitchatMessage]] { get set } - var unreadPrivateMessages: Set { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } + var unreadPrivateMessages: Set { get } var selectedPrivateChatPeer: PeerID? { get } + /// Appends a private message via the single-writer store intent. + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool + /// Clears the peer's unread flag (store unread state only). + func markPrivateChatRead(_ peerID: PeerID) var sentReadReceipts: Set { get } var nickname: String { get } var myPeerID: PeerID { get } @@ -33,7 +38,6 @@ protocol ChatLifecycleContext: AnyObject { /// Schedules main-actor work after a UI-timing delay. Injected so tests /// can run the work synchronously instead of polling wall-clock queues. func scheduleOnMainAfter(_ delay: TimeInterval, _ work: @escaping @MainActor () -> Void) - func synchronizePrivateConversationStore() func addSystemMessage(_ content: String) // MARK: Peers & sessions @@ -72,8 +76,8 @@ extension ChatViewModel: ChatLifecycleContext { // `messages`, `privateChats`, `unreadPrivateMessages`, // `selectedPrivateChatPeer`, `sentReadReceipts`, `nickname`, `myPeerID`, // `activeChannel`, `nostrKeyMapping`, `markReadReceiptSent(_:)`, - // `markPrivateMessagesAsRead(from:)`, - // `synchronizePrivateConversationStore()`, `addSystemMessage(_:)`, + // `markPrivateMessagesAsRead(from:)`, `appendPrivateMessage(_:to:)`, + // `markPrivateChatRead(_:)`, `addSystemMessage(_:)`, // `peerNickname(for:)`, `unifiedPeer(for:)`, `noiseSessionState(for:)`, // the routing/ack members, `isTeleported`, // `deriveNostrIdentity(forGeohash:)`, `recordGeoParticipant(pubkeyHex:)`, @@ -178,7 +182,6 @@ final class ChatLifecycleCoordinator { func markPrivateMessagesAsRead(from peerID: PeerID) { context.markChatAsRead(from: peerID) - context.synchronizePrivateConversationStore() if peerID.isGeoDM, let recipientHex = context.nostrKeyMapping[peerID], @@ -215,7 +218,7 @@ final class ChatLifecycleCoordinator { peerNostrPubkey = favoriteStatus?.peerNostrPublicKey if let noiseKeyHex, context.unreadPrivateMessages.contains(noiseKeyHex) { - context.unreadPrivateMessages.remove(noiseKeyHex) + context.markPrivateChatRead(noiseKeyHex) } } @@ -312,12 +315,7 @@ private extension ChatLifecycleCoordinator { senderPeerID: context.myPeerID ) - var chats = context.privateChats - if chats[peerID] == nil { - chats[peerID] = [] - } - chats[peerID]?.append(notice) - context.privateChats = chats + context.appendPrivateMessage(notice, to: peerID) } func sendPublicGeohashScreenshotMessage(_ message: String, channel: GeohashChannel) { diff --git a/bitchat/ViewModels/ChatMediaTransferCoordinator.swift b/bitchat/ViewModels/ChatMediaTransferCoordinator.swift index fbabf1fb..54ce27ed 100644 --- a/bitchat/ViewModels/ChatMediaTransferCoordinator.swift +++ b/bitchat/ViewModels/ChatMediaTransferCoordinator.swift @@ -25,7 +25,10 @@ protocol ChatMediaTransferContext: AnyObject { func currentPublicSender() -> (name: String, peerID: PeerID) // MARK: Message state - var privateChats: [PeerID: [BitchatMessage]] { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } + /// Appends a private message via the single-writer store intent. + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool func appendTimelineMessage(_ message: BitchatMessage, to channel: ChannelID) func refreshVisibleMessages(from channel: ChannelID?) func trimMessagesIfNeeded() @@ -228,9 +231,7 @@ final class ChatMediaTransferCoordinator { senderPeerID: context.myPeerID, deliveryStatus: .sending ) - var chats = context.privateChats - chats[peerID, default: []].append(message) - context.privateChats = chats + context.appendPrivateMessage(message, to: peerID) context.trimMessagesIfNeeded() } else { let (displayName, senderPeerID) = context.currentPublicSender() diff --git a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift index 0d50ea90..2728afe3 100644 --- a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift +++ b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift @@ -17,8 +17,13 @@ import Foundation @MainActor protocol ChatPeerIdentityContext: AnyObject { // MARK: Conversation state - var privateChats: [PeerID: [BitchatMessage]] { get set } - var unreadPrivateMessages: Set { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } + var unreadPrivateMessages: Set { get } + /// Clears the peer's unread flag (single-writer store intent). + func markPrivateChatRead(_ peerID: PeerID) + /// Moves all messages from `oldPeerID`'s chat into `newPeerID`'s chat + /// (dedup by ID, order preserved, unread carried, old chat removed). + func migratePrivateChat(from oldPeerID: PeerID, to newPeerID: PeerID) var selectedPrivateChatPeer: PeerID? { get set } var selectedPrivateChatFingerprint: String? { get set } var nickname: String { get } @@ -39,7 +44,6 @@ protocol ChatPeerIdentityContext: AnyObject { func syncReadReceiptsForSentMessages(for peerID: PeerID) /// Re-targets the private chat session in the chat manager (no store-sync side effects). func beginPrivateChatSession(with peerID: PeerID) - func synchronizePrivateConversationStore() func synchronizeConversationSelectionStore() func markPrivateMessagesAsRead(from peerID: PeerID) @@ -305,9 +309,7 @@ final class ChatPeerIdentityCoordinator { context.selectedPrivateChatPeer = currentPeerID } - var unread = context.unreadPrivateMessages - unread.remove(currentPeerID) - context.unreadPrivateMessages = unread + context.markPrivateChatRead(currentPeerID) } @MainActor @@ -367,7 +369,6 @@ final class ChatPeerIdentityCoordinator { context.selectedPrivateChatFingerprint = nil } context.beginPrivateChatSession(with: peerID) - context.synchronizePrivateConversationStore() context.synchronizeConversationSelectionStore() context.markPrivateMessagesAsRead(from: peerID) } @@ -553,30 +554,9 @@ private extension ChatPeerIdentityCoordinator { @MainActor func migrateChatState(from oldPeerID: PeerID, to newPeerID: PeerID) { - if let oldMessages = context.privateChats[oldPeerID] { - var chats = context.privateChats - chats[newPeerID, default: []].append(contentsOf: oldMessages) - chats[newPeerID]?.sort { $0.timestamp < $1.timestamp } - - var seenMessageIDs = Set() - chats[newPeerID] = chats[newPeerID]?.filter { message in - if seenMessageIDs.contains(message.id) { - return false - } - seenMessageIDs.insert(message.id) - return true - } - - chats.removeValue(forKey: oldPeerID) - context.privateChats = chats - } - - var unread = context.unreadPrivateMessages - if unread.contains(oldPeerID) { - unread.remove(oldPeerID) - unread.insert(newPeerID) - context.unreadPrivateMessages = unread - } + // The store migration dedups by message ID, preserves timestamp + // order, carries the unread flag, and removes the old chat. + context.migratePrivateChat(from: oldPeerID, to: newPeerID) } @MainActor diff --git a/bitchat/ViewModels/ChatPeerListCoordinator.swift b/bitchat/ViewModels/ChatPeerListCoordinator.swift index 71145fe0..167401a6 100644 --- a/bitchat/ViewModels/ChatPeerListCoordinator.swift +++ b/bitchat/ViewModels/ChatPeerListCoordinator.swift @@ -14,7 +14,9 @@ protocol ChatPeerListContext: AnyObject { // MARK: Connection & chat state var isConnected: Bool { get set } var privateChats: [PeerID: [BitchatMessage]] { get } - var unreadPrivateMessages: Set { get set } + var unreadPrivateMessages: Set { get } + /// Clears the peer's unread flag (single-writer store intent). + func markPrivateChatRead(_ peerID: PeerID) var hasTrackedPrivateChatSelection: Bool { get } func updatePrivateChatPeerIfNeeded() func cleanupOldReadReceipts() @@ -155,7 +157,7 @@ private extension ChatPeerListCoordinator { } idsToRemove.append(staleID) - context.unreadPrivateMessages.remove(staleID) + context.markPrivateChatRead(staleID) } if !idsToRemove.isEmpty { diff --git a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift index 2ae4532c..401536de 100644 --- a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift +++ b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift @@ -14,13 +14,39 @@ import Foundation @MainActor protocol ChatPrivateConversationContext: AnyObject { // MARK: Conversation state - var privateChats: [PeerID: [BitchatMessage]] { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } var sentReadReceipts: Set { get } - var unreadPrivateMessages: Set { get set } + var unreadPrivateMessages: Set { get } var selectedPrivateChatPeer: PeerID? { get } var nickname: String { get } var activeChannel: ChannelID { get } var nostrKeyMapping: [PeerID: String] { get } + + // MARK: Conversation store intents + // The sole mutation paths for private message state (single-writer + // `ConversationStore` ops; see docs/CONVERSATION-STORE-DESIGN.md). + /// Appends a private message in timestamp order; returns `false` on + /// duplicate message ID. + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool + /// Replace-or-append a private message by ID, keeping its position. + func upsertPrivateMessage(_ message: BitchatMessage, in peerID: PeerID) + /// Applies a delivery status by message ID; returns `false` when the + /// message is unknown or the update would downgrade the status. + @discardableResult + func setPrivateDeliveryStatus(_ status: DeliveryStatus, forMessageID messageID: String, peerID: PeerID) -> Bool + func markPrivateChatUnread(_ peerID: PeerID) + func markPrivateChatRead(_ peerID: PeerID) + /// Removes the peer's chat entirely, including unread state. + func removePrivateChat(_ peerID: PeerID) + /// Moves all messages from `oldPeerID`'s chat into `newPeerID`'s chat + /// (dedup by ID, order preserved, unread carried, old chat removed). + func migratePrivateChat(from oldPeerID: PeerID, to newPeerID: PeerID) + /// `true` when any private chat contains a message with `messageID`. + func privateChatsContainMessage(withID messageID: String) -> Bool + /// `true` when `peerID`'s chat contains a message with `messageID`. + func privateChat(_ peerID: PeerID, containsMessageWithID messageID: String) -> Bool + /// Records that a read receipt is being sent for `messageID`. /// Returns `false` when one was already recorded — the caller must skip sending. @discardableResult @@ -65,10 +91,9 @@ protocol ChatPrivateConversationContext: AnyObject { func sendGeohashReadReceipt(_ messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity) func sendDeliveryAckViaNostrEmbedded(_ message: BitchatMessage, wasReadBefore: Bool, senderPubkey: String, key: Data?) - // MARK: System messages & chat hygiene + // MARK: System messages func addSystemMessage(_ content: String) func addMeshOnlySystemMessage(_ content: String) - func sanitizeChat(for peerID: PeerID) // MARK: Favorites & notifications /// The persisted favorite relationship for the peer's Noise static key, if any. @@ -165,10 +190,6 @@ extension ChatViewModel: ChatPrivateConversationContext { addSystemMessage(content, timestamp: Date()) } - func sanitizeChat(for peerID: PeerID) { - privateChatManager.sanitizeChat(for: peerID) - } - func favoriteRelationship(forNoiseKey noiseKey: Data) -> FavoritesPersistenceService.FavoriteRelationship? { FavoritesPersistenceService.shared.getFavoriteStatus(for: noiseKey) } @@ -249,10 +270,7 @@ final class ChatPrivateConversationCoordinator { deliveryStatus: .sending ) - if context.privateChats[peerID] == nil { - context.privateChats[peerID] = [] - } - context.privateChats[peerID]?.append(message) + context.appendPrivateMessage(message, to: peerID) context.notifyUIChanged() if isConnected || isReachable || (isMutualFavorite && hasNostrKey) { @@ -262,15 +280,15 @@ final class ChatPrivateConversationCoordinator { recipientNickname: recipientNickname ?? "user", messageID: messageID ) - if let idx = context.privateChats[peerID]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[peerID]?[idx].deliveryStatus = .sent - } + context.setPrivateDeliveryStatus(.sent, forMessageID: messageID, peerID: peerID) } else { - if let index = context.privateChats[peerID]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[peerID]?[index].deliveryStatus = .failed( + context.setPrivateDeliveryStatus( + .failed( reason: String(localized: "content.delivery.reason.unreachable", comment: "Failure reason when a peer is unreachable") - ) - } + ), + forMessageID: messageID, + peerID: peerID + ) let name = recipientNickname ?? "user" context.addSystemMessage( String( @@ -303,28 +321,28 @@ final class ChatPrivateConversationCoordinator { deliveryStatus: .sending ) - if context.privateChats[peerID] == nil { - context.privateChats[peerID] = [] - } - - context.privateChats[peerID]?.append(message) + context.appendPrivateMessage(message, to: peerID) context.notifyUIChanged() guard let recipientHex = context.nostrKeyMapping[peerID] else { - if let msgIdx = context.privateChats[peerID]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[peerID]?[msgIdx].deliveryStatus = .failed( + context.setPrivateDeliveryStatus( + .failed( reason: String(localized: "content.delivery.reason.unknown_recipient", comment: "Failure reason when the recipient is unknown") - ) - } + ), + forMessageID: messageID, + peerID: peerID + ) return } if context.isNostrBlocked(pubkeyHexLowercased: recipientHex) { - if let msgIdx = context.privateChats[peerID]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[peerID]?[msgIdx].deliveryStatus = .failed( + context.setPrivateDeliveryStatus( + .failed( reason: String(localized: "content.delivery.reason.blocked", comment: "Failure reason when the user is blocked") - ) - } + ), + forMessageID: messageID, + peerID: peerID + ) context.addSystemMessage( String(localized: "system.dm.blocked_generic", comment: "System message when sending fails because user is blocked") ) @@ -334,11 +352,13 @@ final class ChatPrivateConversationCoordinator { do { let identity = try context.deriveNostrIdentity(forGeohash: channel.geohash) if recipientHex.lowercased() == identity.publicKeyHex.lowercased() { - if let idx = context.privateChats[peerID]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[peerID]?[idx].deliveryStatus = .failed( + context.setPrivateDeliveryStatus( + .failed( reason: String(localized: "content.delivery.reason.self", comment: "Failure reason when attempting to message yourself") - ) - } + ), + forMessageID: messageID, + peerID: peerID + ) return } @@ -352,15 +372,15 @@ final class ChatPrivateConversationCoordinator { from: identity, messageID: messageID ) - if let msgIdx = context.privateChats[peerID]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[peerID]?[msgIdx].deliveryStatus = .sent - } + context.setPrivateDeliveryStatus(.sent, forMessageID: messageID, peerID: peerID) } catch { - if let idx = context.privateChats[peerID]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[peerID]?[idx].deliveryStatus = .failed( + context.setPrivateDeliveryStatus( + .failed( reason: String(localized: "content.delivery.reason.send_error", comment: "Failure reason for a generic send error") - ) - } + ), + forMessageID: messageID, + peerID: peerID + ) } } @@ -382,10 +402,7 @@ final class ChatPrivateConversationCoordinator { return } - if context.privateChats[convKey]?.contains(where: { $0.id == messageId }) == true { return } - for (_, arr) in context.privateChats where arr.contains(where: { $0.id == messageId }) { - return - } + if context.privateChatsContainMessage(withID: messageId) { return } let senderName = context.displayNameForNostrPubkey(senderPubkey) let message = BitchatMessage( @@ -400,17 +417,14 @@ final class ChatPrivateConversationCoordinator { deliveryStatus: .delivered(to: context.nickname, at: Date()) ) - if context.privateChats[convKey] == nil { - context.privateChats[convKey] = [] - } - context.privateChats[convKey]?.append(message) + context.appendPrivateMessage(message, to: convKey) let isViewing = context.selectedPrivateChatPeer == convKey let wasReadBefore = context.sentReadReceipts.contains(messageId) let isRecentMessage = Date().timeIntervalSince(messageTimestamp) < 30 let shouldMarkUnread = !wasReadBefore && !isViewing && isRecentMessage if shouldMarkUnread { - context.unreadPrivateMessages.insert(convKey) + context.markPrivateChatUnread(convKey) } if isViewing { @@ -427,10 +441,11 @@ final class ChatPrivateConversationCoordinator { func handleDelivered(_ payload: NoisePayload, senderPubkey: String, convKey: PeerID) { guard let messageID = String(data: payload.data, encoding: .utf8) else { return } - if let idx = context.privateChats[convKey]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[convKey]?[idx].deliveryStatus = .delivered( - to: context.displayNameForNostrPubkey(senderPubkey), - at: Date() + if context.privateChat(convKey, containsMessageWithID: messageID) { + context.setPrivateDeliveryStatus( + .delivered(to: context.displayNameForNostrPubkey(senderPubkey), at: Date()), + forMessageID: messageID, + peerID: convKey ) context.notifyUIChanged() SecureLogger.info( @@ -445,10 +460,11 @@ final class ChatPrivateConversationCoordinator { func handleReadReceipt(_ payload: NoisePayload, senderPubkey: String, convKey: PeerID) { guard let messageID = String(data: payload.data, encoding: .utf8) else { return } - if let idx = context.privateChats[convKey]?.firstIndex(where: { $0.id == messageID }) { - context.privateChats[convKey]?[idx].deliveryStatus = .read( - by: context.displayNameForNostrPubkey(senderPubkey), - at: Date() + if context.privateChat(convKey, containsMessageWithID: messageID) { + context.setPrivateDeliveryStatus( + .read(by: context.displayNameForNostrPubkey(senderPubkey), at: Date()), + forMessageID: messageID, + peerID: convKey ) context.notifyUIChanged() SecureLogger.info("GeoDM: recv READ for mid=\(messageID.prefix(8))… from=\(senderPubkey.prefix(8))…", category: .session) @@ -575,17 +591,9 @@ final class ChatPrivateConversationCoordinator { if stableKeyHex != peerID, let nostrMessages = context.privateChats[stableKeyHex], !nostrMessages.isEmpty { - if context.privateChats[peerID] == nil { - context.privateChats[peerID] = [] - } - - let existingMessageIds = Set(context.privateChats[peerID]?.map { $0.id } ?? []) - for nostrMessage in nostrMessages where !existingMessageIds.contains(nostrMessage.id) { - context.privateChats[peerID]?.append(nostrMessage) - } - - context.privateChats[peerID]?.sort { $0.timestamp < $1.timestamp } - context.privateChats.removeValue(forKey: stableKeyHex) + // Store migration dedups by ID, keeps timestamp order, and + // removes the stable-key chat. + context.migratePrivateChat(from: stableKeyHex, to: peerID) SecureLogger.info( "📥 Consolidated \(nostrMessages.count) Nostr messages from stable key to ephemeral peer \(peerID)", @@ -612,33 +620,23 @@ final class ChatPrivateConversationCoordinator { context.sendMeshReadReceipt(receipt, to: peerID) context.markReadReceiptSent(message.id) } else { - context.unreadPrivateMessages.insert(peerID) + context.markPrivateChatUnread(peerID) context.notifyPrivateMessage(from: message.sender, message: message.content, peerID: peerID) } context.notifyUIChanged() } + /// O(1)-per-conversation dedup via the store's message-ID indexes + /// (replaces the full scan over every private chat). func isDuplicateMessage(_ messageId: String, targetPeerID: PeerID) -> Bool { - if context.privateChats[targetPeerID]?.contains(where: { $0.id == messageId }) == true { - return true - } - for (_, messages) in context.privateChats where messages.contains(where: { $0.id == messageId }) { - return true - } - return false + context.privateChatsContainMessage(withID: messageId) } func addMessageToPrivateChatsIfNeeded(_ message: BitchatMessage, targetPeerID: PeerID) { - if context.privateChats[targetPeerID] == nil { - context.privateChats[targetPeerID] = [] - } - if let idx = context.privateChats[targetPeerID]?.firstIndex(where: { $0.id == message.id }) { - context.privateChats[targetPeerID]?[idx] = message - } else { - context.privateChats[targetPeerID]?.append(message) - } - context.sanitizeChat(for: targetPeerID) + // Store upsert replaces in place by message ID or inserts in + // timestamp order; the old per-append sanitize re-sort is obsolete. + context.upsertPrivateMessage(message, in: targetPeerID) } func mirrorToEphemeralIfNeeded(_ message: BitchatMessage, targetPeerID: PeerID, key: Data?) { @@ -649,15 +647,7 @@ final class ChatPrivateConversationCoordinator { return } - if context.privateChats[ephemeralPeerID] == nil { - context.privateChats[ephemeralPeerID] = [] - } - if let idx = context.privateChats[ephemeralPeerID]?.firstIndex(where: { $0.id == message.id }) { - context.privateChats[ephemeralPeerID]?[idx] = message - } else { - context.privateChats[ephemeralPeerID]?.append(message) - } - context.sanitizeChat(for: ephemeralPeerID) + context.upsertPrivateMessage(message, in: ephemeralPeerID) } func handleViewingThisChat( @@ -666,10 +656,10 @@ final class ChatPrivateConversationCoordinator { key: Data?, senderPubkey: String ) { - context.unreadPrivateMessages.remove(targetPeerID) + context.markPrivateChatRead(targetPeerID) if let key, let ephemeralPeerID = context.ephemeralPeerID(forNoiseKey: key) { - context.unreadPrivateMessages.remove(ephemeralPeerID) + context.markPrivateChatRead(ephemeralPeerID) } guard !context.sentReadReceipts.contains(message.id) else { return } @@ -702,11 +692,11 @@ final class ChatPrivateConversationCoordinator { ) { guard shouldMarkAsUnread else { return } - context.unreadPrivateMessages.insert(targetPeerID) + context.markPrivateChatUnread(targetPeerID) if let key, let ephemeralPeerID = context.ephemeralPeerID(forNoiseKey: key), ephemeralPeerID != targetPeerID { - context.unreadPrivateMessages.insert(ephemeralPeerID) + context.markPrivateChatUnread(ephemeralPeerID) } if isRecentMessage { context.notifyPrivateMessage(from: senderNickname, message: messageContent, peerID: targetPeerID) @@ -778,8 +768,13 @@ final class ChatPrivateConversationCoordinator { let currentFingerprint = context.getFingerprint(for: peerID) if context.privateChats[peerID] == nil || context.privateChats[peerID]?.isEmpty == true { - var migratedMessages: [BitchatMessage] = [] + // Chats migrated wholesale go through the store's + // `migrateConversation` intent; partially-migrated chats keep + // their non-recent tail, so the recent messages are copied in + // via ordered append (dedup by ID) instead. + var partiallyMigratedMessages: [BitchatMessage] = [] var oldPeerIDsToRemove: [PeerID] = [] + var didMigrate = false let cutoffTime = Date().addingTimeInterval(-TransportConfig.uiMigrationCutoffSeconds) for (oldPeerID, messages) in context.privateChats where oldPeerID != peerID { @@ -790,10 +785,11 @@ final class ChatPrivateConversationCoordinator { if let currentFp = currentFingerprint, let oldFp = oldFingerprint, currentFp == oldFp { - migratedMessages.append(contentsOf: recentMessages) + didMigrate = true if recentMessages.count == messages.count { oldPeerIDsToRemove.append(oldPeerID) } else { + partiallyMigratedMessages.append(contentsOf: recentMessages) SecureLogger.info( "📦 Partially migrating \(recentMessages.count) of \(messages.count) messages from \(oldPeerID)", category: .session @@ -811,9 +807,11 @@ final class ChatPrivateConversationCoordinator { } if isRelevantChat { - migratedMessages.append(contentsOf: recentMessages) + didMigrate = true if recentMessages.count == messages.count { oldPeerIDsToRemove.append(oldPeerID) + } else { + partiallyMigratedMessages.append(contentsOf: recentMessages) } SecureLogger.warning( @@ -826,21 +824,22 @@ final class ChatPrivateConversationCoordinator { if !oldPeerIDsToRemove.isEmpty { for oldID in oldPeerIDsToRemove { - context.privateChats.removeValue(forKey: oldID) - context.unreadPrivateMessages.remove(oldID) + // The old behavior dropped the unread flag of removed + // chats instead of transferring it; clear it before the + // migration so the store doesn't carry it over. + context.markPrivateChatRead(oldID) + context.migratePrivateChat(from: oldID, to: peerID) context.clearStoredFingerprint(for: oldID) } context.handOffSelectedPrivateChat(from: oldPeerIDsToRemove, to: peerID) } - if !migratedMessages.isEmpty { - if context.privateChats[peerID] == nil { - context.privateChats[peerID] = [] - } - context.privateChats[peerID]?.append(contentsOf: migratedMessages) - context.privateChats[peerID]?.sort { $0.timestamp < $1.timestamp } - context.sanitizeChat(for: peerID) + for message in partiallyMigratedMessages { + context.appendPrivateMessage(message, to: peerID) + } + + if didMigrate { context.notifyUIChanged() } } diff --git a/bitchat/ViewModels/ChatPublicConversationCoordinator.swift b/bitchat/ViewModels/ChatPublicConversationCoordinator.swift index 41ae4ffd..2cb1ddd9 100644 --- a/bitchat/ViewModels/ChatPublicConversationCoordinator.swift +++ b/bitchat/ViewModels/ChatPublicConversationCoordinator.swift @@ -48,12 +48,17 @@ protocol ChatPublicConversationContext: AnyObject { func setConversationActiveChannel(_ channel: ChannelID) func replaceConversationMessages(_ messages: [BitchatMessage], for channelID: ChannelID) func replaceConversationMessages(_ messages: [BitchatMessage], for conversationID: ConversationID) - func synchronizePrivateConversationStore() func synchronizeConversationSelectionStore() // MARK: Private chats (block cleanup & message removal) - var privateChats: [PeerID: [BitchatMessage]] { get set } - var unreadPrivateMessages: Set { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } + /// Removes the peer's chat entirely, including unread state + /// (single-writer store intent). + func removePrivateChat(_ peerID: PeerID) + /// Removes a message by ID from every private chat containing it, + /// dropping chats that become empty. Returns the removed message. + @discardableResult + func removePrivateMessage(withID messageID: String) -> BitchatMessage? func cleanupLocalFile(forMessage message: BitchatMessage) // MARK: Geohash participants & presence @@ -251,13 +256,7 @@ final class ChatPublicConversationCoordinator: PublicMessagePipelineDelegate { let conversationPeerID = PeerID(nostr_: hex) if context.privateChats[conversationPeerID] != nil { - var privateChats = context.privateChats - privateChats.removeValue(forKey: conversationPeerID) - context.privateChats = privateChats - - var unread = context.unreadPrivateMessages - unread.remove(conversationPeerID) - context.unreadPrivateMessages = unread + context.removePrivateChat(conversationPeerID) } context.removeNostrKeyMappings(matchingPubkeyHexLowercased: hex) @@ -325,21 +324,9 @@ final class ChatPublicConversationCoordinator: PublicMessagePipelineDelegate { synchronizeAllPublicConversationStores() } - var chats = context.privateChats - for (peerID, items) in chats { - let filtered = items.filter { $0.id != messageID } - if filtered.count != items.count { - if filtered.isEmpty { - chats.removeValue(forKey: peerID) - } else { - chats[peerID] = filtered - } - if removedMessage == nil { - removedMessage = items.first(where: { $0.id == messageID }) - } - } + if let removedPrivateMessage = context.removePrivateMessage(withID: messageID) { + removedMessage = removedMessage ?? removedPrivateMessage } - context.privateChats = chats if cleanupFile, let removedMessage { context.cleanupLocalFile(forMessage: removedMessage) @@ -351,7 +338,6 @@ final class ChatPublicConversationCoordinator: PublicMessagePipelineDelegate { func initializeConversationStore() { context.setConversationActiveChannel(context.activeChannel) synchronizePublicConversationStore(for: context.activeChannel) - context.synchronizePrivateConversationStore() context.synchronizeConversationSelectionStore() } diff --git a/bitchat/ViewModels/ChatTransportEventCoordinator.swift b/bitchat/ViewModels/ChatTransportEventCoordinator.swift index ee19a527..33e46b93 100644 --- a/bitchat/ViewModels/ChatTransportEventCoordinator.swift +++ b/bitchat/ViewModels/ChatTransportEventCoordinator.swift @@ -15,9 +15,17 @@ protocol ChatTransportEventContext: AnyObject { var isConnected: Bool { get set } var nickname: String { get } var myPeerID: PeerID { get } - var privateChats: [PeerID: [BitchatMessage]] { get set } - var unreadPrivateMessages: Set { get set } + var privateChats: [PeerID: [BitchatMessage]] { get } + var unreadPrivateMessages: Set { get } var selectedPrivateChatPeer: PeerID? { get set } + /// Appends a private message via the single-writer store intent; + /// returns `false` on duplicate message ID. + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool + /// Removes the peer's chat entirely, including unread state. + func removePrivateChat(_ peerID: PeerID) + func markPrivateChatUnread(_ peerID: PeerID) + func markPrivateChatRead(_ peerID: PeerID) /// Forgets that read receipts were sent for `ids` so READ acks can be /// re-sent after the peer reconnects. (Single mutation path for the /// owner's `sentReadReceipts`; this coordinator never reads the raw set.) @@ -251,13 +259,12 @@ private extension ChatTransportEventCoordinator { to stablePeerID: PeerID, in context: any ChatTransportEventContext ) { - if let messages = context.privateChats[shortPeerID] { - if context.privateChats[stablePeerID] == nil { - context.privateChats[stablePeerID] = [] - } + let hadUnread = context.unreadPrivateMessages.contains(shortPeerID) - let existingIDs = Set(context.privateChats[stablePeerID]?.map(\.id) ?? []) - for message in messages where !existingIDs.contains(message.id) { + if let messages = context.privateChats[shortPeerID] { + for message in messages { + // Rewrite senderPeerID to the stable key so read receipts + // keep working; store append dedups by ID and keeps order. let migrated = BitchatMessage( id: message.id, sender: message.sender, @@ -273,16 +280,15 @@ private extension ChatTransportEventCoordinator { mentions: message.mentions, deliveryStatus: message.deliveryStatus ) - context.privateChats[stablePeerID]?.append(migrated) + context.appendPrivateMessage(migrated, to: stablePeerID) } - context.privateChats[stablePeerID]?.sort { $0.timestamp < $1.timestamp } - context.privateChats.removeValue(forKey: shortPeerID) + context.removePrivateChat(shortPeerID) } - if context.unreadPrivateMessages.contains(shortPeerID) { - context.unreadPrivateMessages.remove(shortPeerID) - context.unreadPrivateMessages.insert(stablePeerID) + if hadUnread { + context.markPrivateChatRead(shortPeerID) + context.markPrivateChatUnread(stablePeerID) } context.selectedPrivateChatPeer = stablePeerID diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index fbd3daab..948514db 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -164,13 +164,19 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele @MainActor var connectedPeers: Set { unifiedPeerService.connectedPeerIDs } @Published var allPeers: [BitchatPeer] = [] + + /// Read-only compat view of all direct conversations in the new + /// `ConversationStore`, keyed by routing peer ID (migration step 2 shim; + /// views/feature models observe `Conversation` objects directly in + /// step 5). All mutations go through the private-chat intent ops below. + /// Rebuilt per access — O(#conversations) thanks to COW message arrays; + /// measured equal to a change-invalidated cache on + /// `pipeline.privateIngest`, so the simpler form wins. + @MainActor var privateChats: [PeerID: [BitchatMessage]] { - get { privateChatManager.privateChats } - set { - privateChatManager.privateChats = newValue - schedulePrivateConversationStoreSynchronization() - } + conversations.directMessagesByRoutingPeerID() } + @MainActor var selectedPrivateChatPeer: PeerID? { get { privateChatManager.selectedPeer } set { @@ -179,19 +185,19 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele } else { privateChatManager.endChat() } - synchronizePrivateConversationStore() synchronizeConversationSelectionStore() } } + /// Read-only compat view of the store's unread direct conversations + /// (migration step 2 shim). Mutate via `markPrivateChatUnread(_:)` / + /// `markPrivateChatRead(_:)`. + @MainActor var unreadPrivateMessages: Set { - get { privateChatManager.unreadMessages } - set { - privateChatManager.unreadMessages = newValue - schedulePrivateConversationStoreSynchronization() - } + conversations.unreadDirectRoutingPeerIDs() } /// Check if there are any unread messages (including from temporary Nostr peer IDs) + @MainActor var hasAnyUnreadMessages: Bool { !unreadPrivateMessages.isEmpty } @@ -271,6 +277,13 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele let idBridge: NostrIdentityBridge let identityManager: SecureIdentityStateManagerProtocol let conversationStore: LegacyConversationStore + /// Single source of truth for conversation message state + /// (docs/CONVERSATION-STORE-DESIGN.md). Owned by `AppRuntime` and passed + /// through, mirroring the legacy store's wiring. + let conversations: ConversationStore + /// Keeps `LegacyConversationStore` fed from `conversations` while feature + /// models still read Legacy (DELETE IN STEP 5). + private var legacyStoreBridge: LegacyConversationStoreBridge? let identityResolver: IdentityResolver let peerIdentityStore: PeerIdentityStore let locationPresenceStore: LocationPresenceStore @@ -374,7 +387,6 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele // MARK: - Message Delivery Tracking var cancellables = Set() - private var pendingPrivateConversationStoreSyncTask: Task? var transferIdToMessageIDs: [String: [String]] { mediaTransferCoordinator.transferIdToMessageIDs @@ -525,6 +537,100 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele selectedPrivateChatPeer = newPeerID } + // MARK: - Private Conversation Store Intents + // The sole mutation paths for private (direct) message state. Each op + // forwards to the single-writer `ConversationStore` + // (docs/CONVERSATION-STORE-DESIGN.md); the read-only `privateChats` / + // `unreadPrivateMessages` shims above are derived from the same store. + + /// Appends a private message in timestamp order. Returns `false` when a + /// message with the same ID is already in that chat (O(1) dedup via the + /// conversation's ID index). + @MainActor + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool { + conversations.append(message, to: .directPeer(peerID)) + } + + /// Replace-or-append a private message by ID (media progress, mirrored + /// copies); an existing message keeps its timeline position. + @MainActor + func upsertPrivateMessage(_ message: BitchatMessage, in peerID: PeerID) { + conversations.upsertByID(message, in: .directPeer(peerID)) + } + + /// Applies a delivery status to a private message by ID. Returns `false` + /// when the message is unknown or the update would downgrade the status + /// (read beats delivered beats sent). + @MainActor + @discardableResult + func setPrivateDeliveryStatus(_ status: DeliveryStatus, forMessageID messageID: String, peerID: PeerID) -> Bool { + conversations.setDeliveryStatus(status, forMessageID: messageID, in: .directPeer(peerID)) + } + + /// Flags the peer's chat as unread (store unread state). + @MainActor + func markPrivateChatUnread(_ peerID: PeerID) { + conversations.markUnread(.directPeer(peerID)) + } + + /// Clears the peer's unread flag (store unread state only; read-receipt + /// sending stays in `PrivateChatManager.markAsRead`). + @MainActor + func markPrivateChatRead(_ peerID: PeerID) { + conversations.markRead(.directPeer(peerID)) + } + + /// Empties the peer's chat but keeps the conversation alive (`/clear`). + @MainActor + func clearPrivateChat(_ peerID: PeerID) { + conversations.clear(.directPeer(peerID)) + } + + /// Removes the peer's chat entirely, including unread state. + @MainActor + func removePrivateChat(_ peerID: PeerID) { + conversations.removeConversation(.directPeer(peerID)) + } + + /// Moves all messages from `oldPeerID`'s chat into `newPeerID`'s chat + /// (ephemeral↔stable peer-ID handoff): dedups by ID, preserves order, + /// carries unread state, removes the old chat. + @MainActor + func migratePrivateChat(from oldPeerID: PeerID, to newPeerID: PeerID) { + conversations.migrateConversation(from: .directPeer(oldPeerID), to: .directPeer(newPeerID)) + } + + /// `true` when any private chat contains a message with `messageID` + /// (O(1) per conversation via the store's ID indexes). + @MainActor + func privateChatsContainMessage(withID messageID: String) -> Bool { + conversations.directConversationsContainMessage(withID: messageID) + } + + /// `true` when `peerID`'s chat contains a message with `messageID`. + @MainActor + func privateChat(_ peerID: PeerID, containsMessageWithID messageID: String) -> Bool { + conversations.conversationsByID[.directPeer(peerID)]?.containsMessage(withID: messageID) ?? false + } + + /// Removes a message by ID from every private chat that contains it, + /// dropping chats that become empty. Returns the removed message, if any. + @MainActor + @discardableResult + func removePrivateMessage(withID messageID: String) -> BitchatMessage? { + var removed: BitchatMessage? + for (id, conversation) in conversations.conversationsByID { + guard case .direct = id, conversation.containsMessage(withID: messageID) else { continue } + let message = conversations.removeMessage(withID: messageID, from: id) + removed = removed ?? message + if conversation.messages.isEmpty { + conversations.removeConversation(id) + } + } + return removed + } + // MARK: - Initialization @MainActor @@ -533,6 +639,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele idBridge: NostrIdentityBridge, identityManager: SecureIdentityStateManagerProtocol, conversationStore: LegacyConversationStore? = nil, + conversations: ConversationStore? = nil, identityResolver: IdentityResolver? = nil, peerIdentityStore: PeerIdentityStore? = nil, locationPresenceStore: LocationPresenceStore? = nil, @@ -546,6 +653,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele identityManager: identityManager, transport: BLEService(keychain: keychain, idBridge: idBridge, identityManager: identityManager), conversationStore: conversationStore, + conversations: conversations, identityResolver: identityResolver, peerIdentityStore: peerIdentityStore ?? PeerIdentityStore(), locationPresenceStore: locationPresenceStore ?? LocationPresenceStore(), @@ -562,12 +670,14 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele identityManager: SecureIdentityStateManagerProtocol, transport: Transport, conversationStore: LegacyConversationStore? = nil, + conversations: ConversationStore? = nil, identityResolver: IdentityResolver? = nil, peerIdentityStore: PeerIdentityStore? = nil, locationPresenceStore: LocationPresenceStore? = nil, locationManager: LocationChannelManager = .shared ) { let conversationStore = conversationStore ?? LegacyConversationStore() + let conversations = conversations ?? ConversationStore() let identityResolver = identityResolver ?? IdentityResolver() let peerIdentityStore = peerIdentityStore ?? PeerIdentityStore() let locationPresenceStore = locationPresenceStore ?? LocationPresenceStore() @@ -582,6 +692,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele self.idBridge = idBridge self.identityManager = identityManager self.conversationStore = conversationStore + self.conversations = conversations self.identityResolver = identityResolver self.peerIdentityStore = peerIdentityStore self.locationPresenceStore = locationPresenceStore @@ -596,6 +707,25 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele self.publicMessagePipeline = services.publicMessagePipeline self.sentReadReceipts = ChatViewModelBootstrapper.loadPersistedReadReceipts() + // Keep the legacy store fed from the new store until the feature + // models cut over (migration step 5). + self.legacyStoreBridge = LegacyConversationStoreBridge( + store: conversations, + legacyStore: conversationStore, + identityResolver: identityResolver + ) + + // Republish on every store change so SwiftUI observers of the + // view model refresh. This replaces the UI-update role of the old + // `PrivateChatManager.@Published` dictionaries (their debounced + // Legacy synchronization sinks are gone; the bridge above feeds + // Legacy instead). + conversations.changes + .sink { [weak self] _ in + self?.objectWillChange.send() + } + .store(in: &cancellables) + ChatViewModelBootstrapper(viewModel: self).configure() initializeConversationStore() } @@ -789,8 +919,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele recipientNickname: meshService.peerNickname(peerID: peerID), senderPeerID: meshService.myPeerID ) - if privateChats[peerID] == nil { privateChats[peerID] = [] } - privateChats[peerID]?.append(systemMessage) + appendPrivateMessage(systemMessage, to: peerID) objectWillChange.send() } @@ -885,8 +1014,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele meshCap: TransportConfig.meshTimelineCap, geohashCap: TransportConfig.geoTimelineCap ) - privateChatManager.privateChats.removeAll() - privateChatManager.unreadMessages.removeAll() + conversations.removeAllDirectConversations() // Delete all keychain data (including Noise and Nostr keys) _ = keychain.deleteAllKeychainData() @@ -1080,24 +1208,13 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele publicConversationCoordinator.synchronizeAllPublicConversationStores() } + /// Full Legacy-store resynchronization from the new `ConversationStore`. + /// Needed when `IdentityResolver` learns new peer associations, which can + /// re-key a direct conversation's canonical handle in Legacy + /// (DELETE IN STEP 5 with the bridge). @MainActor - func schedulePrivateConversationStoreSynchronization() { - guard pendingPrivateConversationStoreSyncTask == nil else { return } - pendingPrivateConversationStoreSyncTask = Task { @MainActor [weak self] in - await Task.yield() - guard let self else { return } - self.pendingPrivateConversationStoreSyncTask = nil - self.synchronizePrivateConversationStore() - } - } - - @MainActor - func synchronizePrivateConversationStore() { - conversationStore.synchronizePrivateChats( - privateChatManager.privateChats, - unreadPeerIDs: privateChatManager.unreadMessages, - identityResolver: identityResolver - ) + func resynchronizeLegacyPrivateConversations() { + legacyStoreBridge?.resynchronizeAll() } @MainActor diff --git a/bitchat/ViewModels/ChatViewModelBootstrapper.swift b/bitchat/ViewModels/ChatViewModelBootstrapper.swift index 3fec30e0..5e1b71d4 100644 --- a/bitchat/ViewModels/ChatViewModelBootstrapper.swift +++ b/bitchat/ViewModels/ChatViewModelBootstrapper.swift @@ -74,6 +74,7 @@ final class ChatViewModelBootstrapper { private extension ChatViewModelBootstrapper { func wireServiceGraph() { + viewModel.privateChatManager.conversationStore = viewModel.conversations viewModel.privateChatManager.messageRouter = viewModel.messageRouter viewModel.privateChatManager.unifiedPeerService = viewModel.unifiedPeerService viewModel.unifiedPeerService.messageRouter = viewModel.messageRouter @@ -89,24 +90,10 @@ private extension ChatViewModelBootstrapper { } .store(in: &viewModel.cancellables) - viewModel.privateChatManager.$privateChats - .receive(on: DispatchQueue.main) - .sink { [weak viewModel] _ in - Task { @MainActor [weak viewModel] in - viewModel?.schedulePrivateConversationStoreSynchronization() - } - } - .store(in: &viewModel.cancellables) - - viewModel.privateChatManager.$unreadMessages - .receive(on: DispatchQueue.main) - .sink { [weak viewModel] _ in - Task { @MainActor [weak viewModel] in - viewModel?.schedulePrivateConversationStoreSynchronization() - } - } - .store(in: &viewModel.cancellables) - + // Private message state now flows: store intent → + // `ConversationStore.changes` → `LegacyConversationStoreBridge` (and + // the ChatViewModel shim-cache sink), so the old `$privateChats` / + // `$unreadMessages` debounced synchronization sinks are gone. viewModel.privateChatManager.$selectedPeer .receive(on: DispatchQueue.main) .sink { [weak viewModel] _ in @@ -192,7 +179,9 @@ private extension ChatViewModelBootstrapper { viewModel.updatePrivateChatPeerIfNeeded() } - viewModel.synchronizePrivateConversationStore() + // Peer registrations can change a conversation's + // canonical handle in the legacy store; re-key it. + viewModel.resynchronizeLegacyPrivateConversations() viewModel.synchronizeConversationSelectionStore() } } diff --git a/bitchatTests/AppArchitectureTests.swift b/bitchatTests/AppArchitectureTests.swift index 975d2ec3..095e864d 100644 --- a/bitchatTests/AppArchitectureTests.swift +++ b/bitchatTests/AppArchitectureTests.swift @@ -631,7 +631,7 @@ struct AppArchitectureTests { transport.reachablePeers.insert(otherPeerID) viewModel.nickname = "builder" viewModel.verifiedFingerprints.insert(verifiedFingerprint) - viewModel.unreadPrivateMessages = Set([otherPeerID]) + viewModel.markPrivateChatUnread(otherPeerID) transport.updatePeerSnapshots([ makeArchitectureSnapshot( peerID: myPeerID, diff --git a/bitchatTests/ChatLifecycleCoordinatorContextTests.swift b/bitchatTests/ChatLifecycleCoordinatorContextTests.swift index 45303bbc..8e5d2a88 100644 --- a/bitchatTests/ChatLifecycleCoordinatorContextTests.swift +++ b/bitchatTests/ChatLifecycleCoordinatorContextTests.swift @@ -38,9 +38,23 @@ private final class MockChatLifecycleContext: ChatLifecycleContext { var nostrKeyMapping: [PeerID: String] = [:] private(set) var ownerLevelReadPasses: [PeerID] = [] private(set) var managerReadMarks: [PeerID] = [] - private(set) var privateStoreSyncCount = 0 private(set) var systemMessages: [String] = [] + // Conversation store intents + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool { + var chat = privateChats[peerID] ?? [] + guard !chat.contains(where: { $0.id == message.id }) else { return false } + let index = chat.firstIndex(where: { $0.timestamp > message.timestamp }) ?? chat.count + chat.insert(message, at: index) + privateChats[peerID] = chat + return true + } + + func markPrivateChatRead(_ peerID: PeerID) { + unreadPrivateMessages.remove(peerID) + } + @discardableResult func markReadReceiptSent(_ messageID: String) -> Bool { sentReadReceipts.insert(messageID).inserted @@ -61,7 +75,6 @@ private final class MockChatLifecycleContext: ChatLifecycleContext { work() } - func synchronizePrivateConversationStore() { privateStoreSyncCount += 1 } func addSystemMessage(_ content: String) { systemMessages.append(content) } // Peers & sessions @@ -234,7 +247,6 @@ struct ChatLifecycleCoordinatorContextTests { coordinator.markPrivateMessagesAsRead(from: convKey) #expect(context.managerReadMarks == [convKey]) - #expect(context.privateStoreSyncCount == 1) // Only the peer's own un-acked, non-relay message gets a READ. #expect(context.geoReadReceipts.map(\.messageID) == ["m1"]) #expect(context.geoReadReceipts.first?.recipientHex == recipientHex) diff --git a/bitchatTests/ChatMediaTransferCoordinatorContextTests.swift b/bitchatTests/ChatMediaTransferCoordinatorContextTests.swift index 12e639a1..32cf8276 100644 --- a/bitchatTests/ChatMediaTransferCoordinatorContextTests.swift +++ b/bitchatTests/ChatMediaTransferCoordinatorContextTests.swift @@ -42,6 +42,16 @@ private final class MockChatMediaTransferContext: ChatMediaTransferContext { // Message state var privateChats: [PeerID: [BitchatMessage]] = [:] + + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool { + var chat = privateChats[peerID] ?? [] + guard !chat.contains(where: { $0.id == message.id }) else { return false } + chat.append(message) + privateChats[peerID] = chat + return true + } + var meshTimeline: [BitchatMessage] = [] private(set) var refreshedChannels: [ChannelID?] = [] private(set) var trimCount = 0 diff --git a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift index e162b77d..ebceeb30 100644 --- a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift +++ b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift @@ -38,11 +38,34 @@ private final class MockChatPeerIdentityContext: ChatPeerIdentityContext { func notifyUIChanged() { notifyUIChangedCount += 1 } func addSystemMessage(_ content: String) { systemMessages.append(content) } + // Conversation store intents (mirror `ConversationStore` migrate + // semantics: dedup by ID, timestamp order, unread carried, old chat + // removed) while recording calls for assertions. + private(set) var migratedChats: [(from: PeerID, to: PeerID)] = [] + + func markPrivateChatRead(_ peerID: PeerID) { + unreadPrivateMessages.remove(peerID) + } + + func migratePrivateChat(from oldPeerID: PeerID, to newPeerID: PeerID) { + migratedChats.append((oldPeerID, newPeerID)) + guard oldPeerID != newPeerID, let source = privateChats[oldPeerID] else { return } + var destination = privateChats[newPeerID] ?? [] + for message in source where !destination.contains(where: { $0.id == message.id }) { + let index = destination.firstIndex(where: { $0.timestamp > message.timestamp }) ?? destination.count + destination.insert(message, at: index) + } + privateChats[newPeerID] = destination + privateChats.removeValue(forKey: oldPeerID) + if unreadPrivateMessages.remove(oldPeerID) != nil { + unreadPrivateMessages.insert(newPeerID) + } + } + // Private chat session lifecycle private(set) var consolidatedPeers: [(peerID: PeerID, peerNickname: String)] = [] private(set) var syncedReadReceiptPeers: [PeerID] = [] private(set) var begunChatSessions: [PeerID] = [] - private(set) var privateStoreSyncCount = 0 private(set) var selectionStoreSyncCount = 0 private(set) var markedReadPeers: [PeerID] = [] @@ -60,7 +83,6 @@ private final class MockChatPeerIdentityContext: ChatPeerIdentityContext { begunChatSessions.append(peerID) } - func synchronizePrivateConversationStore() { privateStoreSyncCount += 1 } func synchronizeConversationSelectionStore() { selectionStoreSyncCount += 1 } func markPrivateMessagesAsRead(from peerID: PeerID) { markedReadPeers.append(peerID) } @@ -261,7 +283,6 @@ struct ChatPeerIdentityCoordinatorContextTests { #expect(context.storedFingerprints.map(\.fingerprint) == ["fp-alice"]) #expect(context.selectedPrivateChatFingerprint == "fp-alice") #expect(context.begunChatSessions == [peerID]) - #expect(context.privateStoreSyncCount == 1) #expect(context.selectionStoreSyncCount == 1) #expect(context.markedReadPeers == [peerID]) diff --git a/bitchatTests/ChatPeerListCoordinatorContextTests.swift b/bitchatTests/ChatPeerListCoordinatorContextTests.swift index 67e433f5..8a81611c 100644 --- a/bitchatTests/ChatPeerListCoordinatorContextTests.swift +++ b/bitchatTests/ChatPeerListCoordinatorContextTests.swift @@ -32,6 +32,10 @@ private final class MockChatPeerListContext: ChatPeerListContext { private(set) var updatePrivateChatPeerIfNeededCount = 0 private(set) var cleanupOldReadReceiptsCount = 0 + func markPrivateChatRead(_ peerID: PeerID) { + unreadPrivateMessages.remove(peerID) + } + func updatePrivateChatPeerIfNeeded() { updatePrivateChatPeerIfNeededCount += 1 } diff --git a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift index 7a6fcf67..2a19afe9 100644 --- a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift +++ b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift @@ -48,6 +48,90 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC notifyUIChangedCount += 1 } + // Conversation store intents (mirror `ConversationStore` semantics: + // ordered insert, dedup by ID, no-downgrade status, unread carry on + // migrate) while recording calls for assertions. + private(set) var upsertedMessages: [(messageID: String, peerID: PeerID)] = [] + private(set) var migratedChats: [(from: PeerID, to: PeerID)] = [] + + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool { + var chat = privateChats[peerID] ?? [] + guard !chat.contains(where: { $0.id == message.id }) else { + privateChats[peerID] = chat + return false + } + let index = chat.firstIndex(where: { $0.timestamp > message.timestamp }) ?? chat.count + chat.insert(message, at: index) + privateChats[peerID] = chat + return true + } + + func upsertPrivateMessage(_ message: BitchatMessage, in peerID: PeerID) { + upsertedMessages.append((message.id, peerID)) + if var chat = privateChats[peerID], + let index = chat.firstIndex(where: { $0.id == message.id }) { + chat[index] = message + privateChats[peerID] = chat + } else { + appendPrivateMessage(message, to: peerID) + } + } + + @discardableResult + func setPrivateDeliveryStatus(_ status: DeliveryStatus, forMessageID messageID: String, peerID: PeerID) -> Bool { + guard var chat = privateChats[peerID], + let index = chat.firstIndex(where: { $0.id == messageID }) else { + return false + } + if Conversation.shouldSkipStatusUpdate(current: chat[index].deliveryStatus, new: status) { + return false + } + chat[index].deliveryStatus = status + privateChats[peerID] = chat + return true + } + + func markPrivateChatUnread(_ peerID: PeerID) { + unreadPrivateMessages.insert(peerID) + } + + func markPrivateChatRead(_ peerID: PeerID) { + unreadPrivateMessages.remove(peerID) + } + + func removePrivateChat(_ peerID: PeerID) { + privateChats.removeValue(forKey: peerID) + unreadPrivateMessages.remove(peerID) + } + + func migratePrivateChat(from oldPeerID: PeerID, to newPeerID: PeerID) { + migratedChats.append((oldPeerID, newPeerID)) + guard oldPeerID != newPeerID, let source = privateChats[oldPeerID] else { return } + for message in source { + appendPrivateMessage(message, to: newPeerID) + } + if privateChats[newPeerID] == nil { + privateChats[newPeerID] = [] + } + let wasUnread = unreadPrivateMessages.contains(oldPeerID) + privateChats.removeValue(forKey: oldPeerID) + unreadPrivateMessages.remove(oldPeerID) + if wasUnread { + unreadPrivateMessages.insert(newPeerID) + } + } + + func privateChatsContainMessage(withID messageID: String) -> Bool { + privateChats.values.contains { chat in + chat.contains { $0.id == messageID } + } + } + + func privateChat(_ peerID: PeerID, containsMessageWithID messageID: String) -> Bool { + privateChats[peerID]?.contains { $0.id == messageID } == true + } + // Peers & identity var myPeerID = PeerID(str: "0011223344556677") var nicknamesByPeerID: [PeerID: String] = [:] @@ -149,10 +233,9 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC privateMessageNotifications.append((senderName, message, peerID)) } - // System messages & chat hygiene + // System messages private(set) var systemMessages: [String] = [] private(set) var meshOnlySystemMessages: [String] = [] - private(set) var sanitizedPeerIDs: [PeerID] = [] func addSystemMessage(_ content: String) { systemMessages.append(content) @@ -162,10 +245,6 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC meshOnlySystemMessages.append(content) } - func sanitizeChat(for peerID: PeerID) { - sanitizedPeerIDs.append(peerID) - } - static let dummyIdentity = NostrIdentity( privateKey: Data(repeating: 0x11, count: 32), publicKey: Data(repeating: 0x22, count: 32), @@ -256,7 +335,9 @@ struct ChatPrivateConversationCoordinatorContextTests { // A different id appends. coordinator.addMessageToPrivateChatsIfNeeded(makeIncomingMessage(id: "m2"), targetPeerID: peerID) #expect(context.privateChats[peerID]?.map(\.id) == ["m1", "m2"]) - #expect(context.sanitizedPeerIDs == [peerID, peerID, peerID]) + // Every add went through the store's upsert intent. + #expect(context.upsertedMessages.map(\.peerID) == [peerID, peerID, peerID]) + #expect(context.upsertedMessages.map(\.messageID) == ["m1", "m1", "m2"]) #expect(coordinator.isDuplicateMessage("m1", targetPeerID: peerID)) #expect(!coordinator.isDuplicateMessage("m3", targetPeerID: peerID)) @@ -436,7 +517,9 @@ struct ChatPrivateConversationCoordinatorContextTests { #expect(context.unreadPrivateMessages.isEmpty) #expect(context.clearedFingerprints == [oldPeerID]) #expect(context.selectedPrivateChatPeer == newPeerID) - #expect(context.sanitizedPeerIDs == [newPeerID]) + // The wholesale move went through the store's migrate intent. + #expect(context.migratedChats.map(\.from) == [oldPeerID]) + #expect(context.migratedChats.map(\.to) == [newPeerID]) #expect(context.notifyUIChangedCount == 1) } diff --git a/bitchatTests/ChatPublicConversationCoordinatorContextTests.swift b/bitchatTests/ChatPublicConversationCoordinatorContextTests.swift index fb661241..55b40079 100644 --- a/bitchatTests/ChatPublicConversationCoordinatorContextTests.swift +++ b/bitchatTests/ChatPublicConversationCoordinatorContextTests.swift @@ -111,7 +111,6 @@ private final class MockChatPublicConversationContext: ChatPublicConversationCon private(set) var conversationActiveChannels: [ChannelID] = [] private(set) var replacedChannelMessages: [(channel: ChannelID, messageIDs: [String])] = [] private(set) var replacedConversationMessages: [(conversation: ConversationID, messageIDs: [String])] = [] - private(set) var privateStoreSyncCount = 0 private(set) var selectionStoreSyncCount = 0 func setConversationActiveChannel(_ channel: ChannelID) { @@ -126,10 +125,6 @@ private final class MockChatPublicConversationContext: ChatPublicConversationCon replacedConversationMessages.append((conversationID, messages.map(\.id))) } - func synchronizePrivateConversationStore() { - privateStoreSyncCount += 1 - } - func synchronizeConversationSelectionStore() { selectionStoreSyncCount += 1 } @@ -137,8 +132,31 @@ private final class MockChatPublicConversationContext: ChatPublicConversationCon // Private chats var privateChats: [PeerID: [BitchatMessage]] = [:] var unreadPrivateMessages: Set = [] + private(set) var removedPrivateChats: [PeerID] = [] private(set) var cleanedUpFileMessageIDs: [String] = [] + func removePrivateChat(_ peerID: PeerID) { + removedPrivateChats.append(peerID) + privateChats.removeValue(forKey: peerID) + unreadPrivateMessages.remove(peerID) + } + + @discardableResult + func removePrivateMessage(withID messageID: String) -> BitchatMessage? { + var removed: BitchatMessage? + for (peerID, chat) in privateChats { + guard let message = chat.first(where: { $0.id == messageID }) else { continue } + removed = removed ?? message + let remaining = chat.filter { $0.id != messageID } + if remaining.isEmpty { + privateChats.removeValue(forKey: peerID) + } else { + privateChats[peerID] = remaining + } + } + return removed + } + func cleanupLocalFile(forMessage message: BitchatMessage) { cleanedUpFileMessageIDs.append(message.id) } diff --git a/bitchatTests/ChatTransportEventCoordinatorContextTests.swift b/bitchatTests/ChatTransportEventCoordinatorContextTests.swift index 9b6ec7ce..e9de3901 100644 --- a/bitchatTests/ChatTransportEventCoordinatorContextTests.swift +++ b/bitchatTests/ChatTransportEventCoordinatorContextTests.swift @@ -33,6 +33,30 @@ private final class MockChatTransportEventContext: ChatTransportEventContext { private(set) var unmarkedReadReceiptBatches: [[String]] = [] private(set) var notifyUIChangedCount = 0 + // Conversation store intents (mirror `ConversationStore` semantics) + @discardableResult + func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool { + var chat = privateChats[peerID] ?? [] + guard !chat.contains(where: { $0.id == message.id }) else { return false } + let index = chat.firstIndex(where: { $0.timestamp > message.timestamp }) ?? chat.count + chat.insert(message, at: index) + privateChats[peerID] = chat + return true + } + + func removePrivateChat(_ peerID: PeerID) { + privateChats.removeValue(forKey: peerID) + unreadPrivateMessages.remove(peerID) + } + + func markPrivateChatUnread(_ peerID: PeerID) { + unreadPrivateMessages.insert(peerID) + } + + func markPrivateChatRead(_ peerID: PeerID) { + unreadPrivateMessages.remove(peerID) + } + func unmarkReadReceiptsSent(_ ids: [String]) { unmarkedReadReceiptBatches.append(ids) } diff --git a/bitchatTests/ChatViewModelDeliveryStatusTests.swift b/bitchatTests/ChatViewModelDeliveryStatusTests.swift index 9730c053..d176b249 100644 --- a/bitchatTests/ChatViewModelDeliveryStatusTests.swift +++ b/bitchatTests/ChatViewModelDeliveryStatusTests.swift @@ -54,7 +54,7 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .read(by: "Peer", at: Date()) ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) // Action: try to downgrade to .delivered viewModel.didUpdateMessageDeliveryStatus(messageID, status: .delivered(to: "Peer", at: Date())) @@ -85,7 +85,7 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) // Action: upgrade to .delivered viewModel.didUpdateMessageDeliveryStatus(messageID, status: .delivered(to: "Peer", at: Date())) @@ -114,7 +114,7 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) let didUpdate = viewModel.deliveryCoordinator.updateMessageDeliveryStatus(messageID, status: .sent) @@ -140,7 +140,7 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .delivered(to: "Peer", at: Date().addingTimeInterval(-60)) ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) // Action: upgrade to .read viewModel.didUpdateMessageDeliveryStatus(messageID, status: .read(by: "Peer", at: Date())) @@ -173,7 +173,7 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) // Action: receive read receipt let receipt = ReadReceipt( @@ -207,7 +207,7 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) viewModel.sentReadReceipts = ["keep-receipt", "drop-receipt"] viewModel.isStartupPhase = false @@ -265,7 +265,7 @@ struct ChatViewModelDeliveryStatusTests { deliveryStatus: .sent ) ] - viewModel.privateChats[firstPeerID] = [ + viewModel.seedPrivateChat([ BitchatMessage( id: messageID, sender: viewModel.nickname, @@ -277,8 +277,8 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .sent ) - ] - viewModel.privateChats[secondPeerID] = [ + ], for: firstPeerID) + viewModel.seedPrivateChat([ BitchatMessage( id: messageID, sender: viewModel.nickname, @@ -290,7 +290,7 @@ struct ChatViewModelDeliveryStatusTests { senderPeerID: transport.myPeerID, deliveryStatus: .sent ) - ] + ], for: secondPeerID) let didUpdate = viewModel.deliveryCoordinator.updateMessageDeliveryStatus( messageID, @@ -331,10 +331,15 @@ struct ChatViewModelDeliveryStatusTests { deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [targetMessage, olderMessage] + viewModel.seedPrivateChat([targetMessage], for: peerID) #expect(isSent(viewModel.deliveryCoordinator.deliveryStatus(for: messageID))) - viewModel.privateChats[peerID] = [olderMessage, targetMessage] + // A late arrival with an older timestamp is inserted before the + // target by the store, shifting its position and invalidating the + // delivery coordinator's location index. + viewModel.seedPrivateChat([olderMessage], for: peerID) + #expect(viewModel.privateChats[peerID]?.map(\.id) == ["older-msg", messageID]) + let didUpdate = viewModel.deliveryCoordinator.updateMessageDeliveryStatus( messageID, status: .read(by: "Peer", at: Date()) @@ -401,6 +406,20 @@ private final class MockChatDeliveryContext: ChatDeliveryContext { func markMessageDelivered(_ messageID: String) { markedDeliveredMessageIDs.append(messageID) } + + @discardableResult + func setPrivateDeliveryStatus(_ status: DeliveryStatus, forMessageID messageID: String, peerID: PeerID) -> Bool { + guard var chat = privateChats[peerID], + let index = chat.firstIndex(where: { $0.id == messageID }) else { + return false + } + if Conversation.shouldSkipStatusUpdate(current: chat[index].deliveryStatus, new: status) { + return false + } + chat[index].deliveryStatus = status + privateChats[peerID] = chat + return true + } } @MainActor diff --git a/bitchatTests/ChatViewModelExtensionsTests.swift b/bitchatTests/ChatViewModelExtensionsTests.swift index b275358c..eb136cfa 100644 --- a/bitchatTests/ChatViewModelExtensionsTests.swift +++ b/bitchatTests/ChatViewModelExtensionsTests.swift @@ -177,7 +177,7 @@ struct ChatViewModelPrivateChatExtensionTests { isPrivate: true, senderPeerID: oldPeerID ) - viewModel.privateChats[oldPeerID] = [oldMessage] + viewModel.seedPrivateChat([oldMessage], for: oldPeerID) viewModel.peerIDToPublicKeyFingerprint[oldPeerID] = fingerprint // Setup new peer fingerprint @@ -444,7 +444,7 @@ struct ChatViewModelNostrExtensionTests { let convKey = PeerID(nostr_: sender.publicKeyHex) let messageID = "geo-ack-delivered" - viewModel.privateChats[convKey] = [ + viewModel.seedPrivateChat([ BitchatMessage( id: messageID, sender: viewModel.nickname, @@ -456,7 +456,7 @@ struct ChatViewModelNostrExtensionTests { senderPeerID: viewModel.meshService.myPeerID, deliveryStatus: .sent ) - ] + ], for: convKey) let content = try ackContent(type: .delivered, messageID: messageID, senderPeerID: PeerID(str: "0123456789abcdef")) let giftWrap = try NostrProtocol.createPrivateMessage( @@ -482,7 +482,7 @@ struct ChatViewModelNostrExtensionTests { let convKey = PeerID(nostr_: sender.publicKeyHex) let messageID = "geo-ack-read" - viewModel.privateChats[convKey] = [ + viewModel.seedPrivateChat([ BitchatMessage( id: messageID, sender: viewModel.nickname, @@ -494,7 +494,7 @@ struct ChatViewModelNostrExtensionTests { senderPeerID: viewModel.meshService.myPeerID, deliveryStatus: .delivered(to: "Friend", at: Date()) ) - ] + ], for: convKey) let content = try ackContent(type: .readReceipt, messageID: messageID, senderPeerID: PeerID(str: "0123456789abcdef")) let giftWrap = try NostrProtocol.createPrivateMessage( @@ -578,7 +578,7 @@ struct ChatViewModelNostrExtensionTests { let convKey = PeerID(nostr_: sender.publicKeyHex) let messageID = "gift-delivered" - viewModel.privateChats[convKey] = [ + viewModel.seedPrivateChat([ BitchatMessage( id: messageID, sender: viewModel.nickname, @@ -590,7 +590,7 @@ struct ChatViewModelNostrExtensionTests { senderPeerID: viewModel.meshService.myPeerID, deliveryStatus: .sent ) - ] + ], for: convKey) let content = try ackContent(type: .delivered, messageID: messageID, senderPeerID: PeerID(str: "0123456789abcdef")) let giftWrap = try NostrProtocol.createPrivateMessage( @@ -1095,7 +1095,7 @@ struct ChatViewModelMediaTransferTests { senderPeerID: viewModel.meshService.myPeerID, deliveryStatus: .sending ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) viewModel.registerTransfer(transferId: "transfer-cancel", messageID: message.id) viewModel.cancelMediaSend(messageID: message.id) @@ -1124,7 +1124,7 @@ struct ChatViewModelMediaTransferTests { senderPeerID: viewModel.meshService.myPeerID, deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) viewModel.registerTransfer(transferId: "transfer-delete", messageID: message.id) viewModel.deleteMediaMessage(messageID: message.id) diff --git a/bitchatTests/ChatViewModelTests.swift b/bitchatTests/ChatViewModelTests.swift index 304b96d5..7b442ed5 100644 --- a/bitchatTests/ChatViewModelTests.swift +++ b/bitchatTests/ChatViewModelTests.swift @@ -136,7 +136,7 @@ struct ChatViewModelIdentityTests { senderPeerID: oldPeerID, mentions: nil ) - viewModel.privateChats[oldPeerID] = [existingMessage] + viewModel.seedPrivateChat([existingMessage], for: oldPeerID) viewModel.startPrivateChat(with: oldPeerID) #expect(viewModel.selectedPrivateChatPeer == oldPeerID) @@ -345,8 +345,8 @@ struct ChatViewModelServiceLifecycleTests { mentions: nil ) - viewModel.privateChats[peerID] = [message] - viewModel.unreadPrivateMessages.insert(peerID) + viewModel.seedPrivateChat([message], for: peerID) + viewModel.markPrivateChatUnread(peerID) viewModel.selectedPrivateChatPeer = peerID viewModel.handleDidBecomeActive() @@ -497,7 +497,7 @@ struct ChatViewModelNoisePayloadTests { mentions: nil, deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) viewModel.didReceiveNoisePayload( from: peerID, @@ -535,7 +535,7 @@ struct ChatViewModelNoisePayloadTests { mentions: nil, deliveryStatus: .sent ) - viewModel.privateChats[peerID] = [message] + viewModel.seedPrivateChat([message], for: peerID) viewModel.didReceiveNoisePayload( from: peerID, @@ -771,7 +771,7 @@ struct ChatViewModelPeerTests { func didUpdatePeerList_removesStaleUnreadPeerWithoutMessages() async { let (viewModel, _) = makeTestableViewModel() let stalePeer = PeerID(str: "00000000000000a2") - viewModel.unreadPrivateMessages = [stalePeer] + viewModel.markPrivateChatUnread(stalePeer) viewModel.didUpdatePeerList([]) @@ -798,8 +798,8 @@ struct ChatViewModelPeerTests { senderPeerID: stablePeer, mentions: nil ) - viewModel.privateChats[stablePeer] = [message] - viewModel.unreadPrivateMessages = [stablePeer] + viewModel.seedPrivateChat([message], for: stablePeer) + viewModel.markPrivateChatUnread(stablePeer) viewModel.didUpdatePeerList([]) try? await Task.sleep(nanoseconds: 100_000_000) @@ -876,33 +876,32 @@ struct ChatViewModelPrivateChatSelectionTests { let older = Date().addingTimeInterval(-120) let newer = Date().addingTimeInterval(-30) - viewModel.privateChats = [ - peerA: [ - BitchatMessage( - id: "a-1", - sender: "A", - content: "Old", - timestamp: older, - isRelay: false, - isPrivate: true, - recipientNickname: "Me", - senderPeerID: peerA - ) - ], - peerB: [ - BitchatMessage( - id: "b-1", - sender: "B", - content: "New", - timestamp: newer, - isRelay: false, - isPrivate: true, - recipientNickname: "Me", - senderPeerID: peerB - ) - ] - ] - viewModel.unreadPrivateMessages = [peerA, peerB] + viewModel.seedPrivateChat([ + BitchatMessage( + id: "a-1", + sender: "A", + content: "Old", + timestamp: older, + isRelay: false, + isPrivate: true, + recipientNickname: "Me", + senderPeerID: peerA + ) + ], for: peerA) + viewModel.seedPrivateChat([ + BitchatMessage( + id: "b-1", + sender: "B", + content: "New", + timestamp: newer, + isRelay: false, + isPrivate: true, + recipientNickname: "Me", + senderPeerID: peerB + ) + ], for: peerB) + viewModel.markPrivateChatUnread(peerA) + viewModel.markPrivateChatUnread(peerB) viewModel.openMostRelevantPrivateChat() @@ -918,32 +917,30 @@ struct ChatViewModelPrivateChatSelectionTests { let older = Date().addingTimeInterval(-200) let newer = Date().addingTimeInterval(-20) - viewModel.privateChats = [ - peerA: [ - BitchatMessage( - id: "a-1", - sender: "A", - content: "Old", - timestamp: older, - isRelay: false, - isPrivate: true, - recipientNickname: "Me", - senderPeerID: peerA - ) - ], - peerB: [ - BitchatMessage( - id: "b-1", - sender: "B", - content: "New", - timestamp: newer, - isRelay: false, - isPrivate: true, - recipientNickname: "Me", - senderPeerID: peerB - ) - ] - ] + viewModel.seedPrivateChat([ + BitchatMessage( + id: "a-1", + sender: "A", + content: "Old", + timestamp: older, + isRelay: false, + isPrivate: true, + recipientNickname: "Me", + senderPeerID: peerA + ) + ], for: peerA) + viewModel.seedPrivateChat([ + BitchatMessage( + id: "b-1", + sender: "B", + content: "New", + timestamp: newer, + isRelay: false, + isPrivate: true, + recipientNickname: "Me", + senderPeerID: peerB + ) + ], for: peerB) viewModel.openMostRelevantPrivateChat() @@ -1011,7 +1008,7 @@ struct ChatViewModelPanicTests { isRelay: false ) ] - viewModel.privateChats[PeerID(str: "PEER1")] = [ + viewModel.seedPrivateChat([ BitchatMessage( id: "pm-1", sender: "Peer", @@ -1022,8 +1019,8 @@ struct ChatViewModelPanicTests { recipientNickname: "Me", senderPeerID: PeerID(str: "PEER1") ) - ] - viewModel.unreadPrivateMessages.insert(PeerID(str: "PEER1")) + ], for: PeerID(str: "PEER1")) + viewModel.markPrivateChatUnread(PeerID(str: "PEER1")) viewModel.panicClearAllData() diff --git a/bitchatTests/CommandProcessorTests.swift b/bitchatTests/CommandProcessorTests.swift index 214d675f..641fdfdd 100644 --- a/bitchatTests/CommandProcessorTests.swift +++ b/bitchatTests/CommandProcessorTests.swift @@ -423,6 +423,12 @@ private final class MockCommandContextProvider: CommandContextProvider { clearCurrentPublicTimelineCallCount += 1 } + private(set) var clearedPrivateChats: [PeerID] = [] + func clearPrivateChat(_ peerID: PeerID) { + clearedPrivateChats.append(peerID) + privateChats[peerID] = [] + } + func sendPublicRaw(_ content: String) { sentPublicRawMessages.append(content) } diff --git a/bitchatTests/LegacyConversationStoreBridgeTests.swift b/bitchatTests/LegacyConversationStoreBridgeTests.swift new file mode 100644 index 00000000..cf1b6ab7 --- /dev/null +++ b/bitchatTests/LegacyConversationStoreBridgeTests.swift @@ -0,0 +1,177 @@ +// +// LegacyConversationStoreBridgeTests.swift +// bitchatTests +// +// Focused tests for migration step 2 (docs/CONVERSATION-STORE-DESIGN.md §4): +// the private write path goes through the single-writer `ConversationStore` +// intents, and `LegacyConversationStoreBridge` keeps the replace-based +// `LegacyConversationStore` consistent — per-message changes via a +// `Task.yield`-coalesced per-conversation mirror, unread flips and +// structural changes (migration/removal) synchronously — until the feature +// models cut over in step 5. +// +// DELETE IN STEP 5 together with the bridge. +// +// This is free and unencumbered software released into the public domain. +// For more information, see +// + +import Testing +import Foundation +import BitFoundation +@testable import bitchat + +@MainActor +private func makeBridgedFixture() -> ( + viewModel: ChatViewModel, + store: ConversationStore, + legacy: LegacyConversationStore, + transport: MockTransport +) { + let keychain = MockKeychain() + let idBridge = NostrIdentityBridge(keychain: MockKeychainHelper()) + let identityManager = MockIdentityManager(keychain) + let transport = MockTransport() + let legacy = LegacyConversationStore() + let store = ConversationStore() + let viewModel = ChatViewModel( + keychain: keychain, + idBridge: idBridge, + identityManager: identityManager, + transport: transport, + conversationStore: legacy, + conversations: store + ) + return (viewModel, store, legacy, transport) +} + +@MainActor +private func makeInboundPrivateMessage( + id: String, + from peerID: PeerID, + sender: String = "alice", + content: String = "hello", + timestamp: Date = Date() +) -> BitchatMessage { + BitchatMessage( + id: id, + sender: sender, + content: content, + timestamp: timestamp, + isRelay: false, + isPrivate: true, + recipientNickname: "me", + senderPeerID: peerID, + deliveryStatus: .delivered(to: "me", at: timestamp) + ) +} + +@Suite("LegacyConversationStoreBridge") +struct LegacyConversationStoreBridgeTests { + + @Test("inbound private message lands in the store and is mirrored into Legacy") + @MainActor + func inboundPrivateMessageLandsInStoreAndLegacy() async { + let (viewModel, store, legacy, _) = makeBridgedFixture() + // Stable Noise-key peer ID, like the perf pipeline fixture. + let peerID = PeerID(str: String(repeating: "ab", count: 32)) + let message = makeInboundPrivateMessage(id: "bridge-pm-1", from: peerID) + + viewModel.handlePrivateMessage(message) + + // New store is the synchronous source of truth. + #expect(store.conversation(for: .directPeer(peerID)).messages.map(\.id) == ["bridge-pm-1"]) + #expect(viewModel.privateChats[peerID]?.map(\.id) == ["bridge-pm-1"]) + // Unread flips mirror into Legacy synchronously. + #expect(viewModel.unreadPrivateMessages.contains(peerID)) + #expect(legacy.unreadDirectPeerIDs().contains(peerID)) + + // Message arrays mirror via the bridge's coalesced flush + // (one Legacy replace per burst, on the next main-actor turn). + let mirrored = await TestHelpers.waitUntil( + { legacy.directMessagesByPeerID()[peerID]?.map(\.id) == ["bridge-pm-1"] }, + timeout: 1.0 + ) + #expect(mirrored) + } + + @Test("store dedups a private message delivered twice") + @MainActor + func storeDedupsDuplicateInboundMessage() async { + let (viewModel, store, legacy, _) = makeBridgedFixture() + let peerID = PeerID(str: String(repeating: "cd", count: 32)) + let message = makeInboundPrivateMessage(id: "bridge-dup-1", from: peerID) + + viewModel.handlePrivateMessage(message) + viewModel.handlePrivateMessage(message) + + #expect(store.conversation(for: .directPeer(peerID)).messages.count == 1) + #expect(viewModel.privateChats[peerID]?.count == 1) + + // The intent itself reports the duplicate. + #expect(!viewModel.appendPrivateMessage(message, to: peerID)) + + let mirrored = await TestHelpers.waitUntil( + { legacy.directMessagesByPeerID()[peerID]?.count == 1 }, + timeout: 1.0 + ) + #expect(mirrored) + } + + @Test("peer-ID migration through store intents keeps Legacy consistent") + @MainActor + func migrationThroughStoreIntentsKeepsLegacyConsistent() { + let (viewModel, store, legacy, _) = makeBridgedFixture() + let oldPeerID = PeerID(str: "aaaaaaaaaaaaaaaa") + let newPeerID = PeerID(str: "bbbbbbbbbbbbbbbb") + + viewModel.seedPrivateChat( + [ + makeInboundPrivateMessage(id: "mig-1", from: oldPeerID, timestamp: Date().addingTimeInterval(-60)), + makeInboundPrivateMessage(id: "mig-2", from: oldPeerID, timestamp: Date().addingTimeInterval(-30)), + ], + for: oldPeerID + ) + viewModel.markPrivateChatUnread(oldPeerID) + + viewModel.migratePrivateChat(from: oldPeerID, to: newPeerID) + + // Store: messages moved in order, old chat gone, unread carried. + #expect(store.conversationsByID[.directPeer(oldPeerID)] == nil) + #expect(store.conversation(for: .directPeer(newPeerID)).messages.map(\.id) == ["mig-1", "mig-2"]) + #expect(viewModel.unreadPrivateMessages == [newPeerID]) + + // Legacy: structural changes resynchronize immediately (stale + // messages removed, destination present, unread re-keyed). The old + // peer may linger as a registered-but-empty handle — Legacy has + // always kept handle registrations from markRead/selection around. + let legacyChats = legacy.directMessagesByPeerID() + #expect(legacyChats[oldPeerID] ?? [] == []) + #expect(legacyChats[newPeerID]?.map(\.id) == ["mig-1", "mig-2"]) + #expect(legacy.unreadDirectPeerIDs() == [newPeerID]) + } + + @Test("coordinator chat migration uses the store migrate intent end to end") + @MainActor + func coordinatorMigrationFlowsThroughStore() { + let (viewModel, store, legacy, _) = makeBridgedFixture() + let oldPeerID = PeerID(str: "1111111111111111") + let newPeerID = PeerID(str: "2222222222222222") + + // Recent messages from "alice" under the old peer ID; no fingerprints + // on either side → nickname-match migration path. + viewModel.seedPrivateChat( + [makeInboundPrivateMessage(id: "coord-mig-1", from: oldPeerID, timestamp: Date().addingTimeInterval(-5))], + for: oldPeerID + ) + + viewModel.migratePrivateChatsIfNeeded(for: newPeerID, senderNickname: "alice") + + #expect(store.conversationsByID[.directPeer(oldPeerID)] == nil) + #expect(store.conversation(for: .directPeer(newPeerID)).messages.map(\.id) == ["coord-mig-1"]) + // Migration resynchronizes Legacy immediately (the old peer may + // linger as a registered-but-empty handle, see above). + #expect(legacy.directMessagesByPeerID()[oldPeerID] ?? [] == []) + #expect(legacy.directMessagesByPeerID()[newPeerID]?.map(\.id) == ["coord-mig-1"]) + } +} diff --git a/bitchatTests/Performance/PerformanceBaselineTests.swift b/bitchatTests/Performance/PerformanceBaselineTests.swift index 839ceb2d..ce1eee4f 100644 --- a/bitchatTests/Performance/PerformanceBaselineTests.swift +++ b/bitchatTests/Performance/PerformanceBaselineTests.swift @@ -655,6 +655,17 @@ private final class PerfDeliveryContext: ChatDeliveryContext { return oldCount - sentReadReceipts.count } + @discardableResult + func setPrivateDeliveryStatus(_ status: DeliveryStatus, forMessageID messageID: String, peerID: PeerID) -> Bool { + guard var chat = privateChats[peerID], + let index = chat.firstIndex(where: { $0.id == messageID }) else { + return false + } + chat[index].deliveryStatus = status + privateChats[peerID] = chat + return true + } + /// 2000 public + `peerCount` x `messagesPerPeer` private messages with /// deterministic IDs and timestamps. static func makeCorpus(publicCount: Int, peerCount: Int, messagesPerPeer: Int) -> PerfDeliveryContext { diff --git a/bitchatTests/Services/PrivateChatManagerTests.swift b/bitchatTests/Services/PrivateChatManagerTests.swift index a75dac5f..a986570d 100644 --- a/bitchatTests/Services/PrivateChatManagerTests.swift +++ b/bitchatTests/Services/PrivateChatManagerTests.swift @@ -3,6 +3,8 @@ // bitchatTests // // Tests for PrivateChatManager read receipt and selection behavior. +// Message storage lives in the single-writer ConversationStore; the +// manager's privateChats/unreadMessages are derived views over it. // import Testing @@ -12,13 +14,20 @@ import BitFoundation struct PrivateChatManagerTests { + @MainActor + private static func makeManager(transport: MockTransport) -> (PrivateChatManager, ConversationStore) { + let store = ConversationStore() + let manager = PrivateChatManager(meshService: transport, conversationStore: store) + return (manager, store) + } + @Test @MainActor func startChat_setsSelectedAndClearsUnread() async { let transport = MockTransport() - let manager = PrivateChatManager(meshService: transport) + let (manager, store) = Self.makeManager(transport: transport) let peerID = PeerID(str: "00000000000000AA") - manager.privateChats[peerID] = [ + store.append( BitchatMessage( id: "pm-1", sender: "Peer", @@ -28,9 +37,10 @@ struct PrivateChatManagerTests { isPrivate: true, recipientNickname: "Me", senderPeerID: peerID - ) - ] - manager.unreadMessages.insert(peerID) + ), + to: .directPeer(peerID) + ) + store.markUnread(.directPeer(peerID)) manager.startChat(with: peerID) @@ -43,13 +53,13 @@ struct PrivateChatManagerTests { func markAsRead_sendsReadReceiptViaRouter() async { let transport = MockTransport() let router = MessageRouter(transports: [transport]) - let manager = PrivateChatManager(meshService: transport) + let (manager, store) = Self.makeManager(transport: transport) manager.messageRouter = router let peerID = PeerID(str: "00000000000000BB") transport.reachablePeers.insert(peerID) - manager.privateChats[peerID] = [ + store.append( BitchatMessage( id: "pm-2", sender: "Peer", @@ -59,9 +69,10 @@ struct PrivateChatManagerTests { isPrivate: true, recipientNickname: "Me", senderPeerID: peerID - ) - ] - manager.unreadMessages.insert(peerID) + ), + to: .directPeer(peerID) + ) + store.markUnread(.directPeer(peerID)) manager.markAsRead(from: peerID) try? await Task.sleep(nanoseconds: 100_000_000) @@ -74,10 +85,10 @@ struct PrivateChatManagerTests { @Test @MainActor func markAsRead_withoutRouterFallsBackToTransport() async { let transport = MockTransport() - let manager = PrivateChatManager(meshService: transport) + let (manager, store) = Self.makeManager(transport: transport) let peerID = PeerID(str: "00000000000000CC") - manager.privateChats[peerID] = [ + store.append( BitchatMessage( id: "pm-fallback", sender: "Peer", @@ -87,8 +98,9 @@ struct PrivateChatManagerTests { isPrivate: true, recipientNickname: "Me", senderPeerID: peerID - ) - ] + ), + to: .directPeer(peerID) + ) manager.markAsRead(from: peerID) @@ -99,7 +111,7 @@ struct PrivateChatManagerTests { @Test @MainActor func consolidateMessages_mergesStableNoiseKeyHistoryAndMarksUnread() async { let transport = MockTransport() - let manager = PrivateChatManager(meshService: transport) + let (manager, store) = Self.makeManager(transport: transport) let identityManager = MockIdentityManager(MockKeychain()) let idBridge = NostrIdentityBridge(keychain: MockKeychainHelper()) let unifiedPeerService = UnifiedPeerService(meshService: transport, idBridge: idBridge, identityManager: identityManager) @@ -120,7 +132,7 @@ struct PrivateChatManagerTests { ]) try? await Task.sleep(nanoseconds: 50_000_000) - manager.privateChats[stablePeerID] = [ + store.append( BitchatMessage( id: "stable-msg", sender: "Alice", @@ -130,9 +142,10 @@ struct PrivateChatManagerTests { isPrivate: true, recipientNickname: "Me", senderPeerID: stablePeerID - ) - ] - manager.unreadMessages.insert(stablePeerID) + ), + to: .directPeer(stablePeerID) + ) + store.markUnread(.directPeer(stablePeerID)) let hadUnread = manager.consolidateMessages(for: peerID, peerNickname: "Alice", persistedReadReceipts: []) @@ -146,11 +159,11 @@ struct PrivateChatManagerTests { @Test @MainActor func consolidateMessages_movesTemporaryGeoDMHistoryByNickname() async { let transport = MockTransport() - let manager = PrivateChatManager(meshService: transport) + let (manager, store) = Self.makeManager(transport: transport) let peerID = PeerID(str: "0011223344556677") let tempPeerID = PeerID(nostr_: "0000000000000000000000000000000000000000000000000000000000000042") - manager.privateChats[tempPeerID] = [ + store.append( BitchatMessage( id: "geo-msg", sender: "Alice", @@ -160,9 +173,10 @@ struct PrivateChatManagerTests { isPrivate: true, recipientNickname: "Me", senderPeerID: tempPeerID - ) - ] - manager.unreadMessages.insert(tempPeerID) + ), + to: .directPeer(tempPeerID) + ) + store.markUnread(.directPeer(tempPeerID)) let hadUnread = manager.consolidateMessages(for: peerID, peerNickname: "alice", persistedReadReceipts: []) @@ -177,10 +191,10 @@ struct PrivateChatManagerTests { @Test @MainActor func syncReadReceiptsForSentMessages_onlyCopiesDeliveredAndRead() async { let transport = MockTransport() - let manager = PrivateChatManager(meshService: transport) + let (manager, store) = Self.makeManager(transport: transport) let peerID = PeerID(str: "00000000000000DD") - manager.privateChats[peerID] = [ + let seeded = [ BitchatMessage( id: "sent-read", sender: "Me", @@ -215,6 +229,9 @@ struct PrivateChatManagerTests { deliveryStatus: .failed(reason: "nope") ) ] + for message in seeded { + store.append(message, to: .directPeer(peerID)) + } var externalReceipts = Set() manager.syncReadReceiptsForSentMessages(peerID: peerID, nickname: "Me", externalReceipts: &externalReceipts) @@ -223,47 +240,36 @@ struct PrivateChatManagerTests { #expect(manager.sentReadReceipts == Set(["sent-read", "sent-delivered"])) } + /// The store replaces `sanitizeChat`: inserts keep chronological order, + /// duplicate IDs are rejected on append, and `upsertByID` replaces the + /// stored message with the latest copy in place. @Test @MainActor - func sanitizeChat_sortsChronologicallyAndKeepsLatestDuplicate() async { + func store_keepsChronologicalOrderAndDedupsByID() async { let transport = MockTransport() - let manager = PrivateChatManager(meshService: transport) + let (manager, store) = Self.makeManager(transport: transport) let peerID = PeerID(str: "00000000000000EE") let base = Date(timeIntervalSince1970: 10) - manager.privateChats[peerID] = [ + func message(_ id: String, _ content: String, offset: TimeInterval) -> BitchatMessage { BitchatMessage( - id: "same", + id: id, sender: "Peer", - content: "Older", - timestamp: base.addingTimeInterval(10), - isRelay: false, - isPrivate: true, - recipientNickname: "Me", - senderPeerID: peerID - ), - BitchatMessage( - id: "first", - sender: "Peer", - content: "First", - timestamp: base, - isRelay: false, - isPrivate: true, - recipientNickname: "Me", - senderPeerID: peerID - ), - BitchatMessage( - id: "same", - sender: "Peer", - content: "Newest", - timestamp: base.addingTimeInterval(20), + content: content, + timestamp: base.addingTimeInterval(offset), isRelay: false, isPrivate: true, recipientNickname: "Me", senderPeerID: peerID ) - ] + } - manager.sanitizeChat(for: peerID) + #expect(store.append(message("same", "Older", offset: 10), to: .directPeer(peerID))) + // Out-of-order arrival is inserted in timestamp order. + #expect(store.append(message("first", "First", offset: 0), to: .directPeer(peerID))) + // Duplicate ID is rejected on append… + #expect(!store.append(message("same", "Newest", offset: 20), to: .directPeer(peerID))) + // …and replaced in place by upsert. + store.upsertByID(message("same", "Newest", offset: 20), in: .directPeer(peerID)) #expect(manager.privateChats[peerID]?.map(\.id) == ["first", "same"]) #expect(manager.privateChats[peerID]?.last?.content == "Newest") diff --git a/bitchatTests/TestUtilities/TestHelpers.swift b/bitchatTests/TestUtilities/TestHelpers.swift index c76f57af..c09c60c5 100644 --- a/bitchatTests/TestUtilities/TestHelpers.swift +++ b/bitchatTests/TestUtilities/TestHelpers.swift @@ -138,6 +138,28 @@ enum TestError: Error { case testFailure(String) } +// MARK: - Private chat seeding (ConversationStore migration) + +extension ChatViewModel { + /// Test-only replacement for the deleted `privateChats` setter: seeds a + /// peer's chat through the single-writer `ConversationStore` intents + /// (upsert keeps re-seeding with updated copies working the way the old + /// dictionary assignment did). + @MainActor + func seedPrivateChat(_ messages: [BitchatMessage], for peerID: PeerID) { + _ = conversations.conversation(for: .directPeer(peerID)) + for message in messages { + conversations.upsertByID(message, in: .directPeer(peerID)) + } + } + + /// Test-only: drops every private chat and unread flag. + @MainActor + func clearAllPrivateChats() { + conversations.removeAllDirectConversations() + } +} + func sleep(_ seconds: TimeInterval) async throws { try await Task.sleep(nanoseconds: UInt64(seconds * 1_000_000_000)) } diff --git a/bitchatTests/ViewSmokeTests.swift b/bitchatTests/ViewSmokeTests.swift index 9a2aaef3..52ef76dd 100644 --- a/bitchatTests/ViewSmokeTests.swift +++ b/bitchatTests/ViewSmokeTests.swift @@ -359,7 +359,7 @@ struct ViewSmokeTests { makeSnapshot(peerID: blockedPeer, nickname: "Mallory", noiseByte: 0x55) ]) try? await Task.sleep(nanoseconds: 50_000_000) - viewModel.unreadPrivateMessages.insert(blockedPeer) + viewModel.markPrivateChatUnread(blockedPeer) _ = mount( MeshPeerList(