Skip to content

Conversation

@msschl
Copy link

@msschl msschl commented Mar 18, 2018

Details

  • Added a feature which allows users to specify a cache store other than the apps default cache.
  • The cache key should not only be serialized, it should also be hashed. Some cache stores have a limited key length, for example the database store has a key length of 255 characters. The md5 hash function seems to be pretty good for this purpose, since it hashes fast and produces a string of 32 characters.
@willdurand willdurand requested a review from mikebronner March 18, 2018 14:31
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.143% when pulling 3cc763a on msschl:feature_SpecifyingCacheStoreToUse into 8487330 on geocoder-php:master.

@mikebronner
Copy link
Member

Hi @msschl, thanks for this PR. This is a good idea. However, I would like to implement it the same way as I have in the laravel-model-caching package:

  • use sha1 for the keys
  • add hash-collision-prevention logic

I can go ahead and implement that on top of your PR, if you like, but I won't be able to get to it until next week.

Thanks for submitting this!

@msschl
Copy link
Author

msschl commented Mar 18, 2018

@mikebronner Thank you very much! That would be nice.

@mikebronner mikebronner changed the base branch from master to feature/improved-caching March 25, 2018 21:53
@mikebronner mikebronner merged commit b9dc865 into geocoder-php:feature/improved-caching Mar 25, 2018
@mikebronner
Copy link
Member

This is now available in release 4.0.7! Thanks for this! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants