diff --git a/lib/persistor/file_persistor/file_data_store_config.dart b/lib/persistor/file_persistor/file_data_store_config.dart index 7a5a2ee..4135d75 100644 --- a/lib/persistor/file_persistor/file_data_store_config.dart +++ b/lib/persistor/file_persistor/file_data_store_config.dart @@ -4,6 +4,45 @@ import 'package:loon/loon.dart'; import 'package:loon/persistor/data_store.dart'; import 'package:loon/persistor/data_store_resolver.dart'; +/// Writes [contents] to [file] atomically: the data is written to a sibling +/// temporary file, flushed to disk, then renamed over the target. A rename on +/// the same filesystem is atomic, so an interrupted write (crash, OOM kill, +/// power loss) can never leave the target torn — it always holds either the +/// complete previous contents or the complete new contents. A plain +/// `writeAsString` truncates the target up front, leaving a window where a +/// crash corrupts it. +/// +/// The `.tmp` suffix is deliberately not matched by the data store file listing +/// (`fileRegex` in the worker), so a temp file orphaned by an interrupted write +/// is ignored and overwritten by the next write to the same target rather than +/// loaded as a store. +Future _writeFileAtomic(File file, String contents) async { + final tmpFile = File('${file.path}.tmp'); + + try { + final raf = await tmpFile.open(mode: FileMode.writeOnly); + try { + await raf.writeString(contents); + // Flush the data to disk before the rename so the rename can't expose an + // empty/partial file after a power loss. + await raf.flush(); + } finally { + await raf.close(); + } + + await tmpFile.rename(file.path); + } catch (_) { + // Best-effort cleanup for normal write failures. This will not run after a + // process crash; the next write to this target overwrites the stale temp. + try { + await tmpFile.delete(); + } on FileSystemException { + // Preserve the original write error if cleanup also fails. + } + rethrow; + } +} + class FileDataStoreConfig extends DataStoreConfig { FileDataStoreConfig( super.name, { @@ -34,7 +73,8 @@ class FileDataStoreConfig extends DataStoreConfig { persist: (store) async { final value = jsonEncode(store.extract()); - await file.writeAsString( + await _writeFileAtomic( + file, encrypted ? encrypter.encrypt(value) : value, ); }, @@ -61,7 +101,8 @@ class FileDataStoreResolverConfig extends DataStoreResolverConfig { return null; } }, - persist: (store) => file.writeAsString(jsonEncode(store.inspect())), + persist: (store) => + _writeFileAtomic(file, jsonEncode(store.inspect())), delete: () async { try { await file.delete(); diff --git a/test/native/file_persistor_test.dart b/test/native/file_persistor_test.dart index fdfbb17..5cf223e 100644 --- a/test/native/file_persistor_test.dart +++ b/test/native/file_persistor_test.dart @@ -1,7 +1,13 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:encrypt/encrypt.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:loon/loon.dart'; +import 'package:loon/persistor/data_store_encrypter.dart'; import 'package:loon/persistor/file_persistor/file_persistor.dart'; +import 'package:loon/persistor/file_persistor/file_persistor_worker.dart' + show fileRegex; // ignore: depend_on_referenced_packages import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; @@ -14,7 +20,7 @@ late Directory testDirectory; class MockPathProvider extends Fake with MockPlatformInterfaceMixin implements PathProviderPlatform { - getApplicationDocumentsDirectory() { + Directory getApplicationDocumentsDirectory() { return testDirectory; } @@ -53,4 +59,81 @@ void main() { factory: FilePersistor.new, ); }); + + group('FilePersistor durability', () { + final loonDir = Directory('${testDirectory.path}/loon'); + + // FlutterSecureStorage does not work in tests, so supply a fixed encrypter. + final encrypter = DataStoreEncrypter( + Encrypter(AES(Key.fromSecureRandom(32), mode: AESMode.cbc)), + ); + + tearDown(() async { + await Loon.clearAll(); + }); + + test('Persists atomically, leaving no .tmp files behind', () async { + final synced = Completer(); + final persistor = FilePersistor( + persistenceThrottle: const Duration(milliseconds: 1), + encrypter: encrypter, + onSync: () { + if (!synced.isCompleted) synced.complete(); + }, + ); + Loon.configure(persistor: persistor); + + Loon.collection('users').doc('1').create({'name': 'User 1'}); + await synced.future; + + expect(await File('${loonDir.path}/__store__.json').exists(), true); + + final tmpFiles = loonDir + .listSync() + .whereType() + .where((file) => file.path.endsWith('.tmp')) + .toList(); + expect(tmpFiles, isEmpty); + }); + + test('Data store file listing ignores orphaned .tmp files', () { + // A temp file orphaned by an interrupted atomic write must not be picked + // up as a data store on the next startup. + expect(fileRegex.hasMatch('users.json'), true); + expect(fileRegex.hasMatch('users.encrypted.json'), true); + expect(fileRegex.hasMatch('users.json.tmp'), false); + expect(fileRegex.hasMatch('__store__.json.tmp'), false); + }); + + test('Overwrites orphaned .tmp files when rewriting their targets', + () async { + final orphanedStoreTmp = File('${loonDir.path}/custom_store.json.tmp'); + final orphanedResolverTmp = File('${loonDir.path}/__resolver__.json.tmp'); + await orphanedStoreTmp.writeAsString('partial store write'); + await orphanedResolverTmp.writeAsString('partial resolver write'); + + final synced = Completer(); + Loon.configure( + persistor: FilePersistor( + persistenceThrottle: const Duration(milliseconds: 1), + encrypter: encrypter, + onSync: () { + if (!synced.isCompleted) synced.complete(); + }, + ), + ); + + final users = Loon.collection( + 'users', + persistorSettings: PersistorSettings( + key: Persistor.key('custom_store'), + ), + ); + users.doc('1').create({'name': 'User 1'}); + await synced.future; + + expect(await orphanedStoreTmp.exists(), false); + expect(await orphanedResolverTmp.exists(), false); + }); + }); }