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

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 <obiefernandez@gmail.com>]
+
 * 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 <mat@absolight.fr>]
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.
       # * <tt>collection=objects</tt> - replaces the collections content by deleting and adding objects as appropriate.
       # * <tt>collection_singular_ids=ids</tt> - replace the collection by the objects identified by the primary keys in +ids+
-      # * <tt>collection.clear</tt> - removes every object from the collection. This does not destroy the objects.
+      # * <tt>collection.clear</tt> - removes every object from the collection. This destroys the associated objects if they
+      #   are <tt>:dependent</tt>, deletes them directly from the database if they are <tt>:exclusively_dependent</tt>,
+      #   and sets their foreign keys to NULL otherwise.
       # * <tt>collection.empty?</tt> - returns true if there are no associated objects.
       # * <tt>collection.size</tt> - returns the number of associated objects.
       # * <tt>collection.find</tt> - 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