search

liip / LiipImagineBundle

Symfony Bundle to assist in image manipulation using the imagine library
http://liip.ch
MIT License
1.67k stars 377 forks source link

Dispatch events on cache storage lifecycle #1280

Open silverbackdan opened 4 years ago

silverbackdan commented 4 years ago

Is your feature request related to a problem? Please describe. In order to save file information about a cached image file, I need to have access to the file

Describe the solution you'd like I think as this information will not change from the tie where it is being stored, a pretty simple PR would be to dispatch an event with the Binary when the CacheManager is storing a file and another when the cached file is being removed.

Describe alternatives you've considered I've thought about altering the interfaces so that the file binary can be retrieved from the configured storage, but it seems like a lot of work and big knock-on consequences. For now I am having to extend or decorate CacheManager in my own project to hook into these points in the caching lifecycle.

Additional context If you'd consider this I'll try to find the time to help by writing up a PR, but would rather not as I see this issue has been raised before (about getting file information about stored cache files) and I could not see a solution yet. I'm not sure what decisions any internal discussions may have resulted in.

Thank you for this very useful bundle!

dbu commented 3 years ago

i am looking at older issues. is this one still relevant to you?

if it is, where would you expect the event to happen? on CacheManager::store? for my understanding, could you please elaborate a bit what you want to do with the cached image?

dbu commented 3 years ago

it looks like somebody did a PR for this once, but then dropped it: https://github.com/liip/LiipImagineBundle/pull/1127

do you want to redo this code? i am trying to be active on this bundle to keep it maintained again ;-)

silverbackdan commented 3 years ago

Hi @dbu thanks for picking this up - currently I am decorating the cache manager: https://github.com/components-web-app/api-components-bundle/blob/master/src/Imagine/CacheManager.php

I am curious as to why the PR was dropped... it seems interesting to have done that and just close it. The difference here is that is just events surrounding the storing of a cached file and not removing it - I could extend it.

Do you have thoughts on if a pre-store and post-store (and pre-remove and post-remove) are all necessary? In my decorated manager I just send an event on pre-store and pre-remove.

silverbackdan commented 3 years ago

I also do not add the resolver into the event, for my use it wasn't necessary but perhaps if it's added to this bundle that'd be good too - I'm happy to create a PR that would be best suited for the bundle. My decorated class is really specific for my needs I think.

c33s commented 3 years ago

if a pre-store and post-store (and pre-remove and post-remove) are all necessary?

all these events can be usefull in some situations. often you come in the situation that you would exactly this one event but it was removed/not added.

silverbackdan commented 3 years ago

OK sure, I'll add pre and post events similar to the PR that was abandoned but for the remove event too. I'll see if I can get time this weekend.

dbu commented 3 years ago

I am curious as to why the PR was dropped

from the time between opening and closing, i assume that the author deleted the fork at some point, after getting no reaction :-|

as you build the event class anyways, i'd do pre and post. maybe something wants to scan the safed image for something, which could be a reason for post.

great if you can work on it! i think you can refactor the legacy-compatible dispatch method to simply accept any event class instead of the specific event...