Fix transactional operations and dump partition/sort keys when they are

of non-native DynamoDB type

Author: andrykonchin

lib/dynamoid/transaction_write/delete_with_instance.rb

@@ -35,10 +35,10 @@ def observable_by_user_result
       end
 
       def action_request
-        key = { @model_class.hash_key => @model.hash_key }
+        key = { @model_class.hash_key => dump_attribute(@model_class.hash_key, @model.hash_key) }
 
         if @model_class.range_key?
-          key[@model_class.range_key] = @model.range_value
+          key[@model_class.range_key] = dump_attribute(@model_class.range_key, @model.range_value)
         end
 
         {
@@ -55,6 +55,11 @@ def validate_model!
         raise Dynamoid::Errors::MissingHashKey if @model.hash_key.nil?
         raise Dynamoid::Errors::MissingRangeKey if @model_class.range_key? && @model.range_value.nil?
       end
+
+      def dump_attribute(name, value)
+        options = @model_class.attributes[name]
+        Dumping.dump_field(value, options)
+      end
     end
   end
 end

lib/dynamoid/transaction_write/delete_with_primary_key.rb

@@ -34,10 +34,10 @@ def observable_by_user_result
       end
 
       def action_request
-        key = { @model_class.hash_key => @hash_key }
+        key = { @model_class.hash_key => dump_attribute(@model_class.hash_key, @hash_key) }
 
         if @model_class.range_key?
-          key[@model_class.range_key] = @range_key
+          key[@model_class.range_key] = dump_attribute(@model_class.range_key, @range_key)
         end
 
         {
@@ -54,6 +54,11 @@ def validate_primary_key!
         raise Dynamoid::Errors::MissingHashKey if @hash_key.nil?
         raise Dynamoid::Errors::MissingRangeKey if @model_class.range_key? && @range_key.nil?
       end
+
+      def dump_attribute(name, value)
+        options = @model_class.attributes[name]
+        Dumping.dump_field(value, options)
+      end
     end
   end
 end

lib/dynamoid/transaction_write/destroy.rb

@@ -54,10 +54,10 @@ def observable_by_user_result
       end
 
       def action_request
-        key = { @model_class.hash_key => @model.hash_key }
+        key = { @model_class.hash_key => dump_attribute(@model_class.hash_key, @model.hash_key) }
 
         if @model_class.range_key?
-          key[@model_class.range_key] = @model.range_value
+          key[@model_class.range_key] = dump_attribute(@model_class.range_key, @model.range_value)
         end
 
         {
@@ -74,6 +74,11 @@ def validate_model!
         raise Dynamoid::Errors::MissingHashKey if @model.hash_key.nil?
         raise Dynamoid::Errors::MissingRangeKey if @model_class.range_key? && @model.range_value.nil?
       end
+
+      def dump_attribute(name, value)
+        options = @model_class.attributes[name]
+        Dumping.dump_field(value, options)
+      end
     end
   end
 end

lib/dynamoid/transaction_write/save.rb

@@ -121,8 +121,8 @@ def action_request_to_update
         changes_dumped = Dynamoid::Dumping.dump_attributes(changes, @model_class.attributes)
 
         # primary key to look up an item to update
-        key = { @model_class.hash_key => @model.hash_key }
-        key[@model_class.range_key] = @model.range_value if @model_class.range_key?
+        key = { @model_class.hash_key => dump_attribute(@model_class.hash_key, @model.hash_key) }
+        key[@model_class.range_key] = dump_attribute(@model_class.range_key, @model.range_value) if @model_class.range_key?
 
         # Build UpdateExpression and keep names and values placeholders mapping
         # in ExpressionAttributeNames and ExpressionAttributeValues.
@@ -159,6 +159,11 @@ def touch_model_timestamps(skip_created_at:)
         @model.updated_at = timestamp unless @options[:touch] == false && !@was_new_record
         @model.created_at ||= timestamp unless skip_created_at
       end
+
+      def dump_attribute(name, value)
+        options = @model_class.attributes[name]
+        Dumping.dump_field(value, options)
+      end
     end
   end
 end

lib/dynamoid/transaction_write/update_fields.rb

@@ -46,8 +46,8 @@ def action_request
         builder = UpdateRequestBuilder.new(@model_class)
 
         # primary key to look up an item to update
-        builder.hash_key = @hash_key
-        builder.range_key = @range_key if @model_class.range_key?
+        builder.hash_key = dump_attribute(@model_class.hash_key, @hash_key)
+        builder.range_key = dump_attribute(@model_class.range_key, @range_key) if @model_class.range_key?
 
         # changed attributes to persist
         changes = @attributes.dup
@@ -111,6 +111,11 @@ def add_timestamps(attributes, skip_created_at: false)
         result
       end
 
+      def dump_attribute(name, value)
+        options = @model_class.attributes[name]
+        Dumping.dump_field(value, options)
+      end
+
       class UpdateRequestBuilder
         attr_writer :hash_key, :range_key, :condition_expression
 

lib/dynamoid/transaction_write/upsert.rb

@@ -44,8 +44,13 @@ def action_request
         changes_dumped = Dynamoid::Dumping.dump_attributes(changes, @model_class.attributes)
 
         # primary key to look up an item to update
-        key = { @model_class.hash_key => @hash_key }
-        key[@model_class.range_key] = @range_key if @model_class.range_key?
+        partition_key_dumped = dump(@model_class.hash_key, @hash_key)
+        key = { @model_class.hash_key => partition_key_dumped }
+
+        if @model_class.range_key?
+          sort_key_dumped = dump(@model_class.range_key, @range_key)
+          key[@model_class.range_key] = sort_key_dumped
+        end
 
         # Build UpdateExpression and keep names and values placeholders mapping
         # in ExpressionAttributeNames and ExpressionAttributeValues.
@@ -91,6 +96,11 @@ def add_timestamps(attributes, skip_created_at: false)
         result[:updated_at] ||= timestamp
         result
       end
+
+      def dump(name, value)
+        options = @model_class.attributes[name]
+        Dumping.dump_field(value, options)
+      end
     end
   end
 end

spec/dynamoid/transaction_write/create_spec.rb

@@ -356,6 +356,35 @@ def around_save_callback
     expect(obj).to be_changed
   end
 
+  it 'uses dumped value of partition key to create item' do
+    klass = new_class(partition_key: { name: :published_on, type: :date }) do
+      field :name
+    end
+    klass.create_table
+
+    described_class.execute do |txn|
+      txn.create klass, published_on: '2018-10-07'.to_date, name: 'Alex'
+    end
+
+    obj = klass.last
+    expect(obj.name).to eql 'Alex'
+  end
+
+  it 'uses dumped value of sort key to create item' do
+    klass = new_class do
+      range :activated_on, :date
+      field :name
+    end
+    klass.create_table
+
+    described_class.execute do |txn|
+      txn.create klass, activated_on: Date.today, name: 'Alex'
+    end
+
+    obj = klass.last
+    expect(obj.name).to eql 'Alex'
+  end
+
   describe 'callbacks' do
     before do
       ScratchPad.clear
@@ -699,4 +728,33 @@ def around_save_callback
     expect(obj_to_save).not_to be_persisted
     expect(obj_to_save).to be_changed
   end
+
+  it 'uses dumped value of partition key to create item' do
+    klass = new_class(partition_key: { name: :published_on, type: :date }) do
+      field :name
+    end
+    klass.create_table
+
+    described_class.execute do |txn|
+      txn.create! klass, published_on: '2018-10-07'.to_date, name: 'Alex'
+    end
+
+    obj = klass.last
+    expect(obj.name).to eql 'Alex'
+  end
+
+  it 'uses dumped value of sort key to create item' do
+    klass = new_class do
+      range :activated_on, :date
+      field :name
+    end
+    klass.create_table
+
+    described_class.execute do |txn|
+      txn.create! klass, activated_on: Date.today, name: 'Alex'
+    end
+
+    obj = klass.last
+    expect(obj.name).to eql 'Alex'
+  end
 end

spec/dynamoid/transaction_write/delete_spec.rb

@@ -137,6 +137,37 @@
     expect(obj).not_to be_destroyed
   end
 
+  it 'uses dumped value of partition key to delete item' do
+    klass = new_class(partition_key: { name: :published_on, type: :date }) do
+      field :name
+    end
+    obj = klass.create!(published_on: '2018-10-07'.to_date, name: 'Alex')
+
+    expect {
+      described_class.execute do |txn|
+        txn.delete obj
+      end
+    }.to change(klass, :count).by(-1)
+
+    expect(klass.exists?(obj.published_on)).to eql false
+  end
+
+  it 'uses dumped value of sort key to delete item' do
+    klass = new_class do
+      range :activated_on, :date
+      field :name
+    end
+    obj = klass.create!(activated_on: Date.today, name: 'Alex')
+
+    expect {
+      described_class.execute do |txn|
+        txn.delete obj
+      end
+    }.to change(klass, :count).by(-1)
+
+    expect(klass.exists?([[obj.id, obj.activated_on]])).to eql false
+  end
+
   describe 'callbacks' do
     it 'does not run any callback' do
       klass_with_callbacks = new_class do
@@ -285,6 +316,37 @@ def around_destroy_callback
     end
   end
 
+  it 'uses dumped value of partition key to delete item' do
+    klass = new_class(partition_key: { name: :published_on, type: :date }) do
+      field :name
+    end
+    obj = klass.create!(published_on: '2018-10-07'.to_date, name: 'Alex')
+
+    expect {
+      described_class.execute do |txn|
+        txn.delete klass, obj.published_on
+      end
+    }.to change(klass, :count).by(-1)
+
+    expect(klass.exists?(obj.published_on)).to eql false
+  end
+
+  it 'uses dumped value of sort key to delete item' do
+    klass = new_class do
+      range :activated_on, :date
+      field :name
+    end
+    obj = klass.create!(activated_on: Date.today, name: 'Alex')
+
+    expect {
+      described_class.execute do |txn|
+        txn.delete klass, obj.id, obj.activated_on
+      end
+    }.to change(klass, :count).by(-1)
+
+    expect(klass.exists?([[obj.id, obj.activated_on]])).to eql false
+  end
+
   describe 'callbacks' do
     it 'does not run any callback' do
       klass_with_callbacks = new_class do

spec/dynamoid/transaction_write/destroy_spec.rb

@@ -197,6 +197,37 @@ def around_destroy_callback
     expect(obj).not_to be_destroyed
   end
 
+  it 'uses dumped value of partition key to destroy item' do
+    klass = new_class(partition_key: { name: :published_on, type: :date }) do
+      field :name
+    end
+    obj = klass.create!(published_on: '2018-10-07'.to_date, name: 'Alex')
+
+    expect(klass.exists?(obj.published_on)).to eql true
+
+    described_class.execute do |txn|
+      txn.destroy obj
+    end
+
+    expect(klass.exists?(obj.published_on)).to eql false
+  end
+
+  it 'uses dumped value of sort key to destroy item' do
+    klass = new_class do
+      range :activated_on, :date
+      field :name
+    end
+    obj = klass.create!(activated_on: Date.today, name: 'Alex')
+
+    expect(klass.exists?([[obj.id, obj.activated_on]])).to eql true
+
+    described_class.execute do |txn|
+      txn.destroy obj
+    end
+
+    expect(klass.exists?([[obj.id, obj.activated_on]])).to eql false
+  end
+
   describe 'callbacks' do
     before do
       ScratchPad.clear