search

craue / CraueConfigBundle

Database-stored settings made available via a service for your Symfony project.
MIT License
173 stars 35 forks source link

[Feature] Cache queries results #32

Closed pesseyjulien closed 7 years ago

pesseyjulien commented 7 years ago

Hi,

Thanks for this great bundle.

I think it would be even more awesome if we could cache the results of the doctrine queries (using redis for example), and of course, invalidate when the value changes.

What do you think ?

-Julien

craue commented 7 years ago

At first, the idea sounds good, but I'm not sure if it can be implemented in a safe way. If settings are modified by the app, cache values could be invalidated properly. But modifying them by e.g. Doctrine migrations would lead to inconsistency between cache and database.

pesseyjulien commented 7 years ago

I agree, but what if we empty the cache in this particular case ?

craue commented 7 years ago

How? A migration was only one example.

pesseyjulien commented 7 years ago

Sorry for the delay of my answer.

If we relay on a third-party to cache, eg Redis, then we can delete every keys in case of a migration or something similar. Or a least warn the user.

In case of an update of an existing setting, then we can just delete the corresponding redis key.

I'm mentioning the feature because the LexikTranslationBundle is doing something similar to your bundle, but with translations, and they are using a cache system.

Maybe it could be an option, deactivated by default ?

craue commented 7 years ago

How about #35?

pesseyjulien commented 7 years ago

Nice ! Thanks !

So when we do $this->get('craue_config')->set($index, $data), the cache is automatically cleared for this index or we need to do it ?

craue commented 7 years ago

This will be done by the bundle, yes. Feel free to test it out and let me know about possible improvements for the PR.

pesseyjulien commented 7 years ago

Alright, I will ! Thanks a bunch again !

pesseyjulien commented 7 years ago

I have just tried it using DoctrineCacheBundle and redis as cache provider, and it's working great ! Thanks a bunch for this cool new feature, it will help save a lot of queries ;) When do you think it will be merged to the master branch ?

craue commented 7 years ago

Aaaand merged. 😏

pesseyjulien commented 7 years ago

Thanks again, great feature ! ;)