Implement Document.delete() method

Author: andrykonchin

lib/dynamoid/persistence.rb

@@ -458,52 +458,49 @@ def inc(hash_key_value, range_key_value = nil, counters)
 
       # Delete a model.
       #
-      # Delete a model by given partition key:
+      # Raises +Dynamoid::Errors::MissingHashKey+ if a partition key has value
+      # +nil+ and raises +Dynamoid::Errors::MissingRangeKey+ if a sort key is
+      # required but has value +nil+ or is missing.
       #
-      #   User.delete(user_id)
+      # @param ids [String|Array] primary key or an array of primary keys
+      # @return nil
       #
-      # Delete a model by given partition and sort keys:
+      # @example Delete a model by given partition key:
+      #   User.delete(user_id)
       #
+      # @example Delete a model by given partition and sort keys:
       #   User.delete(user_id, sort_key)
       #
-      # Delete multiple models by given partition keys:
-      #
+      # @example Delete multiple models by given partition keys:
       #   User.delete([id1, id2, id3])
       #
-      # Delete multiple models by given partition and sort keys:
-      #
+      # @example Delete multiple models by given partition and sort keys:
       #   User.delete([[id1, sk1], [id2, sk2], [id3, sk3]])
-      #
-      # Raises +Dynamoid::Errors::MissingHashKey+ if a partition key has value
-      # +nil+ and raises +Dynamoid::Errors::MissingRangeKey+ if a sort key is
-      # required but has value +nil+ or is missing.
-      #
-      # @return nil
-      def delete(*args)
-        if args.empty?
+
+      def delete(*ids)
+        if ids.empty?
           raise Dynamoid::Errors::MissingHashKey
         end
 
-        # TODO: warn if passed excessive arguments
-        # TODO: handle a case when sort key isn't declared but given in arguments
-        if args[0].is_a?(Array)
-          # given multiple keys:
-          keys = args[0] # ignore other arguments
+        if ids[0].is_a?(Array)
+          # given multiple keys
+          #   Model.delete([id1, id2, id3])
+          keys = ids[0] # ignore other arguments
 
-
-          if keys[0].is_a?(Array)
+          if self.range_key
             # compound primary key
+            # expect [hash key, range key] pairs
 
             ids = []
             range_key = []
 
             # assume all elements are pairs, that's arrays
             keys.each do |pk, sk|
               raise Dynamoid::Errors::MissingHashKey if pk.nil?
-              raise Dynamoid::Errors::MissingRangeKey if self.range_key? && sk.nil?
+              raise Dynamoid::Errors::MissingRangeKey if self.range_key && sk.nil?
 
-              partition_key_dumped = Dumping.dump_field(pk, self.attributes[self.hash_key])
-              sort_key_dumped = Dumping.dump_field(sk, self.attributes[self.range_key])
+              partition_key_dumped = cast_and_dump(self.hash_key, pk)
+              sort_key_dumped = cast_and_dump(self.range_key, sk)
 
               ids << partition_key_dumped
               range_key << sort_key_dumped
@@ -517,22 +514,25 @@ def delete(*args)
             keys.each do |pk|
               raise Dynamoid::Errors::MissingHashKey if pk.nil?
 
-              partition_key_dumped = Dumping.dump_field(pk, self.attributes[self.hash_key])
+              partition_key_dumped = cast_and_dump(self.hash_key, pk)
               ids << partition_key_dumped
             end
           end
 
           options = range_key ? { range_key: range_key } : {}
           Dynamoid.adapter.delete(self.table_name, ids, options)
         else
-          # Model.delete(partition_key, sort_key)
-          partition_key, sort_key = args
+          # given single primary key:
+          #   Model.delete(partition_key)
+          #   Model.delete(partition_key, sort_key)
+
+          partition_key, sort_key = ids
 
           raise Dynamoid::Errors::MissingHashKey if partition_key.nil?
           raise Dynamoid::Errors::MissingRangeKey if self.range_key? && sort_key.nil?
 
-          options = sort_key ? { range_key: Dumping.dump_field(sort_key, self.attributes[self.range_key]) } : {}
-          partition_key_dumped = Dumping.dump_field(partition_key, self.attributes[self.hash_key])
+          options = sort_key ? { range_key: cast_and_dump(self.range_key, sort_key) } : {}
+          partition_key_dumped = cast_and_dump(self.hash_key, partition_key)
 
           Dynamoid.adapter.delete(self.table_name, partition_key_dumped, options)
         end

spec/dynamoid/persistence/delete_spec.rb

