Merge pull request #16989 from Empact/reload-cache-clear

Isolate access to @associations_cache and @aggregations_cache to the Associations and Aggregations modules, respectively.

6 files changed, 53 insertions, 28 deletions

diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb
index 39077aea7e..5d723d6648 100644
--- a/activerecord/lib/active_record/aggregations.rb
+++ b/activerecord/lib/active_record/aggregations.rb
@@ -3,10 +3,27 @@ module ActiveRecord
   module Aggregations # :nodoc:
     extend ActiveSupport::Concern
 
-    def clear_aggregation_cache #:nodoc:
-      @aggregation_cache.clear if persisted?
+    def initialize_dup(*) # :nodoc:
+      @aggregation_cache = {}
+      super
     end
 
+    def reload(*) # :nodoc:
+      clear_aggregation_cache
+      super
+    end
+
+    private
+
+      def clear_aggregation_cache # :nodoc:
+        @aggregation_cache.clear if persisted?
+      end
+
+      def init_internals # :nodoc:
+        @aggregation_cache = {}
+        super
+      end
+
     # Active Record implements aggregation through a macro-like class method called +composed_of+
     # for representing attributes  as value objects. It expresses relationships like "Account [is]
     # composed of Money [among other things]" or "Person [is] composed of [an] address". Each call
@@ -89,7 +106,7 @@ module ActiveRecord
     #
     #   customer.address_street = "Vesterbrogade"
     #   customer.address        # => Address.new("Hyancintvej", "Copenhagen")
-    #   customer.clear_aggregation_cache
+    #   customer.send(:clear_aggregation_cache)
     #   customer.address        # => Address.new("Vesterbrogade", "Copenhagen")
     #
     #   customer.address = Address.new("May Street", "Chicago")
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index a146d78a5a..499b00a815 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -145,20 +145,14 @@ module ActiveRecord
       autoload :AliasTracker,     'active_record/associations/alias_tracker'
     end
 
-    # Clears out the association cache.
-    def clear_association_cache #:nodoc:
-      @association_cache.clear if persisted?
-    end
-
-    # :nodoc:
-    attr_reader :association_cache
-
     # Returns the association instance for the given name, instantiating it if it doesn't already exist
     def association(name) #:nodoc:
       association = association_instance_get(name)
 
       if association.nil?
-        raise AssociationNotFoundError.new(self, name) unless reflection = self.class._reflect_on_association(name)
+        unless reflection = self.class._reflect_on_association(name)
+          raise AssociationNotFoundError.new(self, name)
+        end
         association = reflection.association_class.new(self, reflection)
         association_instance_set(name, association)
       end
@@ -166,7 +160,31 @@ module ActiveRecord
       association
     end
 
+    def association_cached?(name) # :nodoc
+      @association_cache.key?(name)
+    end
+
+    def initialize_dup(*) # :nodoc:
+      @association_cache = {}
+      super
+    end
+
+    def reload(*) # :nodoc:
+      clear_association_cache
+      super
+    end
+
     private
+      # Clears out the association cache.
+      def clear_association_cache # :nodoc:
+        @association_cache.clear if persisted?
+      end
+
+      def init_internals # :nodoc:
+        @association_cache = {}
+        super
+      end
+
       # Returns the specified association instance if it responds to :loaded?, nil otherwise.
       def association_instance_get(name)
         @association_cache[name]
diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb
index fcf06323e6..81eb5136a1 100644
--- a/activerecord/lib/active_record/associations/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/join_dependency.rb
@@ -239,12 +239,10 @@ module ActiveRecord
           if node.reflection.collection?
             other = ar_parent.association(node.reflection.name)
             other.loaded!
-          else
-            if ar_parent.association_cache.key?(node.reflection.name)
-              model = ar_parent.association(node.reflection.name).target
-              construct(model, node, row, rs, seen, model_cache, aliases)
-              next
-            end
+          elsif ar_parent.association_cached?(node.reflection.name)
+            model = ar_parent.association(node.reflection.name).target
+            construct(model, node, row, rs, seen, model_cache, aliases)
+            next
           end
 
           key = aliases.column_alias(node, node.primary_key)
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index eb7ff0ce9e..1244bd6195 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -345,9 +345,6 @@ module ActiveRecord
 
       _run_initialize_callbacks
 
-      @aggregation_cache = {}
-      @association_cache = {}
-
       @new_record  = true
       @destroyed   = false
 
@@ -542,8 +539,6 @@ module ActiveRecord
     end
 
     def init_internals
-      @aggregation_cache        = {}
-      @association_cache        = {}
       @readonly                 = false
       @destroyed                = false
       @marked_for_destruction   = false
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 35d1085f5a..7c076864a3 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -415,9 +415,6 @@ module ActiveRecord
     #   end
     #
     def reload(options = nil)
-      clear_aggregation_cache
-      clear_association_cache
-
       fresh_object =
         if options && options[:lock]
           self.class.unscoped { self.class.lock(options[:lock]).find(id) }
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index de358114ab..1b3bacca64 100644
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -47,7 +47,7 @@ class AssociationsTest < ActiveRecord::TestCase
     firm = Firm.find(1)
     assert_kind_of Firm, firm
 
-    firm.clear_association_cache
+    firm.send(:clear_association_cache)
     assert_equal Firm.find(1).clients.collect(&:name).sort, firm.clients.collect(&:name).sort
   end
 
@@ -61,7 +61,7 @@ class AssociationsTest < ActiveRecord::TestCase
      firm.clients    << clients
      assert_equal clients.map(&:name).to_set, firm.clients.map(&:name).to_set
 
-     firm.clear_association_cache
+     firm.send(:clear_association_cache)
      assert_equal clients.map(&:name).to_set, firm.clients.map(&:name).to_set
   end