fix(#343): keep full message history; window only in memory

The persisted store was being overwritten with the windowed in-memory list, so
any channel or DM with more than _messageWindowSize (200) messages lost
everything older than the newest 200 on the first save after load. Observed
live: Public went 232 -> 201 in one session on a build that already had the
#333 fix, so this was not the race - it was windowing truncating the store.
This is the slow-erosion cause behind the whole 566 -> 231 -> 206 -> 201
history.

saveChannelMessages / saveMessages now MERGE into the persisted set instead of
overwriting: upsert by message identity so older persisted messages are kept,
new ones added, and the in-memory copy wins for edits/reactions/status. If the
existing history fails to decode, the save aborts loudly rather than merging
into an empty base and truncating (SAFELANE 6).

Deletion is now an explicit path - removeChannelMessage / removeMessage - since
the app deletes individual messages. Routing delete through the merging save
would resurrect them; the connector's deleteChannelMessage / deleteMessage now
call the explicit remove.

Windowing stays for display and memory; it no longer dictates what is stored.

Tests: 250-message history survives a 200-window save; new message appends;
edit is captured not duplicated; delete does not resurrect. 508 pass, analyze
and format clean.

Cost: a save now re-reads and re-encodes the channel/contact history. Cheap
with drift's per-key writes (#335); a future append-only schema removes even
that.
integration/290-306-335
Strycher 22 hours ago
parent fa470c1442
commit 2fefb6aa91

@ -699,7 +699,8 @@ class MeshCoreConnector extends ChangeNotifier {
if (messages == null) return;
final removed = messages.remove(message);
if (!removed) return;
await _messageStore.saveMessages(contactKeyHex, messages);
// Explicit delete: saveMessages now merges and never removes (#343).
await _messageStore.removeMessage(contactKeyHex, message);
notifyListeners();
}
@ -833,7 +834,10 @@ class MeshCoreConnector extends ChangeNotifier {
if (messages == null) return;
final removed = messages.remove(message);
if (!removed) return;
await _channelMessageStore.saveChannelMessages(channelIndex, messages);
// Explicit delete path: saveChannelMessages now MERGES and never removes
// (#343), so deletion must go through removeChannelMessage or the message
// would be resurrected on the next save.
await _channelMessageStore.removeChannelMessage(channelIndex, message);
notifyListeners();
}

@ -65,10 +65,83 @@ class ChannelMessageStore {
);
return;
}
final jsonList = messages.map((msg) => _messageToJson(msg)).toList();
// Merge into the persisted full history rather than overwriting it. The
// in-memory list is windowed to the most recent N for memory, so a plain
// overwrite would truncate the store to N and erode old history (#343).
// Upsert by identity: keep older persisted messages, add new ones, and let
// the in-memory copy win so edits/reactions/status updates are captured.
// Deletion has its own path (removeChannelMessage) so this never
// resurrects a message the user deleted.
final key = _storageKey(channelIndex);
final byKey = <String, ChannelMessage>{};
final existing = await BlobStore.instance.readWithPrefsFallback(key);
if (existing != null && existing.isNotEmpty) {
try {
for (final e in jsonDecode(existing) as List<dynamic>) {
final m = _messageFromJson(e as Map<String, dynamic>);
byKey[_mergeKey(m)] = m;
}
} catch (e) {
// SAFELANE 6: a decode failure here must be loud, not silently drop the
// persisted history by merging into an empty base.
appLogger.error(
'Failed to decode existing channel $channelIndex history before '
'merge; aborting save to avoid truncation: $e',
tag: 'Storage',
);
return;
}
}
for (final m in messages) {
byKey[_mergeKey(m)] = m;
}
final merged = byKey.values.toList()
..sort((a, b) => a.timestamp.compareTo(b.timestamp));
await BlobStore.instance.write(
key,
jsonEncode(merged.map(_messageToJson).toList()),
);
}
/// Stable identity for merge/dedupe. messageId when present, else a composite
/// that distinguishes distinct messages that share no id.
String _mergeKey(ChannelMessage m) {
if (m.messageId.isNotEmpty) return 'id:${m.messageId}';
return 'x:${m.packetHash ?? ''}:${m.timestamp.millisecondsSinceEpoch}:${m.text}';
}
/// Removes a single message from the persisted history. The explicit delete
/// path (#343): save merges and never removes, so deletion cannot go through
/// save.
Future<void> removeChannelMessage(
int channelIndex,
ChannelMessage message,
) async {
if (publicKeyHex.isEmpty) return;
final key = _storageKey(channelIndex);
final existing = await BlobStore.instance.readWithPrefsFallback(key);
if (existing == null || existing.isEmpty) return;
final List<dynamic> raw;
try {
raw = jsonDecode(existing) as List<dynamic>;
} catch (e) {
appLogger.error(
'Failed to decode channel $channelIndex history for delete: $e',
tag: 'Storage',
);
return;
}
final target = _mergeKey(message);
final kept = raw
.map((e) => _messageFromJson(e as Map<String, dynamic>))
.where((m) => _mergeKey(m) != target)
.toList();
await BlobStore.instance.write(
_storageKey(channelIndex),
jsonEncode(jsonList),
key,
jsonEncode(kept.map(_messageToJson).toList()),
);
}

