merge rails4 branch and fix conflict

.travis.yml

@@ -4,8 +4,7 @@ rvm:
   - 2.0.0
   - 2.1.0
   - jruby-19mode
-  - rbx
 
 env:
-  - RAILS='~> 4.0.4'
-  - RAILS='~> 4.1.0'
+  - RAILS='~> 4.0.8'
+  - RAILS='~> 4.1.4'

Gemfile

@@ -9,7 +9,7 @@ platforms :rbx do
   gem 'rubinius-developer_tools'
 end
 
-rails = ENV['RAILS'] || '~> 4.0.2'
+rails = ENV['RAILS'] || '~> 4.1.4'
 
 gem 'rails', rails
 

README.md

@@ -6,7 +6,7 @@ You would use either plugin / gem if you wished that when you called `destroy` o
 
 If you wish to actually destroy an object you may call `really_destroy!`. **WARNING**: This will also *really destroy* all `dependent: destroy` records, so please aim this method away from face when using.**
 
-If a record has `has_many` associations defined AND those associations have `dependent: :destroy` set on them, then they will also be soft-deleted if `acts_as_paranoid` is set,  otherwise the normal destroy will be called. 
+If a record has `has_many` associations defined AND those associations have `dependent: :destroy` set on them, then they will also be soft-deleted if `acts_as_paranoid` is set,  otherwise the normal destroy will be called.
 
 ## Installation & Usage
 
@@ -75,9 +75,9 @@ Hey presto, it's there! Calling `destroy` will now set the `deleted_at` column:
 ``` ruby
 >> client.deleted_at
 # => nil
->> client.destroy 
+>> client.destroy
 # => client
->> client.deleted_at 
+>> client.deleted_at
 # => [current timestamp]
 ```
 
@@ -178,6 +178,48 @@ You can replace the older `acts_as_paranoid` methods as follows:
 |`find_with_deleted(:first)` | `Client.with_deleted.first`    |
 |`find_with_deleted(id)`     | `Client.with_deleted.find(id)` |
 
+
+The `recover` method in `acts_as_paranoid` runs `update` callbacks.  Paranoia's
+`restore` method does not do this.
+
+## Support for Unique Keys with Null Values
+
+With most databases, a unique key containing a null value will not be enforced because the null value is unique per row.
+
+``` ruby
+class AddDeletedAtToClients < ActiveRecord::Migration
+  def change
+    add_column :clients, :deleted_at, :datetime
+    add_index :clients, [:username, :deleted_at], unique: true
+  end
+end
+```
+
+Given the migration above, you could have multiple users with username bob given the following inserts: ('bob', null), ('bob', null), ('bob', null). We can agree this is not the expected behavior.
+
+To avoid this problem, we could use a flag column instead of a datetime, but the datetime value has intrinsic usefulness.  Instead, we can add a second column for the unique key that always has a value, in this case 0 or 1:
+
+``` ruby
+class AddDeletedAtToClients < ActiveRecord::Migration
+  def change
+    add_column :clients, :deleted_at, :datetime
+    add_column :clients, :is_deleted, :boolean, null: false, default: 0
+    add_index :clients, [:username, :is_deleted], unique: true
+  end
+end
+```
+
+Support this new column by updating your model as such:
+
+``` ruby
+class Client < ActiveRecord::Base
+  acts_as_paranoid :flag_column => :is_deleted
+  ...
+end
+```
+
+If you create an index on the flag column, and you want paranoia to use that index instead of deleted_at, you can add `:index_column => :is_deleted` to the acts_as_paranoid definition.
+
 ## License
 
 This gem is released under the MIT license.

lib/paranoia.rb

@@ -11,14 +11,14 @@ module Paranoia
 
     def with_deleted
       if ActiveRecord::VERSION::STRING >= "4.1"
-        unscope where: paranoia_column
+        unscope where: paranoia_indexed_column
       else
         all.tap { |x| x.default_scoped = false }
       end
     end
 
     def only_deleted
-      with_deleted.where.not(paranoia_column => nil)
+      with_deleted.where.not(paranoia_indexed_column => paranoia_false_value)
     end
     alias :deleted :only_deleted
 
@@ -29,6 +29,12 @@ module Paranoia
         only_deleted.find(id).restore!(opts)
       end
     end
+
+    private
+
+    def paranoia_false_value
+      (paranoia_indexed_column == paranoia_column) ? nil : 0
+    end
   end
 
   module Callbacks
@@ -50,7 +56,11 @@ module Paranoia
   end
 
   def destroy
-    callbacks_result = run_callbacks(:destroy) { touch_paranoia_column(true) }
+    callbacks_result = transaction do
+      run_callbacks(:destroy) do
+        touch_paranoia_column
+      end
+    end
     callbacks_result ? self : false
   end
 
@@ -70,9 +80,10 @@ module Paranoia
   end
 
   def restore!(opts = {})
-    ActiveRecord::Base.transaction do
+    self.class.transaction do
       run_callbacks(:restore) do
         update_column paranoia_column, nil
