From efaf2af07c4238e911899988871e575d8ef5805b Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 28 Sep 2005 03:52:57 +0000 Subject: r3653@asus: jeremy | 2005-09-28 00:23:49 -0700 Ticket 2221 - model.association.clear should destroy associated objects if :dependent => true instead of nullifying their foreign keys git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2384 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 + activerecord/lib/active_record/associations.rb | 4 +- .../associations/has_many_association.rb | 8 ++- activerecord/test/associations_test.rb | 65 ++++++++++++++++++---- activerecord/test/fixtures/company.rb | 16 ++++++ 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index ada277460d..2eb8c0a98e 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* model.association.clear should destroy associated objects if :dependent => true instead of nullifying their foreign keys. #2221 [joergd@pobox.com, ObieFernandez ] + * Returning false from before_destroy should cancel the action. #1829 [Jeremy Huffman] * Recognize PostgreSQL NOW() default as equivalent to CURRENT_TIMESTAMP or CURRENT_DATE, depending on the column's type. #2256 [mat ] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b2a2ed6e11..dce4f350e8 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -227,7 +227,9 @@ module ActiveRecord # This will also destroy the objects if they're declared as belongs_to and dependent on this model. # * collection=objects - replaces the collections content by deleting and adding objects as appropriate. # * collection_singular_ids=ids - replace the collection by the objects identified by the primary keys in +ids+ - # * collection.clear - removes every object from the collection. This does not destroy the objects. + # * collection.clear - removes every object from the collection. This destroys the associated objects if they + # are :dependent, deletes them directly from the database if they are :exclusively_dependent, + # and sets their foreign keys to NULL otherwise. # * collection.empty? - returns true if there are no associated objects. # * collection.size - returns the number of associated objects. # * collection.find - finds an associated object according to the same rules as Base.find. diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index f9ca655560..2c881290aa 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -87,7 +87,13 @@ module ActiveRecord # Removes all records from this association. Returns +self+ so # method calls may be chained. def clear - @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = #{@owner.quoted_id}") + if @options[:dependent] + each { |associate| associate.destroy } + elsif @options[:exclusively_dependent] + @association_class.delete_all("#{@association_class_primary_key_name} = #{@owner.quoted_id}") + else + @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = #{@owner.quoted_id}") + end @target = [] self end diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index 2bbccb5c9e..d0b560d112 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -23,7 +23,7 @@ raise "ActiveRecord should have barked on bad collection keys" unless bad_collec class AssociationsTest < Test::Unit::TestCase fixtures :accounts, :companies, :developers, :projects, :developers_projects, :computers - + def test_force_reload firm = Firm.new("name" => "A New Firm, Inc") firm.save @@ -257,11 +257,15 @@ end class HasManyAssociationsTest < Test::Unit::TestCase fixtures :accounts, :companies, :developers, :projects, :developers_projects, :topics - + + def setup + Client.destroyed_client_ids.clear + end + def force_signal37_to_load_all_clients_of_firm companies(:first_firm).clients_of_firm.each {|f| } end - + def test_counting assert_equal 2, Firm.find(:first).clients.count end @@ -316,7 +320,6 @@ class HasManyAssociationsTest < Test::Unit::TestCase else assert false, "belongs_to failed unless check" end - end def test_find_ids @@ -499,19 +502,57 @@ class HasManyAssociationsTest < Test::Unit::TestCase force_signal37_to_load_all_clients_of_firm companies(:first_firm).clients_of_firm.create("name" => "Another Client") assert_equal 2, companies(:first_firm).clients_of_firm.size - #companies(:first_firm).clients_of_firm.clear companies(:first_firm).clients_of_firm.delete([companies(:first_firm).clients_of_firm[0], companies(:first_firm).clients_of_firm[1]]) assert_equal 0, companies(:first_firm).clients_of_firm.size assert_equal 0, companies(:first_firm).clients_of_firm(true).size end - def test_deleting_a_association_collection - force_signal37_to_load_all_clients_of_firm - companies(:first_firm).clients_of_firm.create("name" => "Another Client") - assert_equal 2, companies(:first_firm).clients_of_firm.size - companies(:first_firm).clients_of_firm.clear - assert_equal 0, companies(:first_firm).clients_of_firm.size - assert_equal 0, companies(:first_firm).clients_of_firm(true).size + def test_clearing_an_association_collection + firm = companies(:first_firm) + client_id = firm.clients_of_firm.first.id + assert_equal 1, firm.clients_of_firm.size + + firm.clients_of_firm.clear + + assert_equal 0, firm.clients_of_firm.size + assert_equal 0, firm.clients_of_firm(true).size + assert_equal [], Client.destroyed_client_ids[firm.id] + + # Should not be destroyed since the association is not dependent. + assert_not_nil Client.find_by_id(client_id) + end + + def test_clearing_a_dependent_association_collection + firm = companies(:first_firm) + client_id = firm.dependent_clients_of_firm.first.id + assert_equal 1, firm.dependent_clients_of_firm.size + + # :dependent means destroy is called on each client + firm.dependent_clients_of_firm.clear + + assert_equal 0, firm.dependent_clients_of_firm.size + assert_equal 0, firm.dependent_clients_of_firm(true).size + assert_equal [client_id], Client.destroyed_client_ids[firm.id] + + # Should be destroyed since the association is dependent. + assert Client.find_by_id(client_id).nil? + end + + def test_clearing_an_exclusively_dependent_association_collection + firm = companies(:first_firm) + client_id = firm.exclusively_dependent_clients_of_firm.first.id + assert_equal 1, firm.exclusively_dependent_clients_of_firm.size + + # :exclusively_dependent means each client is deleted directly from + # the database without looping through them calling destroy. + firm.exclusively_dependent_clients_of_firm.clear + + assert_equal 0, firm.exclusively_dependent_clients_of_firm.size + assert_equal 0, firm.exclusively_dependent_clients_of_firm(true).size + assert_equal [], Client.destroyed_client_ids[firm.id] + + # Should be destroyed since the association is exclusively dependent. + assert Client.find_by_id(client_id).nil? end def test_deleting_a_item_which_is_not_in_the_collection diff --git a/activerecord/test/fixtures/company.rb b/activerecord/test/fixtures/company.rb index 5af39c1297..748d43eb2e 100755 --- a/activerecord/test/fixtures/company.rb +++ b/activerecord/test/fixtures/company.rb @@ -10,6 +10,8 @@ class Firm < Company has_many :clients, :order => "id", :dependent => true, :counter_sql => "SELECT COUNT(*) FROM companies WHERE firm_id = 1 AND (type = 'Client' OR type = 'SpecialClient' OR type = 'VerySpecialClient' )" has_many :clients_sorted_desc, :class_name => "Client", :order => "id DESC" has_many :clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id" + has_many :dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => true + has_many :exclusively_dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :exclusively_dependent => true has_many :clients_like_ms, :conditions => "name = 'Microsoft'", :class_name => "Client", :order => "id" has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}' has_many :clients_using_counter_sql, :class_name => "Client", @@ -30,6 +32,20 @@ class Client < Company belongs_to :firm_with_basic_id, :class_name => "Firm", :foreign_key => "firm_id" belongs_to :firm_with_other_name, :class_name => "Firm", :foreign_key => "client_of" belongs_to :firm_with_condition, :class_name => "Firm", :foreign_key => "client_of", :conditions => "1 = 1" + + # Record destruction so we can test whether firm.clients.clear has + # is calling client.destroy, deleting from the database, or setting + # foreign keys to NULL. + def self.destroyed_client_ids + @destroyed_client_ids ||= Hash.new { |h,k| h[k] = [] } + end + + before_destroy do |client| + if client.firm + Client.destroyed_client_ids[client.firm.id] << client.id + end + true + end end -- cgit v1.2.3