diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index da7572149caf555c807ee4db855f7dc0a90dc451..e017ade6037ea84304282d925c4c73b1254ab8fd 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Improve performance of calling .create on has_many :through associations. [evan] + * Improved cloning performance by relying less on exception raising #8159 [Blaine] * Added ActiveRecord::Base.inspect to return a column-view like # [DHH] diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index a8ad52bef97e0a8ad1b7c75c98dc01e8825909aa..93f1b2ee6a30bf76999e11bbc5e8ae8f0f2d169e 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -51,8 +51,6 @@ def <<(*records) through = @reflection.through_reflection raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) if @owner.new_record? - load_target - klass = through.klass klass.transaction do flatten_deeper(records).each do |associate| @@ -60,7 +58,7 @@ def <<(*records) raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record? @owner.send(@reflection.through_reflection.name).proxy_target << klass.with_scope(:create => construct_join_attributes(associate)) { klass.create! } - @target << associate + @target << associate if loaded? end end @@ -138,11 +136,11 @@ def construct_owner_attributes(reflection) # Construct attributes for :through pointing to owner and associate. def construct_join_attributes(associate) - returning construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id) do |join_attributes| - if @reflection.options[:source_type] - join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name.to_s) - end + join_attributes = construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id) + if @reflection.options[:source_type] + join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name.to_s) end + join_attributes end # Associate attributes pointing to owner, quoted. diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index 250fa1027cb6aa7238559cdfd3dca23e8a5bc3c6..aec6a47583e660f958ee536a68df5d8eeee1e7e0 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -7,6 +7,7 @@ require 'fixtures/computer' require 'fixtures/customer' require 'fixtures/order' +require 'fixtures/categorization' require 'fixtures/category' require 'fixtures/post' require 'fixtures/author' @@ -69,7 +70,7 @@ def test_storing_in_pstore end class AssociationProxyTest < Test::Unit::TestCase - fixtures :authors, :posts + fixtures :authors, :posts, :categorizations, :categories def test_proxy_accessors welcome = posts(:welcome) @@ -90,6 +91,17 @@ def test_proxy_accessors assert_equal david.posts_with_extension, david.posts_with_extension.testing_proxy_target end + def test_push_does_not_load_target + david = authors(:david) + not_loaded_string = '' + not_loaded_re = Regexp.new(not_loaded_string) + + david.categories << categories(:technology) + assert_match not_loaded_re, david.inspect + assert_equal not_loaded_string, david.categories.inspect + assert david.categories.include?(categories(:technology)) + end + def test_inspect_does_not_load_target david = authors(:david) not_loaded_string = '' @@ -660,7 +672,7 @@ def test_adding_before_save assert_equal 2, new_firm.clients_of_firm.size assert_equal 2, new_firm.clients_of_firm(true).size end - + def test_invalid_adding firm = Firm.find(1) assert !(firm.clients_of_firm << c = Client.new)