Address Jack review: keep composer drafts in memory only (#1569).

This commit is contained in:
Taksh 2026-07-31 17:57:24 +03:00
parent accd28f9f4
commit 3ac4b4232c
4 changed files with 163 additions and 100 deletions

View File

@ -98,6 +98,10 @@ final class ConversationUIModel: ObservableObject {
chatViewModel.unblockMeshPeer(peerID: peerID, displayName: displayName)
}
func getFingerprint(for peerID: PeerID) -> String? {
chatViewModel.getFingerprint(for: peerID)
}
func updateAutocomplete(for text: String, cursorPosition: Int) {
chatViewModel.updateAutocomplete(for: text, cursorPosition: cursorPosition)
}

View File

@ -9,23 +9,37 @@
import Foundation
import BitFoundation
/// Persists unfinished composer text per conversation so switching mesh /
/// Holds unfinished composer text per conversation so switching mesh /
/// geohash / DM channels does not silently discard what someone was typing.
///
/// Drafts are plain text in UserDefaults (same trust boundary as nickname and
/// theme). Panic wipe clears the whole map so a seized phone does not keep
/// half-written messages.
/// Drafts stay **in memory only** message content must not land in
/// UserDefaults (see MessageOutboxStore: only the sealed outbox persists
/// plaintext). Losing a draft on process death is acceptable; surviving a
/// channel/DM switch is the value.
///
/// Panic wipe clears the whole map so a seized phone does not keep
/// half-written messages in RAM either.
enum ComposerDraftStore {
static let storageKey = "composer.drafts.v1"
/// Cap each draft so a pasted novel cannot bloat preferences forever.
/// Cap each draft so a pasted novel cannot bloat the map forever.
static let maxDraftLength = 8_000
/// Cap how many conversations keep a draft; oldest keys fall off first.
/// Cap how many conversations keep a draft; oldest entries fall off first.
static let maxDraftCount = 64
private struct Entry {
var text: String
var updatedAt: Date
}
private static var entries: [String: Entry] = [:]
private static let lock = NSLock()
enum Key: Hashable, Equatable {
case mesh
case location(geohash: String)
case privatePeer(PeerID)
/// Mesh DM keyed by Noise fingerprint when known; falls back to the
/// current peerID only before handshake so drafts do not orphan on
/// peerID rotation mid-session.
case privateChat(stableID: String)
var storageString: String {
switch self {
@ -33,14 +47,21 @@ enum ComposerDraftStore {
return "mesh"
case .location(let geohash):
return "geo:\(geohash.lowercased())"
case .privatePeer(let peerID):
return "dm:\(peerID.id)"
case .privateChat(let stableID):
return "dm:\(stableID.lowercased())"
}
}
static func from(peerID: PeerID?, channel: ChannelID) -> Key {
static func from(
peerID: PeerID?,
fingerprint: String?,
channel: ChannelID
) -> Key {
if let peerID {
return .privatePeer(peerID)
if let fingerprint, !fingerprint.isEmpty {
return .privateChat(stableID: fingerprint)
}
return .privateChat(stableID: peerID.id)
}
switch channel {
case .mesh:
@ -51,42 +72,56 @@ enum ComposerDraftStore {
}
}
static func load(_ key: Key, in defaults: UserDefaults = .standard) -> String {
let map = readMap(in: defaults)
return map[key.storageString] ?? ""
static func load(_ key: Key) -> String {
lock.lock()
defer { lock.unlock() }
return entries[key.storageString]?.text ?? ""
}
static func save(_ text: String, for key: Key, in defaults: UserDefaults = .standard) {
var map = readMap(in: defaults)
static func save(_ text: String, for key: Key) {
lock.lock()
defer { lock.unlock() }
let trimmed = String(text.prefix(maxDraftLength))
if trimmed.isEmpty {
map.removeValue(forKey: key.storageString)
entries.removeValue(forKey: key.storageString)
} else {
map[key.storageString] = trimmed
if map.count > maxDraftCount {
// Drop an arbitrary surplus key that is not the one just written.
let surplus = map.keys.filter { $0 != key.storageString }.prefix(map.count - maxDraftCount)
for doomed in surplus {
map.removeValue(forKey: doomed)
}
}
entries[key.storageString] = Entry(text: trimmed, updatedAt: Date())
evictOldestIfNeededLocked()
}
writeMap(map, in: defaults)
}
static func reset(in defaults: UserDefaults = .standard) {
defaults.removeObject(forKey: storageKey)
static func reset() {
lock.lock()
defer { lock.unlock() }
entries.removeAll(keepingCapacity: false)
}
private static func readMap(in defaults: UserDefaults) -> [String: String] {
defaults.dictionary(forKey: storageKey) as? [String: String] ?? [:]
/// Test helper: replace the in-memory map (and return the previous one).
@discardableResult
static func replaceAllForTesting(_ newEntries: [String: String] = [:]) -> [String: String] {
lock.lock()
defer { lock.unlock() }
let previous = entries.mapValues(\.text)
let now = Date()
entries = Dictionary(uniqueKeysWithValues: newEntries.map { ($0.key, Entry(text: $0.value, updatedAt: now)) })
return previous
}
private static func writeMap(_ map: [String: String], in defaults: UserDefaults) {
if map.isEmpty {
defaults.removeObject(forKey: storageKey)
} else {
defaults.set(map, forKey: storageKey)
static func countForTesting() -> Int {
lock.lock()
defer { lock.unlock() }
return entries.count
}
private static func evictOldestIfNeededLocked() {
guard entries.count > maxDraftCount else { return }
let surplus = entries.count - maxDraftCount
let doomed = entries
.sorted { $0.value.updatedAt < $1.value.updatedAt }
.prefix(surplus)
.map(\.key)
for key in doomed {
entries.removeValue(forKey: key)
}
}
}

View File

@ -251,6 +251,7 @@ struct ContentView: View {
sharedContentImportModel.updateDestination(sharedContentDestination)
activeDraftKey = ComposerDraftStore.Key.from(
peerID: selectedPrivatePeerID,
fingerprint: selectedPrivatePeerID.flatMap { conversationUIModel.getFingerprint(for: $0) },
channel: locationChannelsModel.selectedChannel
)
messageText = ComposerDraftStore.load(activeDraftKey)
@ -273,6 +274,7 @@ struct ContentView: View {
sharedContentImportModel.updateDestination(sharedContentDestination)
switchComposerDraft(to: ComposerDraftStore.Key.from(
peerID: newValue,
fingerprint: newValue.flatMap { conversationUIModel.getFingerprint(for: $0) },
channel: locationChannelsModel.selectedChannel
))
}
@ -283,16 +285,16 @@ struct ContentView: View {
if selectedPrivatePeerID == nil {
switchComposerDraft(to: ComposerDraftStore.Key.from(
peerID: nil,
fingerprint: nil,
channel: newChannel
))
}
}
.onChange(of: scenePhase) { phase in
if phase == .background || phase == .inactive {
// Skip persist when the composer was already cleared (e.g.
// panic wipe): otherwise a half-written message would be
// written back after ComposerDraftStore.reset().
guard !messageText.isEmpty else { return }
// Always save, including empty clearing the composer must
// remove the in-memory draft so it does not resurrect on the
// next switch back into this conversation.
ComposerDraftStore.save(messageText, for: activeDraftKey)
}
}

View File

@ -4,76 +4,98 @@ import Testing
import BitFoundation
struct ComposerDraftStoreTests {
private func makeDefaults() -> UserDefaults {
let suite = "bitchat.tests.drafts.\(UUID().uuidString)"
return UserDefaults(suiteName: suite)!
/// Isolate each test from leftover in-memory drafts.
private func withCleanStore(_ body: () throws -> Void) rethrows {
ComposerDraftStore.replaceAllForTesting([:])
defer { ComposerDraftStore.replaceAllForTesting([:]) }
try body()
}
@Test func emptyDraftIsNotStored() {
let defaults = makeDefaults()
ComposerDraftStore.save("hello", for: .mesh, in: defaults)
ComposerDraftStore.save(" ", for: .mesh, in: defaults)
// Whitespace-only is still a draft the user typed; empty string clears.
ComposerDraftStore.save("", for: .mesh, in: defaults)
#expect(ComposerDraftStore.load(.mesh, in: defaults).isEmpty)
#expect(defaults.object(forKey: ComposerDraftStore.storageKey) == nil)
@Test func emptyDraftIsNotStored() throws {
try withCleanStore {
ComposerDraftStore.save("hello", for: .mesh)
ComposerDraftStore.save(" ", for: .mesh)
// Whitespace-only is still a draft the user typed; empty string clears.
ComposerDraftStore.save("", for: .mesh)
#expect(ComposerDraftStore.load(.mesh).isEmpty)
#expect(ComposerDraftStore.countForTesting() == 0)
}
}
@Test func draftsAreIsolatedPerConversation() {
let defaults = makeDefaults()
@Test func draftsAreIsolatedPerConversation() throws {
try withCleanStore {
let peer = PeerID(str: "aabbccddeeff0011")
ComposerDraftStore.save("mesh draft", for: .mesh)
ComposerDraftStore.save("geo draft", for: .location(geohash: "u4pruy"))
ComposerDraftStore.save("dm draft", for: .privateChat(stableID: peer.id))
#expect(ComposerDraftStore.load(.mesh) == "mesh draft")
#expect(ComposerDraftStore.load(.location(geohash: "u4pruy")) == "geo draft")
#expect(ComposerDraftStore.load(.privateChat(stableID: peer.id)) == "dm draft")
#expect(ComposerDraftStore.load(.location(geohash: "other")).isEmpty)
}
}
@Test func geohashKeysAreCaseInsensitive() throws {
try withCleanStore {
ComposerDraftStore.save("city chat", for: .location(geohash: "U4PRUY"))
#expect(ComposerDraftStore.load(.location(geohash: "u4pruy")) == "city chat")
}
}
@Test func keyFromPeerPrefersFingerprintWhenPresent() {
let peer = PeerID(str: "aabbccddeeff0011")
ComposerDraftStore.save("mesh draft", for: .mesh, in: defaults)
ComposerDraftStore.save("geo draft", for: .location(geohash: "u4pruy"), in: defaults)
ComposerDraftStore.save("dm draft", for: .privatePeer(peer), in: defaults)
#expect(ComposerDraftStore.load(.mesh, in: defaults) == "mesh draft")
#expect(ComposerDraftStore.load(.location(geohash: "u4pruy"), in: defaults) == "geo draft")
#expect(ComposerDraftStore.load(.privatePeer(peer), in: defaults) == "dm draft")
#expect(ComposerDraftStore.load(.location(geohash: "other"), in: defaults).isEmpty)
}
@Test func geohashKeysAreCaseInsensitive() {
let defaults = makeDefaults()
ComposerDraftStore.save("city chat", for: .location(geohash: "U4PRUY"), in: defaults)
#expect(ComposerDraftStore.load(.location(geohash: "u4pruy"), in: defaults) == "city chat")
}
@Test func keyFromPeerAndChannelPrefersPrivate() {
let peer = PeerID(str: "aabbccddeeff0011")
let key = ComposerDraftStore.Key.from(
let withFP = ComposerDraftStore.Key.from(
peerID: peer,
fingerprint: "deadbeefcafebabe",
channel: .location(GeohashChannel(level: .city, geohash: "u4pruy"))
)
#expect(key == .privatePeer(peer))
#expect(withFP == .privateChat(stableID: "deadbeefcafebabe"))
let withoutFP = ComposerDraftStore.Key.from(
peerID: peer,
fingerprint: nil,
channel: .mesh
)
#expect(withoutFP == .privateChat(stableID: peer.id))
}
@Test func longDraftsAreTruncated() {
let defaults = makeDefaults()
let long = String(repeating: "a", count: ComposerDraftStore.maxDraftLength + 50)
ComposerDraftStore.save(long, for: .mesh, in: defaults)
#expect(ComposerDraftStore.load(.mesh, in: defaults).count == ComposerDraftStore.maxDraftLength)
}
@Test func resetClearsAllDrafts() {
let defaults = makeDefaults()
ComposerDraftStore.save("keep quiet", for: .mesh, in: defaults)
ComposerDraftStore.reset(in: defaults)
#expect(ComposerDraftStore.load(.mesh, in: defaults).isEmpty)
#expect(defaults.object(forKey: ComposerDraftStore.storageKey) == nil)
}
@Test func maxDraftCountEvictsSurplusKeys() {
let defaults = makeDefaults()
for index in 0..<ComposerDraftStore.maxDraftCount {
ComposerDraftStore.save(
"d\(index)",
for: .location(geohash: String(format: "gh%04d", index)),
in: defaults
)
@Test func longDraftsAreTruncated() throws {
try withCleanStore {
let long = String(repeating: "a", count: ComposerDraftStore.maxDraftLength + 50)
ComposerDraftStore.save(long, for: .mesh)
#expect(ComposerDraftStore.load(.mesh).count == ComposerDraftStore.maxDraftLength)
}
}
@Test func resetClearsAllDrafts() throws {
try withCleanStore {
ComposerDraftStore.save("keep quiet", for: .mesh)
ComposerDraftStore.reset()
#expect(ComposerDraftStore.load(.mesh).isEmpty)
#expect(ComposerDraftStore.countForTesting() == 0)
}
}
@Test func maxDraftCountEvictsOldestKeys() throws {
try withCleanStore {
for index in 0..<ComposerDraftStore.maxDraftCount {
ComposerDraftStore.save(
"d\(index)",
for: .location(geohash: String(format: "gh%04d", index))
)
}
ComposerDraftStore.save("newest", for: .mesh)
#expect(ComposerDraftStore.countForTesting() == ComposerDraftStore.maxDraftCount)
#expect(ComposerDraftStore.load(.mesh) == "newest")
}
}
@Test func clearingDraftRemovesKeySoItDoesNotResurrect() throws {
try withCleanStore {
ComposerDraftStore.save("old text", for: .mesh)
ComposerDraftStore.save("", for: .mesh)
#expect(ComposerDraftStore.load(.mesh).isEmpty)
}
ComposerDraftStore.save("newest", for: .mesh, in: defaults)
let map = defaults.dictionary(forKey: ComposerDraftStore.storageKey) as? [String: String] ?? [:]
#expect(map.count == ComposerDraftStore.maxDraftCount)
#expect(map["mesh"] == "newest")
}
}