diff --git a/flutter_cache_manager/CHANGELOG.md b/flutter_cache_manager/CHANGELOG.md index 9513fb80..3e2cbf95 100644 --- a/flutter_cache_manager/CHANGELOG.md +++ b/flutter_cache_manager/CHANGELOG.md @@ -1,3 +1,7 @@ +## [3.4.6] - 2026-10-22 + +* Fixes the cache cleanup deleting the cache info before the cached files were removed. When the app was killed in between, files stayed on disk with nothing referencing them. The cache info of a file that could not be removed is now kept, so the next cleanup retries it. + ## [3.4.5] - 2026-09-18 * Prevents image resize decoding from completing more than once and propagates decoding errors ([#400](https://github.com/Baseflow/flutter_cache_manager/issues/400)). diff --git a/flutter_cache_manager/lib/src/cache_store.dart b/flutter_cache_manager/lib/src/cache_store.dart index 44ffbfeb..4d2681e4 100644 --- a/flutter_cache_manager/lib/src/cache_store.dart +++ b/flutter_cache_manager/lib/src/cache_store.dart @@ -146,17 +146,37 @@ class CacheStore { final toRemove = []; final provider = await _cacheInfoRepository; + final removals = >[]; + + Future remove(CacheObject cacheObject) async { + try { + await _removeCachedFile(cacheObject, toRemove); + } catch (_) { + // Keep the cache info of a file that could not be removed, so the next + // cleanup tries again instead of leaving the file behind. + toRemove.remove(cacheObject.id); + rethrow; + } + } + final overCapacity = await provider.getObjectsOverCapacity(_capacity); for (final cacheObject in overCapacity) { - _removeCachedFile(cacheObject, toRemove); + removals.add(remove(cacheObject)); } final oldObjects = await provider.getOldObjects(_maxAge); for (final cacheObject in oldObjects) { - _removeCachedFile(cacheObject, toRemove); + removals.add(remove(cacheObject)); } - await provider.deleteAll(toRemove); + try { + await Future.wait(removals); + } finally { + // Only delete the cache info once the files are gone. When the app is + // killed in between, a cache info without a file is cleaned up on the next + // read, but a file without a cache info would never be found again. + await provider.deleteAll(toRemove); + } } Future emptyCache() async { diff --git a/flutter_cache_manager/pubspec.yaml b/flutter_cache_manager/pubspec.yaml index 3f0c9ce3..8456c4b5 100644 --- a/flutter_cache_manager/pubspec.yaml +++ b/flutter_cache_manager/pubspec.yaml @@ -1,6 +1,6 @@ name: flutter_cache_manager description: Generic cache manager for flutter. Saves web files on the storages of the device and saves the cache info using sqflite. -version: 3.4.5 +version: 3.4.6 homepage: https://github.com/Baseflow/flutter_cache_manager topics: - cache diff --git a/flutter_cache_manager/test/cache_store_test.dart b/flutter_cache_manager/test/cache_store_test.dart index 412a35f1..686580a4 100644 --- a/flutter_cache_manager/test/cache_store_test.dart +++ b/flutter_cache_manager/test/cache_store_test.dart @@ -1,8 +1,12 @@ +import 'dart:async'; + import 'package:clock/clock.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_cache_manager/src/cache_store.dart'; import 'package:flutter_cache_manager/src/storage/cache_object.dart'; +import 'package:flutter_cache_manager/src/storage/file_system/file_system.dart' + as cache_file_system; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; @@ -338,6 +342,108 @@ void main() { ).called(1); }); + test( + 'Store should not remove cache info before the cached file is removed', + () async { + var config = createTestConfig(); + var store = CacheStore(config); + store.cleanupRunMinInterval = const Duration(milliseconds: 1); + var file = await config.returnsFile('testimage.png'); + + var cacheObject = CacheObject( + 'baseflow.com/test.png', + relativePath: 'testimage.png', + id: 1, + validTill: clock.now().add(const Duration(days: 7)), + ); + + when( + config.mockRepo.getObjectsOverCapacity(any), + ).thenAnswer((_) => Future.value([])); + when( + config.mockRepo.getOldObjects(any), + ).thenAnswer((_) => Future.value([cacheObject])); + when( + config.mockRepo.get('baseflow.com/test.png'), + ).thenAnswer((_) => Future.value(cacheObject)); + + expect(await store.getFile('baseflow.com/test.png'), isNotNull); + + // From here on, getting to the file takes a while, like a slow disk. + final fileAvailable = Completer(); + store.fileSystem = _DelayedFileSystem( + store.fileSystem, + fileAvailable.future, + ); + + // Give the cleanup time to run. It must not delete the cache info + // while the file is still on disk. + await Future.delayed(const Duration(milliseconds: 50)); + verifyNever(config.mockRepo.deleteAll(any)); + expect(file.existsSync(), isTrue); + + fileAvailable.complete(); + await untilCalled(config.mockRepo.deleteAll(any)); + + expect(file.existsSync(), isFalse); + verify( + config.mockRepo.deleteAll(argThat(contains(cacheObject.id))), + ).called(1); + }, + ); + + test( + 'Store should keep cache info of a file that cannot be removed', + () async { + final fileSystem = _FailingDeleteFileSystem('broken.png'); + final config = createTestConfig(); + final store = CacheStore(config); + store.cleanupRunMinInterval = const Duration(milliseconds: 1); + store.fileSystem = fileSystem; + await fileSystem.createFileWithData('ok.png'); + await fileSystem.createFileWithData('broken.png'); + + final okObject = CacheObject( + 'baseflow.com/ok.png', + relativePath: 'ok.png', + id: 1, + validTill: clock.now().add(const Duration(days: 7)), + ); + final brokenObject = CacheObject( + 'baseflow.com/broken.png', + relativePath: 'broken.png', + id: 2, + validTill: clock.now().add(const Duration(days: 7)), + ); + + when( + config.mockRepo.getObjectsOverCapacity(any), + ).thenAnswer((_) => Future.value([])); + when( + config.mockRepo.getOldObjects(any), + ).thenAnswer((_) => Future.value([okObject, brokenObject])); + when( + config.mockRepo.get('baseflow.com/ok.png'), + ).thenAnswer((_) => Future.value(okObject)); + + // The failed removal is still reported as an error from the cleanup. + final errors = []; + await runZonedGuarded(() async { + expect(await store.getFile('baseflow.com/ok.png'), isNotNull); + await untilCalled(config.mockRepo.deleteAll(any)); + await Future.delayed(const Duration(milliseconds: 5)); + }, (error, stack) => errors.add(error)); + + expect(errors, hasLength(1)); + expect(errors.single, isA()); + verify( + config.mockRepo.deleteAll( + argThat(allOf(contains(okObject.id), isNot(contains(2)))), + ), + ).called(1); + }, + ); + test('Store should recheck cache info when file is removed', () async { var config = createTestConfig(); var store = CacheStore(config); @@ -464,3 +570,41 @@ Future createDir() async { final fileSystem = MemoryFileSystem(); return fileSystem.systemTempDirectory.createTemp('test'); } + +/// A file system that only hands out files once [_available] has completed. +class _DelayedFileSystem extends cache_file_system.FileSystem { + _DelayedFileSystem(this._inner, this._available); + + final cache_file_system.FileSystem _inner; + final Future _available; + + @override + Future createFile(String name) async { + await _available; + return _inner.createFile(name); + } +} + +/// A file system in which deleting the file called [undeletableName] fails. +class _FailingDeleteFileSystem extends cache_file_system.FileSystem { + _FailingDeleteFileSystem(String undeletableName) + : _fileSystem = MemoryFileSystem( + opHandle: (path, operation) { + if (operation == FileSystemOp.delete && + path.endsWith(undeletableName)) { + throw FileSystemException('Cannot delete', path); + } + }, + ); + + final MemoryFileSystem _fileSystem; + late final Directory _directory = _fileSystem.systemTempDirectory + .createTempSync('test'); + + @override + Future createFile(String name) async => _directory.childFile(name); + + Future createFileWithData(String name) async { + await (await createFile(name)).writeAsBytes([1, 3]); + } +}