Skip to content
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

Merged
merged 1 commit into from Feb 26, 2015

Conversation

nemphys
Copy link
Contributor

@nemphys nemphys commented Feb 24, 2015

No description provided.

@nemphys nemphys force-pushed the diskcache-clean branch 2 times, most recently from e8293ba to 65a8aba Compare February 24, 2015 22:08
@nemphys
Copy link
Contributor Author

nemphys commented Feb 24, 2015

PR updated, please check

@@ -293,6 +293,10 @@ public void onResourceReleased(Key cacheKey, EngineResource resource) {
}
}

public void clearDiskCache() {
diskCacheProvider.getDiskCache().clear();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this happen asynchronously?

Copy link
Collaborator

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?

Copy link
Collaborator

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.

Copy link
Collaborator

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...

@FrancoisBlavoet
Copy link
Contributor

My use-case is a music streaming app. We provide a setting allowing users to flush all the app caches.
This is pretty common among music streaming apps in order to let user with limited storage space reclaim some of it.

@sjudd
Copy link
Collaborator

sjudd commented Feb 26, 2015

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() {
Copy link
Collaborator

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.

@sjudd
Copy link
Collaborator

sjudd commented Feb 26, 2015

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!

@nemphys
Copy link
Contributor Author

nemphys commented Feb 26, 2015

PR updated, please check

@nemphys
Copy link
Contributor Author

nemphys commented Feb 26, 2015

Sorry, one more update for the synchronization

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.24%) to 84.7% when pulling f3af6ea on nemphys:diskcache-clean into 34474ac on bumptech:master.

@sjudd
Copy link
Collaborator

sjudd commented Feb 26, 2015

Looks good to me, thanks!

sjudd added a commit that referenced this pull request Feb 26, 2015
New Glide class static methods that allow disk cache & all cache types cleaning.
@sjudd sjudd merged commit 1d3ca05 into bumptech:master Feb 26, 2015
@nemphys nemphys deleted the diskcache-clean branch March 7, 2015 10:27
@sjudd sjudd added this to the 3.6.0 milestone Apr 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants