Allow write_options to be specified for FaradayMiddleware::Caching
#155
Conversation
`FaradayMiddleware::Caching` is configured with a cache with responds to `#read`, `#fetch` and `#write`. Usually, this will be an instance of a class which conforms to the abstract cache store class [`ActiveSupport::Cache::Store`](http://api.rubyonrails.org/classes/ActiveSupport/Cache/Store.html). In this pattern, the third argument of `#write` is an optional hash of options - for example, you can specify an `expires_in` to set how long until the cache key expires. Cache stores may support a range of options. This allows you, when configuring the middleware, to specify a set of `write_options` which will be passed in as this third argument when writing to the cache. The alternative to this would be passing in some kind of wrapper around your chosen cache store which does this for you, but it feels nicer for the middleware to do it.
| @@ -48,7 +51,11 @@ def call(env) | |||
| response = @app.call(env) | |||
|
|
|||
| if CACHEABLE_STATUS_CODES.include?(response.status) | |||
| cache.write(key, response) | |||
| if @options[:write_options] | |||
There was a problem hiding this comment.
I've added this if statement with backwards compatibility in mind - if people are using ActiveSupport::Cache::Store-confirming objects, this will work fine, but people may not be, especially in tests, so we shouldn't start passing options if the user hasn't set some.
| @@ -27,6 +27,9 @@ class Caching < Faraday::Middleware | |||
| # :ignore_params - String name or Array names of query params | |||
| # that should be ignored when forming the cache | |||
| # key (default: []). | |||
| # :write_options - Hash of settings that should be passed as the third | |||
There was a problem hiding this comment.
I did consider other names, but this felt like the best one, being the most explicit about where exactly these options go
|
@iMacTia Can you take a look at this? Looks like you've been active on this project most recently. Thanks ❤️ |
There was a problem hiding this comment.
Thanks @timrogers for the PR.
I like the idea of this new feature and I appreciate the focus on backward compatibility.
I marked a quick refactor to make code a little DRYer 😄
| cache.write(key, response_env.response, @options[:write_options]) | ||
| else | ||
| cache.write(key, response_env.response) | ||
| end |
There was a problem hiding this comment.
Lines 90-95 are exactly the same as lines 53-58, the only difference is the variable they use: response_env.response for the former, response for the latter (but they should be exactly the same).
Please refactor these lines into a method
|
@iMacTia Done! |
|
Great job, thanks @timrogers 👍 |
|
@iMacTia What is the planned release schedule for 0.12? |
FaradayMiddleware::Cachingis configured with a cache with responds to#read,#fetchand#write. Usually, this will be an instance of a class which conforms to the abstract cache store classActiveSupport::Cache::Store.In this pattern, the third argument of
#writeis an optional hash of options - for example, you can specify anexpires_into set how long until the cache key expires (this is the case where we're interested for a gem I maintain, https://github.com/ejholmes/restforce). Cache stores may support a range of options.This allows you, when configuring the middleware, to specify a set of
write_optionswhich will be passed in as this third argument when writing to the cache.The alternative to this would be passing in some kind of wrapper around your chosen cache store which does this for you, but it feels nicer for the middleware to do it.