aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test
diff options
context:
space:
mode:
authorSeb Jacobs <me@sebjacobs.com>2015-01-15 23:03:23 +0000
committerSeb Jacobs <me@sebjacobs.com>2015-01-16 11:41:43 +0000
commitd5bf649a535948e70692e521ce3070595334e71b (patch)
treed1f56e9ff058eceba35785bbeb1d88e54e15e910 /activerecord/test
parent090c5211ced7b728df6176d5c9fc7437c107beaf (diff)
downloadrails-d5bf649a535948e70692e521ce3070595334e71b.tar.gz
rails-d5bf649a535948e70692e521ce3070595334e71b.tar.bz2
rails-d5bf649a535948e70692e521ce3070595334e71b.zip
Add support for bidirectional destroy dependencies
Prior to this commit if you defined a bidirectional relationship between two models with destroy dependencies on both sides, a call to `destroy` would result in an infinite callback loop. Take the following relationship. class Content < ActiveRecord::Base has_one :content_position, dependent: :destroy end class ContentPosition < ActiveRecord::Base belongs_to :content, dependent: :destroy end Calling `Content#destroy` or `ContentPosition#destroy` would result in an infinite callback loop. This commit changes the behaviour of `ActiveRecord::Callbacks#destroy` so that it guards against subsequent callbacks. Thanks to @zetter for demonstrating the issue with failing tests[1]. [1] rails#13609
Diffstat (limited to 'activerecord/test')
-rw-r--r--activerecord/test/cases/associations/bidirectional_destroy_dependencies_test.rb41
-rw-r--r--activerecord/test/fixtures/content.yml3
-rw-r--r--activerecord/test/fixtures/content_positions.yml3
-rw-r--r--activerecord/test/models/content.rb40
-rw-r--r--activerecord/test/schema/schema.rb8
5 files changed, 95 insertions, 0 deletions
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