@ -1,3 +1,5 @@
import 'package:flutter/foundation.dart';
import '../../utils/app_logger.dart';
import '../prefs_manager.dart';
import 'offband_database.dart';
@ -19,10 +21,19 @@ class BlobStore {
final OffbandDatabase _db;
static OffbandDatabase? _sharedDb;
static BlobStore? _override;
/// Process-wide instance. The database must be opened once; opening it twice
/// is an error on the web backends.
static BlobStore get instance => BlobStore(_sharedDb ??= OffbandDatabase());
static BlobStore get instance =>
_override ?? BlobStore(_sharedDb ??= OffbandDatabase());
/// Test seam: point the singleton at an in-memory database.
@visibleForTesting
static void overrideForTest(BlobStore store) => _override = store;
@visibleForTesting
static void clearTestOverride() => _override = null;
/// Key families that hold bulk data. Everything else stays in
/// SharedPreferences, which is what it is for.

@ -24,9 +24,71 @@ class MessageStore {
appLogger.warn('Public key hex is not set. Cannot save messages.');
return;
}
// Merge into the persisted full history rather than overwriting it (#343).
// The in-memory list is windowed for memory, so a plain overwrite would
// truncate the store. Upsert by identity; deletion is explicit
// (removeMessage).
final key = '$keyFor$contactKeyHex';
final jsonList = messages.map(_messageToJson).toList();
await BlobStore.instance.write(key, jsonEncode(jsonList));
final byKey = <String, Message>{};
final existing = await BlobStore.instance.readWithPrefsFallback(key);
if (existing != null && existing.isNotEmpty) {
try {
for (final e in jsonDecode(existing) as List<dynamic>) {
final m = _messageFromJson(e as Map<String, dynamic>);
byKey[_mergeKey(m)] = m;
}
} catch (e) {
appLogger.error(
'Failed to decode existing DM history before merge; aborting save '
'to avoid truncation: $e',
tag: 'Storage',
);
return;
}
}
for (final m in messages) {
byKey[_mergeKey(m)] = m;
}
final merged = byKey.values.toList()
..sort((a, b) => a.timestamp.compareTo(b.timestamp));
await BlobStore.instance.write(
key,
jsonEncode(merged.map(_messageToJson).toList()),
);
}
String _mergeKey(Message m) {
if (m.messageId.isNotEmpty) return 'id:${m.messageId}';
return 'x:${m.senderKeyHex}:${m.timestamp.millisecondsSinceEpoch}:${m.text}';
}
/// Explicit delete path (#343): save merges and never removes.
Future<void> removeMessage(String contactKeyHex, Message message) async {
if (publicKeyHex.isEmpty) return;
final key = '$keyFor$contactKeyHex';
final existing = await BlobStore.instance.readWithPrefsFallback(key);
if (existing == null || existing.isEmpty) return;
final List<dynamic> raw;
try {
raw = jsonDecode(existing) as List<dynamic>;
} catch (e) {
appLogger.error(
'Failed to decode DM history for delete: $e',
tag: 'Storage',
);
return;
}
final target = _mergeKey(message);
final kept = raw
.map((e) => _messageFromJson(e as Map<String, dynamic>))
.where((m) => _mergeKey(m) != target)
.toList();
await BlobStore.instance.write(
key,
jsonEncode(kept.map(_messageToJson).toList()),
);
}
Future<List<Message>> loadMessages(String contactKeyHex) async {

@ -0,0 +1,103 @@
import 'package:drift/native.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:meshcore_open/models/channel_message.dart';
import 'package:meshcore_open/storage/channel_message_store.dart';
import 'package:meshcore_open/storage/drift/blob_store.dart';
import 'package:meshcore_open/storage/drift/offband_database.dart';
import 'package:meshcore_open/storage/prefs_manager.dart';
import 'package:shared_preferences/shared_preferences.dart';
/// #343: saving a windowed in-memory list must not truncate the persisted
/// history. The store keeps full history; deletion is explicit.
void main() {
late OffbandDatabase db;
late ChannelMessageStore store;
setUp(() async {
SharedPreferences.setMockInitialValues({});
PrefsManager.reset();
await PrefsManager.initialize();
db = OffbandDatabase(NativeDatabase.memory());
BlobStore.overrideForTest(BlobStore(db));
store = ChannelMessageStore();
store.setPublicKeyHex = 'a' * 20;
// No PSK resolver -> uses the slot-index key, which is fine for the test.
});
tearDown(() async {
await db.close();
BlobStore.clearTestOverride();
});
ChannelMessage msg(int i) => ChannelMessage(
senderName: 's',
text: 'm$i',
timestamp: DateTime.fromMillisecondsSinceEpoch(1000 + i),
isOutgoing: false,
channelIndex: 0,
messageId: 'id$i',
);
test('saving a 200-window does not truncate a 250-message history', () async {
// Seed 250 messages, as if a full history were persisted.
await store.saveChannelMessages(0, [for (var i = 0; i < 250; i++) msg(i)]);
expect((await store.loadChannelMessages(0)).length, 250);
// The app now saves only the most-recent 200 (its in-memory window).
final window = [for (var i = 50; i < 250; i++) msg(i)];
await store.saveChannelMessages(0, window);
// Full history must survive: the older 50 are still there.
final all = await store.loadChannelMessages(0);
expect(all.length, 250, reason: 'the older 50 must not be dropped');
expect(all.first.text, 'm0');
expect(all.last.text, 'm249');
});
test('a new message appends without dropping old history', () async {
await store.saveChannelMessages(0, [for (var i = 0; i < 10; i++) msg(i)]);
await store.saveChannelMessages(0, [msg(10)]);
final all = await store.loadChannelMessages(0);
expect(all.length, 11);
expect(all.last.text, 'm10');
});
test('an edit to a message is captured, not duplicated', () async {
await store.saveChannelMessages(0, [msg(1)]);
final edited = ChannelMessage(
senderName: 's',
text: 'm1',
timestamp: DateTime.fromMillisecondsSinceEpoch(1001),
isOutgoing: false,
channelIndex: 0,
messageId: 'id1',
reactions: {'thumbsup': 2},
);
await store.saveChannelMessages(0, [edited]);
final all = await store.loadChannelMessages(0);
expect(all, hasLength(1));
expect(all.single.reactions['thumbsup'], 2);
});
test('deleting a message does NOT resurrect it on the next save', () async {
await store.saveChannelMessages(0, [for (var i = 0; i < 5; i++) msg(i)]);
// Delete via the explicit path.
await store.removeChannelMessage(0, msg(2));
expect(
(await store.loadChannelMessages(0)).map((m) => m.text),
isNot(contains('m2')),
);
// A later save of the remaining in-memory list must not bring it back.
final remaining = [
for (var i = 0; i < 5; i++)
if (i != 2) msg(i),
];
await store.saveChannelMessages(0, remaining);
final all = await store.loadChannelMessages(0);
expect(all.map((m) => m.text), isNot(contains('m2')));
expect(all, hasLength(4));
});
}
Loading…
Cancel
Save

Powered by TurnKey Linux.