Merge pull request #794 from Dynamoid/add-configuration-option-to-per…

…sist-empty-strings

Add configuration option to persist empty strings as is

.github/workflows/ci.yml

@@ -156,12 +156,12 @@ jobs:
 
     - name: Start dynamodb-local
       run: |
-        docker-compose up -d
+        docker compose up -d
 
     - name: Run RSpec tests
       run: |
         bundle exec rspec
 
     - name: Stop dynamodb-local
       run: |
-        docker-compose down
+        docker compose down

.github/workflows/coverage.yml

@@ -66,15 +66,15 @@ jobs:
 
       - name: Start dynamodb-local
         run: |
-          docker-compose up -d
+          docker compose up -d
 
       - name: Run RSpec tests
         run: |
           bundle exec rspec
 
       - name: Stop dynamodb-local
         run: |
-          docker-compose down
+          docker compose down
 
       - name: CodeClimate Post-build Notification
         run: cc-test-reporter after-build

.rubocop_rspec.yml

@@ -1,4 +1,7 @@
-RSpec/FilePath:
+RSpec/SpecFilePathFormat:
+  Enabled: false
+
+RSpec/SpecFilePathSuffix:
   Enabled: false
 
 RSpec/MultipleExpectations:
@@ -28,6 +31,9 @@ RSpec/NestedGroups:
 RSpec/ExpectInHook:
   Enabled: false
 
+RSpec/ExpectInLet:
+  Enabled: false
+
 # NOTE: for many tests of equality `eql` works, while `be` does not, because
 #        expected #<Fixnum:...> => 101
 #            got #<BigDecimal:...> => 101.0 (0.101e3)

README.md

@@ -1134,6 +1134,7 @@ Listed below are all configuration options.
   fields in ISO 8601 string format. Default is `false`
 * `store_date_as_string` - if `true` then Dynamoid stores :date fields
   in ISO 8601 string format. Default is `false`
+* `store_empty_string_as_nil` - store attribute's empty String value as NULL. Default is `true`
 * `store_boolean_as_native` - if `true` Dynamoid stores boolean fields
   as native DynamoDB boolean values. Otherwise boolean fields are stored
   as string values `'t'` and `'f'`. Default is `true`

dynamoid.gemspec

@@ -51,9 +51,9 @@ Gem::Specification.new do |spec|
   spec.metadata['wiki_uri'] = 'https://github.com/Dynamoid/dynamoid/wiki'
   spec.metadata['rubygems_mfa_required'] = 'true'
 
-  spec.add_runtime_dependency 'activemodel', '>=4'
-  spec.add_runtime_dependency 'aws-sdk-dynamodb', '~> 1.0'
-  spec.add_runtime_dependency 'concurrent-ruby', '>= 1.0'
+  spec.add_dependency 'activemodel', '>=4'
+  spec.add_dependency 'aws-sdk-dynamodb', '~> 1.0'
+  spec.add_dependency 'concurrent-ruby', '>= 1.0'
 
   spec.add_development_dependency 'appraisal'
   spec.add_development_dependency 'bundler'

lib/dynamoid/adapter_plugin/aws_sdk_v3.rb

@@ -667,8 +667,7 @@ def sanitize_item(attributes)
         store_attribute_with_nil_value = config_value.nil? ? false : !!config_value
 
         attributes.reject do |_, v|
-          ((v.is_a?(Set) || v.is_a?(String)) && v.empty?) ||
-            (!store_attribute_with_nil_value && v.nil?)
+          !store_attribute_with_nil_value && v.nil?
         end.transform_values do |v|
           v.is_a?(Hash) ? v.stringify_keys : v
         end

lib/dynamoid/adapter_plugin/aws_sdk_v3/item_updater.rb

@@ -97,18 +97,19 @@ def attribute_updates
 
         private
 
-        # Keep in sync with AwsSdkV3.sanitize_item.
+        # It's a single low level component available in a public API (with
+        # Document#update/#update! methods). So duplicate sanitizing to some
+        # degree.
         #
-        # The only difference is that to update item we need to track whether
-        # attribute value is nil or not.
+        # Keep in sync with AwsSdkV3.sanitize_item.
         def sanitize_attributes(attributes)
           # rubocop:disable Lint/DuplicateBranch
           attributes.transform_values do |v|
             if v.is_a?(Hash)
               v.stringify_keys
             elsif v.is_a?(Set) && v.empty?
               nil
-            elsif v.is_a?(String) && v.empty?
+            elsif v.is_a?(String) && v.empty? && Config.store_empty_string_as_nil
               nil
             else
               v

lib/dynamoid/config.rb

