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

Need to be able to make it longer than 7 days #9

Closed
FastNinja opened this issue Jan 3, 2018 · 8 comments
Closed

Need to be able to make it longer than 7 days #9

FastNinja opened this issue Jan 3, 2018 · 8 comments

Comments

@FastNinja
Copy link

Hi @renefloor ,

7 days is not long enough. Imagine chat app where people share images.
if person does not open app for 7 days - next time everything will be extremely slow.
I actually do not understand what goals 7 days are achieving - since you have CacheManager that controls its size and removes oldest files automatically.

The easiest solution would be to move that into cache manager - so it will be configurable:

var ageDuration = new Duration(days: 7);

p.s. happy to make PR

@FastNinja
Copy link
Author

sorry this issue might actually belong to cache manager package.

but basically cache manager does that:

By default it removes objects that haven't been used for 30 days.

but CacheEntity does :

var ageDuration = new Duration(days: 7);

it is hard to reason about whether or not the object will be used from the cache.

This is the current state :
Will the object be used from the cache:

  1. Has it been downloaded at least once? Yes
  2. Has it been used in last 30 days Yes

OK I now assume the image will be used from the cache.. well now another rule kicks in:

  1. Has it been used 8 days ago and does not contain cache headers Ooops

@renefloor
Copy link
Collaborator

True, I can make that configurable, but that also gives the feeling that it is a real option. In any case using cache-control headers is better, also because when the validity expires the server can indicate that the file didn't change so you don't have to download it again. It also makes the harder to understand as you also indicate.
So I also say in the readme that it uses the http headers to retrieve the file. That 7 days is only a fallback but shouldn't really be used. I could log a warning when it doesn't have a header.

Do you have a lot of files that don't have a cache-control header? Do you serve them from your own server, or are you depending on someone elses?

@FastNinja
Copy link
Author

all the images are loaded into Firebase Storage. Currently Flutter Firebase package does not allow to set any additional metadata. so they are default.
let me check - these are the headers:

Cache-Control →private, max-age=0
Content-Disposition →inline; filename*=utf-8''08cbd260_d2d0_11e7_ddf0_1723f66f7baf.jpg
Content-Length →362812
Content-Type →application/octet-stream
Date →Wed, 03 Jan 2018 11:30:50 GMT
ETag →"55a0ca4c626c52b644e13aebcf1390db"
Expires →Wed, 03 Jan 2018 11:30:50 GMT

max-age=0 sounds quite bad. does it mean it will never use cached data ?? :(

@renefloor
Copy link
Collaborator

That's not really good no.
It kind of uses the cache. It uses the eTag to get the file after it has been cached. The server should respond with a 304 and that contains hardly any data. So the image is cached and used from the cache, but still it makes an api call every time you use it.
Can't you use something like firebase functions to set the metadata after the upload?

@FastNinja
Copy link
Author

yes good idea - I can potentially trigger a function that will alter the metadata (will find that out).

the proper fix would be create a Pull Request on Flutter firebase package - also happy to do that but waiting for an answer from Flutter team on that.

still would be good to make Cache manager ignore max age and be able to override it..

another option - give ability to inject you own Cache manager into NetworkCacheImageProvider.

as long as it satisfy certain interface that shall work....

I still feel that Cache Manager is playing by server rules. whatever server tells it sort of trust it. however sometimes it is good to have control on a client side...?

@renefloor
Copy link
Collaborator

Hmm.. I am not sure about that. The server is supposed to know for how long a file is valid. For the application it would be guessing when to fetch a file again.
What I might do is to read 'max-age=0' as no data set and use the default value for that.

@FastNinja
Copy link
Author

FastNinja commented Jan 3, 2018 via email

@FastNinja
Copy link
Author

I have fixed images so they have proper max-age value. I think this issue can be closed

@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