@@ -24,20 +24,22 @@
       obj1 = klass.create!
       obj2 = klass.create!
 
-      klass.delete([obj1.id, obj2.id])
-
-      expect(klass.exists?(obj1.id)).to eql(false)
-      expect(klass.exists?(obj2.id)).to eql(false)
+      expect {
+        expect {
+          klass.delete([obj1.id, obj2.id])
+        }.to change { klass.where(id: obj1.id).first }.to(nil)
+      }.to change { klass.where(id: obj2.id).first }.to(nil)
     end
 
     it 'deletes multiple items when given multiple partition and sort keys' do
       obj1 = klass_with_composite_key.create!(age: 1)
       obj2 = klass_with_composite_key.create!(age: 2)
 
-      klass_with_composite_key.delete([[obj1.id, obj1.age], [obj2.id, obj2.age]])
-
-      expect(klass_with_composite_key.exists?([[obj1.id, obj1.age]])).to eql(false)
-      expect(klass_with_composite_key.exists?([[obj2.id, obj2.age]])).to eql(false)
+      expect {
+        expect {
+          klass_with_composite_key.delete([[obj1.id, obj1.age], [obj2.id, obj2.age]])
+        }.to change { klass_with_composite_key.where(id: obj1.id, age: obj1.age).first }.to(nil)
+      }.to change { klass_with_composite_key.where(id: obj2.id, age: obj2.age).first }.to(nil)
     end
 
     it 'uses dumped value of partition key to delete item' do
@@ -47,9 +49,20 @@
 
       expect {
         klass.delete(obj.published_on)
-      }.to change {
-        klass.where(published_on: obj.published_on).first
-      }.to(nil)
+      }.to change { klass.where(published_on: obj.published_on).first }.to(nil)
+    end
+
+    it 'uses dumped value of partition key to delete item when given multiple primary keys' do
+      klass = new_class(partition_key: { name: :published_on, type: :date })
+
+      obj1 = klass.create!(published_on: '2018-10-07'.to_date)
+      obj2 = klass.create!(published_on: '2018-10-13'.to_date)
+
+      expect {
+        expect {
+          klass.delete([obj1.published_on, obj2.published_on])
+        }.to change { klass.where(published_on: obj1.published_on).first }.to(nil)
+      }.to change { klass.where(published_on: obj2.published_on).first }.to(nil)
     end
 
     it 'uses dumped value of sort key to delete item' do
@@ -61,11 +74,25 @@
 
       expect {
         klass.delete(obj.id, obj.activated_on)
-      }.to change {
-        klass.where(id: obj.id, activated_on: obj.activated_on).first
-      }.to(nil)
+      }.to change { klass.where(id: obj.id, activated_on: obj.activated_on).first }.to(nil)
     end
 
+    it 'uses dumped value of sort key to delete item when given multiple primary keys' do
+      klass = new_class do
+        range :activated_on, :date
+      end
+
+      obj1 = klass.create!(activated_on: Date.today)
+      obj2 = klass.create!(activated_on: Date.tomorrow)
+
+      expect {
+        expect {
+          klass.delete([[obj1.id, obj1.activated_on], [obj2.id, obj2.activated_on]])
+        }.to change { klass.where(id: obj1.id, activated_on: obj1.activated_on).first }.to(nil)
+      }.to change { klass.where(id: obj2.id, activated_on: obj2.activated_on).first }.to(nil)
+    end
+
+
     it 'does not raise exception when model was concurrently deleted' do
       klass = new_class
       klass.create_table
@@ -78,6 +105,7 @@
         it 'requires partition key to be specified' do
           klass = new_class
           expect { klass.delete(nil) }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+          expect { klass.delete() }.to raise_exception(Dynamoid::Errors::MissingHashKey)
         end
       end
 
@@ -86,10 +114,19 @@
           expect { klass_with_composite_key.delete(nil, 1) }.to raise_exception(Dynamoid::Errors::MissingHashKey)
         end
 
+        it 'requires partition key to be specified when given multiple primary keys' do
+          expect { klass_with_composite_key.delete([nil]) }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+        end
+
         it 'requires sort key to be specified' do
           expect { klass_with_composite_key.delete("abc", nil) }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
           expect { klass_with_composite_key.delete("abc") }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
         end
+
+        it 'requires sort key to be specified when given multiple primary keys' do
+          expect { klass_with_composite_key.delete([["abc", nil]]) }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+          expect { klass_with_composite_key.delete([["abc"]]) }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+        end
       end
     end