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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants