diff --git a/lib/storage/drift/offband_database.dart b/lib/storage/drift/offband_database.dart index 2d7d209..b35b2d1 100644 --- a/lib/storage/drift/offband_database.dart +++ b/lib/storage/drift/offband_database.dart @@ -2,8 +2,10 @@ import 'dart:io'; import 'package:drift/drift.dart'; import 'package:drift_flutter/drift_flutter.dart'; +import 'package:flutter/foundation.dart'; import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; +import 'package:sqlite3/sqlite3.dart'; import '../../utils/app_logger.dart'; @@ -37,17 +39,14 @@ class OffbandDatabase extends _$OffbandDatabase { static QueryExecutor _defaultExecutor() { return driftDatabase( name: _dbName, - // Native default is getApplicationDocumentsDirectory(), which on Windows - // is the user's Documents folder — redirected into OneDrive on most - // machines. A live SQLite file syncing to OneDrive risks lock contention - // and corruption, and it is the wrong place for app data. Use the - // application-support dir instead (%APPDATA% on Windows), where - // SharedPreferences already lives. path_provider has no web - // implementation, so this only applies off web; web ignores - // databaseDirectory and uses OPFS/IndexedDB. - native: DriftNativeOptions( - databaseDirectory: _appSupportDatabaseDirectory, - ), + // The DB directory is PINNED (see _pinnedDatabaseDirectory): a constant + // path that never depends on the exe's version metadata or launch + // location, so builds cannot diverge onto different databases (#363). + // This also keeps the file out of the Documents/OneDrive dir (the native + // default), where syncing a live SQLite file risks lock contention and + // corruption. path_provider has no web implementation, so this only + // applies off web; web ignores databaseDirectory and uses OPFS/IndexedDB. + native: DriftNativeOptions(databaseDirectory: _pinnedDatabaseDirectory), web: DriftWebOptions( sqlite3Wasm: Uri.parse('sqlite3.wasm'), // Filename matches the asset published by the drift release, which is @@ -59,82 +58,239 @@ class OffbandDatabase extends _$OffbandDatabase { ); } - /// Resolves the application-support directory, and relocates a database left - /// in the old documents location by an earlier build. + /// Resolves the ONE fixed directory the database lives in, and migrates a DB + /// left in an older location by a previous build into it. + /// + /// The path is pinned so it never depends on the executable's version + /// metadata or launch location (#363). `getApplicationSupportDirectory()` on + /// Windows derives the folder from the exe's embedded Company/Product + /// strings, so two builds - or a future rebrand - would read DIFFERENT + /// databases and the history would appear to vanish. On Windows the DB is + /// therefore pinned to `%APPDATA%\Offband MeshCore`, a constant. /// - /// Builds before this fix opened the DB under getApplicationDocumentsDirectory - /// (OneDrive on Windows). Existing installs already have data there, so this - /// moves the file - and its `-wal` / `-shm` sidecars - to the new location on - /// first run, once, before drift opens it. Never deletes the source without a - /// successful move; a failed move leaves the old file so no data is stranded. - static Future _appSupportDatabaseDirectory() async { - final support = await getApplicationSupportDirectory(); - final newPath = p.join(support.path, '$_dbName.sqlite'); - - if (!File(newPath).existsSync()) { + /// On first run at the pinned location, the richest `offband_store.sqlite` + /// from the old locations (the metadata-derived support dir; the + /// documents/OneDrive dir; the earlier nested support path) is copied in - a + /// copy, never a move: the source is left intact so nothing is stranded if + /// anything goes wrong. + static Future _pinnedDatabaseDirectory() async { + final dir = await _resolvePinnedDir(); + if (!dir.existsSync()) dir.createSync(recursive: true); + + final target = p.join(dir.path, '$_dbName.sqlite'); + if (!File(target).existsSync()) { try { - final docs = await getApplicationDocumentsDirectory(); - final oldPath = p.join(docs.path, '$_dbName.sqlite'); - if (File(oldPath).existsSync() && oldPath != newPath) { - // Copy ALL files first, verify, and only then delete the sources. - // Deleting per-file mid-loop (Gemini review) could strand the main - // .sqlite in one place and its -wal in another if a later copy - // failed, corrupting the DB. Copy-all-then-delete-all makes a - // partial failure recoverable: the original stays intact and the - // half-written destination is cleaned up. - final suffixes = ['', '-wal', '-shm']; - final present = suffixes - .where((s) => File('$oldPath$s').existsSync()) - .toList(); - try { - for (final s in present) { - File('$oldPath$s').copySync('$newPath$s'); - } - // Verify every copy exists with a matching size before deleting. - for (final s in present) { - final src = File('$oldPath$s'); - final dst = File('$newPath$s'); - if (!dst.existsSync() || dst.lengthSync() != src.lengthSync()) { - throw StateError('copy verification failed for $newPath$s'); - } - } - for (final s in present) { - File('$oldPath$s').deleteSync(); - } - appLogger.info( - 'Relocated drift DB out of the documents dir (OneDrive on ' - 'Windows) into the app-support dir: $oldPath -> $newPath', - tag: 'Storage', - ); - } catch (e) { - // Roll back any partial destination so drift does not open a - // half-copied DB; the original in the documents dir is untouched. - for (final s in present) { - final dst = File('$newPath$s'); - if (dst.existsSync()) { - try { - dst.deleteSync(); - } catch (_) {} - } - } - rethrow; - } - } + await _importRichestLegacyDatabase(target); } catch (e) { - // Relocation is best-effort. On failure the ORIGINAL DB is left intact - // in the documents dir (the destination was rolled back), so no data is - // lost - drift opens a fresh DB at the support location and the user's - // history remains recoverable from the old path. Loud, never silent. + // Best-effort: on failure every SOURCE is left intact and any partial + // destination is rolled back, so drift opens a fresh DB and the old + // data stays recoverable. Loud, never silent (SAFELANE 6). appLogger.error( - 'Failed to relocate the drift DB from the documents dir: $e. The ' - 'original at the old path is intact; a fresh DB will open at the ' - 'app-support location. Recover the old DB manually if needed.', + 'Failed to migrate an existing drift DB into the pinned directory ' + '($target): $e. Sources are intact; a fresh DB will open here and the ' + 'old data remains recoverable.', tag: 'Storage', ); } } - return support; + return dir; + } + + /// The fixed directory: `/Offband MeshCore`, a constant. + /// + /// Windows uses `%APPDATA%`. Linux is pinned too, because its support dir + /// derives from the executable NAME and so has the same divergence bug this + /// fixes; it uses `$XDG_DATA_HOME` (or `~/.local/share`). macOS is left on + /// `getApplicationSupportDirectory()`, which is keyed by the stable bundle id. + static Future _resolvePinnedDir() async { + if (Platform.isWindows) { + final appData = Platform.environment['APPDATA']; + if (appData != null && appData.isNotEmpty) { + return Directory(p.join(appData, _pinnedFolder)); + } + } + if (Platform.isLinux) { + final xdg = Platform.environment['XDG_DATA_HOME']; + final home = Platform.environment['HOME']; + final base = (xdg != null && xdg.isNotEmpty) + ? xdg + : (home != null && home.isNotEmpty + ? p.join(home, '.local', 'share') + : null); + if (base != null) return Directory(p.join(base, _pinnedFolder)); + } + return getApplicationSupportDirectory(); + } + + static const String _pinnedFolder = 'Offband MeshCore'; + + /// Snapshots the legacy DB (chosen from the known and scanned locations) into + /// [target] (which must not yet exist), leaving the source intact. A no-op on + /// a fresh install where no legacy DB exists. + static Future _importRichestLegacyDatabase(String target) async { + // Prefer the KNOWN canonical locations in priority order: these are where a + // normal prior build wrote (the metadata-derived support dir, then the + // documents/OneDrive dir the first builds used). Using them directly avoids + // the size heuristic picking a larger-but-junk DB from some other build. + final known = []; + try { + final support = await getApplicationSupportDirectory(); + known.add(p.join(support.path, '$_dbName.sqlite')); + } catch (_) {} + try { + final docs = await getApplicationDocumentsDirectory(); + known.add(p.join(docs.path, '$_dbName.sqlite')); + } catch (_) {} + + final scanned = _scanAppDataForDatabases() + .where((c) => c != target) + .toList(); + final source = chooseMigrationSource( + known.where((c) => c != target).toList(), + scanned, + ); + if (source == null) return; // fresh install, nothing to migrate + + snapshotDatabase(source, target); + appLogger.info( + 'Migrated drift DB into the pinned directory: $source -> $target ' + '(source left intact)', + tag: 'Storage', + ); + } + + /// Writes a consistent snapshot of the [source] DB to [target] via SQLite + /// `VACUUM INTO`, which takes a read transaction and so produces a correct + /// snapshot even if the source is being written by another process (an old + /// build left running) - the failure mode a raw file copy of a live WAL + /// database has (Gemini review). + /// + /// On ANY failure it deletes the partial output and RETHROWS - it does NOT + /// fall back to a file copy. VACUUM most often fails precisely because a + /// writer holds a lock, which is exactly when a raw copy would be unsafe, so + /// the safe move is to abort: the source is untouched and drift opens a fresh + /// DB, keeping the history recoverable. + /// + /// The source's main and `-wal` files are not written; SQLite may create or + /// touch the temporary `-shm` file next to the source when opening a WAL DB. + /// Visible for testing. + @visibleForTesting + static void snapshotDatabase(String source, String target) { + try { + final db = sqlite3.open(source, mode: OpenMode.readOnly); + try { + db.execute("VACUUM INTO '${target.replaceAll("'", "''")}'"); + } finally { + db.close(); + } + final out = File(target); + if (!out.existsSync() || out.lengthSync() == 0) { + throw StateError('VACUUM INTO produced no output at $target'); + } + } catch (_) { + final out = File(target); + if (out.existsSync()) { + try { + out.deleteSync(); + } catch (_) {} + } + rethrow; + } + } + + /// Bounded scan of the app-data roots for the DB file, so one left by a build + /// whose folder name we do not know (its metadata differed) is still found. + static List _scanAppDataForDatabases() { + final roots = []; + for (final env in const ['APPDATA', 'LOCALAPPDATA']) { + final root = Platform.environment[env]; + if (root == null || root.isEmpty) continue; + final dir = Directory(root); + if (dir.existsSync()) roots.add(dir); + } + return findDatabaseFiles(roots, '$_dbName.sqlite'); + } + + /// Depth-limited, entry-capped search of [roots] for files named [fileName]. + /// + /// No directory-name blocklist: a build's data can legitimately sit under a + /// folder named after ANY company string (e.g. `%APPDATA%\Microsoft\...` for + /// a runner that never set custom metadata), so skipping by name would lose + /// exactly the data this migration exists to preserve. Instead the walk is + /// bounded by [maxDepth] and a hard [maxEntries] visit cap so a pathological + /// tree (a folder with hundreds of thousands of files, a slow network drive) + /// cannot stall startup. Symlinks are not followed, so it cannot cycle; + /// unreadable directories are skipped. Visible for testing. + @visibleForTesting + static List findDatabaseFiles( + List roots, + String fileName, { + int maxDepth = 2, + int maxEntries = 50000, + }) { + final found = []; + var visited = 0; + + void walk(Directory dir, int depth) { + List entries; + try { + entries = dir.listSync(followLinks: false); + } catch (_) { + return; // permission denied etc. + } + for (final e in entries) { + if (visited++ >= maxEntries) return; + if (e is File) { + if (p.basename(e.path) == fileName) found.add(e.path); + } else if (e is Directory && depth < maxDepth) { + walk(e, depth + 1); + } + } + } + + for (final root in roots) { + walk(root, 0); + } + return found; + } + + /// Picks the DB to migrate: the first existing, non-empty path in + /// [knownInPriority] (the canonical locations a normal prior build wrote to), + /// else the largest DB in [scanned] (the unknown-folder fallback). + /// + /// Preferring the known locations stops a larger-but-junk DB from some other + /// build (a size-heuristic misfire) from beating the real one. Visible for + /// testing. + @visibleForTesting + static String? chooseMigrationSource( + List knownInPriority, + List scanned, + ) { + for (final k in knownInPriority) { + final f = File(k); + if (f.existsSync() && f.lengthSync() > 0) return k; + } + return richestExisting(scanned); + } + + /// Returns the existing, non-empty candidate path with the LARGEST main file, + /// or null if none exist. Choosing by size guarantees a small/fresh DB never + /// wins over one holding real history. Visible for testing. + @visibleForTesting + static String? richestExisting(List candidatePaths) { + String? best; + int bestSize = 0; + for (final path in candidatePaths) { + final f = File(path); + if (!f.existsSync()) continue; + final size = f.lengthSync(); + if (size > bestSize) { + bestSize = size; + best = path; + } + } + return best; } @override diff --git a/pubspec.lock b/pubspec.lock index e31ec5c..e358930 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -1102,6 +1102,46 @@ packages: url: "https://pub.dev" source: hosted version: "0.28.0" + screen_retriever: + dependency: "direct main" + description: + name: screen_retriever + sha256: ace919117a7520c13a50a6259e60c4a0d4cbe98809468792a91b5c5adada2aa6 + url: "https://pub.dev" + source: hosted + version: "0.2.2" + screen_retriever_linux: + dependency: transitive + description: + name: screen_retriever_linux + sha256: "7b52006a5ceae1f3d5af7f77188c3290d6e7d8ded16d99809bea84967c65c257" + url: "https://pub.dev" + source: hosted + version: "0.2.2" + screen_retriever_macos: + dependency: transitive + description: + name: screen_retriever_macos + sha256: a1489b99cce597c45a54b9aae1cd94c8d4705353b7e0bb2457a6e4de44e0ad8a + url: "https://pub.dev" + source: hosted + version: "0.2.2" + screen_retriever_platform_interface: + dependency: transitive + description: + name: screen_retriever_platform_interface + sha256: "94a5535277510a63184ca178ce12a1449bc0b38618879aa1c18bf57369c5064a" + url: "https://pub.dev" + source: hosted + version: "0.2.2" + screen_retriever_windows: + dependency: transitive + description: + name: screen_retriever_windows + sha256: dafc6922b0bfbf1d48cf3ccbf519b4fff47bdcb820da1728ea6db675fecc9324 + url: "https://pub.dev" + source: hosted + version: "0.2.2" share_plus: dependency: "direct main" description: @@ -1300,7 +1340,7 @@ packages: source: hosted version: "0.7.0+eol" sqlite3: - dependency: transitive + dependency: "direct main" description: name: sqlite3 sha256: c73fd75df1332d76a6257f4823ae4df9c791f522b97e4a60cbcad214de1becf4 @@ -1579,6 +1619,14 @@ packages: url: "https://pub.dev" source: hosted version: "5.15.0" + window_manager: + dependency: "direct main" + description: + name: window_manager + sha256: "05c231fd7b23d2380f14c5cc10b7b93d60d4fa4a2fb4e0f032de27e44b5560e9" + url: "https://pub.dev" + source: hosted + version: "0.5.2" wkt_parser: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index 0da6b94..293f929 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -77,6 +77,10 @@ dependencies: # from the drift release. Bump deliberately, then re-download both assets. drift: 2.34.2 drift_flutter: 0.3.1 + # Direct use for a consistent DB snapshot during the pinned-dir migration + # (#363, VACUUM INTO). Pinned to drift 2.34.2's resolved sqlite3 so it does + # not drift out of lockstep with the web wasm assets. + sqlite3: 3.5.0 path: ^1.9.1 window_manager: ^0.5.2 screen_retriever: ^0.2.2 diff --git a/test/storage/db_path_migration_test.dart b/test/storage/db_path_migration_test.dart new file mode 100644 index 0000000..9e8b32f --- /dev/null +++ b/test/storage/db_path_migration_test.dart @@ -0,0 +1,190 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/storage/drift/offband_database.dart'; +import 'package:sqlite3/sqlite3.dart'; + +/// #363: the drift DB directory is pinned to one fixed location, and an existing +/// DB from an older/unknown location is migrated into it. These pin the +/// data-safety-critical helpers: choosing the richest source, finding a DB under +/// any folder name, and copying without stranding or truncating data. +void main() { + late Directory tmp; + + setUp(() => tmp = Directory.systemTemp.createTempSync('dbpath_test')); + tearDown(() => tmp.deleteSync(recursive: true)); + + String write(String relPath, int bytes) { + final f = File('${tmp.path}/$relPath') + ..createSync(recursive: true) + ..writeAsBytesSync(List.filled(bytes, 0)); + return f.path; + } + + // listSync returns backslash paths on Windows; normalize both sides so the + // comparison is separator-agnostic. + String norm(String s) => s.replaceAll('\\', '/'); + List normAll(List xs) => xs.map(norm).toList(); + + group('richestExisting', () { + test('picks the largest existing candidate', () { + final small = write('small.sqlite', 100); + final big = write('big.sqlite', 5000); + final medium = write('medium.sqlite', 1000); + expect(OffbandDatabase.richestExisting([small, big, medium]), big); + }); + + test('ignores candidates that do not exist', () { + final real = write('real.sqlite', 200); + expect( + OffbandDatabase.richestExisting(['${tmp.path}/nope.sqlite', real]), + real, + ); + }); + + test('returns null when none exist (fresh install)', () { + expect( + OffbandDatabase.richestExisting([ + '${tmp.path}/a.sqlite', + '${tmp.path}/b.sqlite', + ]), + isNull, + ); + }); + + test('a 0-byte DB never wins', () { + final empty = write('empty.sqlite', 0); + final real = write('real.sqlite', 300); + expect(OffbandDatabase.richestExisting([empty, real]), real); + expect(OffbandDatabase.richestExisting([empty]), isNull); + }); + }); + + group('findDatabaseFiles', () { + test('finds the DB under ANY company folder, incl. "Microsoft"', () { + // The exact case the old blocklist would have dropped: a runner with + // default metadata writes under %APPDATA%\Microsoft\\. + final target = write( + 'Microsoft/Offband MeshCore/offband_store.sqlite', + 9, + ); + final found = OffbandDatabase.findDatabaseFiles([ + tmp, + ], 'offband_store.sqlite'); + expect(normAll(found), contains(norm(target))); + }); + + test('respects maxDepth (does not descend past the limit)', () { + write('a/b/c/offband_store.sqlite', 9); // depth 3 from tmp + final found = OffbandDatabase.findDatabaseFiles( + [tmp], + 'offband_store.sqlite', + maxDepth: 2, + ); + expect(found, isEmpty); + }); + + test('finds a DB at the allowed depth', () { + final target = write('company/product/offband_store.sqlite', 9); + final found = OffbandDatabase.findDatabaseFiles( + [tmp], + 'offband_store.sqlite', + maxDepth: 2, + ); + expect(normAll(found), contains(norm(target))); + }); + + test('stops at the entry cap rather than scanning unbounded', () { + for (var i = 0; i < 50; i++) { + write('dir/file$i.txt', 1); + } + write('dir/offband_store.sqlite', 9); + // A tiny cap must return without walking everything (no throw, bounded). + final found = OffbandDatabase.findDatabaseFiles( + [tmp], + 'offband_store.sqlite', + maxEntries: 5, + ); + expect(found.length, lessThanOrEqualTo(1)); + }); + }); + + group('chooseMigrationSource', () { + test('prefers a known location over a larger scanned DB', () { + final known = write('support/offband_store.sqlite', 100); + final biggerJunk = write('elsewhere/offband_store.sqlite', 999999); + expect( + OffbandDatabase.chooseMigrationSource([known], [biggerJunk, known]), + known, + reason: 'a real DB at a known location beats a larger junk one', + ); + }); + + test('falls back to the largest scanned DB when no known one exists', () { + final missing = '${tmp.path}/support/offband_store.sqlite'; + final small = write('a/offband_store.sqlite', 100); + final big = write('b/offband_store.sqlite', 5000); + expect( + OffbandDatabase.chooseMigrationSource([missing], [small, big]), + big, + ); + }); + + test('skips an empty known DB and uses the scan', () { + final emptyKnown = write('support/offband_store.sqlite', 0); + final scanned = write('b/offband_store.sqlite', 500); + expect( + OffbandDatabase.chooseMigrationSource([emptyKnown], [scanned]), + scanned, + ); + }); + + test('returns null when nothing exists', () { + expect( + OffbandDatabase.chooseMigrationSource(['${tmp.path}/nope.sqlite'], []), + isNull, + ); + }); + }); + + group('snapshotDatabase', () { + test('VACUUM INTO round-trips real rows and leaves the source', () { + final src = '${tmp.path}/real/offband_store.sqlite'; + Directory('${tmp.path}/real').createSync(recursive: true); + final db = sqlite3.open(src); + db.execute('CREATE TABLE stored_blobs(key TEXT PRIMARY KEY, value TEXT)'); + db.execute("INSERT INTO stored_blobs VALUES('a','1'),('b','2')"); + db.close(); + + final dst = '${tmp.path}/snap/offband_store.sqlite'; + Directory('${tmp.path}/snap').createSync(); + OffbandDatabase.snapshotDatabase(src, dst); + + final out = sqlite3.open(dst, mode: OpenMode.readOnly); + final rows = out.select( + 'SELECT key, value FROM stored_blobs ORDER BY key', + ); + out.close(); + expect(rows.map((r) => '${r['key']}=${r['value']}').toList(), [ + 'a=1', + 'b=2', + ]); + expect(File(src).existsSync(), isTrue, reason: 'source untouched'); + }); + + test('aborts (throws) and leaves no target on a non-DB source', () { + // A garbage file is not a valid SQLite DB: VACUUM INTO must fail, the + // partial target must be cleaned up, and the error must propagate rather + // than fall back to any unsafe path. + final src = write('garbage/offband_store.sqlite', 64); + final dst = '${tmp.path}/snap2/offband_store.sqlite'; + Directory('${tmp.path}/snap2').createSync(); + + expect( + () => OffbandDatabase.snapshotDatabase(src, dst), + throwsA(anything), + ); + expect(File(dst).existsSync(), isFalse, reason: 'no partial left behind'); + }); + }); +}