Skip to content

Commit 9ec5616

Browse files
agrbergclaude
andcommitted
Warn when default cache key format is used with arguments
The default key format ignores method arguments, meaning different arguments return the same cached value. This adds a deprecation warning (once per method) when a cached method is called with arguments but uses the default key format, guiding users to provide a :key_format proc. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2f88816 commit 9ec5616

3 files changed

Lines changed: 38 additions & 9 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ Fetching data from GitHub
135135

136136
#### Default
137137

138-
By default, Cacheable will construct a key in the format `[cache_key || class_name, method_name]` without using method arguments.
138+
By default, Cacheable will construct a key in the format `[cache_key || class_name, method_name]` without using method arguments. If a cached method is called with arguments while using the default key format, Cacheable will emit a warning to stderr since different arguments will return the same cached value. To silence the warning, provide a `:key_format` proc that includes the arguments in the cache key.
139139

140140
If the object responds to `cache_key` its return value will be the first element in the array. `ActiveRecord` provides [`cache_key`](https://api.rubyonrails.org/classes/ActiveRecord/Integration.html#method-i-cache_key) but it can be added to any Ruby object or overwritten. If the object does not respond to it, the name of the class will be used instead. The second element will be the name of the method as a symbol.
141141

lib/cacheable/method_generator.rb

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,16 @@ def create_cacheable_methods(original_method_name, opts = {})
5353
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength
5454

5555
def default_key_format
56-
proc do |target, method_name, _method_args, **_kwargs|
57-
# By default, we omit the _method_args from the cache key because there is no acceptable default behavior
56+
warned = false
57+
58+
proc do |target, method_name, method_args, **kwargs|
59+
if !warned && (!method_args.empty? || !kwargs.empty?)
60+
warn "Cacheable WARNING: '#{method_name}' is using the default key format but was called with " \
61+
'arguments. Arguments are NOT included in the cache key, so different arguments will return ' \
62+
'the same cached value. Provide a :key_format proc to include arguments in the cache key.'
63+
warned = true
64+
end
65+
5866
class_name = (target.is_a?(Module) ? target.name : target.class.name)
5967
cache_key = target.respond_to?(:cache_key) ? target.cache_key : class_name
6068
[cache_key, method_name].compact

spec/cacheable/cacheable_spec.rb

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
arg = 'an argument'
4444
expect(cacheable_object).to receive(cacheable_method_inner).with(arg)
4545

46-
cacheable_object.send(cacheable_method, arg)
46+
expect { cacheable_object.send(cacheable_method, arg) }.to output.to_stderr
4747
end
4848

4949
it 'creates a method that can skip the cache' do
@@ -223,9 +223,28 @@
223223
.to change { described_class.cache_adapter.exist?(key) }.from(false).to(true)
224224
end
225225

226-
it 'does not use the arguments to the method to determine the cache key' do
227-
args = [1]
228-
expect(cacheable_object.cacheable_method_key_format(*args)).to eq([cacheable_method])
226+
it 'does not use positional arguments in the cache key and warns' do
227+
cache_key = nil
228+
expect { cache_key = cacheable_object.cacheable_method_key_format(1) }
229+
.to output(/default key format.*arguments are NOT included/i).to_stderr
230+
expect(cache_key).to eq([cacheable_method])
231+
end
232+
233+
it 'does not use keyword arguments in the cache key and warns' do
234+
cache_key = nil
235+
expect { cache_key = cacheable_object.cacheable_method_key_format(foo: 1) }
236+
.to output(/default key format.*arguments are NOT included/i).to_stderr
237+
expect(cache_key).to eq([cacheable_method])
238+
end
239+
240+
it 'only warns once per method' do
241+
expect { cacheable_object.cacheable_method_key_format(1) }
242+
.to output(/default key format/i).to_stderr
243+
expect { cacheable_object.cacheable_method_key_format(2) }.not_to output.to_stderr
244+
end
245+
246+
it 'does not warn when called without arguments' do
247+
expect { cacheable_object.cacheable_method_key_format }.not_to output.to_stderr
229248
end
230249

231250
it 'uses different keys for different cached values' do
@@ -244,8 +263,10 @@
244263
expect(cacheable_object).to receive(inner_method).with(arg1).once.and_call_original
245264
expect(cacheable_object).to receive(inner_method).with(arg2).once.and_call_original
246265

247-
2.times { expect(cacheable_object.send(cacheable_method, arg1)).to include(arg1) }
248-
2.times { expect(cacheable_object.send(another_cacheable_method, arg2)).to include(arg2) }
266+
expect do
267+
2.times { expect(cacheable_object.send(cacheable_method, arg1)).to include(arg1) }
268+
2.times { expect(cacheable_object.send(another_cacheable_method, arg2)).to include(arg2) }
269+
end.to output.to_stderr
249270
end
250271

251272
it 'uses the value of `cache_key` if the method is defined instead of the class' do

0 commit comments

Comments
 (0)