From dd67391a0e8afd1ce298094f6fd710a662ac4f23 Mon Sep 17 00:00:00 2001 From: Strycher Date: Mon, 20 Jul 2026 15:41:34 -0400 Subject: [PATCH] feat(#335): switch bulk stores over to drift Wires the migration into startup and points the four bulk stores at drift. SharedPreferences keeps the settings, which is what it is for. Startup runs the migration after prefs are up and BEFORE any store reads, and awaits it. Letting stores race a half-finished migration is precisely the shape of #333, where a storage path silently chose the wrong key and 566 real messages read as empty. Converted: message_store, channel_message_store, contact_store, contact_discovery_store. All four now have ZERO prefs get/set for bulk data. Three safety properties, deliberately built in: 1. readWithPrefsFallback - if a key is somehow not in drift, the prefs copy is still served rather than reading as empty. It logs a WARNING when it fires, because a fallback during normal operation means the migration is incomplete and someone needs to know. Silence here is what made #333 look like data loss. 2. deleteEverywhere / keysWithPrefix span BOTH backends. A clear that only removed the drift row would leave a pre-migration prefs copy to reappear through the fallback, resurrecting deleted history. 3. The legacy-key migrations inside the stores now check both backends and only touch prefs when a legacy key actually exists. This also carries the #306 fix into this branch: the unconditional prefs.remove was still present here, since this branched from dev rather than from the #306 work. Verified: analyze clean, format clean, 504 tests pass, Windows and web both build. Migration rehearsed earlier against a copy of a real 7 MB store: 69 keys, 5.21 MB, zero failures. NOT yet run against live data - that happens on first launch of this build, and the owner should have a prefs backup before that. --- lib/main.dart | 9 +++++ lib/storage/channel_message_store.dart | 35 ++++++++++-------- lib/storage/contact_discovery_store.dart | 18 ++++++--- lib/storage/contact_store.dart | 32 +++++++++------- lib/storage/drift/blob_store.dart | 47 ++++++++++++++++++++++++ lib/storage/message_store.dart | 36 +++++++++++------- 6 files changed, 128 insertions(+), 49 deletions(-) diff --git a/lib/main.dart b/lib/main.dart index 52f3a6f..4e0c17e 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -27,6 +27,7 @@ import 'services/ui_view_state_service.dart'; import 'services/timeout_prediction_service.dart'; import 'services/observer_config_service.dart'; import 'services/block_service.dart'; +import 'storage/drift/blob_store.dart'; import 'storage/prefs_manager.dart'; import 'utils/app_logger.dart'; @@ -36,6 +37,14 @@ void main() async { // Initialize SharedPreferences cache await PrefsManager.initialize(); + // Move bulk data (message history, contacts) out of SharedPreferences into + // drift (#335). Must run after prefs are up and BEFORE any store reads, so + // no code sees a half-migrated state. Idempotent: a no-op once done. + // + // Deliberately awaited: the alternative is stores racing the migration, and + // this is the failure mode that produced #333. + await BlobStore.instance.migrateFromPrefs(); + // Start always-on file logging (#97); no-op on web. await FileLogService.instance.init(); diff --git a/lib/storage/channel_message_store.dart b/lib/storage/channel_message_store.dart index dfef8c3..39507fe 100644 --- a/lib/storage/channel_message_store.dart +++ b/lib/storage/channel_message_store.dart @@ -5,7 +5,7 @@ import 'package:meshcore_open/utils/app_logger.dart'; import '../models/channel_message.dart'; import '../models/translation_support.dart'; import '../helpers/smaz.dart'; -import 'prefs_manager.dart'; +import 'drift/blob_store.dart'; class ChannelMessageStore { static const String _keyPrefix = 'channel_messages_'; @@ -51,9 +51,11 @@ class ChannelMessageStore { ); return; } - final prefs = PrefsManager.instance; final jsonList = messages.map((msg) => _messageToJson(msg)).toList(); - await prefs.setString(_storageKey(channelIndex), jsonEncode(jsonList)); + await BlobStore.instance.write( + _storageKey(channelIndex), + jsonEncode(jsonList), + ); } /// Load messages for a specific channel @@ -64,9 +66,11 @@ class ChannelMessageStore { ); return []; } - final prefs = PrefsManager.instance; + final blobs = BlobStore.instance; final key = _storageKey(channelIndex); - String? jsonString = prefs.getString(key); + // Bulk data lives in drift (#335); the fallback covers an unmigrated key + // and logs loudly if it fires. + String? jsonString = await blobs.readWithPrefsFallback(key); // One-time migration into the PSK-identity key. Only runs when the PSK is // known (key != index key). Adopts pre-#194 history keyed by slot index — @@ -79,13 +83,15 @@ class ChannelMessageStore { _indexKey(channelIndex), '$_keyPrefix$channelIndex', // pre-device-scoping, unscoped index key ]) { - final legacy = prefs.getString(legacyKey); + // Legacy keys may sit in either backend depending on when this + // install last ran, so check both. + final legacy = await blobs.readWithPrefsFallback(legacyKey); if (legacy != null && legacy.isNotEmpty) { appLogger.info( 'Migrating channel messages $legacyKey -> $key (PSK-keyed, #194)', ); - await prefs.setString(key, legacy); - await prefs.remove(legacyKey); + await blobs.write(key, legacy); + await blobs.deleteEverywhere(legacyKey); jsonString = legacy; break; } @@ -108,17 +114,16 @@ class ChannelMessageStore { /// history gone, and setChannel's reuse-clear (#193) needs any stale /// slot-index history gone so it can't be migrated onto the new occupant. Future clearChannelMessages(int channelIndex) async { - final prefs = PrefsManager.instance; - await prefs.remove(_storageKey(channelIndex)); - await prefs.remove(_indexKey(channelIndex)); + final blobs = BlobStore.instance; + await blobs.deleteEverywhere(_storageKey(channelIndex)); + await blobs.deleteEverywhere(_indexKey(channelIndex)); } /// Clear all channel messages Future clearAllChannelMessages() async { - final prefs = PrefsManager.instance; - final keys = prefs.getKeys().where((k) => k.startsWith(keyFor)).toList(); - for (var key in keys) { - await prefs.remove(key); + final blobs = BlobStore.instance; + for (final key in await blobs.keysWithPrefix(keyFor)) { + await blobs.deleteEverywhere(key); } } diff --git a/lib/storage/contact_discovery_store.dart b/lib/storage/contact_discovery_store.dart index 3f6f171..7622fdb 100644 --- a/lib/storage/contact_discovery_store.dart +++ b/lib/storage/contact_discovery_store.dart @@ -2,14 +2,16 @@ import 'dart:convert'; import 'dart:typed_data'; import '../models/contact.dart'; -import 'prefs_manager.dart'; +import '../utils/app_logger.dart'; +import 'drift/blob_store.dart'; class ContactDiscoveryStore { static const String _keyPrefix = 'discovered_contacts'; Future> loadContacts() async { - final prefs = PrefsManager.instance; - final jsonStr = prefs.getString(_keyPrefix); + // Bulk data lives in drift (#335), not SharedPreferences. The fallback + // covers a key the migration has not moved yet and logs loudly if it fires. + final jsonStr = await BlobStore.instance.readWithPrefsFallback(_keyPrefix); if (jsonStr == null) return []; try { @@ -17,15 +19,19 @@ class ContactDiscoveryStore { return jsonList .map((entry) => _fromJson(entry as Map)) .toList(); - } catch (_) { + } catch (e) { + // SAFELANE 6: a decode failure is not "no data". + appLogger.error( + 'Failed to decode discovered contacts: $e', + tag: 'Storage', + ); return []; } } Future saveContacts(List contacts) async { - final prefs = PrefsManager.instance; final jsonList = contacts.map(_toJson).toList(); - await prefs.setString(_keyPrefix, jsonEncode(jsonList)); + await BlobStore.instance.write(_keyPrefix, jsonEncode(jsonList)); } Map _toJson(Contact contact) { diff --git a/lib/storage/contact_store.dart b/lib/storage/contact_store.dart index 5d1805d..d907752 100644 --- a/lib/storage/contact_store.dart +++ b/lib/storage/contact_store.dart @@ -3,6 +3,7 @@ import 'dart:typed_data'; import '../models/contact.dart'; import '../utils/app_logger.dart'; +import 'drift/blob_store.dart'; import 'prefs_manager.dart'; class ContactStore { @@ -19,23 +20,27 @@ class ContactStore { appLogger.warn('Public key hex is not set. Cannot load contacts.'); return []; } - final prefs = PrefsManager.instance; - String? jsonString = prefs.getString(keyFor); + // Bulk data lives in drift (#335). The fallback covers a key the migration + // has not moved yet, and logs loudly if it fires. + final blobs = BlobStore.instance; + String? jsonString = await blobs.readWithPrefsFallback(keyFor); + if (jsonString == null || jsonString.isEmpty) { - // Attempt migration from legacy unscoped key on first load - final legacyJsonString = prefs.getString(_keyPrefix); - prefs.remove(_keyPrefix); - if (legacyJsonString != null && legacyJsonString.isNotEmpty) { + // Pre-device-scoping data still sits under the legacy unscoped key in + // SharedPreferences. Only touch prefs when it actually exists: an + // unconditional remove costs a full-file rewrite on Windows and a + // 5 MiB-capped synchronous write on web (#306). + final prefs = PrefsManager.instance; + final legacy = prefs.get(_keyPrefix); + if (legacy is String && legacy.isNotEmpty) { appLogger.info( - 'Migrating contacts from legacy key $_keyPrefix to scoped key $keyFor', + 'Migrating contacts from legacy key $_keyPrefix to $keyFor (drift)', ); - await prefs.setString(keyFor, legacyJsonString); - jsonString = legacyJsonString; + await blobs.write(keyFor, legacy); + await prefs.remove(_keyPrefix); + jsonString = legacy; } } - if (jsonString == null || jsonString.isEmpty) { - jsonString = prefs.getString(keyFor); - } if (jsonString == null || jsonString.isEmpty) { return []; } @@ -55,9 +60,8 @@ class ContactStore { appLogger.warn('Public key hex is not set. Cannot save contacts.'); return; } - final prefs = PrefsManager.instance; final jsonList = contacts.map(_toJson).toList(); - await prefs.setString(keyFor, jsonEncode(jsonList)); + await BlobStore.instance.write(keyFor, jsonEncode(jsonList)); } Map _toJson(Contact contact) { diff --git a/lib/storage/drift/blob_store.dart b/lib/storage/drift/blob_store.dart index d92f73f..6cab09b 100644 --- a/lib/storage/drift/blob_store.dart +++ b/lib/storage/drift/blob_store.dart @@ -35,6 +35,29 @@ class BlobStore { static bool isBulkKey(String key) => migratedPrefixes.any(key.startsWith); + /// Reads a bulk key, falling back to SharedPreferences if drift does not + /// have it. + /// + /// Belt and braces for the switchover: if migration ever missed a key, the + /// data must still be reachable rather than silently reading as empty, which + /// is exactly how #333 presented. The fallback is LOUD, because a fallback + /// that fires in normal operation means the migration is incomplete and + /// somebody needs to know. + Future readWithPrefsFallback(String key) async { + final fromDrift = await read(key); + if (fromDrift != null) return fromDrift; + + final raw = PrefsManager.instance.get(key); + if (raw is! String || raw.isEmpty) return null; + + appLogger.warn( + 'Blob read for $key fell back to SharedPreferences: it is NOT in drift. ' + 'Migration is incomplete for this key; serving the prefs copy.', + tag: 'Storage', + ); + return raw; + } + Future read(String key) async { final row = await (_db.select( _db.storedBlobs, @@ -50,6 +73,30 @@ class BlobStore { ); } + /// Keys beginning with [prefix], across BOTH backends. + /// + /// Callers that clear a family of keys must see prefs-resident keys too, or + /// a pre-migration copy survives the clear and reappears through the read + /// fallback. + Future> keysWithPrefix(String prefix) async { + // Filtered in Dart rather than SQL: the table holds tens of rows, so the + // cost is irrelevant and it avoids depending on a version-specific LIKE + // API for a pinned dependency. + final rows = await _db.select(_db.storedBlobs).get(); + + return { + ...rows.map((r) => r.key).where((k) => k.startsWith(prefix)), + ...PrefsManager.instance.getKeys().where((k) => k.startsWith(prefix)), + }.toList(); + } + + /// Removes a key from BOTH backends, so a clear cannot be undone by a + /// leftover prefs copy surfacing through the fallback. + Future deleteEverywhere(String key) async { + await delete(key); + await PrefsManager.instance.remove(key); + } + Future delete(String key) async { await (_db.delete(_db.storedBlobs)..where((t) => t.key.equals(key))).go(); } diff --git a/lib/storage/message_store.dart b/lib/storage/message_store.dart index ccaf9ea..2d8b02d 100644 --- a/lib/storage/message_store.dart +++ b/lib/storage/message_store.dart @@ -4,6 +4,7 @@ import '../models/message.dart'; import '../models/translation_support.dart'; import '../helpers/smaz.dart'; import '../utils/app_logger.dart'; +import 'drift/blob_store.dart'; import 'prefs_manager.dart'; class MessageStore { @@ -23,10 +24,9 @@ class MessageStore { appLogger.warn('Public key hex is not set. Cannot save messages.'); return; } - final prefs = PrefsManager.instance; final key = '$keyFor$contactKeyHex'; final jsonList = messages.map(_messageToJson).toList(); - await prefs.setString(key, jsonEncode(jsonList)); + await BlobStore.instance.write(key, jsonEncode(jsonList)); } Future> loadMessages(String contactKeyHex) async { @@ -34,24 +34,30 @@ class MessageStore { appLogger.warn('Public key hex is not set. Cannot load messages.'); return []; } - final prefs = PrefsManager.instance; final key = '$keyFor$contactKeyHex'; final oldKey = '$_keyPrefix$contactKeyHex'; - String? jsonString = prefs.getString(key); + // Bulk data lives in drift (#335); fallback covers an unmigrated key and + // logs loudly if it fires. + final blobs = BlobStore.instance; + String? jsonString = await blobs.readWithPrefsFallback(key); + if (jsonString == null || jsonString.isEmpty) { - // Attempt migration from legacy unscoped key on first load - final legacyJsonString = prefs.getString(oldKey); - prefs.remove(oldKey); - if (legacyJsonString != null && legacyJsonString.isNotEmpty) { + // Only touch prefs when the legacy key actually exists. An unconditional + // remove here ran once per contact and cost a full-file rewrite each + // time on Windows (#306). + final prefs = PrefsManager.instance; + final legacy = prefs.get(oldKey); + if (legacy is String && legacy.isNotEmpty) { appLogger.info( - 'Migrating messages from legacy key $oldKey to scoped key $key', + 'Migrating messages from legacy key $oldKey to $key (drift)', ); - await prefs.setString(key, legacyJsonString); - jsonString = legacyJsonString; + await blobs.write(key, legacy); + await prefs.remove(oldKey); + jsonString = legacy; } } if (jsonString == null || jsonString.isEmpty) { - jsonString = prefs.getString(keyFor); + jsonString = await blobs.readWithPrefsFallback(keyFor); } if (jsonString == null || jsonString.isEmpty) { return []; @@ -70,9 +76,11 @@ class MessageStore { appLogger.warn('Public key hex is not set. Cannot clear messages.'); return; } - final prefs = PrefsManager.instance; final key = '$keyFor$contactKeyHex'; - await prefs.remove(key); + // Clear both backends: drift is authoritative, but a pre-migration prefs + // copy must not survive a clear and reappear via the read fallback. + await BlobStore.instance.delete(key); + await PrefsManager.instance.remove(key); } Map _messageToJson(Message msg) {