From 134d0533056e31b91687b35a072ac09bdad502d2 Mon Sep 17 00:00:00 2001 From: Peter Woo Date: Wed, 27 Aug 2014 16:47:34 -0700 Subject: [PATCH 1/3] Implement :dependent_recovery_window option --- lib/paranoia.rb | 26 ++++++++++++++++++++------ test/paranoia_test.rb | 36 +++++++++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/lib/paranoia.rb b/lib/paranoia.rb index 889a3a1d..099a0831 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -2,6 +2,7 @@ module Paranoia @@default_sentinel_value = nil + @@default_dependent_recovery_window = 120 # Change default_sentinel_value in a rails initilizer def self.default_sentinel_value=(val) @@ -12,6 +13,10 @@ def self.default_sentinel_value @@default_sentinel_value end + def self.default_dependent_recovery_window + @@default_dependent_recovery_window + end + def self.included(klazz) klazz.extend Query klazz.extend Callbacks @@ -63,7 +68,7 @@ def self.extended(klazz) def destroy callbacks_result = transaction do run_callbacks(:destroy) do - touch_paranoia_column + touch_paranoia_column unless destroyed? end end callbacks_result ? self : false @@ -85,16 +90,20 @@ def delete end def restore!(opts = {}) + opts.merge!(:recovery_window => paranoia_dependent_recovery_window) self.class.transaction do run_callbacks(:restore) do # Fixes a bug where the build would error because attributes were frozen. # This only happened on Rails versions earlier than 4.1. noop_if_frozen = ActiveRecord.version < Gem::Version.new("4.1") + deleted_at = send(paranoia_column) if (noop_if_frozen && !@attributes.frozen?) || !noop_if_frozen write_attribute paranoia_column, paranoia_sentinel_value update_column paranoia_column, paranoia_sentinel_value end - restore_associated_records if opts[:recursive] + if opts[:recursive] + restore_associated_records(deleted_at, opts[:recovery_window]) + end end end end @@ -125,7 +134,7 @@ def touch_paranoia_column(with_transaction=false) # restore associated records that have been soft deleted when # we called #destroy - def restore_associated_records + def restore_associated_records(deleted_at, window) destroyed_associations = self.class.reflect_on_all_associations.select do |association| association.options[:dependent] == :destroy end @@ -136,9 +145,12 @@ def restore_associated_records unless association_data.nil? if association_data.paranoid? if association.collection? - association_data.only_deleted.each { |record| record.restore(:recursive => true) } + x = association_data.only_deleted. + where("#{association.quoted_table_name}.#{paranoia_column} < ?", deleted_at + window). + where("#{association.quoted_table_name}.#{paranoia_column} > ?", deleted_at - window). + each { |record| record.restore(:recursive => true) } else - association_data.restore(:recursive => true) + association_data.restore(:recursive => true, :recovery_window => window) end end end @@ -172,10 +184,12 @@ def really_destroy! end include Paranoia - class_attribute :paranoia_column, :paranoia_sentinel_value + class_attribute :paranoia_column, :paranoia_sentinel_value, :paranoia_dependent_recovery_window self.paranoia_column = options[:column] || :deleted_at self.paranoia_sentinel_value = options.fetch(:sentinel_value) { Paranoia.default_sentinel_value } + self.paranoia_dependent_recovery_window = options[:dependent_recovery_window] || Paranoia.default_dependent_recovery_window + default_scope { where(paranoia_column => paranoia_sentinel_value) } before_restore { diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index 4c2f9060..e95ac70d 100644 --- a/test/paranoia_test.rb +++ b/test/paranoia_test.rb @@ -43,6 +43,19 @@ def setup end end + def with_stubbed_current_time(value, &block) + metaclass = Time.instance_eval{ class << self; self; end } + metaclass.send(:alias_method, :__original_time_now, :now) + metaclass.send(:define_method, :now){ value } + begin + block.call + ensure + metaclass.send(:undef_method, :now) + metaclass.send(:alias_method, :now, :__original_time_now) + metaclass.send(:undef_method, :__original_time_now) + end + end + def test_plain_model_class_is_not_paranoid assert_equal false, PlainModel.paranoid? end @@ -410,6 +423,7 @@ def test_restore_with_associations parent = ParentModel.create first_child = parent.very_related_models.create second_child = parent.non_paranoid_models.create + third_child = parent.very_related_models.create parent.destroy assert_equal false, parent.deleted_at.nil? @@ -432,6 +446,18 @@ def test_restore_with_associations assert_equal true, parent.reload.deleted_at.nil? assert_equal true, first_child.reload.deleted_at.nil? assert_equal true, second_child.destroyed? + + with_stubbed_current_time(Time.at(0)) do + first_child.destroy + end + with_stubbed_current_time(Time.at(3600)) do + parent.destroy + end + ParentModel.restore(parent.id, :recursive => true, :recovery_window => 5.minute) + assert_equal true, parent.reload.deleted_at.nil? + assert_equal false, first_child.reload.deleted_at.nil? + assert_equal true, second_child.destroyed? + assert_equal true, third_child.reload.deleted_at.nil? end # regression tests for #118 @@ -463,22 +489,22 @@ def test_restore_with_nil_has_one_association # Does it raise NoMethodException on restore of nil hasOne.restore(:recursive => true) - + assert hasOne.reload.deleted_at.nil? end - + # covers #131 def test_has_one_really_destroy_with_nil model = ParanoidModelWithHasOne.create model.really_destroy! - + refute ParanoidModelWithBelong.unscoped.exists?(model.id) end - + def test_has_one_really_destroy_with_record model = ParanoidModelWithHasOne.create { |record| record.build_paranoid_model_with_belong } model.really_destroy! - + refute ParanoidModelWithBelong.unscoped.exists?(model.id) end From 9d49362f2c2ef532646e88e8fef944bca7645f23 Mon Sep 17 00:00:00 2001 From: Patrick Koperwas Date: Wed, 3 Dec 2014 16:33:52 -0800 Subject: [PATCH 2/3] Create failing test Restoring polymorphic has_one relationships errored because paranoia was not correctly looking up the foreign_key. Output from failing test - ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: parent_model_id: SELECT "polymorphic_models".* FROM "polymorphic_models" WHERE ("polymorphic_models"."deleted_at" IS NOT NULL) AND (parent_model_id) ORDER BY "polymorphic_models"."id" ASC LIMIT 1 The test sets up a PolymorphicModel, which has_many parents. The ParentModel then has a has_one relationship with PolymorphicModel. When restoring, the foreign key is set as - `self.class.name.to_s.underscore_id` which will be parent_model_id, instead of the :as option. --- test/paranoia_test.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index 63cb9b67..9a0cb6a9 100644 --- a/test/paranoia_test.rb +++ b/test/paranoia_test.rb @@ -28,6 +28,7 @@ def connect! ActiveRecord::Base.connection.execute 'CREATE TABLE custom_column_models (id INTEGER NOT NULL PRIMARY KEY, destroyed_at DATETIME)' ActiveRecord::Base.connection.execute 'CREATE TABLE custom_sentinel_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME NOT NULL)' ActiveRecord::Base.connection.execute 'CREATE TABLE non_paranoid_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER)' + ActiveRecord::Base.connection.execute 'CREATE TABLE polymorphic_models (id INTEGER NOT NULL PRIMARY KEY, parent_id INTEGER, parent_type STRING, deleted_at DATETIME)' end class WithDifferentConnection < ActiveRecord::Base @@ -646,6 +647,19 @@ def test_restore_clear_association_cache_if_associations_present assert_equal 3, parent.very_related_models.size end + def test_restore_recursive_on_polymorphic_has_one_association + parent = ParentModel.create + polymorphic = PolymorphicModel.create(parent: parent) + + parent.destroy + + assert_equal 0, polymorphic.class.count + + parent.restore(recursive: true) + + assert_equal 1, polymorphic.class.count + end + private def get_featureful_model FeaturefulModel.new(:name => "not empty") @@ -698,6 +712,7 @@ class ParentModel < ActiveRecord::Base has_many :very_related_models, :class_name => 'RelatedModel', dependent: :destroy has_many :non_paranoid_models, dependent: :destroy has_many :asplode_models, dependent: :destroy + has_one :polymorphic_model, as: :parent, dependent: :destroy end class RelatedModel < ActiveRecord::Base @@ -784,3 +799,8 @@ class AsplodeModel < ActiveRecord::Base raise StandardError, 'ASPLODE!' end end + +class PolymorphicModel < ActiveRecord::Base + acts_as_paranoid + belongs_to :parent, polymorphic: true +end From 99999dddcbcceb8438a9f4064f1849b04a2824e0 Mon Sep 17 00:00:00 2001 From: Patrick Koperwas Date: Wed, 3 Dec 2014 16:41:40 -0800 Subject: [PATCH 3/3] Fixes restoring polymorphic has_one relationships If association is a has_one relationship with an :as option, it will have a type attribute (see https://github.com/rails/docrails/blob/master/activerecord/lib/active_record/reflection.rb). If it is present, that will be the type column on the polymorphic model. The foreign key can be found as an attribute on association. --- lib/paranoia.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/paranoia.rb b/lib/paranoia.rb index 633c0160..1ac56846 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -155,8 +155,16 @@ def restore_associated_records(deleted_at, window) if association_data.nil? && association.macro.to_s == "has_one" association_class_name = association.options[:class_name].present? ? association.options[:class_name] : association.name.to_s.camelize - association_foreign_key = association.options[:foreign_key].present? ? association.options[:foreign_key] : "#{self.class.name.to_s.underscore}_id" - Object.const_get(association_class_name).only_deleted.where(association_foreign_key, self.id).first.try(:restore, recursive: true) + association_foreign_key = association.foreign_key.present? ? association.foreign_key : "#{self.class.name.to_s.underscore}_id" + + if association.type + association_polymorphic_type = association.type + where_conditions = { association_polymorphic_type => self.class.name.to_s, association_foreign_key => self.id } + else + where_conditions = { association_foreign_key => self.id } + end + + Object.const_get(association_class_name).only_deleted.where(where_conditions).first.try(:restore, recursive: true) end end