throw :abort does not propagate to parent class

[gorodscy]

Steps to reproduce

• Define the following classes

class Parent
  has_one :child, dependent: :destroy
end

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

• Instanciate as follows:

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

• Then, destroy the parent

p.destroy

Expected behavior

Neither the parent nor the child should be deleted.
throw :abort should propagate to the parent who called :destroy on the child.

p.destroyed? # false
p.child.destroyed? # false

Actual behavior

The parent is being destroyed.

p.destroyed? # true
p.child.destroyed? # false

PS: If a foreign key is set (which might not be the case for polymorphic associations), then parent is not deleted because it would violate a foreign key contraint, but rails would try it anyways.

System configuration

Rails version: 5.1.5

Ruby version: 2.5.0

[pixeltrix]

This is specifically an issue with has_one - has_many works as expected.

Here's a reproduction script:

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "5.1.5"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :children, force: true do |t|
    t.references :parent
  end

  create_table :parents, force: true do |t|
  end
end

class Parent < ActiveRecord::Base
  has_one :child, dependent: :destroy
end

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

class BugTest < Minitest::Test
  def test_destroy
    parent = Parent.create!
    child = parent.create_child!

    parent.destroy

    assert !parent.destroyed?
  end
end

This is also an issue with the 5.2 rc gem as well

[claudiob]

🔬 git bisect shows that the behavior changed in 3a25cdc

[gorodscy]

@claudiob I believe that change is only for returning false in the callback. throw :abort is preferred now. throw :abort should halt the callback chain

[claudiob]

@fegorodscy What you describe is correct.

My comment was referring to @pixeltrix's script, which passes before 3a25cdc and fails after.
That could help identify the root cause of the problem.

[rafaelfranca]

But if is failing on 5.1.5, that doesn't include that commit.

[gorodscy]

@rafaelfranca I just tested with 5.2.0.rc1 and the issue is still there

[pixeltrix]

@fegorodscy I can see what the problem is - has_many uses destroy! to propagate the abort back up to the parent but has_one and belongs_to only call destroy. Unfortunately it isn't as simple as just using destroy! for the singular associations as that breaks another test. 😞

[gorodscy]

@pixeltrix The failing test is 'test_bidirectional_dependence_when_destroying_item_with_belongs_to_association' right? Did you get any other failing test with this change? re: changing destroy to destroy!

So, I tested with gem "activerecord", "5.0" and it passes the test (using the script above). This bug was probably introduced in 5.1 when support for biderectional_dependence was added.

[pixeltrix]

@fegorodscy yes, that's the failing test but removing those changes doesn't fix it - it's just that returning false terminated the callback chain in 5.0 and not 5.1, we still need to call destroy!.