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

Cache Manager - separate concern #5

Closed
FastNinja opened this issue Dec 26, 2017 · 6 comments
Closed

Cache Manager - separate concern #5

FastNinja opened this issue Dec 26, 2017 · 6 comments

Comments

@FastNinja
Copy link

Two thoughts crossed my mind:

  1. I really couldn't find a place where you eliminate/delete images from cache. it looks like it will grow infinitely.
  2. I actually need similar caching capabilities for other assets- like documents and even maybe JSON data

These leads me to think that your Cache Manager actually will be helpful as a separate package.
Caching data on a mobile phone is very common practice/concern so I can see it is much more widely used as CachedNetworkImageProvider

Proposal: extract Cache Manager as a separate package. Add ability to eliminate entities. Make it generic so it is not coupled with HttpHeaders or any other concerns.

@renefloor
Copy link
Collaborator

About your points

  1. Cleaning the cache is indeed my number 1 priority at the moment, but I was thinking about how best to do that. For now I only used this cachemanager for images that I use all the time and that almost never change. This is also the reason that I didn't yet publish it as a separate package.

  2. Like we already discussed in Throws exception when content-type is not image #4 it is very well possible to eliminate the header for content type. I will do that for sure when I publish it separately. About HttpHeaders in general, I will keep the check for 'cache-control', but now I download the file every time when it has no information about that, I can change that to never download again or set that to a default length.

@FastNinja
Copy link
Author

@renefloor I think my point was nothing to do with header but more to the point that CacheManager sounds like pretty useful package. any data/documents can be cached.
Elimination strategy can be configured or passed as a custom class:

  1. for example cache manager can have a size of only 2000 entities and it will remove the most not used one, or the oldest etc..

@renefloor
Copy link
Collaborator

Yes I understand, I also think it is very useful as a separate package. Using a count is a good idea. I will probably do that in combination with a maximum age. I will make them both configurable by the user, but I think a default of 2000 items is a bit much.

@renefloor
Copy link
Collaborator

I just made the needed changes in separate feature branch. Would like to hear your opinion.
https://github.com/renefloor/flutter_cached_network_image/tree/feature/generic_cache_manager.
I made one commit for the cleaning of the cache: e1711d0
And one to remove the forced check of the image content type: 2d0343c

@FastNinja
Copy link
Author

FastNinja commented Dec 27, 2017

@renefloor looks good, but can be even better.
I created a PR so it easier to discuss
#6

@renefloor
Copy link
Collaborator

@FastNinja I just moved the cache manager to its own repo and released a first version of it.
https://github.com/renefloor/flutter_cache_manager

Let's move the discussion there. I already refactored some bit and opened an issue for error reporting. I don't have much time coming period, so if you have any breaking issues, please make a pull request for it.

@expilu expilu mentioned this issue Jul 23, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants