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

Add support for 'last-modified' and 'etag' cache headers #40

Closed
advancingu opened this issue Nov 28, 2011 · 5 comments
Closed

Add support for 'last-modified' and 'etag' cache headers #40

advancingu opened this issue Nov 28, 2011 · 5 comments

Comments

@advancingu
Copy link

The current caching implementation of LiipImagineBundle relies on the web server to possess writable disc space to "cache" processed images. I found some of the issues with this approach to be that there is no cache expiry available in case a filter is modified and that the web server might run out of disc space if many images are filtered.

As remedy, I've hacked up an implementation of 'last-modified' and 'etag' cache headers to be served by LiipImagineBundle. This allows for external caching techniques instead of relying on server-side storage. Feel free to merge this patch if of interest.

https://github.com/advancingu/LiipImagineBundle/commit/bd0f8cecead5da7815f04700410a6daecf3f20a5

@stof
Copy link
Contributor

stof commented Nov 28, 2011

You should send a pull request instead of linking to the commit. It would make it easier to provide feedback and then to merge it

@lsmith77
Copy link
Contributor

i am not sure if we really want to add this or at least not like this.
we can add a method into the data loader interface to get metadata on the resource.
the actual handling of setting cache headers should be done else where though imho. either inside a customized controller or via LiipCacheControlBundle.

see also #16
cc @MHeleniak

@ghost
Copy link

ghost commented Nov 28, 2011

In my app I have handled this via another application dedicated for serving photos. Request to pic.example.com/filter_set/path/to/image.jpg is translated to www.example.com/media/cache/filter_set/path/to/image.jpg. In my case I have disabled caching in LiipImagineBundle as this is hadled by app behind pic.example.com.

Maybe it's not ideal (extra request) but it's much easier. @lsmith77 you mentioned in #12 that you done this the same way, right?

Btw. Don't send both ETag and Last-Modified headers. Only one of these is sufficent.

@advancingu
Copy link
Author

@stof I couldn't immediately find out how to request a cherry-pick on Github so I posted the commit. My branch contains some other commits that wouldn't be of interest here.

@lsmith77 Yes, the design is not clean. The cache headers could be integrated into the cache resolver but they would also have to be set by the filter on its response to a non-cached request.

@MHeleniak Thanks for pointing out the headers. I actually realized that with my current code, only eTag should be used since last-modified wouldn't change if a filter configuration was updated.
The idea behind my implementation is that I don't want to handle storing filtered images on my server. I'll leave this to proxies.

@lsmith77
Copy link
Contributor

lsmith77 commented Dec 2, 2011

@advancingu there is nothing special to do on github. simply create a new branch locally from the current master of this repo, cherry-pick your commit and push and open a PR.

"The cache headers could be integrated into the cache resolver but they would also have to be set by the filter on its response to a non-cached request."

This should be entirely possible inside a custom cache resolver.

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

3 participants