@@ -48,6 +48,7 @@ module Config
     option :dynamodb_timezone, default: :utc # available values - :utc, :local, time zone name like "Hawaii"
     option :store_datetime_as_string, default: false # store Time fields in ISO 8601 string format
     option :store_date_as_string, default: false # store Date fields in ISO 8601 string format
+    option :store_empty_string_as_nil, default: true # store attribute's empty String value as null
     option :store_boolean_as_native, default: true
     option :backoff, default: nil # callable object to handle exceeding of table throughput limit
     option :backoff_strategies, default: {

lib/dynamoid/dumping.rb

@@ -70,7 +70,8 @@ def sanitize_array(array)
       end
 
       def invalid_value?(value)
-        (value.is_a?(Set) || value.is_a?(String)) && value.empty?
+        (value.is_a?(Set) && value.empty?) ||
+          (value.is_a?(String) && value.empty? && Config.store_empty_string_as_nil)
       end
     end
 
@@ -86,6 +87,12 @@ def process(value)
 
     # string -> string
     class StringDumper < Base
+      def process(string)
+        return nil if string.nil?
+        return nil if string.empty? && Config.store_empty_string_as_nil
+
+        string
+      end
     end
 
     # integer -> number
@@ -101,6 +108,8 @@ class SetDumper < Base
       ALLOWED_TYPES = %i[string integer number date datetime serialized].freeze
 
       def process(set)
+        return nil if set.is_a?(Set) && set.empty?
+
         if @options.key?(:of)
           process_typed_collection(set)
         else
@@ -112,13 +121,13 @@ def process(set)
 
       def process_typed_collection(set)
         if allowed_type?
-          dumper = Dumping.find_dumper(element_options)
-          result = set.map { |el| dumper.process(el) }
-
-          if element_type == :string
-            result.reject!(&:empty?)
+          # StringDumper may replace "" with nil so we cannot distinguish it from an explicit nil
+          if element_type == :string && Config.store_empty_string_as_nil
+            set.reject! { |s| s && s.empty? }
           end
 
+          dumper = Dumping.find_dumper(element_options)
+          result = set.map { |el| dumper.process(el) }
           result.to_set
         else
           raise ArgumentError, "Set element type #{element_type} isn't supported"
@@ -164,14 +173,13 @@ def process(array)
 
       def process_typed_collection(array)
         if allowed_type?
-          dumper = Dumping.find_dumper(element_options)
-          result = array.map { |el| dumper.process(el) }
-
-          if element_type == :string
-            result.reject!(&:empty?)
+          # StringDumper may replace "" with nil so we cannot distinguish it from an explicit nil
+          if element_type == :string && Config.store_empty_string_as_nil
+            array.reject! { |s| s && s.empty? }
           end
 
-          result
+          dumper = Dumping.find_dumper(element_options)
+          array.map { |el| dumper.process(el) }
         else
           raise ArgumentError, "Array element type #{element_type} isn't supported"
         end

lib/dynamoid/errors.rb

@@ -16,7 +16,7 @@ class MissingIndex < Error; end
     class InvalidIndex < Error
       def initialize(item)
         if item.is_a? String
-          super(item)
+          super
         else
           super("Validation failed: #{item.errors.full_messages.join(', ')}")
         end

lib/dynamoid/persistence.rb

@@ -10,6 +10,7 @@
 require 'dynamoid/persistence/save'
 require 'dynamoid/persistence/inc'
 require 'dynamoid/persistence/update_validations'
+require 'dynamoid/persistence/item_updater_with_dumping'
 
 # encoding: utf-8
 module Dynamoid
@@ -669,12 +670,12 @@ def update!(conditions = {})
           update_item_options = options.merge(conditions: conditions)
 
           new_attrs = Dynamoid.adapter.update_item(table_name, hash_key, update_item_options) do |t|
-            t.add(lock_version: 1) if self.class.attributes[:lock_version]
+            item_updater = ItemUpdaterWithDumping.new(self.class, t)
+
+            item_updater.add(lock_version: 1) if self.class.attributes[:lock_version]
 
             if self.class.timestamps_enabled?
-              time_now = DateTime.now.in_time_zone(Time.zone)
-              time_now_dumped = Dumping.dump_field(time_now, self.class.attributes[:updated_at])
-              t.set(updated_at: time_now_dumped)
+              item_updater.set(updated_at: DateTime.now.in_time_zone(Time.zone))
             end
 
             yield t

lib/dynamoid/persistence/inc.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'item_updater_with_casting_and_dumping'
+
 module Dynamoid
   module Persistence
     # @private
@@ -21,15 +23,17 @@ def call
         touch = @counters.delete(:touch)
 
         Dynamoid.adapter.update_item(@model_class.table_name, @hash_key, update_item_options) do |t|
+          item_updater = ItemUpdaterWithCastingAndDumping.new(@model_class, t)
+
           @counters.each do |name, value|
-            t.add(name => cast_and_dump_attribute_value(name, value))
+            item_updater.add(name => value)
           end
 
           if touch
             value = DateTime.now.in_time_zone(Time.zone)
 
             timestamp_attributes_to_touch(touch).each do |name|
-              t.set(name => cast_and_dump_attribute_value(name, value))
+              item_updater.set(name => value)
             end
           end
         end
@@ -48,11 +52,6 @@ def update_item_options
         end
       end
 
-      def cast_and_dump_attribute_value(name, value)
-        value_casted = TypeCasting.cast_field(value, @model_class.attributes[name])
-        Dumping.dump_field(value_casted, @model_class.attributes[name])
-      end
-
       def timestamp_attributes_to_touch(touch)
         return [] unless touch
 

lib/dynamoid/persistence/item_updater_with_casting_and_dumping.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module Dynamoid
+  module Persistence
+    # @private
+    class ItemUpdaterWithCastingAndDumping
+      def initialize(model_class, item_updater)
+        @model_class = model_class
+        @item_updater = item_updater
+      end
+
+      def add(attributes)
+        @item_updater.add(cast_and_dump(attributes))
+      end
+
+      def set(attributes)
+        @item_updater.set(cast_and_dump(attributes))
+      end
+
+      private
+
+      def cast_and_dump(attributes)
+        casted_and_dumped = {}
+
+        attributes.each do |name, value|
+          value_casted = TypeCasting.cast_field(value, @model_class.attributes[name])
+          value_dumped = Dumping.dump_field(value_casted, @model_class.attributes[name])
+
+          casted_and_dumped[name] = value_dumped
+        end
+
+        casted_and_dumped
+      end
+    end
+  end
+end

lib/dynamoid/persistence/item_updater_with_dumping.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Dynamoid
+  module Persistence
+    # @private
+    class ItemUpdaterWithDumping
+      def initialize(model_class, item_updater)
+        @model_class = model_class
+        @item_updater = item_updater
+      end
+
+      def add(attributes)
+        @item_updater.add(dump(attributes))
+      end
+
+      def set(attributes)
+        @item_updater.set(dump(attributes))
+      end
+
+      private
+
+      def dump(attributes)
+        dumped = {}
+
+        attributes.each do |name, value|
+          dumped[name] = Dumping.dump_field(value, @model_class.attributes[name])
+        end
+
+        dumped
+      end
+    end
+  end
+end

lib/dynamoid/persistence/save.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'item_updater_with_dumping'
+
 module Dynamoid
   module Persistence
     # @private
@@ -36,9 +38,10 @@ def call
           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|
+            item_updater = ItemUpdaterWithDumping.new(@model.class, t)
+
             attributes_to_persist.each do |name, value|
-              value_dumped = Dumping.dump_field(value, @model.class.attributes[name])
-              t.set(name => value_dumped)
+              item_updater.set(name => value)
             end
           end
         end

lib/dynamoid/persistence/update_fields.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'item_updater_with_casting_and_dumping'
+
 module Dynamoid
   module Persistence
     # @private
@@ -32,10 +34,10 @@ def call
 
       def update_item
         Dynamoid.adapter.update_item(@model_class.table_name, @partition_key, options_to_update_item) do |t|
+          item_updater = ItemUpdaterWithCastingAndDumping.new(@model_class, t)
+
           @attributes.each do |k, v|
-            value_casted = TypeCasting.cast_field(v, @model_class.attributes[k])
-            value_dumped = Dumping.dump_field(value_casted, @model_class.attributes[k])
-            t.set(k => value_dumped)
+            item_updater.set(k => v)
           end
         end
       end

lib/dynamoid/persistence/upsert.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'item_updater_with_casting_and_dumping'
+
 module Dynamoid
   module Persistence
     # @private
@@ -32,11 +34,10 @@ def call
 
       def update_item
         Dynamoid.adapter.update_item(@model_class.table_name, @partition_key, options_to_update_item) do |t|
-          @attributes.each do |k, v|
-            value_casted = TypeCasting.cast_field(v, @model_class.attributes[k])
-            value_dumped = Dumping.dump_field(value_casted, @model_class.attributes[k])
+          item_updater = ItemUpdaterWithCastingAndDumping.new(@model_class, t)
 
-            t.set(k => value_dumped)
+          @attributes.each do |k, v|
+            item_updater.set(k => v)
           end
         end
       end