Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SystemStackError when paranoid models have a circular dependent: :destroy #446

Open
wants to merge 1 commit into
base: core
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
sudo: false
language: ruby
before_install: gem update --system
before_install:
- gem uninstall -v '>= 2' -i $(rvm gemdir)@global -ax bundler || true
- gem install bundler -v '< 2'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

taken from here

cache: bundler
rvm:
- 2.2
Expand Down
5 changes: 1 addition & 4 deletions lib/paranoia.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
require 'active_record' unless defined? ActiveRecord

if [ActiveRecord::VERSION::MAJOR, ActiveRecord::VERSION::MINOR] == [5, 2] ||
ActiveRecord::VERSION::MAJOR > 5
require 'paranoia/active_record_5_2'
end
require 'paranoia/active_record_5_2'

module Paranoia
@@default_sentinel_value = nil
Expand Down
6 changes: 6 additions & 0 deletions lib/paranoia/active_record_5_2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ def handle_dependency

case options[:dependent]
when :destroy
@_destroy_callback_already_called ||= false
return if @_destroy_callback_already_called
@_destroy_callback_already_called = true
target.destroy
if target.respond_to?(:paranoia_destroyed?)
raise ActiveRecord::Rollback unless target.paranoia_destroyed?
Expand All @@ -23,7 +26,10 @@ def delete(method = options[:dependent])
when :delete
target.delete
when :destroy
@_destroy_callback_already_called ||= false
return if @_destroy_callback_already_called
target.destroyed_by_association = reflection
@_destroy_callback_already_called = true
target.destroy
if target.respond_to?(:paranoia_destroyed?)
throw(:abort) unless target.paranoia_destroyed?
Expand Down
28 changes: 26 additions & 2 deletions test/paranoia_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ def setup!
'active_column_models' => 'deleted_at DATETIME, active BOOLEAN',
'active_column_model_with_uniqueness_validations' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN',
'paranoid_model_with_belongs_to_active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN, active_column_model_with_has_many_relationship_id INTEGER',
'active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN',
'without_default_scope_models' => 'deleted_at DATETIME'
'active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN',
'without_default_scope_models' => 'deleted_at DATETIME',
'paranoid_has_one_dependent_destroys' => 'deleted_at DATETIME',
'paranoid_belongs_to_dependent_destroys' => 'deleted_at DATETIME, paranoid_has_one_dependent_destroy_id INTEGER'
}.each do |table_name, columns_as_sql_string|
ActiveRecord::Base.connection.execute "CREATE TABLE #{table_name} (id INTEGER NOT NULL PRIMARY KEY, #{columns_as_sql_string})"
end
Expand Down Expand Up @@ -1053,6 +1055,18 @@ def test_counter_cache_column_on_restore
related_model.restore
assert_equal 1, parent_model_with_counter_cache_column.reload.related_models_count
end

def test_circular_dependent_destroy_has_one_belongs_to
has_one_dependent_destroy = ParanoidHasOneDependentDestroy.create
belongs_to_dependent_destroy = ParanoidBelongsToDependentDestroy.create
belongs_to_dependent_destroy.paranoid_has_one_dependent_destroy = has_one_dependent_destroy
belongs_to_dependent_destroy.save!

belongs_to_dependent_destroy.destroy

refute has_one_dependent_destroy.reload.deleted_at.nil?
refute belongs_to_dependent_destroy.reload.deleted_at.nil?
end
end

private
Expand Down Expand Up @@ -1356,6 +1370,16 @@ class PolymorphicModel < ActiveRecord::Base
belongs_to :parent, polymorphic: true
end

class ParanoidHasOneDependentDestroy < ActiveRecord::Base
acts_as_paranoid
has_one :paranoid_belongs_to_dependent_destroy, dependent: :destroy
end

class ParanoidBelongsToDependentDestroy < ActiveRecord::Base
acts_as_paranoid
belongs_to :paranoid_has_one_dependent_destroy, dependent: :destroy
end

module Namespaced
def self.table_name_prefix
"namespaced_"
Expand Down