Isolate access to @associations_cache and @aggregations cache to the...

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

This includes replacing the `association_cache` accessor with a more
limited `association_cached?` accessor and making `clear_association_cache`
and `clear_aggregation_cache` private.

activerecord/CHANGELOG.md

+*   Isolate access to @associations_cache and @aggregations cache to the
+    Associations and Aggregations modules, respectively.
+
+    This includes replacing the `association_cache` accessor with an
+    `association_cached?` accessor and making `clear_association_cache`
+    and `clear_aggregation_cache` private.
+
+    *Ben Woosley*
+
 *   Honor overridden `rack.test` in Rack environment for the connection
     management middleware.
 

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 @@ def clear_aggregation_cache #:nodoc:
     #
     #   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")

activerecord/lib/active_record/associations.rb

@@ -138,20 +138,14 @@ module Builder #:nodoc:
       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
@@ -159,7 +153,31 @@ def association(name) #:nodoc:
       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]

activerecord/lib/active_record/associations/join_dependency.rb

@@ -230,12 +230,10 @@ def construct(ar_parent, parent, row, rs, seen, model_cache, aliases)
           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)

activerecord/lib/active_record/core.rb

@@ -333,9 +333,6 @@ def initialize_dup(other) # :nodoc:
 
       run_callbacks(:initialize) unless _initialize_callbacks.empty?
 
-      @aggregation_cache = {}
-      @association_cache = {}
-
       @new_record  = true
       @destroyed   = false
 
@@ -529,8 +526,6 @@ def to_ary # :nodoc:
     def init_internals
       @attributes.ensure_initialized(self.class.primary_key)
 
-      @aggregation_cache        = {}
-      @association_cache        = {}
       @readonly                 = false
       @destroyed                = false
       @marked_for_destruction   = false

activerecord/lib/active_record/persistence.rb

@@ -403,9 +403,6 @@ def toggle!(attribute)
     #   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) }

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -92,14 +92,14 @@ def test_eager_loading_with_primary_key
     Firm.create("name" => "Apple")
     Client.create("name" => "Citibank", :firm_name => "Apple")
     citibank_result = Client.all.merge!(:where => {:name => "Citibank"}, :includes => :firm_with_primary_key).first
-    assert citibank_result.association_cache.key?(:firm_with_primary_key)
+    assert citibank_result.association_cached?(:firm_with_primary_key)
   end
 
   def test_eager_loading_with_primary_key_as_symbol
     Firm.create("name" => "Apple")
     Client.create("name" => "Citibank", :firm_name => "Apple")
     citibank_result = Client.all.merge!(:where => {:name => "Citibank"}, :includes => :firm_with_primary_key_symbols).first
-    assert citibank_result.association_cache.key?(:firm_with_primary_key_symbols)
+    assert citibank_result.association_cached?(:firm_with_primary_key_symbols)
   end
 
   def test_creating_the_belonging_object

activerecord/test/cases/associations_test.rb

@@ -46,7 +46,7 @@ def test_clear_association_cache_stored
     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{ |x| x.name }.sort, firm.clients.collect{ |x| x.name }.sort
   end
 
@@ -60,7 +60,7 @@ def test_clear_association_cache_new_record
      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
 

activerecord/test/cases/inheritance_test.rb

@@ -294,12 +294,12 @@ def test_alt_complex_inheritance
 
   def test_eager_load_belongs_to_something_inherited
     account = Account.all.merge!(:includes => :firm).find(1)
-    assert account.association_cache.key?(:firm), "nil proves eager load failed"
+    assert account.association_cached?(:firm), "nil proves eager load failed"
   end
 
   def test_alt_eager_loading
     cabbage = RedCabbage.all.merge!(:includes => :seller).find(4)
-    assert cabbage.association_cache.key?(:seller), "nil proves eager load failed"
+    assert cabbage.association_cached?(:seller), "nil proves eager load failed"
   end
 
   def test_eager_load_belongs_to_primary_key_quoting