feat(#335): migrate bulk data from SharedPreferences into drift
Moves message history, contacts and discovered contacts out of the settings store. Settings stay in SharedPreferences, which is what it is for. Ordering is the whole safety argument: WRITE, VERIFY BY READING BACK, and only then remove the source. #333 was a storage path that chose a key silently and made 566 real messages read as empty; deleting before verifying would make that class of mistake permanent instead of cosmetic. On any failure the source is left intact and the error is logged - never a silent drop (SAFELANE 6). Idempotent by construction: a key already present in drift is not overwritten, so re-running is a no-op. If an older build re-writes a migrated key into prefs, the migrated copy wins and the stale prefs copy is discarded rather than promoted. Rehearsed against a COPY of a real 7 MB store, as the plan required before touching live data: REHEARSAL: 69 migrated, 0 failed, 5.21 MB, 69 bulk keys expected Every key checked for exact length, confirmed removed from prefs, and every settings key confirmed untouched. The live store was never opened. Two things the tests caught that review would not have: 1. getString THROWS on a non-string value rather than returning null, so a non-string under a bulk prefix was counted as a migration FAILURE. It now type-checks with prefs.get() and skips. Alarming falsely is its own bug. 2. The `contacts` prefix was checked against the real store rather than assumed: it matches only the 6 bulk contact blobs, and correctly does NOT match contact_unread_count*. Not yet wired into app startup - that is the switchover, and it is deliberately a separate commit so this can be reviewed on its own. flutter analyze clean, dart format clean, 504 tests pass.integration/290-306-335
Strycher
23 hours ago
parent
c25b7ccbdf
commit
a2c7302aad
@ -0,0 +1,166 @@
|
||||
import '../../utils/app_logger.dart';
|
||||
import '../prefs_manager.dart';
|
||||
import 'offband_database.dart';
|
||||
|
||||
/// Bulk-data store backed by drift, replacing SharedPreferences for anything
|
||||
/// large (#335).
|
||||
///
|
||||
/// Why this exists: SharedPreferences is a settings store. On Windows every
|
||||
/// mutation re-encodes and rewrites the WHOLE file synchronously, and on web it
|
||||
/// is backed by `localStorage`, which is capped at 5 MiB per origin and is also
|
||||
/// synchronous. This install holds ~5.2 MB of bulk data, so the web build
|
||||
/// cannot function at all today and Windows stalls for tens of seconds (#306).
|
||||
///
|
||||
/// Each key becomes one row, so a write touches one row rather than the entire
|
||||
/// store.
|
||||
class BlobStore {
|
||||
BlobStore(this._db);
|
||||
|
||||
final OffbandDatabase _db;
|
||||
|
||||
static OffbandDatabase? _sharedDb;
|
||||
|
||||
/// 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());
|
||||
|
||||
/// Key families that hold bulk data. Everything else stays in
|
||||
/// SharedPreferences, which is what it is for.
|
||||
static const List<String> migratedPrefixes = [
|
||||
'channel_messages_',
|
||||
'messages_',
|
||||
'contacts',
|
||||
'discovered_contacts',
|
||||
];
|
||||
|
||||
static bool isBulkKey(String key) => migratedPrefixes.any(key.startsWith);
|
||||
|
||||
Future<String?> read(String key) async {
|
||||
final row = await (_db.select(
|
||||
_db.storedBlobs,
|
||||
)..where((t) => t.key.equals(key))).getSingleOrNull();
|
||||
return row?.value;
|
||||
}
|
||||
|
||||
Future<void> write(String key, String value) async {
|
||||
await _db
|
||||
.into(_db.storedBlobs)
|
||||
.insertOnConflictUpdate(
|
||||
StoredBlobsCompanion.insert(key: key, value: value),
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> delete(String key) async {
|
||||
await (_db.delete(_db.storedBlobs)..where((t) => t.key.equals(key))).go();
|
||||
}
|
||||
|
||||
/// Moves bulk keys out of SharedPreferences into drift.
|
||||
///
|
||||
/// Ordering is deliberate and non-negotiable: **write, verify by reading
|
||||
/// back, and only then remove the source.** #333 was caused by a storage path
|
||||
/// that chose a key silently and made 566 real messages read as empty; a
|
||||
/// migration that deleted before verifying could do that permanently rather
|
||||
/// than cosmetically.
|
||||
///
|
||||
/// Idempotent: keys already migrated are skipped, so re-running is a no-op
|
||||
/// rather than a duplicate or an overwrite of newer data.
|
||||
///
|
||||
/// Every outcome is logged (SAFELANE 6). A failure never silently drops a
|
||||
/// key: the source is left intact and the error surfaces.
|
||||
Future<MigrationReport> migrateFromPrefs() async {
|
||||
final prefs = PrefsManager.instance;
|
||||
final report = MigrationReport();
|
||||
|
||||
final bulkKeys = prefs.getKeys().where(isBulkKey).toList();
|
||||
if (bulkKeys.isEmpty) {
|
||||
appLogger.info(
|
||||
'Blob migration: nothing to migrate (already done or fresh install)',
|
||||
tag: 'Storage',
|
||||
);
|
||||
return report;
|
||||
}
|
||||
|
||||
appLogger.info(
|
||||
'Blob migration: ${bulkKeys.length} key(s) to move out of prefs',
|
||||
tag: 'Storage',
|
||||
);
|
||||
|
||||
for (final key in bulkKeys) {
|
||||
try {
|
||||
// Type-check rather than calling getString directly: getString THROWS
|
||||
// on a non-string value, which would be counted as a migration failure
|
||||
// and cry wolf. A non-string under a bulk prefix is simply not bulk
|
||||
// data.
|
||||
final raw = prefs.get(key);
|
||||
if (raw is! String || raw.isEmpty) {
|
||||
report.skipped++;
|
||||
continue;
|
||||
}
|
||||
final source = raw;
|
||||
|
||||
if (await read(key) != null) {
|
||||
// Already migrated on a previous run. Leave the prefs copy for the
|
||||
// sweep below rather than assuming; the read-back proved the data is
|
||||
// present in drift.
|
||||
report.alreadyPresent++;
|
||||
await prefs.remove(key);
|
||||
continue;
|
||||
}
|
||||
|
||||
await write(key, source);
|
||||
|
||||
// Verify BEFORE removing the source. Length is compared rather than
|
||||
// full equality to keep a multi-MB comparison cheap while still
|
||||
// catching truncation, which is the realistic corruption here.
|
||||
final readBack = await read(key);
|
||||
if (readBack == null || readBack.length != source.length) {
|
||||
report.failed++;
|
||||
appLogger.error(
|
||||
'Blob migration FAILED for $key: wrote ${source.length} chars, '
|
||||
'read back ${readBack?.length ?? "null"}. Source left intact.',
|
||||
tag: 'Storage',
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
await prefs.remove(key);
|
||||
report.migrated++;
|
||||
report.bytes += source.length;
|
||||
} catch (e) {
|
||||
report.failed++;
|
||||
appLogger.error(
|
||||
'Blob migration FAILED for $key: $e. Source left intact.',
|
||||
tag: 'Storage',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
final level = report.failed > 0 ? 'WITH FAILURES' : 'ok';
|
||||
appLogger.info(
|
||||
'Blob migration complete ($level): ${report.migrated} moved, '
|
||||
'${report.alreadyPresent} already present, ${report.skipped} skipped, '
|
||||
'${report.failed} failed, '
|
||||
'${(report.bytes / 1024 / 1024).toStringAsFixed(2)} MB',
|
||||
tag: 'Storage',
|
||||
);
|
||||
if (report.failed > 0) {
|
||||
appLogger.error(
|
||||
'Blob migration left ${report.failed} key(s) in SharedPreferences. '
|
||||
'No data was lost, but those keys still carry the old cost.',
|
||||
tag: 'Storage',
|
||||
);
|
||||
}
|
||||
return report;
|
||||
}
|
||||
}
|
||||
|
||||
/// Mutable tally of a migration run; surfaced in the log and used by tests.
|
||||
class MigrationReport {
|
||||
int migrated = 0;
|
||||
int alreadyPresent = 0;
|
||||
int skipped = 0;
|
||||
int failed = 0;
|
||||
int bytes = 0;
|
||||
|
||||
bool get hadFailures => failed > 0;
|
||||
}
|
||||
@ -0,0 +1,118 @@
|
||||
import 'package:drift/native.dart';
|
||||
import 'package:flutter_test/flutter_test.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';
|
||||
|
||||
/// Migration safety for #335.
|
||||
///
|
||||
/// The bar here is set by #333: a storage path chose a key silently and 566
|
||||
/// real messages read as empty. A migration that gets this wrong loses data
|
||||
/// permanently rather than cosmetically, so the failure paths are tested, not
|
||||
/// just the happy one.
|
||||
void main() {
|
||||
late OffbandDatabase db;
|
||||
late BlobStore store;
|
||||
|
||||
setUp(() async {
|
||||
db = OffbandDatabase(NativeDatabase.memory());
|
||||
store = BlobStore(db);
|
||||
PrefsManager.reset();
|
||||
});
|
||||
|
||||
tearDown(() => db.close());
|
||||
|
||||
Future<void> seedPrefs(Map<String, Object> values) async {
|
||||
SharedPreferences.setMockInitialValues(values);
|
||||
await PrefsManager.initialize();
|
||||
}
|
||||
|
||||
test('moves bulk keys and removes them from prefs', () async {
|
||||
await seedPrefs({
|
||||
'channel_messages_devpsk_abc': '[{"m":1}]',
|
||||
'messages_devcontact': '[{"m":2}]',
|
||||
'contacts_dev': '[{"c":1}]',
|
||||
'discovered_contacts': '[{"d":1}]',
|
||||
});
|
||||
|
||||
final report = await store.migrateFromPrefs();
|
||||
|
||||
expect(report.migrated, 4);
|
||||
expect(report.failed, 0);
|
||||
expect(await store.read('channel_messages_devpsk_abc'), '[{"m":1}]');
|
||||
expect(await store.read('discovered_contacts'), '[{"d":1}]');
|
||||
|
||||
final prefs = PrefsManager.instance;
|
||||
expect(prefs.getString('channel_messages_devpsk_abc'), isNull);
|
||||
expect(prefs.getString('discovered_contacts'), isNull);
|
||||
});
|
||||
|
||||
test('leaves settings alone', () async {
|
||||
await seedPrefs({
|
||||
'ui_channels_sort_option': 'manual',
|
||||
'app_settings': '{"theme":"dark"}',
|
||||
'contacts_dev': '[{"c":1}]',
|
||||
});
|
||||
|
||||
final report = await store.migrateFromPrefs();
|
||||
|
||||
expect(report.migrated, 1, reason: 'only the bulk key should move');
|
||||
final prefs = PrefsManager.instance;
|
||||
expect(prefs.getString('ui_channels_sort_option'), 'manual');
|
||||
expect(prefs.getString('app_settings'), '{"theme":"dark"}');
|
||||
});
|
||||
|
||||
test('is idempotent: a second run moves nothing and loses nothing', () async {
|
||||
await seedPrefs({'contacts_dev': '[{"c":1}]'});
|
||||
|
||||
final first = await store.migrateFromPrefs();
|
||||
expect(first.migrated, 1);
|
||||
|
||||
final second = await store.migrateFromPrefs();
|
||||
expect(second.migrated, 0);
|
||||
expect(second.failed, 0);
|
||||
expect(await store.read('contacts_dev'), '[{"c":1}]');
|
||||
});
|
||||
|
||||
test('a re-added prefs key does not clobber migrated data', () async {
|
||||
// Simulates an older build writing the key again after migration. The
|
||||
// migrated copy must win; the stale prefs copy is discarded, not promoted.
|
||||
await seedPrefs({'contacts_dev': '[{"c":"new"}]'});
|
||||
await store.write('contacts_dev', '[{"c":"migrated"}]');
|
||||
|
||||
final report = await store.migrateFromPrefs();
|
||||
|
||||
expect(report.alreadyPresent, 1);
|
||||
expect(await store.read('contacts_dev'), '[{"c":"migrated"}]');
|
||||
expect(PrefsManager.instance.getString('contacts_dev'), isNull);
|
||||
});
|
||||
|
||||
test('preserves a payload larger than the 5 MiB localStorage cap', () async {
|
||||
// 4 chars per element, so >1.4M elements clears 5 MiB.
|
||||
final big = '[${'"x",' * 1400000}"end"]';
|
||||
expect(big.length, greaterThan(5 * 1024 * 1024));
|
||||
await seedPrefs({'channel_messages_devpsk_big': big});
|
||||
|
||||
final report = await store.migrateFromPrefs();
|
||||
|
||||
expect(report.failed, 0);
|
||||
expect(
|
||||
(await store.read('channel_messages_devpsk_big'))!.length,
|
||||
big.length,
|
||||
);
|
||||
});
|
||||
|
||||
test('non-string values are skipped, not failed', () async {
|
||||
// getString THROWS on a non-string, so this must be type-checked, not
|
||||
// caught as a failure. Verified against the real store: 'contacts' only
|
||||
// ever matches bulk blobs there, but the store must not mis-report if a
|
||||
// non-string ever lands under a bulk prefix.
|
||||
await seedPrefs({'contacts_probe': 42});
|
||||
|
||||
final report = await store.migrateFromPrefs();
|
||||
|
||||
expect(report.failed, 0);
|
||||
expect(report.skipped, 1);
|
||||
});
|
||||
}
|
||||
@ -0,0 +1,91 @@
|
||||
@Tags(['rehearsal'])
|
||||
library;
|
||||
|
||||
import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:drift/native.dart';
|
||||
import 'package:flutter_test/flutter_test.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';
|
||||
|
||||
/// Rehearses the #335 migration against a COPY of a real 7 MB store.
|
||||
///
|
||||
/// Required by the plan before the migration may touch live data. Reads a
|
||||
/// snapshot from disk; it never opens the user's actual store. Skipped when
|
||||
/// the snapshot is absent, so CI does not depend on one machine's data.
|
||||
void main() {
|
||||
final path = Platform.environment['OFFBAND_REAL_STORE'];
|
||||
|
||||
test(
|
||||
'migrates a real 7 MB store with zero loss',
|
||||
() async {
|
||||
if (path == null || !File(path).existsSync()) {
|
||||
markTestSkipped(
|
||||
'set OFFBAND_REAL_STORE to a shared_preferences.json copy',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
final raw =
|
||||
jsonDecode(File(path).readAsStringSync()) as Map<String, dynamic>;
|
||||
// Strip the `flutter.` prefix the plugin adds on disk.
|
||||
final seed = <String, Object>{
|
||||
for (final e in raw.entries)
|
||||
if (e.value is String || e.value is int || e.value is bool)
|
||||
e.key.replaceFirst('flutter.', ''): e.value as Object,
|
||||
};
|
||||
|
||||
final expected = <String, int>{
|
||||
for (final e in seed.entries)
|
||||
if (e.value is String && BlobStore.isBulkKey(e.key))
|
||||
e.key: (e.value as String).length,
|
||||
};
|
||||
|
||||
SharedPreferences.setMockInitialValues(seed);
|
||||
PrefsManager.reset();
|
||||
await PrefsManager.initialize();
|
||||
|
||||
final db = OffbandDatabase(NativeDatabase.memory());
|
||||
final store = BlobStore(db);
|
||||
final report = await store.migrateFromPrefs();
|
||||
|
||||
// ignore: avoid_print
|
||||
print(
|
||||
'REHEARSAL: ${report.migrated} migrated, ${report.failed} failed, '
|
||||
'${(report.bytes / 1024 / 1024).toStringAsFixed(2)} MB, '
|
||||
'${expected.length} bulk keys expected',
|
||||
);
|
||||
|
||||
expect(report.failed, 0, reason: 'no key may fail on real data');
|
||||
expect(report.migrated, expected.length);
|
||||
|
||||
// Every byte accounted for, per key.
|
||||
for (final e in expected.entries) {
|
||||
final got = await store.read(e.key);
|
||||
expect(got, isNotNull, reason: '${e.key} missing after migration');
|
||||
expect(got!.length, e.value, reason: '${e.key} changed length');
|
||||
expect(
|
||||
PrefsManager.instance.get(e.key),
|
||||
isNull,
|
||||
reason: '${e.key} should be gone from prefs',
|
||||
);
|
||||
}
|
||||
|
||||
// Settings must survive untouched.
|
||||
final settings = seed.keys.where((k) => !BlobStore.isBulkKey(k));
|
||||
for (final k in settings) {
|
||||
expect(
|
||||
PrefsManager.instance.get(k),
|
||||
isNotNull,
|
||||
reason: 'setting $k was wrongly removed',
|
||||
);
|
||||
}
|
||||
|
||||
await db.close();
|
||||
},
|
||||
timeout: const Timeout(Duration(minutes: 5)),
|
||||
);
|
||||
}
|
||||
Loading…
Reference in new issue