Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions flutter_cache_manager/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## [Unreleased]

* Awaits cache-info persist in `putFile`, `putFileStream`, and downloads so the stored object has an id before those calls return
* 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
Expand Down
4 changes: 2 additions & 2 deletions flutter_cache_manager/lib/src/cache_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ class CacheManager implements BaseCacheManager {

final file = await _config.fileSystem.createFile(cacheObject.relativePath);
await file.writeAsBytes(fileBytes);
_store.putFile(cacheObject);
await _store.putFile(cacheObject);
return file;
}

Expand Down Expand Up @@ -287,7 +287,7 @@ class CacheManager implements BaseCacheManager {
.map((event) => event)
.pipe(sink);

_store.putFile(cacheObject);
await _store.putFile(cacheObject);
return file;
}

Expand Down
9 changes: 4 additions & 5 deletions flutter_cache_manager/lib/src/web/web_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,10 @@ class WebHelper {
newCacheObject = newCacheObject.copyWith(length: savedBytes);
}

_store.putFile(newCacheObject).then((_) {
if (newCacheObject.relativePath != oldCacheObject.relativePath) {
_removeOldFile(oldCacheObject.relativePath);
}
});
await _store.putFile(newCacheObject);
if (newCacheObject.relativePath != oldCacheObject.relativePath) {
await _removeOldFile(oldCacheObject.relativePath);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function _removeOldFile lacks error handling, can you add that?
This would probably do:

try {
    if (await file.exists()) {
      await file.delete();
    }
  } on FileSystemException {
    // Already deleted (see #184) or not deletable. The cache info no longer
    // points at this path, so there is nothing to recover here.
  }

}

final file = await _store.fileSystem.createFile(
newCacheObject.relativePath,
Expand Down
29 changes: 29 additions & 0 deletions flutter_cache_manager/test/cache_manager_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,35 @@ void main() {
expect(arg.key, fileKey);
expect(arg.url, fileUrl);
});

test('putFile waits for store persist before returning', () async {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I suggest the following, since we try to test persistence without relying on timers:

test('putFile waits for store persist before returning', () async {
  final persisted = Completer<void>();
  final store = MockCacheStore();
  when(store.putFile(any)).thenAnswer((_) => persisted.future);
  final cacheManager = TestCacheManager(createTestConfig(), store: store);
  var returned = false;
  final put = cacheManager.putFile('baseflow.com/test', Uint8List(8))
    ..whenComplete(() => returned = true);
  await pumpEventQueue();
  expect(returned, isFalse, reason: 'putFile returned before the store persisted');
  persisted.complete();
  await put;
});

var persistDone = false;
var store = MockCacheStore();
when(store.putFile(any)).thenAnswer((_) async {
await Future<void>.delayed(const Duration(milliseconds: 40));
persistDone = true;
});

var cacheManager = TestCacheManager(createTestConfig(), store: store);
await cacheManager.putFile('baseflow.com/test', Uint8List(8));
expect(persistDone, isTrue);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a test that actually tests if the file removal works:

test('removeFile deletes the entry right after putFile', () async {
  final repo = JsonCacheInfoRepository.withFile(
    await JsonRepoHelpers.createDatabaseFile(),
  );
  final config = Config(
    'test',
    fileSystem: TestFileSystem(),
    repo: repo,
    fileService: MockFileService(),
  );
  final cacheManager = TestCacheManager(config);
  const url = 'baseflow.com/test';
  final file = await cacheManager.putFile(url, Uint8List(8), fileExtension: 'jpg');
  await cacheManager.removeFile(url);
  await pumpEventQueue();
  expect(await repo.get(url), isNull);
  expect(await file.exists(), isFalse);
});

test('putFileStream waits for store persist before returning', () async {
var persistDone = false;
var store = MockCacheStore();
when(store.putFile(any)).thenAnswer((_) async {
await Future<void>.delayed(const Duration(milliseconds: 40));
persistDone = true;
});

var cacheManager = TestCacheManager(createTestConfig(), store: store);
await cacheManager.putFileStream(
'baseflow.com/test',
Stream<List<int>>.value([1, 2, 3]),
);
expect(persistDone, isTrue);
});
});

group('Testing remove files from cache', () {
Expand Down
34 changes: 34 additions & 0 deletions flutter_cache_manager/test/web_helper_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,40 @@ void main() {
verify(store.putFile(any)).called(1);
});

test('downloadFile waits for persist before yielding FileInfo', () async {
const imageUrl = 'baseflow.com/testimage';

var persistDone = false;
var config = createTestConfig();
var store = _createStore(config);
when(store.putFile(any)).thenAnswer((_) async {
await Future<void>.delayed(const Duration(milliseconds: 40));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here prevent using a timer and go with the Completer instead.

persistDone = true;
});

final fileService = MockFileService();
when(fileService.get(imageUrl, headers: anyNamed('headers'))).thenAnswer((
_,
) {
return Future.value(
MockFileFetcherResponse(
Stream.value([0, 1, 2, 3, 4, 5]),
6,
'testv1',
'.jpg',
200,
DateTime.now(),
),
);
});

var webHelper = WebHelper(store, fileService);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var webHelper = WebHelper(store, fileService);
final webHelper = WebHelper(store, fileService);

await webHelper
.downloadFile(imageUrl)
.firstWhere((r) => r is FileInfo, orElse: null);
expect(persistDone, isTrue);
});

test('File should be removed if extension changed', () async {
const imageUrl = 'baseflow.com/testimage';
var imageName = 'image.png';
Expand Down