diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 210151a2ad75c8167dbbcd79003a1f36cd44b618..4d2aeda41e1a1c80f3222f9ec3669dbf8d549e35 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* 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. diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index e576ec4d40f6b69fa6db089ae0774c9b13093485..b2e83824be207254844f9e2fc0078d8ce749fa3a 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 @@ 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") diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 12ca3a48a918c37258b3d41dc7426dec4cb637d5..5f80f485af25fbf3187b79391136d986ab53572a 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/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] diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index ec5c189cd31c5849533f30311753c3c7c02599f3..65b6ada7370011db052fcfc2ffdf8ccdb30131b7 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/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) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 069aa977bf0ad07374dca7ae0b8ab20f42a346e7..2ce5ff7f8e121bc8dd3416eccf71b352718b0a2a 100644 --- a/activerecord/lib/active_record/core.rb +++ b/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 diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 755ff2b2f12f18fc0c3611a41b185305a750959d..ea951f083e754e3711c475c950a42a202e3bcb93 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/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) } diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 25555bd75cfa24df541bb1d6033c87e9b612d3cf..e3ddcd98c0d82100c78ffed89f8c4fa28de532d2 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/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 diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 9b0cf4c18f7178a23dbbb7ecb1df2aaa8b452970..c40bcd068cc443dcdd936cb9bfb10db2ae05b1c8 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/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 diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 792950d24d86333b28c80b7f5d5b4dd34ba2c0eb..969e71898a08000de9c3bc8c8c4be7cb71b45e2a 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/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