Cacheable feature improvements and documentation updates#21
Cacheable feature improvements and documentation updates#21
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the Cacheable gem’s caching interceptor to better support modern Ruby calling patterns (kwargs + blocks), improves safety and configurability of caching backends, and updates docs/examples accordingly.
Changes:
- Forward
**kwargsand blocks through cacheable wrappers; pass**kwargsintokey_format/unless. - Add a stderr warning when the default cache key format is used with arguments.
- Make
MemoryAdapterthread-safe and add per-class cache adapter support; switch interceptor modules to anonymous (noCacheable::...constants).
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/spec_helper.rb | Switch test bootstrapping to bundler/setup. |
| spec/cacheable/cacheable_spec.rb | Add specs for kwargs/block forwarding, warnings, and per-class adapters; adjust interceptor naming assertions. |
| lib/cacheable/method_generator.rb | Forward kwargs/blocks, use class-specific adapters, and warn when default key format is used with args. |
| lib/cacheable/cache_adapters/memory_adapter.rb | Add Monitor synchronization for thread-safety. |
| lib/cacheable/cache_adapter.rb | Implement per-class adapter fallback to global adapter. |
| lib/cacheable.rb | Create per-class anonymous interceptor module and store it on the class (no namespace constants). |
| examples/simple_example.rb | Refresh example output values. |
| examples/custom_key_example.rb | Update example to use kwargs-based key_format and date: kwarg. |
| examples/conditional_example.rb | Update conditional caching example for kwargs and date: kwarg. |
| examples/class_method_example.rb | Refresh example output values. |
| README.md | Document kwargs/block behavior, default-key-format warning, and per-class adapters; refresh examples/output. |
| Gemfile.lock | Normalize platforms and ffi entry. |
| CLAUDE.md | Add repository guidance for Claude Code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Generated methods only accepted *args, silently dropping keyword arguments and blocks. This caused methods with kwargs to break when wrapped with `cacheable`. Changes: - All generated methods now accept *args, **kwargs, &block - key_format procs receive kwargs as well - unless proc is resolved once at definition time instead of per-call - Use Bundler.setup instead of Bundler.require in spec_helper for Ruby 4.0 compatibility Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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>
The MemoryAdapter used a plain Hash with no synchronization, causing race conditions in multi-threaded servers (Puma, Sidekiq). fetch did a non-atomic exist?-then-write, allowing concurrent threads to duplicate work or corrupt state. Changes: - Wrap all cache operations with Monitor#synchronize - Make fetch atomic (inline key check + write under one lock) - Add frozen_string_literal pragma - Update tests to assert cache state directly instead of mocking internal write calls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously all classes shared a single global Cacheable.cache_adapter. Now each class that includes Cacheable can set its own adapter via MyClass.cache_adapter=, falling back to the global adapter when not configured. Changes: - Extend including classes with CacheAdapter - CacheAdapter#cache_adapter falls back to Cacheable.cache_adapter when no class-level adapter is set - Generated methods resolve adapter from the class instead of referencing Cacheable directly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, each class that included Cacheable created a constant on the Cacheable module (e.g. Cacheable::MyClassCacher) via const_set. This polluted the namespace and leaked memory for anonymous classes. Re-including Cacheable used remove_const which left ghost modules in the MRO. Now interceptor modules are stored as instance variables on the including class (@_cacheable_interceptor). The module gets a descriptive to_s/inspect for debugging without creating constants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The only suggestion is rubocop-rake which adds no value for our trivial Rakefile. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8985f24 to
4eb4a46
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@rofreg Claude and I worked thought a number of improvements I'd had on my mind and some I didn't even consider previously! Let me know if you'd rather pull any of these additions out. They were made with the intention that it should not require a major version bump / break any existing integrations (save maybe the warning in folk's logs). |
Summary
key_formatandunlessprocs now receive**kwargs, and blocks are forwarded through the caching layerMonitorfor safe concurrent accessself.cache_adapter =instead of only using the global**kwargswith adate:keyword argument, star counts refreshedTest plan
bundle exec rakepasses (RuboCop clean, 64 specs, 0 failures)🤖 Generated with Claude Code