Merge pull request #681 from Dynamoid/fix-updating-attributes-with-ni…

…l-values

Fix saving persisted model and delete item attributes with nil values

lib/dynamoid/adapter_plugin/aws_sdk_v3/item_updater.rb

@@ -50,7 +50,20 @@ def delete(values)
         # Replaces the values of one or more attributes
         #
         def set(values)
-          @updates.merge!(sanitize_attributes(values))
+          values_sanitized = sanitize_attributes(values)
+
+          if Dynamoid.config.store_attribute_with_nil_value
+            @updates.merge!(values_sanitized)
+          else
+            # delete explicitly attributes if assigned nil value and configured
+            # to not store nil values
+            values_to_update = values_sanitized.select { |_, v| !v.nil? }
+            values_to_delete = values_sanitized.select { |_, v| v.nil? }
+
+            @updates.merge!(values_to_update)
+            @deletions.merge!(values_to_delete)
+          end
+
         end
 
         #
@@ -85,6 +98,10 @@ def attribute_updates
 
         private
 
+        # Keep in sync with AwsSdkV3.sanitize_item.
+        #
+        # The only difference is that to update item we need to track whether
+        # attribute value is nil or not.
         def sanitize_attributes(attributes)
           attributes.transform_values do |v|
             if v.is_a?(Hash)

spec/dynamoid/persistence_spec.rb

@@ -2902,32 +2902,58 @@ def around_save_callback
       end
 
       context 'true', config: { store_attribute_with_nil_value: true } do
-        it 'keeps document attribute with nil' do
+        it 'keeps document attribute with nil when model is not persisted' do
           obj = klass.new(age: nil)
           obj.save
 
           expect(raw_attributes(obj)).to include(age: nil)
         end
+
+        it 'keeps document attribute with nil when model is persisted' do
+          obj = klass.create(age: 42)
+          obj.age = nil
+          obj.save
+
+          expect(raw_attributes(obj)).to include(age: nil)
+        end
       end
 
       context 'false', config: { store_attribute_with_nil_value: false } do
-        it 'does not keep document attribute with nil' do
+        it 'does not keep document attribute with nil when model is not persisted' do
           obj = klass.new(age: nil)
           obj.save
 
           # doesn't contain :age key
           expect(raw_attributes(obj).keys).to contain_exactly(:id, :created_at, :updated_at)
         end
+
+        it 'does not keep document attribute with nil when model is persisted' do
+          obj = klass.create!(age: 42)
+          obj.age = nil
+          obj.save
+
+          # doesn't contain :age key
+          expect(raw_attributes(obj).keys).to contain_exactly(:id, :created_at, :updated_at)
+        end
       end
 
       context 'by default', config: { store_attribute_with_nil_value: nil } do
-        it 'does not keep document attribute with nil' do
+        it 'does not keep document attribute with nil when model is not persisted' do
           obj = klass.new(age: nil)
           obj.save
 
           # doesn't contain :age key
           expect(raw_attributes(obj).keys).to contain_exactly(:id, :created_at, :updated_at)
         end
+
+        it 'does not keep document attribute with nil when model is persisted' do
+          obj = klass.create!(age: 42)
+          obj.age = nil
+          obj.save
+
+          # doesn't contain :age key
+          expect(raw_attributes(obj).keys).to contain_exactly(:id, :created_at, :updated_at)
+        end
       end
     end