From b260275dfcecd05f2c23959d37897c2f94d8c713 Mon Sep 17 00:00:00 2001 From: Andy Klimczak Date: Sun, 10 Jun 2018 13:15:42 -0400 Subject: [PATCH] Fix `SystemStackError` when paranoid models have a circular `dependent: :destroy` - Previously, paranoid models with a circular `dependent: :destroy` would recursively try to delete eachother - Add a variable to check if the destroy callback has already been called for that model - Similar fix to rails/rails#18548 --- .travis.yml | 25 ++++++++++++++++------- CHANGELOG.md | 6 ++++++ Gemfile | 16 ++++++++++++--- lib/paranoia.rb | 6 ++---- lib/paranoia/active_record_5_2.rb | 6 ++++++ lib/paranoia/version.rb | 2 +- paranoia.gemspec | 4 ++-- test/paranoia_test.rb | 34 ++++++++++++++++++++++++++----- 8 files changed, 77 insertions(+), 22 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8828fbfa..78b46b95 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,25 +4,36 @@ before_install: gem update --system cache: bundler rvm: - 2.2 - - 2.3.5 - - 2.4.3 - - 2.5.0 + - 2.3.8 + - 2.4.5 + - 2.5.3 + - 2.6.3 - jruby-9.1.6.0 env: matrix: - - RAILS='~> 4.2.0' - - RAILS='~> 5.0.0' + - RAILS='~> 4.2.0' SQLITE_VERSION='~> 1.3.6' + - RAILS='~> 5.0.0' SQLITE_VERSION='~> 1.3.6' - RAILS='~> 5.1.0' - RAILS='~> 5.2.0' + - RAILS='master' matrix: allow_failures: - - env: RAILS='~> 4.2.0' + - env: RAILS='~> 4.2.0' SQLITE_VERSION='~> 1.3.6' rvm: jruby-9.1.6.0 - - env: RAILS='~> 5.0.0' + - env: RAILS='~> 5.0.0' SQLITE_VERSION='~> 1.3.6' rvm: jruby-9.1.6.0 - env: RAILS='~> 5.1.0' rvm: jruby-9.1.6.0 - env: RAILS='~> 5.2.0' rvm: jruby-9.1.6.0 + - env: RAILS='master' + rvm: jruby-9.1.6.0 + exclude: + - rvm: 2.2 + env: RAILS='master' + - rvm: 2.3.8 + env: RAILS='master' + - rvm: 2.4.5 + env: RAILS='master' diff --git a/CHANGELOG.md b/CHANGELOG.md index 35d25547..08ac9d61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # paranoia Changelog +## 2.4.2 + +* [#470](https://github.com/rubysherpas/paranoia/pull/470) Add support for ActiveRecord 6.0 + + [Anton Kolodii](https://github.com/iggant), [Jared Norman](https://github.com/jarednorman) + ## 2.4.1 * [#435](https://github.com/rubysherpas/paranoia/pull/435) Monkeypatch activerecord relations to work with rails 5.2.0 diff --git a/Gemfile b/Gemfile index fbc29caa..6c061d29 100644 --- a/Gemfile +++ b/Gemfile @@ -1,20 +1,30 @@ source 'https://rubygems.org' -gem 'sqlite3', platforms: [:ruby] +sqlite = ENV['SQLITE_VERSION'] + +if sqlite + gem 'sqlite3', sqlite, platforms: [:ruby] +else + gem 'sqlite3', platforms: [:ruby] +end platforms :jruby do gem 'activerecord-jdbcsqlite3-adapter' end platforms :rbx do + gem 'rubinius-developer_tools' gem 'rubysl', '~> 2.0' gem 'rubysl-test-unit' - gem 'rubinius-developer_tools' end rails = ENV['RAILS'] || '~> 5.2.0' -gem 'rails', rails +if rails == 'master' + gem 'rails', github: 'rails/rails' +else + gem 'rails', rails +end # Specify your gem's dependencies in paranoia.gemspec gemspec diff --git a/lib/paranoia.rb b/lib/paranoia.rb index 4ba4c389..ac8b6f0f 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -1,8 +1,6 @@ require 'active_record' unless defined? ActiveRecord -if [ActiveRecord::VERSION::MAJOR, ActiveRecord::VERSION::MINOR] == [5, 2] - require 'paranoia/active_record_5_2' -end +require 'paranoia/active_record_5_2' module Paranoia @@default_sentinel_value = nil @@ -305,7 +303,7 @@ def build_relation(klass, *args) class UniquenessValidator < ActiveModel::EachValidator prepend UniquenessParanoiaValidator end - + class AssociationNotSoftDestroyedValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) # if association is soft destroyed, add an error diff --git a/lib/paranoia/active_record_5_2.rb b/lib/paranoia/active_record_5_2.rb index c9c5463f..f28202d5 100644 --- a/lib/paranoia/active_record_5_2.rb +++ b/lib/paranoia/active_record_5_2.rb @@ -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? @@ -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? diff --git a/lib/paranoia/version.rb b/lib/paranoia/version.rb index 1b2fb121..046171d6 100644 --- a/lib/paranoia/version.rb +++ b/lib/paranoia/version.rb @@ -1,3 +1,3 @@ module Paranoia - VERSION = '2.4.1'.freeze + VERSION = '2.4.2'.freeze end diff --git a/paranoia.gemspec b/paranoia.gemspec index 18a9768f..a6a932b0 100644 --- a/paranoia.gemspec +++ b/paranoia.gemspec @@ -24,9 +24,9 @@ Gem::Specification.new do |s| s.required_ruby_version = '>= 2.0' - s.add_dependency 'activerecord', '>= 4.0', '< 5.3' + s.add_dependency 'activerecord', '>= 4.0', '< 6.1' - s.add_development_dependency "bundler", ">= 1.0.0" + s.add_development_dependency "bundler", ">= 1.3.0" s.add_development_dependency "rake" s.files = `git ls-files`.split("\n") diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index e08f9a7b..ec7e20f7 100644 --- a/test/paranoia_test.rb +++ b/test/paranoia_test.rb @@ -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 @@ -189,11 +191,11 @@ def test_scoping_behavior_for_paranoid_models p2 = ParanoidModel.create(:parent_model => parent2) p1.destroy p2.destroy - + assert_equal 0, parent1.paranoid_models.count assert_equal 1, parent1.paranoid_models.only_deleted.count - assert_equal 2, ParanoidModel.only_deleted.joins(:parent_model).count + assert_equal 2, ParanoidModel.only_deleted.joins(:parent_model).count assert_equal 1, parent1.paranoid_models.deleted.count assert_equal 0, parent1.paranoid_models.without_deleted.count p3 = ParanoidModel.create(:parent_model => parent1) @@ -206,7 +208,7 @@ def test_only_deleted_with_joins c1 = ActiveColumnModelWithHasManyRelationship.create(name: 'Jacky') c2 = ActiveColumnModelWithHasManyRelationship.create(name: 'Thomas') p1 = ParanoidModelWithBelongsToActiveColumnModelWithHasManyRelationship.create(name: 'Hello', active_column_model_with_has_many_relationship: c1) - + c1.destroy assert_equal 1, ActiveColumnModelWithHasManyRelationship.count assert_equal 1, ActiveColumnModelWithHasManyRelationship.only_deleted.count @@ -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 @@ -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_"