More cleanup and improvements - #25
Merged
Merged
Conversation
ArgumentKeys joined the argument values with no separator and dropped the
field names, so three different finders shared one cache entry:
Collide.cached_find_by_alpha_and_beta("x", "y") # => {alpha: "x", beta: "y"}
Collide.cached_find_by_delta_and_gamma("x", "y") # => {alpha: "x", beta: "y"}
Collide.cached_find_by_guid("xy") # => {alpha: "x", beta: "y"}
The second and third calls returned the first record.
ArgumentKeys.for_fields now names each field and escapes the separators
inside a value, so the key is one-to-one with the arguments. The separators
survive both the remove-characters and the replace-characters option.
This changes every cache key. Existing entries become a miss.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two problems in the generated cached_search_by_*! method: raise ::ActiveRemote::RemoteRecordNotFound.new(self.class) if results.first.nil? self.class is Class inside a `def self.` body, so the message read "Class does not exist" and error.remote_record_class returned Class. results.first raised NoMethodError when search returned nil, where the caller expects RemoteRecordNotFound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each generated method built its options through Hash#to_s:
.merge(#{cached_finder_options})
The result has to parse as Ruby source, and a common value does not:
cached_finders_for :guid, :expires_in => 5.minutes # => SyntaxError
5.minutes.to_s produces "5 minutes". Any object without a literal form
failed the same way.
The failure also left the class broken. cached_methods << method_name ran
before class_eval, so the name was registered with no method behind it.
method_missing then dispatched to that same missing name and recursed until
Ruby raised SystemStackError.
The options now live in a per-class registry that the generated method reads
at call time, and the name is registered only after class_eval returns. The
lookup walks the superclass chain, so a subclass keeps the declared options.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_args_in_sorted_order looked each field name up by index and passed the
result through without a check:
index = orignal_args_name.index(arg_name)
args_in_order << args[index]
A missing positional argument became nil with no error, and the finder then
cached a record under a key built from that nil:
E.cached_find_by_beta_and_alpha("only_one") # => {alpha: nil, beta: "only_one"}
E.cached_find_by_alpha_and_beta("only_one") # => ArgumentError
The same call raised in the defined order and passed nil in the reordered
order. A field name absent from the called name raised TypeError from
args[nil] rather than a useful error.
Also corrects the orignal_args_name typo and the match_1/match_2 names, which
drops the Naming/VariableNumber and Metrics/AbcSize suppressions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cache#fetch let the provider write the value, then deleted it when the value was not valid. Three searches that returned nil produced this call sequence: [:fetch, :write, :delete, :fetch, :write, :delete, :fetch, :write, :delete] Two of every three calls were waste. Against Redis or Memcached that is two network round trips per request, on the cache-miss path. The provider now receives :skip_nil unless the caller passed :allow_nil, so the nil is never written and needs no delete. The empty-value case still writes and deletes, because ActiveSupport has no matching option. The generated delete method also built the same ArgumentKeys twice. It now builds the key once and uses it for both the find and the search key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
validate_provider_method_present raised a bare String, so callers got a RuntimeError whose message carried the heredoc indentation. It now raises ActiveRemote::Cached::Cache::InvalidCacheProvider with a single-line message. The raise was also indented two columns past the return above it. That one line was why .rubocop.yml disabled Layout/IndentationConsistency for the whole repository. Six cop exclusions are now dead and removed. I forced each one on and got no offenses: Layout/AccessModifierIndentation, Layout/CommentIndentation, Layout/IndentationConsistency, Style/AccessModifierDeclarations, Style/MissingRespondToMissing (respond_to_missing? is defined), and Naming/VariableNumber. The global Metrics/MethodLength disable becomes a per-file exclusion, so a long method in a new file is still flagged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- homepage was an empty string; set the GitHub URL. - No license was declared, though LICENSE.txt says MIT. gem build warned. - test_files is deprecated; removed. - files called git ls-files with no fallback, so a build from a released tarball produced an empty file list. - activesupport had no version floor, though the code calls ActiveSupport::Cache::NullStore and reads ActiveSupport::VERSION::STRING. Set >= 6.1 to match the active_remote floor. - Added metadata with rubygems_mfa_required. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ActiveRemote::Cached.cache(provider) builds a new Cache, and a new Cache starts with a NullStore as its nested provider. A nested cache that enable_nested_caching! had turned on was silently lost: cache.enable_nested_caching! ActiveRemote::Cached.cache(other_store) # nested provider is a NullStore again The Railtie calls enable_nested_caching! after it sets the provider, so Rails was unaffected. An application that swapped the store later was not. Adds Cache#nested_caching? and carries the setting to the new Cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
REMOVE_CHARACTERS uses [[:space:]], so it strips a tab and a newline. REPLACE_MAP listed only a plain space, so it left them in the key: remove "a\tb" # => "ab" replace "a\tb" # => "a\tb" tab survives into the key A raw tab or newline in a cache key breaks the Memcached protocol. REPLACE_MAP now covers tab, newline, carriage return, form feed, and vertical tab. It is also a Hash rather than an array of pairs, so #cache_key makes one gsub pass over the string instead of 13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cache key format changed, so every existing entry becomes a miss. A dynamic finder called with too few arguments now raises ArgumentError rather than caching a record under a nil, and the cache provider validator raises InvalidCacheProvider rather than a bare RuntimeError. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.