Merge pull request #34094 from christophemaximin/fix-activerecord-clearing-of-query-cache

Fix inconsistent behavior by clearing QueryCache when reloading associations

activerecord/CHANGELOG.md

@@ -1,3 +1,20 @@
+*   Reloading associations now clears the Query Cache like `Persistence#reload` does.
+
+    ```
+    class Post < ActiveRecord::Base
+      has_one :category
+      belongs_to :author
+      has_many :comments
+    end
+
+    # Each of the following will now clear the query cache.
+    post.reload_category
+    post.reload_author
+    post.comments.reload
+    ```
+
+    *Christophe Maximin*
+
 *   Added `index` option for `change_table` migration helpers.
     With this change you can create indexes while adding new
     columns into the existing tables.

activerecord/lib/active_record/associations/association.rb

@@ -40,7 +40,9 @@ module ActiveRecord
       end
 
       # Reloads the \target and returns +self+ on success.
-      def reload
+      # The QueryCache is cleared if +force+ is true.
+      def reload(force = false)
+        klass.connection.clear_query_cache if force && klass
         reset
         reset_scope
         load_target

activerecord/lib/active_record/associations/collection_proxy.rb

@@ -1088,7 +1088,7 @@ module ActiveRecord
       #   person.pets.reload # fetches pets from the database
       #   # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>]
       def reload
-        proxy_association.reload
+        proxy_association.reload(true)
         reset_scope
       end
 

activerecord/lib/active_record/associations/singular_association.rb

@@ -26,7 +26,7 @@ module ActiveRecord
       # Implements the reload reader method, e.g. foo.reload_bar for
       # Foo.has_one :bar
       def force_reload_reader
-        klass.uncached { reload }
+        reload(true)
         target
       end
 

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -367,6 +367,30 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert_equal "ODEGY", odegy_account.reload_firm.name
   end
 
+  def test_reload_the_belonging_object_with_query_cache
+    odegy_account_id = accounts(:odegy_account).id
+
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    # Populate the cache with a query
+    odegy_account = Account.find(odegy_account_id)
+
+    # Populate the cache with a second query
+    odegy_account.firm
+
+    assert_equal 2, connection.query_cache.size
+
+    # Clear the cache and fetch the firm again, populating the cache with a query
+    assert_queries(1) { odegy_account.reload_firm }
+
+    # This query is not cached anymore, so it should make a real SQL query
+    assert_queries(1) { Account.find(odegy_account_id) }
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
   def test_natural_assignment_to_nil
     client = Client.find(3)
     client.firm = nil

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -829,6 +829,48 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_not_same original_object, collection.first, "Expected #first after #reload to return a new object"
   end
 
+  def test_reload_with_query_cache
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    # Populate the cache with a query
+    firm = Firm.first
+    # Populate the cache with a second query
+    firm.clients.load
+
+    assert_equal 2, connection.query_cache.size
+
+    # Clear the cache and fetch the clients again, populating the cache with a query
+    assert_queries(1) { firm.clients.reload }
+    # This query is cached, so it shouldn't make a real SQL query
+    assert_queries(0) { firm.clients.load }
+
+    assert_equal 1, connection.query_cache.size
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
+  def test_reloading_unloaded_associations_with_query_cache
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    firm = Firm.create!(name: "firm name")
+    client = firm.clients.create!(name: "client name")
+    firm.clients.to_a # add request to cache
+
+    connection.uncached do
+      client.update!(name: "new client name")
+    end
+
+    firm = Firm.find(firm.id)
+
+    assert_equal [client.name], firm.clients.reload.map(&:name)
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
   def test_find_all_with_include_and_conditions
     assert_nothing_raised do
       Developer.all.merge!(joins: :audit_logs, where: { "audit_logs.message" => nil, :name => "Smith" }).to_a

activerecord/test/cases/associations/has_one_associations_test.rb

@@ -333,6 +333,29 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     assert_equal 80, odegy.reload_account.credit_limit
   end
 
+  def test_reload_association_with_query_cache
+    odegy_id = companies(:odegy).id
+
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    # Populate the cache with a query
+    odegy = Company.find(odegy_id)
+    # Populate the cache with a second query
+    odegy.account
+
+    assert_equal 2, connection.query_cache.size
+
+    # Clear the cache and fetch the account again, populating the cache with a query
+    assert_queries(1) { odegy.reload_account }
+
+    # This query is not cached anymore, so it should make a real SQL query
+    assert_queries(1) { Company.find(odegy_id) }
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
   def test_build
     firm = Firm.new("name" => "GlobalMegaCorp")
     firm.save