Geocoder::CacheStore::Generic.new(Rails.cache, {})) does not support expiration

[Saoma1]

Hello 👋

I've encountered an issue when using Geocoder::CacheStore::Generic.new(Rails.cache, {}) and passing an expiration in the cache_options. The @cache_hit is always true (after the first request). I expected @cache_hit to return false one the first request after the cache expired.

Upon reviewing the implementation here, it appears that expires_in is never passed to Rails.cache, which handles expiration logic.

I came across a related issue, but it focuses mainly on updating the documentation. Would it be possible to implement a proper Geocoder::CacheStore::RailsCache? This would eliminate the need for users to extend or monkey patch the gem.

For context, here is my configuration:

Geocoder.configure(
  timeout: 3, # geocoding service timeout (secs)
  lookup: :nominatim,
  language: :de, # ISO-639 language code
  use_https: true, # use HTTPS for lookup requests? (if supported)
  cache: Geocoder::CacheStore::Generic.new(Rails.cache, {}),
  cache_options: {
    expiration: 2.seconds
  },
  units: :km, # :km for kilometers or :mi for miles
)

I am happy to work on a PR!

Thank you for considering this! 😊

[alexreisner]

Good point. I think this was done because we didn't want to make it seem like expiration is supported by cache back-ends that don't actually support it. But yes, I'm open to a PR For a proper Rails cache solution. (Thanks!)

[DeedleFake]

I just ran into this. The Rails cache in the project I'm working on at work is backed by Redis, but the redirect through the Rails cache causes the expiration config to get silently ignored, and I only found out because there is now gigabytes of data cached in Redis. Worse, the documentation does not at all explain how to configure the cache well. Reading the actual code, it seems that the cache backend is chosen based on the class name of the value given for the cache in the config, which means that if it's not a class exactly named Redis that is not in a module in a module then you get the generic, no expiration cache layer. At the very least the documentation needs to be improved.