-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
New Glide class static methods that allow disk cache & all cache types cleaning. #352
Conversation
e8293ba
to
65a8aba
Compare
PR updated, please check |
65a8aba
to
2e1f3ba
Compare
@@ -293,6 +293,10 @@ public void onResourceReleased(Key cacheKey, EngineResource resource) { | |||
} | |||
} | |||
|
|||
public void clearDiskCache() { | |||
diskCacheProvider.getDiskCache().clear(); |
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.
Should this happen asynchronously?
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.
The name implies blocking work. Thing about how would you use it: call clear
then call .with.into
, how would you make clear
finish before checking the cache during processing the request?
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.
I think I'm not 100% clear on the use case. If you're trying to make sure all entries are removed, there's no guarantee (asynchronous or not) that in flight requests won't still complete having loaded data from the cache after this method is called.
Can anyone share a use case or two? What's the goal of clearing the cache?
I'm still inclined to say a version number might be the best way to do this, because it would at least let you provide some guarantees (ie no request will complete with old data if you pass in an incremented disk cache version number to Glide as part of the GlideBuilder). Again though I'm not 100% clear on the use case.
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.
I found a use case: ApplicationManifest.xml > application > @manageSpaceActivity, this attribute replaces the "Clear Data" button with a "Manage Storage" button which starts the above activity. An example I could think of is if a user needs space and sees that the app is huge, pressing "Clear Data" would wipe out all personalization/data. I know there's a "Clear Cache", but a lot of apps don't put their volatile data there for some reason. Anyway, I would put a "Clear image cache" button in my storage manager to allow freeing up space. Weirdly the API for "Clear Data" button was added in API 19, while the attribute existed since API 1...
My use-case is a music streaming app. We provide a setting allowing users to flush all the app caches. |
Thanks for commenting, that makes sense. As long as no one expects this to be a promise as to where requests will or won't complete from, this looks good to me. |
@@ -383,6 +383,13 @@ public void trimMemory(int level) { | |||
} | |||
|
|||
/** | |||
* Clears disk cache. | |||
*/ | |||
public void clearDiskCache() { |
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.
Add an assertion that this is on a background thread (https://github.com/bumptech/glide/blob/master/library/src/main/java/com/bumptech/glide/util/Util.java#L135), and a comment that says it must be called from a background thread.
Thanks for putting this up, if you can add the assertion/comment on the method in Glide and fix the findbugs error, I'd appreciate it! |
2e1f3ba
to
69040b3
Compare
PR updated, please check |
69040b3
to
f3af6ea
Compare
Sorry, one more update for the synchronization |
Looks good to me, thanks! |
New Glide class static methods that allow disk cache & all cache types cleaning.
No description provided.