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

Dispatch events on cache storage lifecycle #1280

Open
silverbackdan opened this issue May 1, 2020 · 7 comments
Open

Dispatch events on cache storage lifecycle #1280

silverbackdan opened this issue May 1, 2020 · 7 comments
Labels
Level: New Feature 🆕 This item involves the introduction of new functionality.

Comments

@silverbackdan
Copy link
Contributor

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!

@silverbackdan silverbackdan changed the title Dispatch an event with the Binary from the CacheManager when store is called Dispatch an events on cache storage lifecycle May 1, 2020
@dbu dbu changed the title Dispatch an events on cache storage lifecycle Dispatch an event on cache storage lifecycle Oct 6, 2021
@dbu dbu changed the title Dispatch an event on cache storage lifecycle Dispatch events on cache storage lifecycle Oct 6, 2021
@dbu dbu added the Level: New Feature 🆕 This item involves the introduction of new functionality. label Oct 6, 2021
@dbu
Copy link
Member

dbu commented Oct 6, 2021

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 dbu added the Status: Waiting for Information This discussion required additional information before it can be resolved. label Oct 6, 2021
@dbu
Copy link
Member

dbu commented Oct 6, 2021

it looks like somebody did a PR for this once, but then dropped it: #1127

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

@silverbackdan
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link

c33s commented Oct 6, 2021

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
Copy link
Contributor Author

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
Copy link
Member

dbu commented Oct 7, 2021

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

@dbu dbu removed the Status: Waiting for Information This discussion required additional information before it can be resolved. label Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Level: New Feature 🆕 This item involves the introduction of new functionality.
Projects
None yet
Development

No branches or pull requests

3 participants