+        update_column(paranoia_flag_column, false) if paranoia_flag_column
         restore_associated_records if opts[:recursive]
       end
     end
@@ -86,7 +97,12 @@ module Paranoia
 
   private
 
-  # touch paranoia column.
+  def mark_columns_deleted
+    update_column(paranoia_flag_column, true) if paranoia_flag_column
+    touch(paranoia_column)
+  end
+
+  # touch paranoia column, update flag column if necessary
   # insert time to paranoia column.
   # @param with_transaction [Boolean] exec with ActiveRecord Transactions.
   def touch_paranoia_column(with_transaction=false)
@@ -95,9 +111,11 @@ module Paranoia
     # Let's not touch it if it's frozen.
     unless self.frozen?
       if with_transaction
-        with_transaction_returning_status { touch(paranoia_column) }
+        with_transaction_returning_status do
+          mark_columns_deleted
+        end
       else
-        touch(paranoia_column)
+        mark_columns_deleted
       end
     end
   end
@@ -136,7 +154,7 @@ class ActiveRecord::Base
     alias :destroy! :destroy
     alias :delete! :delete
     def really_destroy!
-      dependent_reflections = self.reflections.select do |name, reflection|
+      dependent_reflections = self.class.reflections.select do |name, reflection|
         reflection.options[:dependent] == :destroy
       end
       if dependent_reflections.any?
@@ -145,16 +163,22 @@ class ActiveRecord::Base
           # Paranoid models will have this method, non-paranoid models will not
           associated_records = associated_records.with_deleted if associated_records.respond_to?(:with_deleted)
           associated_records.each(&:really_destroy!)
+          self.send(name).reload
         end
       end
+      touch_paranoia_column if ActiveRecord::VERSION::STRING >= "4.1"
       destroy!
     end
 
     include Paranoia
     class_attribute :paranoia_column
+    class_attribute :paranoia_flag_column
+    class_attribute :paranoia_indexed_column
 
     self.paranoia_column = options[:column] || :deleted_at
-    default_scope { where(paranoia_column => nil) }
+    self.paranoia_flag_column = options[:flag_column] || nil
+    self.paranoia_indexed_column = options[:indexed_column] || paranoia_column
+    default_scope { where(paranoia_indexed_column => paranoia_false_value) }
 
     before_restore {
       self.class.notify_observers(:before_restore, self) if self.class.respond_to?(:notify_observers)
@@ -187,6 +211,14 @@ class ActiveRecord::Base
 
   private
 
+  def paranoia_flag_column
+    self.class.paranoia_flag_column
+  end
+
+  def paranoia_indexed_column
+    self.class.paranoia_indexed_column
+  end
+
   def paranoia_column
     self.class.paranoia_column
   end

test/paranoia_test.rb

@@ -9,22 +9,33 @@ else
 end
 require File.expand_path(File.dirname(__FILE__) + "/../lib/paranoia")
 
-ActiveRecord::Base.establish_connection :adapter => 'sqlite3', database: ':memory:'
-ActiveRecord::Base.connection.execute 'CREATE TABLE parent_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_model_with_belongs (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME, paranoid_model_with_has_one_id INTEGER)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_model_with_anthor_class_name_belongs (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME, paranoid_model_with_has_one_id INTEGER)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_model_with_foreign_key_belongs (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME, has_one_foreign_key_id INTEGER)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE featureful_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME, name VARCHAR(32))'
-ActiveRecord::Base.connection.execute 'CREATE TABLE plain_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE callback_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE fail_callback_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE related_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER NOT NULL, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE employers (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE employees (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE jobs (id INTEGER NOT NULL PRIMARY KEY, employer_id INTEGER NOT NULL, employee_id INTEGER NOT NULL, deleted_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE custom_column_models (id INTEGER NOT NULL PRIMARY KEY, destroyed_at DATETIME)'
-ActiveRecord::Base.connection.execute 'CREATE TABLE non_paranoid_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER)'
+def connect!
+  ActiveRecord::Base.establish_connection :adapter => 'sqlite3', database: ':memory:'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE parent_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_model_with_belongs (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME, paranoid_model_with_has_one_id INTEGER)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_model_with_anthor_class_name_belongs (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME, paranoid_model_with_has_one_id INTEGER)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE paranoid_model_with_foreign_key_belongs (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME, has_one_foreign_key_id INTEGER)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE featureful_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME, name VARCHAR(32))'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE plain_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME, is_deleted tinyint(1) not null default 0)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE callback_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE fail_callback_models (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE related_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER NOT NULL, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE asplode_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE employers (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE employees (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE jobs (id INTEGER NOT NULL PRIMARY KEY, employer_id INTEGER NOT NULL, employee_id INTEGER NOT NULL, deleted_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE custom_column_models (id INTEGER NOT NULL PRIMARY KEY, destroyed_at DATETIME)'
+  ActiveRecord::Base.connection.execute 'CREATE TABLE non_paranoid_models (id INTEGER NOT NULL PRIMARY KEY, parent_model_id INTEGER)'
+end
+
+class WithDifferentConnection < ActiveRecord::Base
+  establish_connection adapter: 'sqlite3', database: ':memory:'
+  connection.execute 'CREATE TABLE with_different_connections (id INTEGER NOT NULL PRIMARY KEY, deleted_at DATETIME)'
+  acts_as_paranoid
+end
+
+connect!
 
 class ParanoiaTest < test_framework
   def setup
