@wip Add missing specs (destroy_spec.rb)

spec/dynamoid/persistence_spec.rb

@@ -4815,9 +4815,20 @@ def around_update_callback
     end
   end
 
-  context 'destroy' do
+  describe 'destroy' do
     # TODO: adopt test cases for the `delete` method
 
+    it 'does not raise any error when model is already deleted' do
+      klass = new_class
+      obj = klass.create
+      obj2 = klass.find(obj.id)
+      obj.delete
+      expect(klass.exists?(obj.id)).to eql false
+
+      obj2.destroy
+      expect(obj2.destroyed?).to eql true
+    end
+
     describe 'callbacks' do
       it 'runs before_destroy callback' do
         klass_with_callback = new_class do
@@ -4853,10 +4864,42 @@ def around_destroy_callback
 
         expect { obj.destroy }.to output('start around_destroyfinish around_destroy').to_stdout
       end
+
+      it 'aborts destroying and returns false if a before_destroy callback throws :abort' do
+        klass = new_class do
+          before_destroy { throw :abort }
+        end
+        obj = klass.create!
+
+        result = nil
+        expect {
+          result = obj.destroy
+        }.not_to change { klass.count }
+
+        expect(result).to eql false
+        # expect(obj.destroyed?).to eql false # FIXME
+      end
+    end
+  end
+
+  describe 'destroy!' do
+    it 'aborts destroying and raises RecordNotDestroyed if a before_destroy callback throws :abort' do
+      klass = new_class do
+        before_destroy { throw :abort }
+      end
+      obj = klass.create!
+
+      expect {
+        expect {
+          obj.destroy!
+        }.to raise_error(Dynamoid::Errors::RecordNotDestroyed)
+      }.not_to change { klass.count }
+
+      # expect(obj.destroyed?).to eql false # FIXME
     end
   end
 
-  context 'delete' do
+  describe 'delete' do
     it 'deletes an item' do
       klass = new_class
       obj = klass.create
@@ -4886,7 +4929,7 @@ def around_destroy_callback
     context 'with lock version' do
       it 'deletes a record if lock version matches' do
         address.save!
-        expect { address.destroy }.not_to raise_error
+        expect { address.delete }.not_to raise_error
       end
 
       it 'does not delete a record if lock version does not match' do
@@ -4897,15 +4940,15 @@ def around_destroy_callback
         a1.city = 'Seattle'
         a1.save!
 
-        expect { a2.destroy }.to raise_exception(Dynamoid::Errors::StaleObjectError)
+        expect { a2.delete }.to raise_exception(Dynamoid::Errors::StaleObjectError)
       end
 
       it 'uses the correct lock_version even if it is modified' do
         address.save!
         a1 = address
         a1.lock_version = 100
 
-        expect { a1.destroy }.not_to raise_error
+        expect { a1.delete }.not_to raise_error
       end
     end
 

spec/dynamoid/transaction_write/delete_spec.rb

@@ -29,7 +29,7 @@
     end
 
     expect(result).to be_a(klass)
-    expect(result).to eql(obj)
+    expect(result).to equal(obj)
     # expect(result).to be_destroyed # FIXME
   end
 

spec/dynamoid/transaction_write/destroy_spec.rb

@@ -8,113 +8,158 @@
 describe Dynamoid::TransactionWrite, '.destroy' do
   include_context 'transaction_write'
 
-  context 'destroys' do
-    context 'simple primary key' do
-      before do
-        klass.create_table
-      end
+  it 'deletes a model and accepts a model itself' do
+    obj = klass.create!(name: 'one')
 
-      it 'with instance' do
-        obj1 = klass.create!(name: 'one')
-        expect(obj1.persisted?).to eql(true)
-        expect(obj1).not_to be_destroyed
-        described_class.execute do |txn|
-          txn.destroy! obj1
-        end
-        expect(obj1.destroyed?).to eql(true)
-        expect(obj1.persisted?).to eql(false)
-        expect(klass).not_to exist(obj1.id)
+    expect {
+      described_class.execute do |txn|
+        txn.destroy obj
       end
+    }.to change { klass.count }.by(-1)
 
-      it 'with id' do
-        obj2 = klass.create!(name: 'two')
-        described_class.execute do |txn|
-          txn.destroy! klass, obj2.id
-        end
-        expect(klass).not_to exist(obj2.id)
-      end
-    end
+    expect(klass.exists?(obj.id)).to eql false
+  end
 
-    context 'composite key' do
-      before do
-        klass_with_composite_key.create_table
-      end
+  it 'returns a model' do
+    obj = klass.create!(name: 'one')
 
-      it 'with instance' do
-        obj1 = klass_with_composite_key.create!(name: 'one', age: 1)
-        obj2 = klass_with_composite_key.create!(name: 'two', age: 2)
-        described_class.execute do |txn|
-          txn.destroy! obj1
-        end
-        expect(klass_with_composite_key).not_to exist({ id: obj1.id, age: 1 })
-      end
+    result = nil
+    described_class.execute do |txn|
+      result = txn.destroy obj
+    end
 
