From 4cf8bf7312a124cb96e615923ebb373380fb60bd Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 21 Dec 2006 23:28:12 +0000 Subject: [PATCH] Pushing a record on an association collection doesn't unnecessarily load all the associated records. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5769 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 ++ .../associations/association_collection.rb | 7 +++---- activerecord/test/associations_test.rb | 10 +++++++--- activerecord/test/base_test.rb | 6 +++--- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 357a88a6d4..c5c6925e24 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Pushing a record on an association collection doesn't unnecessarily load all the associated records. [Obie Fernandez, Jeremy Kemper] + * Oracle: fix connection reset failure. #6846 [leonlleslie] * Subclass instantiation doesn't try to explicitly require the corresponding subclass. #6840 [leei, Jeremy Kemper] diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index d3b76e2087..bda72637c3 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -7,7 +7,7 @@ module ActiveRecord load_target @target.to_ary end - + def reset reset_target! @loaded = false @@ -17,7 +17,6 @@ module ActiveRecord # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically. def <<(*records) result = true - load_target @owner.transaction do flatten_deeper(records).each do |record| @@ -34,7 +33,7 @@ module ActiveRecord alias_method :push, :<< alias_method :concat, :<< - + # Remove all records from this association def delete_all load_target @@ -103,7 +102,7 @@ module ActiveRecord # calling collection.size if it has. If it's more likely than not that the collection does have a size larger than zero # and you need to fetch that collection afterwards, it'll take one less SELECT query if you use length. def size - if loaded? && !@reflection.options[:uniq] + if @owner.new_record? || (loaded? && !@reflection.options[:uniq]) @target.size elsif !loaded? && !@reflection.options[:uniq] && @target.is_a?(Array) unsaved_records = Array(@target.detect { |r| r.new_record? }) diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index 511653dfce..c5d89bb216 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -637,12 +637,15 @@ class HasManyAssociationsTest < Test::Unit::TestCase def test_adding_before_save no_of_firms = Firm.count no_of_clients = Client.count + new_firm = Firm.new("name" => "A New Firm, Inc") + c = Client.new("name" => "Apple") + new_firm.clients_of_firm.push Client.new("name" => "Natural Company") - new_firm.clients_of_firm << (c = Client.new("name" => "Apple")) - assert new_firm.new_record? - assert c.new_record? + assert_equal 1, new_firm.clients_of_firm.size + new_firm.clients_of_firm << c assert_equal 2, new_firm.clients_of_firm.size + assert_equal no_of_firms, Firm.count # Firm was not saved to database. assert_equal no_of_clients, Client.count # Clients were not saved to database. assert new_firm.save @@ -651,6 +654,7 @@ class HasManyAssociationsTest < Test::Unit::TestCase assert_equal new_firm, c.firm assert_equal no_of_firms+1, Firm.count # Firm was saved to database. assert_equal no_of_clients+2, Client.count # Clients were saved to database. + assert_equal 2, new_firm.clients_of_firm.size assert_equal 2, new_firm.clients_of_firm(true).size end diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb index bd9666bc79..3335ab6019 100755 --- a/activerecord/test/base_test.rb +++ b/activerecord/test/base_test.rb @@ -1288,12 +1288,12 @@ class BasicsTest < Test::Unit::TestCase client_new = Client.new client_new.name = "The Joneses" clients = [ client_stored, client_new ] - + firm.clients << clients + assert_equal clients.map(&:name).to_set, firm.clients.map(&:name).to_set firm.clear_association_cache - - assert_equal firm.clients.collect{ |x| x.name }.sort, clients.collect{ |x| x.name }.sort + assert_equal clients.map(&:name).to_set, firm.clients.map(&:name).to_set end def test_interpolate_sql