From a286c32cc0ccf253b495abe983ffffd5146153d3 Mon Sep 17 00:00:00 2001
From: Fernando Gorodscy <fegorodscy@gmail.com>
Date: Mon, 26 Feb 2018 14:20:43 -0500
Subject: Fix dependence on has_one/belongs_to relationships

When a class has a belongs_to or has_one relationship with dependent: :destroy
option enabled, objects of this class should not be deleted if it's dependents
cannot be deleted.

Example:

    class Parent
      has_one :child, dependent: :destroy
    end

    class Child
      belongs_to :parent, inverse_of: :child
      before_destroy { throw :abort }
    end

    c = Child.create
    p = Parent.create(child: c)

    p.destroy

    p.destroyed? # expected: false; actual: true;

Fixes #32022
---
 .../associations/belongs_to_association.rb         | 10 ++++++++-
 .../associations/has_one_association.rb            |  1 +
 .../associations/belongs_to_associations_test.rb   | 24 ++++++++++++++++++++++
 .../associations/has_one_associations_test.rb      | 24 ++++++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index bd2012df84..1109fee462 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -5,7 +5,15 @@ module ActiveRecord
     # = Active Record Belongs To Association
     class BelongsToAssociation < SingularAssociation #:nodoc:
       def handle_dependency
-        target.send(options[:dependent]) if load_target
+        return unless load_target
+
+        case options[:dependent]
+        when :destroy
+          target.destroy
+          raise ActiveRecord::Rollback unless target.destroyed?
+        else
+          target.send(options[:dependent])
+        end
       end
 
       def replace(record)
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 7953b89f61..090b082cb0 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -60,6 +60,7 @@ module ActiveRecord
           when :destroy
             target.destroyed_by_association = reflection
             target.destroy
+            throw(:abort) unless target.destroyed?
           when :nullify
             target.update_columns(reflection.foreign_key => nil) if target.persisted?
           end
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index 9e6d94191b..6b4f826766 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -931,6 +931,30 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert_equal error.message, "The :dependent option must be one of [:destroy, :delete], but is :nullify"
   end
 
+  class DestroyableBook < ActiveRecord::Base
+    self.table_name = "books"
+    belongs_to :author, class_name: "UndestroyableAuthor", dependent: :destroy
+  end
+
+  class UndestroyableAuthor < ActiveRecord::Base
+    self.table_name = "authors"
+    has_one :book, class_name: "DestroyableBook", foreign_key: "author_id"
+    before_destroy :dont
+
+    def dont
+      throw(:abort)
+    end
+  end
+
+  def test_dependency_should_halt_parent_destruction
+    author = UndestroyableAuthor.create!(name: "Test")
+    book = DestroyableBook.create!(author: author)
+
+    assert_no_difference ["UndestroyableAuthor.count", "DestroyableBook.count"] do
+      assert_not book.destroy
+    end
+  end
+
   def test_attributes_are_being_set_when_initialized_from_belongs_to_association_with_where_clause
     new_firm = accounts(:signals37).build_firm(name: "Apple")
     assert_equal new_firm.name, "Apple"
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 1a213ef7e4..602fe52701 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -725,4 +725,28 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
 
     assert_not DestroyByParentBook.exists?(book.id)
   end
+
+  class UndestroyableBook < ActiveRecord::Base
+    self.table_name = "books"
+    belongs_to :author, class_name: "DestroyableAuthor"
+    before_destroy :dont
+
+    def dont
+      throw(:abort)
+    end
+  end
+
+  class DestroyableAuthor < ActiveRecord::Base
+    self.table_name = "authors"
+    has_one :book, class_name: "UndestroyableBook", foreign_key: "author_id", dependent: :destroy
+  end
+
+  def test_dependency_should_halt_parent_destruction
+    author = DestroyableAuthor.create!(name: "Test")
+    UndestroyableBook.create!(author: author)
+
+    assert_no_difference ["DestroyableAuthor.count", "UndestroyableBook.count"] do
+      assert_not author.destroy
+    end
+  end
 end
-- 
cgit v1.2.3