Validate primary key in #save

lib/dynamoid/transaction_write/save.rb

@@ -14,6 +14,8 @@ def initialize(model, **options)
       end
 
       def on_registration
+        validate_model!
+
         unless @valid = @model.valid?
           if @options[:raise_error]
             raise Dynamoid::Errors::DocumentNotValid, @model
@@ -72,6 +74,11 @@ def action_request
 
       private
 
+      def validate_model!
+        raise Dynamoid::Errors::MissingHashKey if !@was_new_record && @model.hash_key.nil?
+        raise Dynamoid::Errors::MissingRangeKey if @model_class.range_key? && @model.range_value.nil?
+      end
+
       def action_request_to_create
         @model.hash_key = SecureRandom.uuid if @model.hash_key.nil?
         touch_model_timestamps(skip_created_at: false)

spec/dynamoid/transaction_write/create_spec.rb

@@ -105,6 +105,20 @@ def around_save_callback
     end
   end
 
+  describe 'primary key validation' do
+    context 'composite key' do
+      it 'requires sort key to be specified' do
+        klass_with_composite_key.create_table
+
+        expect {
+          described_class.execute do |txn|
+            txn.create klass_with_composite_key, name: 'Alex', age: nil
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+      end
+    end
+  end
+
   describe 'timestamps' do
     before do
       klass.create_table

spec/dynamoid/transaction_write/delete_spec.rb

@@ -66,6 +66,45 @@
     end
   end
 
+  describe 'primary key validation' do
+    context 'simple primary key' do
+      it 'requires partition key to be specified' do
+        obj = klass.create!(name: 'one')
+        obj.id = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.delete obj
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+    end
+
+    context 'composite key' do
+      it 'requires partition key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'one', age: 1)
+        obj.id = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.delete obj
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+
+      it 'requires sort key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'one', age: 1)
+        obj.age = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.delete obj
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+      end
+    end
+  end
+
   context 'when an issue detected on the DynamoDB side' do
     it 'does not roll back the changes when a model to delete does not exist' do
       obj1 = klass.create!(name: 'one', id: '1')

spec/dynamoid/transaction_write/destroy_spec.rb

@@ -80,6 +80,45 @@ def around_destroy_callback
     end
   end
 
+  describe 'primary key validation' do
+    context 'simple primary key' do
+      it 'requires partition key to be specified' do
+        obj = klass.create!(name: 'one')
+        obj.id = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.destroy obj
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+    end
+
+    context 'composite key' do
+      it 'requires partition key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'one', age: 1)
+        obj.id = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.destroy obj
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+
+      it 'requires sort key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'one', age: 1)
+        obj.age = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.destroy obj
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+      end
+    end
+  end
+
   context 'when an issue detected on the DynamoDB side' do
     it 'does not raise exception and does not roll back the transaction when a model to destroy does not exist' do
       obj_to_destroy = klass.create!(name: 'one', id: '1')

spec/dynamoid/transaction_write/save_spec.rb

@@ -141,6 +141,64 @@ def around_save_callback
     end
   end
 
+  describe 'primary key validation' do
+    context 'simple primary key' do
+      context 'persisted model' do
+        it 'requires partition key to be specified' do
+          obj = klass.create!(name: 'Alex')
+          obj.id = nil
+          obj.name = 'Alex [Updated]'
+
+          expect {
+            described_class.execute do |txn|
+              txn.save obj
+            end
+          }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+        end
+      end
+    end
+
+    context 'composite key' do
+      context 'new model' do
+        it 'requires sort key to be specified' do
+          obj = klass_with_composite_key.new name: 'Alex', age: nil
+
+          expect {
+            described_class.execute do |txn|
+              txn.save obj
+            end
+          }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+        end
+      end
+
+      context 'persisted model' do
+        it 'requires partition key to be specified' do
+          obj = klass_with_composite_key.create!(name: 'Alex', age: 3)
+          obj.id = nil
+          obj.name = 'Alex [Updated]'
+
+          expect {
+            described_class.execute do |txn|
+              txn.save obj
+            end
+          }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+        end
+
+        it 'requires sort key to be specified' do
+          obj = klass_with_composite_key.create!(name: 'Alex', age: 3)
+          obj.age = nil
+          obj.name = 'Alex [Updated]'
+
+          expect {
+            described_class.execute do |txn|
+              txn.save obj
+            end
+          }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+        end
+      end
+    end
+  end
+
   describe 'timestamps' do
     context 'new model' do
       before do

spec/dynamoid/transaction_write/update_attributes_spec.rb

@@ -96,6 +96,45 @@ def around_save_callback
     end
   end
 
+  describe 'primary key validation' do
+    context 'simple primary key' do
+      it 'requires partition key to be specified' do
+        obj = klass.create!(name: 'Alex')
+        obj.id = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.update_attributes obj, name: 'Alex [Updated]'
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+    end
+
+    context 'composite key' do
+      it 'requires partition key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'Alex', age: 3)
+        obj.id = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.update_attributes obj, name: 'Alex [Updated]'
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+
+      it 'requires sort key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'Alex', age: 3)
+        obj.age = nil
+
+        expect {
+          described_class.execute do |txn|
+            txn.update_attributes obj, name: 'Alex [Updated]'
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+      end
+    end
+  end
+
   describe 'timestamps' do
     it 'sets updated_at if Config.timestamps=true', config: { timestamps: true } do
       obj = klass.create!

spec/dynamoid/transaction_write/update_fields_spec.rb

@@ -68,6 +68,42 @@
     end
   end
 
+  describe 'primary key validation' do
+    context 'simple primary key' do
+      it 'requires partition key to be specified' do
+        obj = klass.create!(name: 'Alex')
+
+        expect {
+          described_class.execute do |txn|
+            txn.update_fields klass_with_composite_key, nil, name: 'Alex [Updated]'
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+    end
+
+    context 'composite key' do
+      it 'requires partition key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'Alex', age: 3)
+
+        expect {
+          described_class.execute do |txn|
+            txn.update_fields klass_with_composite_key, nil, 3, name: 'Alex [Updated]'
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+
+      it 'requires sort key to be specified' do
+        obj = klass_with_composite_key.create!(name: 'Alex', age: 3)
+
+        expect {
+          described_class.execute do |txn|
+            txn.update_fields klass_with_composite_key, obj.id, nil, name: 'Alex [Updated]'
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingRangeKey)
+      end
+    end
+  end
+
   describe 'timestamps' do
     it 'sets updated_at if Config.timestamps=true', config: { timestamps: true } do
       obj = klass.create!

spec/dynamoid/transaction_write/upsert_spec.rb

@@ -86,14 +86,6 @@
           end
         }.to change { klass.find(obj.id).name }.to('Alex [Updated]')
       end
-
-      it 'requires partition key to be specified' do
-        expect {
-          described_class.execute do |txn|
-            txn.upsert klass, nil, { name: 'threethree' }
-          end
-        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
-      end
     end
 
     context 'composite key' do
@@ -117,7 +109,21 @@
           end
         }.to change { obj.reload.name }.to('Alex [Updated]')
       end
+    end
+  end
 
+  describe 'primary key validation' do
+    context 'simple primary key' do
+      it 'requires partition key to be specified' do
+        expect {
+          described_class.execute do |txn|
+            txn.upsert klass, nil, { name: 'threethree' }
+          end
+        }.to raise_exception(Dynamoid::Errors::MissingHashKey)
+      end
+    end
+
+    context 'composite key' do
       it 'requires partition key to be specified' do
         expect {
           described_class.execute do |txn|