diff --git a/.gitignore b/.gitignore index f894cace..c4b9f885 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ devtools_options.yaml .fvmrc .fvm/ +AGENTS.md # Environment files ios/Flutter/Dart-Defines.xcconfig diff --git a/flutter_cache_manager/CHANGELOG.md b/flutter_cache_manager/CHANGELOG.md index eb4d067e..97c1a155 100644 --- a/flutter_cache_manager/CHANGELOG.md +++ b/flutter_cache_manager/CHANGELOG.md @@ -1,5 +1,6 @@ ## [Unreleased] +* Fixes `JsonCacheInfoRepository` losing metadata when the app exits within 3 seconds of a cache change by writing through promptly with serialized, atomic file writes ([#491](https://github.com/Baseflow/flutter_cache_manager/issues/491)) * Modernizes GitHub Actions CI (combined quality job, pinned Flutter 3.44.4, Dependabot for actions) * Updates example Android project to AGP 9.0.1 / Gradle 9.1 / Kotlin 2.3.20 * Migrates example Android app to built-in Kotlin diff --git a/flutter_cache_manager/lib/src/storage/cache_info_repositories/json_cache_info_repository.dart b/flutter_cache_manager/lib/src/storage/cache_info_repositories/json_cache_info_repository.dart index e8ee293f..5d24b24a 100644 --- a/flutter_cache_manager/lib/src/storage/cache_info_repositories/json_cache_info_repository.dart +++ b/flutter_cache_manager/lib/src/storage/cache_info_repositories/json_cache_info_repository.dart @@ -1,9 +1,9 @@ -import 'dart:async'; import 'dart:convert'; import 'dart:io'; import 'dart:math'; import 'package:collection/collection.dart'; +import 'package:file/file.dart' as pf; import 'package:flutter/widgets.dart'; import 'package:flutter_cache_manager/src/storage/cache_info_repositories/cache_info_repository.dart'; import 'package:flutter_cache_manager/src/storage/cache_info_repositories/helper_methods.dart'; @@ -31,6 +31,9 @@ class JsonCacheInfoRepository extends CacheInfoRepository final Map _cacheObjects = {}; final Map> _jsonCache = {}; + bool _dirty = false; + Future _writeQueue = Future.value(); + @override Future open() async { if (!shouldOpenOnNewConnection()) { @@ -77,7 +80,7 @@ class JsonCacheInfoRepository extends CacheInfoRepository if (cacheObject.id == null) { throw ArgumentError('Updated objects should have an existing id.'); } - _put(cacheObject, setTouchedToNow); + await _put(cacheObject, setTouchedToNow); return 1; } @@ -104,13 +107,10 @@ class JsonCacheInfoRepository extends CacheInfoRepository @override Future delete(int id) async { - final cacheObject = _cacheObjects.values.firstWhereOrNull( - (element) => element.id == id, - ); - if (cacheObject == null) { + if (!_removeById(id)) { return 0; } - _remove(cacheObject); + await _schedulePersist(); return 1; } @@ -118,18 +118,23 @@ class JsonCacheInfoRepository extends CacheInfoRepository Future deleteAll(Iterable ids) async { var deleted = 0; for (final id in ids) { - deleted += await delete(id); + if (_removeById(id)) deleted++; + } + if (deleted > 0) { + await _schedulePersist(); } return deleted; } @override Future close() async { - if (!shouldClose()) { - return false; + final shouldCloseRepo = shouldClose(); + if (_dirty) { + await _schedulePersist(); + } else { + await _writeQueue; } - await _saveFile(); - return true; + return shouldCloseRepo; } Future _readFile(File file) async { @@ -162,33 +167,85 @@ class JsonCacheInfoRepository extends CacheInfoRepository } } - CacheObject _put(CacheObject cacheObject, bool setTouchedToNow) { + Future _put( + CacheObject cacheObject, + bool setTouchedToNow, + ) async { final map = cacheObject.toMap(setTouchedToNow: setTouchedToNow); _jsonCache[cacheObject.id!] = map; final updatedCacheObject = CacheObject.fromMap(map); _cacheObjects[cacheObject.key] = updatedCacheObject; - _cacheUpdated(); + await _schedulePersist(); return updatedCacheObject; } - void _remove(CacheObject cacheObject) { + bool _removeById(int id) { + final cacheObject = _cacheObjects.values.firstWhereOrNull( + (element) => element.id == id, + ); + if (cacheObject == null) { + return false; + } _cacheObjects.remove(cacheObject.key); _jsonCache.remove(cacheObject.id); - _cacheUpdated(); + return true; } - void _cacheUpdated() { - timer?.cancel(); - timer = Timer(timerDuration, _saveFile); + /// Queues a write of the current cache info. + /// + /// The returned future completes when the changes are on disk. Changes made + /// while a write is in progress are written by that same write or the one + /// directly after it, so a burst of changes doesn't cause a write per change. + Future _schedulePersist() { + _dirty = true; + _writeQueue = _writeQueue.then((_) => _flushIfDirty()); + return _writeQueue; } - Timer? timer; - Duration timerDuration = const Duration(seconds: 3); + Future _flushIfDirty() async { + while (_dirty) { + _dirty = false; + try { + await _saveFile(); + } on Object catch (e, stacktrace) { + // Keep the changes dirty so a later change or close retries the write, + // but stop here to avoid retrying a persistent failure in a loop. + _dirty = true; + FlutterError.reportError( + FlutterErrorDetails( + exception: e, + stack: stacktrace, + library: 'flutter cache manager', + context: ErrorDescription( + 'Thrown when writing the file containing cache info. ' + 'The cache info could not be persisted and may be lost when the ' + 'app is closed.', + ), + ), + ); + return; + } + } + } Future _saveFile() async { - timer?.cancel(); - timer = null; - await _file!.writeAsString(jsonEncode(_jsonCache.values.toList())); + final file = await _getFile(); + final content = jsonEncode(_jsonCache.values.toList()); + final tempFile = _createSiblingFile('${file.path}.tmp'); + await tempFile.writeAsString(content, flush: true); + await tempFile.rename(file.path); + } + + /// Creates a file next to [_file] on the same file system. + /// + /// [_file] can be backed by an alternative [pf.FileSystem], in which case a + /// plain [File] would resolve against the local file system instead. + File _createSiblingFile(String siblingPath) { + final file = _file!; + if (file is pf.File) { + return file.fileSystem.file(siblingPath); + } + return File(siblingPath); } @override @@ -197,6 +254,10 @@ class JsonCacheInfoRepository extends CacheInfoRepository if (await file.exists()) { await file.delete(); } + final tempFile = _createSiblingFile('${file.path}.tmp'); + if (await tempFile.exists()) { + await tempFile.delete(); + } } @override @@ -206,18 +267,20 @@ class JsonCacheInfoRepository extends CacheInfoRepository } Future _getFile() async { - if (_file == null) { - if (path != null) { - directory = File(path!).parent; - } else { - directory ??= await getApplicationSupportDirectory(); - } - await directory!.create(recursive: true); - if (path == null || !path!.endsWith('.json')) { - path = join(directory!.path, '$databaseName.json'); - } - _file = File(path!); + if (_file != null) { + return _file!; + } + + if (path != null) { + directory = File(path!).parent; + } else { + directory ??= await getApplicationSupportDirectory(); + } + await directory!.create(recursive: true); + if (path == null || !path!.endsWith('.json')) { + path = join(directory!.path, '$databaseName.json'); } + _file = File(path!); return _file!; } } diff --git a/flutter_cache_manager/test/helpers/json_repo_helpers.dart b/flutter_cache_manager/test/helpers/json_repo_helpers.dart index d31ef552..c725618f 100644 --- a/flutter_cache_manager/test/helpers/json_repo_helpers.dart +++ b/flutter_cache_manager/test/helpers/json_repo_helpers.dart @@ -18,13 +18,17 @@ class JsonRepoHelpers { static Future createRepository({ bool open = true, }) async { - var directory = await _createDirectory(); - var file = await _createFile(directory); + var file = await createDatabaseFile(); var repository = JsonCacheInfoRepository.withFile(file); if (open) await repository.open(); return repository; } + static Future createDatabaseFile() async { + var directory = await _createDirectory(); + return _createFile(directory); + } + static Future _createDirectory() async { var testDir = await MemoryFileSystem().systemTempDirectory.createTemp( 'testFolder', diff --git a/flutter_cache_manager/test/repositories/json_file_repository_test.dart b/flutter_cache_manager/test/repositories/json_file_repository_test.dart index 941738f5..1daf59b6 100644 --- a/flutter_cache_manager/test/repositories/json_file_repository_test.dart +++ b/flutter_cache_manager/test/repositories/json_file_repository_test.dart @@ -1,6 +1,8 @@ +import 'dart:convert'; import 'dart:io'; import 'package:collection/collection.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter_cache_manager/src/storage/cache_info_repositories/json_cache_info_repository.dart'; import 'package:flutter_cache_manager/src/storage/cache_object.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -230,6 +232,56 @@ void main() { JsonRepoHelpers.startCacheObjects.length + 1, ); }); + + test('Changes are persisted when the mutating future completes', () async { + final file = await JsonRepoHelpers.createDatabaseFile(); + final repo = JsonCacheInfoRepository.withFile(file); + await repo.open(); + await repo.insert(JsonRepoHelpers.extraCacheObject); + + // New instance reads from disk without closing the first repository. + final repo2 = JsonCacheInfoRepository.withFile(file); + await repo2.open(); + final allObjects = await repo2.getAllObjects(); + expect(allObjects.length, JsonRepoHelpers.startCacheObjects.length + 1); + }); + + test('Persist does not leave a temp file', () async { + final file = await JsonRepoHelpers.createDatabaseFile(); + final repo = JsonCacheInfoRepository.withFile(file); + await repo.open(); + await repo.insert(JsonRepoHelpers.extraCacheObject); + + final tempFile = file.fileSystem.file('${file.path}.tmp'); + expect(await tempFile.exists(), false); + expect(await file.exists(), true); + expect(jsonDecode(await file.readAsString()), isA>()); + }); + + test('A failing write is reported and retried on close', () async { + final file = await JsonRepoHelpers.createDatabaseFile(); + final repo = JsonCacheInfoRepository.withFile(file); + await repo.open(); + + // Writing is impossible while the containing directory is gone. + await file.parent.delete(recursive: true); + + final errors = []; + final originalOnError = FlutterError.onError; + FlutterError.onError = errors.add; + await repo.insert(JsonRepoHelpers.extraCacheObject); + FlutterError.onError = originalOnError; + + expect(errors, hasLength(1)); + + await file.parent.create(recursive: true); + expect(await repo.close(), true); + + final repo2 = JsonCacheInfoRepository.withFile(file); + await repo2.open(); + final allObjects = await repo2.getAllObjects(); + expect(allObjects.length, JsonRepoHelpers.startCacheObjects.length + 1); + }); }); }