Bypass the support table cache when finding on has_many associations

.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

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 correctly bypass the cache and directly query the database.
+
 ## 1.1.3
 
 ### Fixed

VERSION

@@ -1 +1 @@
-1.1.3
+1.1.4

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<String, Symbol>, 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<String, Symbol>, 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<String>] 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?

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]
 

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?

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

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<Object>] 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<Object>] 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?

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

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) }
 

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

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,13 +95,25 @@
 
       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)
       expect(TestModel.where(group: "First").find_by(code: "one").value).to eq 1
       expect(TestModel.where(code: "one").find_by(group: "First").value).to eq 1
     end
 
+    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)
+
+      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