Require a for config.cache_store = :file_store

Currently if you do `config.cache_store = :file_store` in an initializer, your app will boot and the cache contents will be stored in a directory called `{}` in your project root. This is unlikely to be the intended outcome.

This PR raises an error if you call `config.cache_store = :file_store` without a path. The correct way to use the default cache path is `config.cache_store = :file_store, "#{config.root}/tmp/cache/"`.

To preserve the current behavior:

```ruby
config.cache_store = :file_store, "{}"
```

Co-authored-by: Ryuta Kamizono <kamipo@gmail.com>
This commit is contained in:
Alex Ghiculescu 2021-02-22 15:02:28 -06:00
parent 90049a4107
commit aedb2a7bae
3 changed files with 14 additions and 2 deletions

@ -63,7 +63,13 @@ def lookup_store(store = nil, *parameters)
case store
when Symbol
options = parameters.extract_options!
retrieve_store_class(store).new(*parameters, **options)
# clean this up once Ruby 2.7 support is dropped
# see https://github.com/rails/rails/pull/41522#discussion_r581186602
if options.empty?
retrieve_store_class(store).new(*parameters)
else
retrieve_store_class(store).new(*parameters, **options)
end
when Array
lookup_store(*store)
when nil

@ -20,7 +20,7 @@ class FileStore < Store
FILEPATH_MAX_SIZE = 900 # max is 1024, plus some room
GITKEEP_FILES = [".gitkeep", ".keep"].freeze
def initialize(cache_path, options = nil)
def initialize(cache_path, **options)
super(options)
@cache_path = cache_path.to_s
end

@ -21,6 +21,12 @@ def test_file_fragment_cache_store
assert_equal "/path/to/cache/directory", store.cache_path
end
def test_file_store_requires_a_path
assert_raises(ArgumentError) do
ActiveSupport::Cache.lookup_store :file_store
end
end
def test_mem_cache_fragment_cache_store
assert_called_with(Dalli::Client, :new, [%w[localhost], {}]) do
store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost"