Optimise #save method - call PutItem only if there are changed attrib…

…utes

Gemfile.lock

@@ -126,6 +126,7 @@ GEM
 
 PLATFORMS
   x86_64-darwin-19
+  x86_64-darwin-21
   x86_64-darwin-22
   x86_64-linux
 

lib/dynamoid/components.rb

@@ -13,8 +13,6 @@ module Components
 
       define_model_callbacks :create, :save, :destroy, :initialize, :update
 
-      before_create :set_created_at
-      before_save :set_updated_at
       before_save :set_expires_field
       after_initialize :set_inheritance_field
     end

lib/dynamoid/fields.rb

@@ -354,23 +354,6 @@ def read_attribute_before_type_cast(name)
 
     private
 
-    # Automatically called during the created callback to set the created_at time.
-    #
-    # @since 0.2.0
-    def set_created_at
-      self.created_at ||= DateTime.now.in_time_zone(Time.zone) if self.class.timestamps_enabled?
-    end
-
-    # Automatically called during the save callback to set the updated_at time.
-    #
-    # @since 0.2.0
-    def set_updated_at
-      # @_touch_record=false means explicit disabling
-      if self.class.timestamps_enabled? && changed? && !updated_at_changed? && @_touch_record != false
-        self.updated_at = DateTime.now.in_time_zone(Time.zone)
-      end
-    end
-
     def set_expires_field
       options = self.class.options[:expires]
 

lib/dynamoid/persistence.rb

@@ -482,14 +482,11 @@ def persisted?
     # @since 0.2.0
     def save(options = {})
       self.class.create_table(sync: true)
-
-      @_touch_record = options[:touch]
-
       create_or_update = new_record? ? :create : :update
 
       run_callbacks(create_or_update) do
         run_callbacks(:save) do
-          Save.call(self)
+          Save.call(self, touch: options[:touch])
         end
       end
     end

lib/dynamoid/persistence/save.rb

@@ -4,24 +4,44 @@ module Dynamoid
   module Persistence
     # @private
     class Save
-      def self.call(model)
-        new(model).call
+      def self.call(model, **options)
+        new(model, **options).call
       end
 
-      def initialize(model)
+      def initialize(model, touch: nil)
         @model = model
+        @touch = touch # touch=false means explicit disabling of updating the `updated_at` attribute
       end
 
       def call
         @model.hash_key = SecureRandom.uuid if @model.hash_key.blank?
 
+        return true unless @model.changed?
+
+        @model.created_at ||= DateTime.now.in_time_zone(Time.zone) if @model.class.timestamps_enabled?
+
+        if @model.class.timestamps_enabled? && @model.changed? && !@model.updated_at_changed? && @touch != false
+          @model.updated_at = DateTime.now.in_time_zone(Time.zone)
+        end
+
         # Add an optimistic locking check if the lock_version column exists
         if @model.class.attributes[:lock_version]
           @model.lock_version = (@model.lock_version || 0) + 1
         end
 
-        attributes_dumped = Dumping.dump_attributes(@model.attributes, @model.class.attributes)
-        Dynamoid.adapter.write(@model.class.table_name, attributes_dumped, conditions_for_write)
+        if @model.new_record?
+          attributes_dumped = Dumping.dump_attributes(@model.attributes, @model.class.attributes)
+          Dynamoid.adapter.write(@model.class.table_name, attributes_dumped, conditions_for_write)
+        else
+          attributes_to_persist = @model.attributes.slice(*@model.changed.map(&:to_sym))
+
+          Dynamoid.adapter.update_item(@model.class.table_name, @model.hash_key, options_to_update_item) do |t|
+            attributes_to_persist.each do |name, value|
+              value_dumped = Dumping.dump_field(value, @model.class.attributes[name])
+              t.set(name => value_dumped)
+            end
+          end
+        end
 
         @model.new_record = false
         true
@@ -59,6 +79,33 @@ def conditions_for_write
 
         conditions
       end
+
+      def options_to_update_item
+        options = {}
+
+        if @model.class.range_key
+          value_dumped = Dumping.dump_field(@model.range_value, @model.class.attributes[@model.class.range_key])
+          options[:range_key] = value_dumped
+        end
+
+        conditions = {}
+        conditions[:if_exists] ||= {}
+        conditions[:if_exists][@model.class.hash_key] = @model.hash_key
+
+        # Add an optimistic locking check if the lock_version column exists
+        if @model.class.attributes[:lock_version]
+          # Uses the original lock_version value from Dirty API
+          # in case user changed 'lock_version' manually
+          if @model.changes[:lock_version][0]
+            conditions[:if] ||= {}
+            conditions[:if][:lock_version] = @model.changes[:lock_version][0]
+          end
+        end
+
+        options[:conditions] = conditions
+
+        options
+      end
     end
   end
 end

spec/dynamoid/persistence_spec.rb

@@ -1903,22 +1903,66 @@ def log_message
     end
   end
 
-  describe '.save' do
+  describe '#save' do
     let(:klass) do
       new_class do
         field :name
       end
     end
 
-    it 'saves model' do
+    let(:klass_with_range_key) do
+      new_class do
+        field :name
+        range :age, :integer
+      end
+    end
+
+    let(:klass_with_range_key_and_custom_type) do
+      new_class do
+        field :name
+        range :tags, :serialized
+      end
+    end
+
+    it 'persists new model' do
       obj = klass.new(name: 'Alex')
       obj.save
 
       expect(klass.exists?(obj.id)).to eq true
       expect(klass.find(obj.id).name).to eq 'Alex'
     end
 
-    it 'marks it as persisted' do
+    it 'saves changes of already persisted model' do
+      obj = klass.create!(name: 'Alex')
+
+      obj.name = 'Michael'
+      obj.save
+
+      obj_loaded = klass.find(obj.id)
+      expect(obj_loaded.name).to eql 'Michael'
+    end
+
+    it 'saves changes of already persisted model if range key is declared' do
+      obj = klass_with_range_key.create!(name: 'Alex', age: 21)
+
+      obj.name = 'Michael'
+      obj.save
+
+      obj_loaded = klass_with_range_key.find(obj.id, range_key: obj.age)
+      expect(obj_loaded.name).to eql 'Michael'
+    end
+
+    it 'saves changes of already persisted model if range key is declared and its type is not supported by DynamoDB natively' do
+      obj = klass_with_range_key_and_custom_type.create!(name: 'Alex', tags: ['a', 'b'])
+
+      obj.name = 'Michael'
+      obj.save
+
+      obj_loaded = klass_with_range_key_and_custom_type.find(obj.id, range_key: obj.tags)
+      expect(obj_loaded.name).to eql 'Michael'
+    end
+
+    it 'marks persisted new model as persisted' do
       obj = klass.new(name: 'Alex')
       expect { obj.save }.to change { obj.persisted? }.from(false).to(true)
     end
@@ -1969,6 +2013,81 @@ def log_message
       expect(obj_loaded.name).to eql nil
     end
 
+    it 'does not make a request to persist a model if there is no any changed attribute' do
+      expect(Dynamoid.adapter).to receive(:write).and_call_original
+      obj = klass.create(name: 'Alex')
+
+      obj_loaded = klass.find(obj.id)
+
+      expect(Dynamoid.adapter).not_to receive(:write)
+
+      obj.save
+      obj_loaded.save
+    end
+
+    it 'returns true if there is no any changed attribute' do
+      obj = klass.create(name: 'Alex')
+      obj_loaded = klass.find(obj.id)
+
+      expect(obj.save).to eql(true)
+      expect(obj_loaded.save).to eql(true)
+    end
+
+    it "calls PutItem for a new record" do
+      expect(Dynamoid.adapter).to receive(:write).and_call_original
+      klass.create(name: 'Alex')
+    end
+
+    it "calls UpdateItem for already persisted record" do
+      klass = new_class do
+        field :name
+        field :age, :integer
+      end
+
+      obj = klass.create!(name: 'Alex', age: 21)
+      obj.age = 31
+
+      expect(Dynamoid.adapter).to receive(:update_item).and_call_original
+      obj.save
+    end
+
+    it 'does not persist changes if a model was deleted' do
+      obj = klass.create!(name: 'Alex')
+      Dynamoid.adapter.delete_item(klass.table_name, obj.id)
+
+      obj.name = 'Michael'
+
+      expect do
+        expect { obj.save }.to raise_error(Dynamoid::Errors::StaleObjectError)
+      end.not_to change { klass.count }
+    end
+
+    it 'does not persist changes if a model was deleted and range key is declared' do
+      obj = klass_with_range_key.create!(name: 'Alex', age: 21)
+      Dynamoid.adapter.delete_item(klass_with_range_key.table_name, obj.id, range_key: obj.age)
+
+      obj.name = 'Michael'
+
+      expect do
+        expect { obj.save }.to raise_error(Dynamoid::Errors::StaleObjectError)
+      end.not_to change { klass_with_range_key.count }
+    end
+
+    it 'does not persist changes if a model was deleted, range key is declared and its type is not supported by DynamoDB natively' do
+      obj = klass_with_range_key_and_custom_type.create!(name: 'Alex', tags: ['a', 'b'])
+      Dynamoid.adapter.delete_item(
+        obj.class.table_name,
+        obj.id,
+        range_key: Dynamoid::Dumping.dump_field(obj.tags, klass_with_range_key_and_custom_type.attributes[:tags])
+      )
+
+      obj.name = 'Michael'
+
+      expect do
+        expect { obj.save }.to raise_error(Dynamoid::Errors::StaleObjectError)
+      end.not_to change { obj.class.count }
+    end
+
     describe 'partition key value' do
       it 'generates "id" for new model' do
         obj = klass.new