diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/has_many_through_association.rb | 12 | ||||
| -rwxr-xr-x | activerecord/test/associations_test.rb | 16 |
3 files changed, 21 insertions, 9 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index da7572149c..e017ade603 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 #<Post id:integer, title:string, body:text> [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 a8ad52bef9..93f1b2ee6a 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord # 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 250fa1027c..aec6a47583 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -7,6 +7,7 @@ require 'fixtures/reply' 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 @@ class AssociationsTest < Test::Unit::TestCase 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 @@ class AssociationProxyTest < Test::Unit::TestCase 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 = '<categories not loaded yet>' + 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 = '<posts not loaded yet>' @@ -660,7 +672,7 @@ class HasManyAssociationsTest < Test::Unit::TestCase 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) |