Fixed paranoid types

* Fixed hooks to work with Resource#destroy when paranoid type used
* Specified behaviour of Resource#destroy and Resource#destroy! when a
  paranoid type is used
* Specified behaviour of Model#with_deleted when a paranoia property is
  declared. Previous behaviour seemed broken, since it would only have
  returned deleted resources, but "with deleted" implies you will be
  returning all resources, including the deleted ones.
* Moved common code from paranoid types into a module

[#520 state:resolved]
[#883 state:resolved]

dm-core.gemspec

@@ -78,6 +78,7 @@ Gem::Specification.new do |s|
      "lib/dm-core/types/boolean.rb",
      "lib/dm-core/types/discriminator.rb",
      "lib/dm-core/types/object.rb",
+     "lib/dm-core/types/paranoid/base.rb",
      "lib/dm-core/types/paranoid_boolean.rb",
      "lib/dm-core/types/paranoid_datetime.rb",
      "lib/dm-core/types/serial.rb",
@@ -103,6 +104,8 @@ Gem::Specification.new do |s|
      "spec/public/model/relationship_spec.rb",
      "spec/public/model_spec.rb",
      "spec/public/property/object_spec.rb",
+     "spec/public/property/paranoid_boolean_spec.rb",
+     "spec/public/property/paranoid_datetime_spec.rb",
      "spec/public/property/text_spec.rb",
      "spec/public/property_spec.rb",
      "spec/public/resource_spec.rb",
@@ -171,6 +174,8 @@ Gem::Specification.new do |s|
      "spec/public/model/relationship_spec.rb",
      "spec/public/model_spec.rb",
      "spec/public/property/object_spec.rb",
+     "spec/public/property/paranoid_boolean_spec.rb",
+     "spec/public/property/paranoid_datetime_spec.rb",
      "spec/public/property/text_spec.rb",
      "spec/public/property_spec.rb",
      "spec/public/resource_spec.rb",

lib/dm-core.rb

@@ -34,6 +34,7 @@
 require dir / 'types' / 'boolean'
 require dir / 'types' / 'discriminator'
 require dir / 'types' / 'text'
+require dir / 'types' / 'paranoid' / 'base'
 require dir / 'types' / 'paranoid_datetime'     # TODO: move to dm-more
 require dir / 'types' / 'paranoid_boolean'      # TODO: move to dm-more
 require dir / 'types' / 'object'

lib/dm-core/model/hook.rb

@@ -6,7 +6,7 @@ module Hook
       def self.included(model)
         model.send(:include, Extlib::Hook)
         model.extend Methods
-        model.register_instance_hooks :create_hook, :update_hook, :destroy
+        model.register_instance_hooks :create_hook, :update_hook, :destroy_hook
       end
 
       module Methods
@@ -29,10 +29,11 @@ def after(target_method, *args, &block)
         # @api private
         def remap_target_method(target_method)
           case target_method
-            when :create then [ :create_hook               ]
-            when :update then [ :update_hook               ]
-            when :save   then [ :create_hook, :update_hook ]
-            else              [ target_method              ]
+            when :create  then [ :create_hook               ]
+            when :update  then [ :update_hook               ]
+            when :save    then [ :create_hook, :update_hook ]
+            when :destroy then [ :destroy_hook              ]
+            else               [ target_method              ]
           end
         end
       end

lib/dm-core/resource.rb

@@ -370,7 +370,8 @@ def save!
     #
     # @api public
     def destroy
-      destroy!
+      return true if destroyed?
+      destroy_hook
     end
 
     # Destroy the instance, remove it from the repository, bypassing hooks
@@ -381,15 +382,7 @@ def destroy
     # @api public
     def destroy!
       return true if destroyed?
-
-      if saved?
-        repository.delete(collection_for_self)
-        reset
-        @_readonly  = true
-        @_destroyed = true
-      else
-        false
-      end
+      _destroy(false)
     end
 
     # Compares another Resource for equality
@@ -634,6 +627,16 @@ def update_hook
       _update
     end
 
+    # Method for hooking callbacks on resource destruction
+    #
+    # @return [Boolean]
+    #   true if the destroy was successful, false if not
+    #
+    # @api private
+    def destroy_hook
+      _destroy(true)
+    end
+
     private
 
     # Initialize a new instance of this Resource using the provided values
@@ -901,6 +904,21 @@ def _update
       end
     end
 
+    # @api private
+    def _destroy(safe)
+      return false unless saved?
+      repository.delete(collection_for_self)
+      set_destroyed_state
+      true
+    end
+
+    # @api private
+    def set_destroyed_state
+      reset
+      @_readonly  = true
+      @_destroyed = true
+    end
+
     # @api private
     def _save(safe)
       run_once(true) do

lib/dm-core/types/paranoid/base.rb

@@ -0,0 +1,38 @@
+module DataMapper
+  module Types
+    module Paranoid
+      module Base
+        def self.included(type)
+          type.extend ClassMethods
+        end
+
+        def paranoid_destroy
+          return false unless saved?
+          model.paranoid_properties.each do |name, block|
+            attribute_set(name, block.call(self))
+          end
+          save_self
+          set_destroyed_state
+          true
+        end
+
+        private
+
+        # @api private
+        def _destroy(safe)
+          if safe
+            paranoid_destroy
+          else
+            super
+          end
+        end
+      end # module Methods
+
+      module ClassMethods
+        def with_deleted
+          with_exclusive_scope({}) { block_given? ? yield : all }
+        end
+      end # module ClassMethods
+    end # module Paranoid
+  end # module Types
+end # module DataMapper

lib/dm-core/types/paranoid_boolean.rb

@@ -7,35 +7,17 @@ class ParanoidBoolean < Type
 
       # @api private
       def self.bind(property)
-        repository_name = property.repository_name
+        repository_name = property.repository_name.inspect
         model           = property.model
-        property_name   = property.name
-
-        model.send(:set_paranoid_property, property_name){true}
+        property_name   = property.name.inspect
 
         model.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-          def self.with_deleted
-            with_exclusive_scope(#{property_name.inspect} => true) do
-              yield
-            end
-          end
+          include Paranoid::Base
 
-          def destroy
-            paranoid_destroy
-          end
+          set_paranoid_property(#{property_name}) { true }
 
-          def paranoid_destroy
-            self.class.paranoid_properties.each do |name, blk|
-              attribute_set(name, blk.call(self))
-            end
-            save_self
-            @_destroyed = true
-            @_readonly  = true
-            reset
-          end
+          default_scope(#{repository_name}).update(#{property_name} => false)
         RUBY
-
-        model.default_scope(repository_name).update(property_name => false)
       end
     end # class ParanoidBoolean
   end # module Types

lib/dm-core/types/paranoid_datetime.rb

@@ -6,35 +6,17 @@ class ParanoidDateTime < Type
 
       # @api private
       def self.bind(property)
-        repository_name = property.repository_name
+        repository_name = property.repository_name.inspect
         model           = property.model
-        property_name   = property.name
-
-        model.send(:set_paranoid_property, property_name){DateTime.now}
+        property_name   = property.name.inspect
 
         model.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-          def self.with_deleted
-            with_exclusive_scope(#{property_name.inspect}.not => nil) do
-              yield
-            end
-          end
+          include Paranoid::Base
 
-          def destroy
-            paranoid_destroy
-          end
+          set_paranoid_property(#{property_name}) { DateTime.now }
 
-          def paranoid_destroy
-            self.class.paranoid_properties.each do |name, blk|
-              attribute_set(name, blk.call(self))
-            end
-            save_self
-            @_destroyed = true
-            @_readonly  = true
-            reset
-          end
+          default_scope(#{repository_name}).update(#{property_name} => nil)
         RUBY
-
-        model.default_scope(repository_name).update(property_name => nil)
       end
     end # class ParanoidDateTime
   end # module Types

spec/public/property/paranoid_boolean_spec.rb

@@ -0,0 +1,136 @@
+require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'spec_helper'))
+
+describe DataMapper::Property, 'ParanoidBoolean type' do
+  before :all do
+    module ::Blog
+      class Article
+        include DataMapper::Resource
+
+        attr_reader :hook_called
+
+        property :id,      Serial
+        property :deleted, ParanoidBoolean
+
+        before :destroy do
+          @hook_called ||= 0
+          @hook_called += 1
+        end
+      end
+    end
+
+    @model = Blog::Article
+  end
+
+  supported_by :all do
+    describe 'Resource#destroy' do
+      subject { @resource.destroy }
+
+      describe 'with a new resource' do
+        before do
+          @resource = @model.new
+        end
+
+        it { should be_false }
+
+        it 'should not delete the resource from the datastore' do
+          method(:subject).should_not change { @model.with_deleted.size }.from(0)
+        end
+
+        it 'should not set the paranoid column' do
+          method(:subject).should_not change { @resource.deleted }.from(false)
+        end
+
+        it 'should run the destroy hook' do
+          method(:subject).should change { @resource.hook_called }.from(nil).to(1)
+        end
+      end
+
+      describe 'with a saved resource' do
+        before do
+          @resource = @model.create
+        end
+
+        it { should be_true }
+
+        it 'should not delete the resource from the datastore' do
+          method(:subject).should_not change { @model.with_deleted.size }.from(1)
+        end
+
+        it 'should set the paranoid column' do
+          method(:subject).should change { @resource.deleted }.from(false).to(true)
+        end
+
+        it 'should run the destroy hook' do
+          method(:subject).should change { @resource.hook_called }.from(nil).to(1)
+        end
+      end
+    end
+
+    describe 'Resource#destroy!' do
+      subject { @resource.destroy! }
+
+      describe 'with a new resource' do
+        before do
+          @resource = @model.new
+        end
+
+        it { should be_false }
+
+        it 'should not delete the resource from the datastore' do
+          method(:subject).should_not change { @model.with_deleted.size }.from(0)
+        end
+
+        it 'should not set the paranoid column' do
+          method(:subject).should_not change { @resource.deleted }.from(false)
+        end
+
+        it 'should not run the destroy hook when #destroy called' do
+          method(:subject).should_not change { @resource.hook_called }.from(nil)
+        end
+      end
+
+      describe 'with a saved resource' do
+        before do
+          @resource = @model.create
+        end
+
+        it { should be_true }
+
+        it 'should delete the resource from the datastore' do
+          method(:subject).should change { @model.with_deleted.size }.from(1).to(0)
+        end
+
+        it 'should not set the paranoid column' do
+          method(:subject).should_not change { @resource.deleted }.from(false)
+        end
+
+        it 'should not run the destroy hook when #destroy called' do
+          method(:subject).should_not change { @resource.hook_called }.from(nil)
+        end
+      end
+    end
+
+    describe 'Model#with_deleted' do
+      before do
+        @resource = @model.create
+        @resource.destroy
+      end
+
+      describe 'with a block' do
+        subject { @model.with_deleted { @model.all } }
+
+        it 'should scope the block to return all resources' do
+          subject.map { |resource| resource.key }.should == [ @resource.key ]
+        end
+      end
+
+      describe 'without a block' do
+        subject { @model.with_deleted }
+
+        it 'should return a collection scoped to return all resources' do
+          subject.map { |resource| resource.key }.should == [ @resource.key ]
+        end
+      end
+    end
+  end
+end