@@ -101,7 +112,7 @@ class ParanoiaTest < test_framework
     assert_equal nil, model.instance_variable_get(:@validate_called)
     assert_equal nil, model.instance_variable_get(:@destroy_callback_called)
     assert_equal nil, model.instance_variable_get(:@after_destroy_callback_called)
-    assert_equal nil, model.instance_variable_get(:@after_commit_callback_called)
+    assert model.instance_variable_get(:@after_commit_callback_called)
   end
 
   def test_destroy_behavior_for_paranoid_models
@@ -481,9 +492,8 @@ class ParanoiaTest < test_framework
     assert_equal false, hasOne.reload.deleted_at.nil?
 
     # Does it raise NoMethodException on restore of nil
-    assert_nothing_raised do
-      hasOne.restore(:recursive => true)
-    end
+    hasOne.restore(:recursive => true)
+    
     assert hasOne.reload.deleted_at.nil?
   end
 
@@ -503,6 +513,32 @@ class ParanoiaTest < test_framework
     # essentially, we're just ensuring that this doesn't crash
   end
 
+  def test_destroy_flagged_model
+    a = FlaggedModel.create
+    assert_equal false, a.is_deleted?
+
+    a.destroy
+    assert_equal true, a.is_deleted?
+  end
+
+  def test_restore_flagged_model
+    a = FlaggedModel.create
+    a.destroy
+    a.restore!
+    assert_equal false, a.is_deleted?
+    assert_equal false, a.deleted?
+  end
+
+  def test_uses_flagged_index
+    a = FlaggedModelWithCustomIndex.create(is_deleted: 0)
+    assert_equal 1, FlaggedModelWithCustomIndex.count
+    a.destroy
+    assert_equal 0, FlaggedModelWithCustomIndex.count
+    assert FlaggedModelWithCustomIndex.all.to_sql.index(FlaggedModelWithCustomIndex.paranoia_indexed_column.to_s)
+    assert !FlaggedModelWithCustomIndex.all.to_sql.index( FlaggedModelWithCustomIndex.paranoia_column.to_s)
+  end
+
+
   def test_i_am_the_destroyer
     output = capture(:stdout) { ParanoidModel.I_AM_THE_DESTROYER! }
     assert_equal %Q{
@@ -512,6 +548,36 @@ class ParanoiaTest < test_framework
 }, output
   end
 
+  def test_destroy_fails_if_callback_raises_exception
+    parent = AsplodeModel.create
+
+    assert_raises(StandardError) { parent.destroy }
+
+    #transaction should be rolled back, so parent NOT deleted
+    refute parent.destroyed?, 'Parent record was destroyed, even though AR callback threw exception'
+  end
+
+  def test_destroy_fails_if_association_callback_raises_exception
+    parent = ParentModel.create
+    children = []
+    3.times { children << parent.asplode_models.create }
+
+    assert_raises(StandardError) { parent.destroy }
+
+    #transaction should be rolled back, so parent and children NOT deleted
+    refute parent.destroyed?, 'Parent record was destroyed, even though AR callback threw exception'
+    refute children.any?(&:destroyed?), 'Child record was destroyed, even though AR callback threw exception'
+  end
+
+  def test_restore_model_with_different_connection
+    ActiveRecord::Base.remove_connection # Disconnect the main connection
+    a = WithDifferentConnection.create
+    a.destroy!
+    a.restore!
+    # This test passes if no exception is raised
+    connect! # Reconnect the main connection
+  end
+
   private
   def get_featureful_model
     FeaturefulModel.new(:name => "not empty")
@@ -563,6 +629,7 @@ class ParentModel < ActiveRecord::Base
   has_many :related_models
   has_many :very_related_models, :class_name => 'RelatedModel', dependent: :destroy
   has_many :non_paranoid_models, dependent: :destroy
+  has_many :asplode_models, dependent: :destroy
 end
 
 class RelatedModel < ActiveRecord::Base
@@ -630,3 +697,18 @@ class ParanoidModelWithForeignKeyBelong < ActiveRecord::Base
   acts_as_paranoid
   belongs_to :paranoid_model_with_has_one
 end
+
+class FlaggedModel < PlainModel
+  acts_as_paranoid :flag_column => :is_deleted
+end
+
+class FlaggedModelWithCustomIndex < PlainModel
+  acts_as_paranoid :flag_column => :is_deleted, :indexed_column => :is_deleted
+end
+
+class AsplodeModel < ActiveRecord::Base
+  acts_as_paranoid
+  before_destroy do |r|
+    raise StandardError, 'ASPLODE!'
+  end
+end