diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c777af342f..d0dcff801b 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Add support for bidirectional destroy dependencies. + + Fixes #13609. + + Example: + + class Content < ActiveRecord::Base + has_one :position, dependent: :destroy + end + + class Position < ActiveRecord::Base + belongs_to :content, dependent: :destroy + end + + *Seb Jacobs* + * `time` columns can now affected by `time_zone_aware_attributes`. If you have set `config.time_zone` to a value other than `'UTC'`, they will be treated as in that time zone by default in Rails 5.0. If this is not the desired diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index f44e5af5de..07769e8952 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -289,7 +289,14 @@ module ActiveRecord end def destroy #:nodoc: - _run_destroy_callbacks { super } + begin + @_destroy_callback_already_called ||= false + return if @_destroy_callback_already_called + @_destroy_callback_already_called = true + _run_destroy_callbacks { super } + ensure + @_destroy_callback_already_called = false + end end def touch(*) #:nodoc: diff --git a/activerecord/test/cases/associations/bidirectional_destroy_dependencies_test.rb b/activerecord/test/cases/associations/bidirectional_destroy_dependencies_test.rb new file mode 100644 index 0000000000..2b867965ba --- /dev/null +++ b/activerecord/test/cases/associations/bidirectional_destroy_dependencies_test.rb @@ -0,0 +1,41 @@ +require 'cases/helper' +require 'models/content' + +class BidirectionalDestroyDependenciesTest < ActiveRecord::TestCase + fixtures :content, :content_positions + + def setup + Content.destroyed_ids.clear + ContentPosition.destroyed_ids.clear + end + + def test_bidirectional_dependence_when_destroying_item_with_belongs_to_association + content_position = ContentPosition.find(1) + content = content_position.content + assert_not_nil content + + content_position.destroy + + assert_equal [content_position.id], ContentPosition.destroyed_ids + assert_equal [content.id], Content.destroyed_ids + end + + def test_bidirectional_dependence_when_destroying_item_with_has_one_association + content = Content.find(1) + content_position = content.content_position + assert_not_nil content_position + + content.destroy + + assert_equal [content.id], Content.destroyed_ids + assert_equal [content_position.id], ContentPosition.destroyed_ids + end + + def test_bidirectional_dependence_when_destroying_item_with_has_one_association_fails_first_time + content = ContentWhichRequiresTwoDestroyCalls.find(1) + + 2.times { content.destroy } + + assert_equal content.destroyed?, true + end +end diff --git a/activerecord/test/fixtures/content.yml b/activerecord/test/fixtures/content.yml new file mode 100644 index 0000000000..0d12ee03dc --- /dev/null +++ b/activerecord/test/fixtures/content.yml @@ -0,0 +1,3 @@ +content: + id: 1 + title: How to use Rails diff --git a/activerecord/test/fixtures/content_positions.yml b/activerecord/test/fixtures/content_positions.yml new file mode 100644 index 0000000000..9e85773f8e --- /dev/null +++ b/activerecord/test/fixtures/content_positions.yml @@ -0,0 +1,3 @@ +content_positions: + id: 1 + content_id: 1 diff --git a/activerecord/test/models/content.rb b/activerecord/test/models/content.rb new file mode 100644 index 0000000000..140e1dfc78 --- /dev/null +++ b/activerecord/test/models/content.rb @@ -0,0 +1,40 @@ +class Content < ActiveRecord::Base + self.table_name = 'content' + has_one :content_position, dependent: :destroy + + def self.destroyed_ids + @destroyed_ids ||= [] + end + + before_destroy do |object| + Content.destroyed_ids << object.id + end +end + +class ContentWhichRequiresTwoDestroyCalls < ActiveRecord::Base + self.table_name = 'content' + has_one :content_position, foreign_key: 'content_id', dependent: :destroy + + after_initialize do + @destroy_count = 0 + end + + before_destroy do + @destroy_count += 1 + if @destroy_count == 1 + throw :abort + end + end +end + +class ContentPosition < ActiveRecord::Base + belongs_to :content, dependent: :destroy + + def self.destroyed_ids + @destroyed_ids ||= [] + end + + before_destroy do |object| + ContentPosition.destroyed_ids << object.id + end +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 21b23d8e0c..477afe6feb 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -216,6 +216,14 @@ ActiveRecord::Schema.define do add_index :companies, [:firm_id, :type], name: "company_partial_index", where: "rating > 10" add_index :companies, :name, name: 'company_name_index', using: :btree + create_table :content, force: true do |t| + t.string :title + end + + create_table :content_positions, force: true do |t| + t.integer :content_id + end + create_table :vegetables, force: true do |t| t.string :name t.integer :seller_id