From 9cad2cedea783ee20888e4b16ef68a4c7b96f9d4 Mon Sep 17 00:00:00 2001 From: Brian Durand Date: Wed, 17 Dec 2025 17:45:03 -0500 Subject: [PATCH 1/3] Bypass the support table cache when finding on has_many associations --- .github/workflows/continuous_integration.yml | 4 +++ CHANGELOG.md | 6 ++++ VERSION | 2 +- lib/support_table_cache.rb | 29 +++++++++++-------- lib/support_table_cache/associations.rb | 2 +- lib/support_table_cache/find_by_override.rb | 8 +++-- lib/support_table_cache/memory_cache.rb | 26 +++++++++++++++++ lib/support_table_cache/relation_override.rb | 19 +++++++++--- spec/spec_helper.rb | 22 ++++++++++++++ spec/support_table_cache/associations_spec.rb | 4 +-- spec/support_table_cache/memory_cache_spec.rb | 4 +-- spec/support_table_cache_spec.rb | 16 ++++++++-- 12 files changed, 115 insertions(+), 27 deletions(-) diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index 1025c9a..ebe9388 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -24,6 +24,7 @@ jobs: include: - ruby: "ruby" standardrb: true + yard: true - ruby: "3.4" appraisal: "activerecord_8" - ruby: "3.1" @@ -60,3 +61,6 @@ jobs: - name: standardrb if: matrix.standardrb == true run: bundle exec rake standard + - name: yard + if: matrix.yard == true + run: bundle exec yard --fail-on-warning diff --git a/CHANGELOG.md b/CHANGELOG.md index 59d7a7b..b5c7246 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## 1.1.4 + +### Fixed + +- Fixed issue where using `find_by` on a `has_many` relation would not take the scope of the relation into account when looking up the cached record. Now chaining a `find_by` onto a `has_many` relation will correcty bypass the cache and directly query the database. + ## 1.1.3 ### Fixed diff --git a/VERSION b/VERSION index 781dcb0..65087b4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.3 +1.1.4 diff --git a/lib/support_table_cache.rb b/lib/support_table_cache.rb index 8eae28d..5e4a065 100644 --- a/lib/support_table_cache.rb +++ b/lib/support_table_cache.rb @@ -39,7 +39,8 @@ module ClassMethods # for a class will always take precedence over the global setting. # # @param disabled [Boolean] Caching will be disabled if this is true and enabled if false. - # @yieldreturn The return value of the block. + # @yield Executes the provided block with caching disabled or enabled. + # @return [Object] The return value of the block. def disable_cache(disabled = true, &block) varname = "support_table_cache_disabled:#{name}" save_val = Thread.current.thread_variable_get(varname) @@ -54,7 +55,8 @@ def disable_cache(disabled = true, &block) # Enable the caching behavior for this class within the block. The enabled setting # for a class will always take precedence over the global setting. # - # @yieldreturn The return value of the block. + # @yield Executes the provided block with caching enabled. + # @return [Object] The return value of the block. def enable_cache(&block) disable_cache(false, &block) end @@ -89,16 +91,16 @@ def support_table_cache=(cache) protected # Specify which attributes can be used for looking up records in the cache. Each value must - # define a unique key, Multiple unique keys can be specified. + # define a unique key. Multiple unique keys can be specified. # # If multiple attributes are used to make up a unique key, then they should be passed in as an array. # # If you need to remove caching setup in a superclass, you can pass in the value false to reset # cache behavior on the class. # - # @param attributes [String, Symbol, Array, FalseClass] Attributes that make up a unique key. - # @param case_sensitive [Boolean] Indicate if strings should treated as case insensitive in the key. - # @param where [Hash] A hash representing a hard coded set of attributes that must match a query in order + # @param attributes [String, Symbol, Array, false] Attributes that make up a unique key. + # @param case_sensitive [Boolean] Indicate if strings should be treated as case insensitive in the key. + # @param where [Hash, nil] A hash representing a hard coded set of attributes that must match a query in order # to cache the result. If a model has a default scope, then this value should be set to match the # where clause in that scope. # @return [void] @@ -144,7 +146,8 @@ class << self # disabled for that block. If no block is specified, then caching is disabled globally. # # @param disabled [Boolean] Caching will be disabled if this is true and enabled if false. - # @yieldreturn The return value of the block. + # @yield Executes the provided block with caching disabled or enabled (if block is given). + # @return [Object, nil] The return value of the block if a block is given, nil otherwise. def disable(disabled = true, &block) if block save_val = Thread.current.thread_variable_get(:support_table_cache_disabled) @@ -162,7 +165,8 @@ def disable(disabled = true, &block) # Enable the caching behavior for all classes. If a block is specified, then caching is only # enabled for that block. If no block is specified, then caching is enabled globally. # - # @yieldreturn The return value of the block. + # @yield Executes the provided block with caching enabled (if block is given). + # @return [Object, nil] The return value of the block if a block is given, nil otherwise. def enable(&block) disable(false, &block) end @@ -204,7 +208,8 @@ def cache # can use this to wrap your test methods so that cached values from one test don't show up # in subsequent tests. # - # @return [void] + # @yield Executes the provided block in test mode. + # @return [Object] The return value of the block. def testing!(&block) save_val = Thread.current.thread_variable_get(:support_table_cache_test_cache) if save_val.nil? @@ -219,7 +224,7 @@ def testing!(&block) # Get the current test mode cache. This will only return a value inside of a `testing!` block. # - # @return [SupportTableCache::MemoryCache] + # @return [SupportTableCache::MemoryCache, nil] The test cache or nil if not in test mode. # @api private def testing_cache unless defined?(@cache) && @cache.nil? @@ -232,9 +237,9 @@ def testing_cache # # @param klass [Class] The class that is being cached. # @param attributes [Hash] The attributes used to find a record. - # @param key_attribute_names [Array] List of attributes that can be used as a key in the cache. + # @param key_attribute_names [Array] List of attributes that can be used as a key in the cache. # @param case_sensitive [Boolean] Indicator if string values are case-sensitive in the cache key. - # @return [String] + # @return [Array(String, Hash), nil] A two-element array with the class name and attributes hash, or nil if not cacheable. # @api private def cache_key(klass, attributes, key_attribute_names, case_sensitive) return nil if attributes.blank? || key_attribute_names.blank? diff --git a/lib/support_table_cache/associations.rb b/lib/support_table_cache/associations.rb index bce5eb9..46bbada 100644 --- a/lib/support_table_cache/associations.rb +++ b/lib/support_table_cache/associations.rb @@ -14,7 +14,7 @@ module ClassMethods # # @param association_name [Symbol, String] The association name to cache. # @return [void] - # @raise ArgumentError if the association is not defined or if it has a runtime scope. + # @raise [ArgumentError] if the association is not defined or if it has a runtime scope. def cache_belongs_to(association_name) reflection = reflections[association_name.to_s] diff --git a/lib/support_table_cache/find_by_override.rb b/lib/support_table_cache/find_by_override.rb index 2ac915c..31fc027 100644 --- a/lib/support_table_cache/find_by_override.rb +++ b/lib/support_table_cache/find_by_override.rb @@ -39,7 +39,8 @@ def find_by(*args) # Same as find_by, but performs a safety check to confirm the query will hit the cache. # # @param attributes [Hash] Attributes to find the record by. - # @raise ArgumentError if the query cannot use the cache. + # @return [ActiveRecord::Base, nil] The found record or nil if not found. + # @raise [ArgumentError] if the query cannot use the cache. def fetch_by(attributes) find_by_attribute_names = support_table_find_by_attribute_names(attributes) unless support_table_cache_by_attributes.any? { |attribute_names, _ci, _where| attribute_names == find_by_attribute_names } @@ -51,8 +52,9 @@ def fetch_by(attributes) # Same as find_by!, but performs a safety check to confirm the query will hit the cache. # # @param attributes [Hash] Attributes to find the record by. - # @raise ArgumentError if the query cannot use the cache. - # @raise ActiveRecord::RecordNotFound if no record is found. + # @return [ActiveRecord::Base] The found record. + # @raise [ArgumentError] if the query cannot use the cache. + # @raise [ActiveRecord::RecordNotFound] if no record is found. def fetch_by!(attributes) value = fetch_by(attributes) if value.nil? diff --git a/lib/support_table_cache/memory_cache.rb b/lib/support_table_cache/memory_cache.rb index 8030e93..44405fb 100644 --- a/lib/support_table_cache/memory_cache.rb +++ b/lib/support_table_cache/memory_cache.rb @@ -8,11 +8,20 @@ module SupportTableCache # This cache will not store nil values. This is to prevent the cache from filling up with # cache misses because there is no purging mechanism. class MemoryCache + # Create a new memory cache. + # + # @return [SupportTableCache::MemoryCache] def initialize @cache = {} @mutex = Mutex.new end + # Fetch a value from the cache. If the key is not found or has expired, yields to get a new value. + # + # @param key [Object] The cache key. + # @param expires_in [Integer, nil] Time in seconds until the cached value expires. + # @yield Block to execute to get a new value if the key is not cached. + # @return [Object, nil] The cached value or the result of the block, or nil if no value is found. def fetch(key, expires_in: nil) serialized_value, expire_at = @cache[key] if serialized_value.nil? || (expire_at && expire_at < Process.clock_gettime(Process::CLOCK_MONOTONIC)) @@ -24,10 +33,20 @@ def fetch(key, expires_in: nil) Marshal.load(serialized_value) end + # Read a value from the cache. + # + # @param key [Object] The cache key. + # @return [Object, nil] The cached value or nil if not found. def read(key) fetch(key) end + # Write a value to the cache. + # + # @param key [Object] The cache key. + # @param value [Object] The value to cache. Nil values are not cached. + # @param expires_in [Integer, nil] Time in seconds until the cached value expires. + # @return [void] def write(key, value, expires_in: nil) return if value.nil? @@ -42,10 +61,17 @@ def write(key, value, expires_in: nil) end end + # Delete a value from the cache. + # + # @param key [Object] The cache key. + # @return [void] def delete(key) @cache.delete(key) end + # Clear all values from the cache. + # + # @return [void] def clear @cache.clear end diff --git a/lib/support_table_cache/relation_override.rb b/lib/support_table_cache/relation_override.rb index 54192b7..6970562 100644 --- a/lib/support_table_cache/relation_override.rb +++ b/lib/support_table_cache/relation_override.rb @@ -5,12 +5,18 @@ module SupportTableCache module RelationOverride # Override for the find_by method that looks in the cache first. + # + # @param args [Array] Arguments passed to find_by. + # @return [ActiveRecord::Base, nil] The found record or nil if not found. def find_by(*args) return super unless klass.include?(SupportTableCache) cache = klass.send(:current_support_table_cache) return super unless cache + # Skip caching for has_many or has_many :through associations + return super if is_a?(ActiveRecord::Associations::CollectionProxy) + return super if select_values.present? cache_key = nil @@ -43,7 +49,10 @@ def find_by(*args) end # Override for the find_by! method that looks in the cache first. - # @raise ActiveRecord::RecordNotFound if no record is found. + # + # @param args [Array] Arguments passed to find_by!. + # @return [ActiveRecord::Base] The found record. + # @raise [ActiveRecord::RecordNotFound] if no record is found. def find_by!(*args) value = find_by(*args) unless value @@ -55,7 +64,8 @@ def find_by!(*args) # Same as find_by, but performs a safety check to confirm the query will hit the cache. # # @param attributes [Hash] Attributes to find the record by. - # @raise ArgumentError if the query cannot use the cache. + # @return [ActiveRecord::Base, nil] The found record or nil if not found. + # @raise [ArgumentError] if the query cannot use the cache. def fetch_by(attributes) find_by_attribute_names = support_table_find_by_attribute_names(attributes) unless klass.support_table_cache_by_attributes.any? { |attribute_names, _ci| attribute_names == find_by_attribute_names } @@ -67,8 +77,9 @@ def fetch_by(attributes) # Same as find_by!, but performs a safety check to confirm the query will hit the cache. # # @param attributes [Hash] Attributes to find the record by. - # @raise ArgumentError if the query cannot use the cache. - # @raise ActiveRecord::RecordNotFound if no record is found. + # @return [ActiveRecord::Base] The found record. + # @raise [ArgumentError] if the query cannot use the cache. + # @raise [ActiveRecord::RecordNotFound] if no record is found. def fetch_by!(attributes) value = fetch_by(attributes) if value.nil? diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1624b4f..c81e8a7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -29,6 +29,15 @@ t.string :label t.datetime :deleted_at, null: true end + + connection.create_table(:things) do |t| + t.string :name + end + + connection.create_table(:other_things) do |t| + t.integer :thing_id, index: true + t.integer :test_model_id, index: true + end end class TestModel < ActiveRecord::Base @@ -61,10 +70,23 @@ class DefaultScopeModel < ActiveRecord::Base default_scope { where(deleted_at: nil) } end +class Thing < ActiveRecord::Base + has_many :other_things + has_many :test_models, through: :other_things +end + +class OtherThing < ActiveRecord::Base + belongs_to :test_model + belongs_to :thing +end + SupportTableCache.cache = ActiveSupport::Cache::MemoryStore.new RSpec.configure do |config| + config.disable_monkey_patching! + config.default_formatter = "doc" if config.files_to_run.one? config.order = :random + Kernel.srand config.seed config.before do TestModel.delete_all diff --git a/spec/support_table_cache/associations_spec.rb b/spec/support_table_cache/associations_spec.rb index 152286b..a690dca 100644 --- a/spec/support_table_cache/associations_spec.rb +++ b/spec/support_table_cache/associations_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require_relative "../spec_helper" +require "spec_helper" -describe SupportTableCache::Associations do +RSpec.describe SupportTableCache::Associations do let!(:record) { TestModel.create!(name: "One", code: "one", group: "First", value: 1) } let!(:parent) { ParentModel.create!(test_model: record) } diff --git a/spec/support_table_cache/memory_cache_spec.rb b/spec/support_table_cache/memory_cache_spec.rb index 71a933c..15606e5 100644 --- a/spec/support_table_cache/memory_cache_spec.rb +++ b/spec/support_table_cache/memory_cache_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require_relative "../spec_helper" +require "spec_helper" -describe SupportTableCache::MemoryCache do +RSpec.describe SupportTableCache::MemoryCache do let(:cache) { SupportTableCache::MemoryCache.new } # rubocop:disable Style/RedundantFetchBlock diff --git a/spec/support_table_cache_spec.rb b/spec/support_table_cache_spec.rb index 8abf23d..3cd0d4e 100644 --- a/spec/support_table_cache_spec.rb +++ b/spec/support_table_cache_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require_relative "spec_helper" +require "spec_helper" -describe SupportTableCache do +RSpec.describe SupportTableCache do let!(:record_1) { TestModel.create!(name: "One", code: "one", group: "First", value: 1) } let!(:record_2) { TestModel.create!(name: "Two", code: "two", group: "Second", value: 2) } @@ -95,6 +95,7 @@ expect(TestModel.where(group: "First").find_by(code: "one")).to eq record_1 expect(TestModel.where(group: "Second").find_by(code: "two")).to eq record_2 + expect(TestModel.where(group: "Second").find_by(code: "one")).to be_nil expect(TestModel.where(group: "First").find_by(code: "one").value).to eq 1 record_1.update_columns(value: 3) @@ -102,6 +103,17 @@ expect(TestModel.where(code: "one").find_by(group: "First").value).to eq 1 end + it "uses the cache when finding on an association" do + thing = Thing.create!(name: "Thing One") + OtherThing.create!(thing: thing, test_model: record_1) + + TestModel.find_by(name: "One") # prime the cache + TestModel.find_by(name: "Two") # prime the cache + + expect(thing.test_models.find_by(name: "One")).to eq record_1 + expect(thing.test_models.find_by(name: "Two")).to be_nil + end + it "uses the cache when finding by multiple cacheable attributes with a relation chain with find_by!" do expect(TestModel.where(group: "First").find_by!(code: "one")).to eq record_1 expect(SupportTableCache.cache.read(SupportTableCache.cache_key(TestModel, {code: "one", group: "First"}, ["code", "group"], false))).to eq record_1 From 562b2da180486c09817e098e92aa96a7b2724d98 Mon Sep 17 00:00:00 2001 From: Brian Durand Date: Wed, 17 Dec 2025 17:06:20 -0800 Subject: [PATCH 2/3] Update CHANGELOG.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c7246..95cb874 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Fixed issue where using `find_by` on a `has_many` relation would not take the scope of the relation into account when looking up the cached record. Now chaining a `find_by` onto a `has_many` relation will correcty bypass the cache and directly query the database. +- Fixed issue where using `find_by` on a `has_many` relation would not take the scope of the relation into account when looking up the cached record. Now chaining a `find_by` onto a `has_many` relation will correctly bypass the cache and directly query the database. ## 1.1.3 From f9120779a170454a2e17053455209c9a545d0c94 Mon Sep 17 00:00:00 2001 From: Brian Durand Date: Wed, 17 Dec 2025 17:06:56 -0800 Subject: [PATCH 3/3] Update spec/support_table_cache_spec.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- spec/support_table_cache_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support_table_cache_spec.rb b/spec/support_table_cache_spec.rb index 3cd0d4e..ee188c1 100644 --- a/spec/support_table_cache_spec.rb +++ b/spec/support_table_cache_spec.rb @@ -103,7 +103,7 @@ expect(TestModel.where(code: "one").find_by(group: "First").value).to eq 1 end - it "uses the cache when finding on an association" do + it "does not use the cache when finding on an association" do thing = Thing.create!(name: "Thing One") OtherThing.create!(thing: thing, test_model: record_1)