Await store.putFile so cache info is persisted #492 - #518
Conversation
rickdijk
left a comment
rickdijk
left a comment
There was a problem hiding this comment.
Please address my comments so we can proceed with this fix
|
Added the try/catch on _removeOldFile, switched the persist tests to Completers, and added the removeFile-after-putFile case. |
a0cab87 to
6312786
Compare
968f35c to
a4509a4
Compare
|
Dropped the extra Config import, wrapped the test lines dart format was failing on, rebased onto develop, and noted the breaking persist-error behavior in the PR body and CHANGELOG. |
a4509a4 to
4c82fd8
Compare
putFile, putFileStream, and downloads started persist without waiting. After those calls returned, CacheObject.id could still be null, so removeFile skipped the entry and a process exit could lose the info. Fixes Baseflow#492
Catch FileSystemException when deleting a stale cache file.
Drop the extra Config import, wrap the long test lines, and document that a failed cache-info write now surfaces to the caller.
4c82fd8 to
8a674ca
Compare
rickdijk
left a comment
rickdijk
left a comment
There was a problem hiding this comment.
Please move CHANGELOG to a new version 3.4.4 and update the pubspec.yaml of this package to that version too. Then we can merge
Move the persist-await changelog entry out of 3.4.3 and set the package version.
|
Moved the changelog entry to 3.4.4 and bumped the package version in pubspec.yaml. |
Seems like the formatter still requests fixing. |
|
Ran dart format on the persist-await test. |
2 participants
✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)
Bug fix
putFile,putFileStream, andWebHelperstartstore.putFileand return without waiting. The file is on disk, but the cache-info row (and its id) may not be written yet.That matches #492:
removeFileaftergetFileStreamcan no-op becauseCacheObject.idis still null, and a crash right after download can drop the entry.🆕 What is the new behavior (if this is a feature change)?
Those paths now await persist. After they complete, the store has the object and an id, so a follow-up
removeFileactually deletes it.💥 Does this PR introduce a breaking change?
Yes. If the cache-info repository write fails,
putFile/putFileStreamnow throw and downloads/getFileStreamerror the stream. On develop those calls still returned the file. Bytes may already be on disk.🐛 Recommendations for testing
flutter testinflutter_cache_manager. New cases delaystore.putFileand fail if the caller returns early.📝 Links to relevant issues/docs
Fixes #492
🤔 Checklist before submitting