-
-
Notifications
You must be signed in to change notification settings - Fork 511
Await store.putFile so cache info is persisted #492 #518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -424,6 +424,35 @@ void main() { | |
| expect(arg.key, fileKey); | ||
| expect(arg.url, fileUrl); | ||
| }); | ||
|
|
||
| test('putFile waits for store persist before returning', () async { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
| }); | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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', () { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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)); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here prevent using a timer and go with the |
||||||
| 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); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| 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'; | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function
_removeOldFilelacks error handling, can you add that?This would probably do: