From 9d569585a20ddd9ddb3602921f2ccffc208998d8 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Sat, 20 Sep 2014 09:03:03 -0700 Subject: 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 | 9 ++++++ activerecord/lib/active_record/aggregations.rb | 23 ++++++++++++-- activerecord/lib/active_record/associations.rb | 36 ++++++++++++++++------ .../active_record/associations/join_dependency.rb | 10 +++--- activerecord/lib/active_record/core.rb | 5 --- activerecord/lib/active_record/persistence.rb | 3 -- .../associations/belongs_to_associations_test.rb | 4 +-- activerecord/test/cases/associations_test.rb | 4 +-- activerecord/test/cases/inheritance_test.rb | 4 +-- 9 files changed, 66 insertions(+), 32 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 210151a2ad..4d2aeda41e 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 e576ec4d40..b2e83824be 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 12ca3a48a9..5f80f485af 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -138,20 +138,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 @@ -159,7 +153,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 ec5c189cd3..65b6ada737 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -230,12 +230,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 069aa977bf..2ce5ff7f8e 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -333,9 +333,6 @@ module ActiveRecord run_callbacks(:initialize) unless _initialize_callbacks.empty? - @aggregation_cache = {} - @association_cache = {} - @new_record = true @destroyed = false @@ -529,8 +526,6 @@ module ActiveRecord 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 755ff2b2f1..ea951f083e 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -403,9 +403,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/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 25555bd75c..e3ddcd98c0 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 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase 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 9b0cf4c18f..c40bcd068c 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -46,7 +46,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{ |x| x.name }.sort, firm.clients.collect{ |x| x.name }.sort end @@ -60,7 +60,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 diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 792950d24d..969e71898a 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -294,12 +294,12 @@ class InheritanceTest < ActiveRecord::TestCase 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 -- cgit v1.2.3