-      it 'with id' do
-        obj2 = klass_with_composite_key.create!(name: 'two', age: 2)
-        described_class.execute do |txn|
-          txn.destroy! klass_with_composite_key, { id: obj2.id, age: 2 }
-        end
-        expect(klass_with_composite_key).not_to exist({ id: obj2.id, age: 1 })
-      end
+    expect(result).to be_a(klass)
+    expect(result).to equal(obj)
+    # expect(result).to be_destroyed # FIXME
+  end
+
+  describe 'primary key schemas' do
+    context 'simple primary key' do
+      it 'deletes a model' do
+        obj = klass.create!(name: 'one')
 
-      it 'requires hash key' do
         expect {
           described_class.execute do |txn|
-            txn.destroy! klass_with_composite_key, age: 5
+            txn.destroy obj
           end
-        }.to raise_exception(Dynamoid::Errors::MissingHashKey) # not ArgumentError
+        }.to change { klass.count }.by(-1)
       end
+    end
+
+    context 'composite key' do
+      it 'deletes a model' do
+        obj = klass_with_composite_key.create!(name: 'one', age: 1)
 
-      it 'requires range key' do
         expect {
           described_class.execute do |txn|
-            txn.destroy! klass_with_composite_key, id: 'bananas'
+            txn.destroy obj
           end
-        }.to raise_exception(Dynamoid::Errors::MissingRangeKey) # not ArgumentError
+        }.to change { klass_with_composite_key.count }.by(-1)
       end
     end
+  end
 
-    it 'uses callbacks' do
-      klass_with_callbacks.create_table
-      obj1 = klass_with_callbacks.create!(name: 'one')
-      expect {
-        described_class.execute do |txn|
-          txn.destroy! obj1
-        end
-      }.to output('destroying destroyed ').to_stdout
-    end
+  context 'when an issue detected on the DynamoDB side' do
+    it 'rolls back the changes when item to delete with specified id does not exist' do
+      obj1 = klass.create!(name: 'one', id: '1')
+      obj1.id = 'not-existing'
+      obj2 = klass.new(name: 'two', id: '2')
 
-    it 'uses around callbacks' do
-      klass_with_around_callbacks.create_table
-      obj1 = klass_with_around_callbacks.create!(name: 'one')
       expect {
-        described_class.execute do |txn|
-          txn.destroy! obj1
-        end
-      }.to output('destroying destroyed ').to_stdout
-    end
-
-    context 'when an issue detected on the DynamoDB side' do
-      it 'does not roll back the changes when item to delete with specified id does not exist' do
-        obj1 = klass.create!(name: 'one', id: '1')
-        obj1.id = 'not-existing'
-        obj2 = klass.new(name: 'two', id: '2')
-
         expect {
           described_class.execute do |txn|
             txn.destroy obj1
             txn.save obj2
           end
         }.to raise_error(Aws::DynamoDB::Errors::TransactionCanceledException)
+      }.not_to change { klass.count }
+
+      # expect(obj1.destroyed?).to eql nil # FIXME
+      expect(obj2.persisted?).to eql false
+    end
+  end
+
+  it 'uses callbacks' do
+    obj = klass_with_callbacks.create!
+
+    expect {
+      described_class.execute do |txn|
+        txn.destroy obj
+      end
+    }.to output('destroying destroyed ').to_stdout
+  end
+
+  it 'uses around callbacks' do
+    obj = klass_with_around_callbacks.create!
+
+    expect {
+      described_class.execute do |txn|
+        txn.destroy obj
+      end
+    }.to output('destroying destroyed ').to_stdout
+  end
+
+  it 'aborts destroying and returns false if a before_destroy callback throws :abort' do
+    klass = new_class do
+      before_destroy { throw :abort }
+    end
+    obj = klass.create!
+
+    result = nil
+    expect {
+      described_class.execute do |txn|
+        result = txn.destroy obj
+      end
+    }.not_to change { klass.count }
 
-        expect(klass.count).to eql 1
-        expect(klass.all.to_a.map(&:id)).to contain_exactly('1')
+    # expect(result).to eql false # FIXME
+    expect(obj.destroyed?).to eql nil
+  end
+end
+
+describe Dynamoid::TransactionWrite, '.destroy!' do
+  include_context 'transaction_write'
 
-        # expect(obj1.destroyed?).to eql nil # FIXME
-        expect(obj2.persisted?).to eql false
+  it 'deletes a model and accepts a model itself' do
+    obj = klass.create!(name: 'one')
+
+    expect {
+      described_class.execute do |txn|
+        txn.destroy! obj
       end
+    }.to change { klass.count }.by(-1)
+
+    expect(klass.exists?(obj.id)).to eql false
+  end
+
+  it 'returns a model' do
+    obj = klass.create!(name: 'one')
+
+    result = nil
+    described_class.execute do |txn|
+      result = txn.destroy! obj
+    end
+
+    expect(result).to be_a(klass)
+    expect(result).to equal(obj)
+    # expect(result).to be_destroyed # FIXME
+  end
+
+  it 'aborts destroying and raises RecordNotDestroyed if a before_destroy callback throws :abort' do
+    klass = new_class do
+      before_destroy { throw :abort }
     end
+    obj = klass.create!
+
+    expect {
+      # FIXME
+      #expect {
+        described_class.execute do |txn|
+          txn.destroy! obj
+        end
+      #}.to raise_error(Dynamoid::Errors::RecordNotDestroyed)
+    }.not_to change { klass.count }
 
-    # TODO: test destroy! vs. destroy i.e. when an :abort is raised in a callback
+    expect(obj.destroyed?).to eql nil
   end
 end