Merge pull request #822 from dalibor/binary

Support binary type natively

Author: andrykonchin

.rubocop_thread_safety.yml

@@ -2,5 +2,5 @@
 # TODO: Comment out the following to see code needing to be refactored for thread safety!
 ThreadSafety/ClassAndModuleAttributes:
   Enabled: false
-ThreadSafety/InstanceVariableInClassMethod:
+ThreadSafety/ClassInstanceVariable:
   Enabled: false

README.md

@@ -342,6 +342,24 @@ class Document
 end
 ```
 
+#### Note on binary type
+
+By default binary fields are persisted as DynamoDB String value encoded
+in the Base64 encoding. DynamoDB supports binary data natively. To use
+it instead of String a `store_binary_as_native` field option should be
+set:
+
+```ruby
+class Document
+  include Dynamoid::Document
+
+  field :image, :binary, store_binary_as_native: true
+end
+```
+
+There is also a global config option `store_binary_as_native` that is
+`false` by default as well.
+
 #### Magic Columns
 
 You get magic columns of `id` (`string`), `created_at` (`datetime`), and
@@ -1138,6 +1156,9 @@ Listed below are all configuration options.
 * `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`
+* `store_binary_as_native` - if `true` Dynamoid stores binary fields
+  as native DynamoDB binary values. Otherwise binary fields are stored
+  as Base64 encoded string values. Default is `false`
 * `backoff` - is a hash: key is a backoff strategy (symbol), value is
   parameters for the strategy. Is used in batch operations. Default id
   `nil`

lib/dynamoid/config.rb

@@ -50,6 +50,7 @@ module Config
     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 :store_binary_as_native, default: false
     option :backoff, default: nil # callable object to handle exceeding of table throughput limit
     option :backoff_strategies, default: {
       constant: BackoffStrategies::ConstantBackoff,

lib/dynamoid/dumping.rb

@@ -297,10 +297,24 @@ def process(value)
       end
     end
 
-    # string -> string
+    # string -> StringIO
     class BinaryDumper < Base
       def process(value)
-        Base64.strict_encode64(value)
+        store_as_binary = if @options[:store_as_native_binary].nil?
+                            Dynamoid.config.store_binary_as_native
+                          else
+                            @options[:store_as_native_binary]
+                          end
+
+        if store_as_binary
+          if value.is_a?(StringIO) || value.is_a?(IO)
+            value
+          else
+            StringIO.new(value)
+          end
+        else
+          Base64.strict_encode64(value)
+        end
       end
     end
 

lib/dynamoid/type_casting.rb

@@ -288,7 +288,9 @@ def process(value)
 
     class BinaryTypeCaster < Base
       def process(value)
-        if value.is_a? String
+        if value.is_a?(StringIO) || value.is_a?(IO)
+          value
+        elsif value.is_a?(String)
           value.dup
         else
           value.to_s

lib/dynamoid/undumping.rb

@@ -284,7 +284,17 @@ def process(value)
 
     class BinaryUndumper < Base
       def process(value)
-        Base64.strict_decode64(value)
+        store_as_binary = if @options[:store_as_native_binary].nil?
+                            Dynamoid.config.store_binary_as_native
+                          else
+                            @options[:store_as_native_binary]
+                          end
+
+        if store_as_binary
+          value.string # expect StringIO here
+        else
+          Base64.strict_decode64(value)
+        end
       end
     end
 

spec/dynamoid/dumping_spec.rb

@@ -1606,20 +1606,87 @@ def self.dynamoid_field_type
   end
 
   describe 'Binary field' do
-    let(:klass) do
-      new_class do
-        field :image, :binary
+    let(:unfrozen_string) { +"\x00\x88\xFF" }
+    let(:binary_value) { unfrozen_string.force_encoding('ASCII-8BIT') }
+
+    context 'default non-native binary' do
+      let(:klass) do
+        new_class do
+          field :image, :binary
+        end
+      end
+
+      it 'encodes a string in base64-encoded format' do
+        obj = klass.create(image: binary_value)
+
+        expect(reload(obj).image).to eql(binary_value)
+        expect(raw_attributes(obj)[:image]).to eql(Base64.strict_encode64(binary_value))
       end
     end
 
-    let(:unfrozen_string) { +"\x00\x88\xFF" }
-    let(:binary_value) { unfrozen_string.force_encoding('ASCII-8BIT') }
+    context 'native binary' do
+      let(:klass) do
+        new_class do
+          field :image, :binary, store_as_native_binary: true
+        end
+      end
+
+      it 'converts string to StringIO object' do
+        obj = klass.create(image: binary_value)
+
+        expect(reload(obj).image).to eql(binary_value)
+        expect(raw_attributes(obj)[:image].class).to eql(StringIO)
+        expect(raw_attributes(obj)[:image].string).to eql(binary_value)
+      end
 
-    it 'encodes a string in base64-encoded format' do
-      obj = klass.create(image: binary_value)
+      it 'accepts StringIO object' do
+        image = StringIO.new(binary_value)
+        obj = klass.create(image: image)
 
-      expect(reload(obj).image).to eql(binary_value)
-      expect(raw_attributes(obj)[:image]).to eql(Base64.strict_encode64(binary_value))
+        expect(reload(obj).image).to eql(binary_value)
+        expect(raw_attributes(obj)[:image].class).to eql(StringIO)
+        expect(raw_attributes(obj)[:image].string).to eql(binary_value)
+      end
+
+      it 'accepts IO object' do
+        Tempfile.create('image') do |image|
+          image.write(binary_value)
+          image.rewind
+
+          obj = klass.create(image: image)
+
+          expect(reload(obj).image).to eql(binary_value)
+          expect(raw_attributes(obj)[:image].class).to eql(StringIO)
+          expect(raw_attributes(obj)[:image].string).to eql(binary_value)
+        end
+      end
+    end
+
+    context 'store_binary_as_native config option' do
+      it 'is stored as binary if store_binary_as_native config option is true',
+         config: { store_binary_as_native: true } do
+        klass = new_class do
+          field :image, :binary
+        end
+
+        obj = klass.create(image: binary_value)
+
+        expect(reload(obj).image).to eql(binary_value)
+        expect(raw_attributes(obj)[:image].class).to eql(StringIO)
+        expect(raw_attributes(obj)[:image].string).to eql(binary_value)
+      end
+
+      it 'is not stored as binary if store_binary_as_native config option is false',
+         config: { store_binary_as_native: false } do
+        klass = new_class do
+          field :image, :binary
+        end
+
+        obj = klass.create(image: binary_value)
+
+        expect(reload(obj).image).to eql(binary_value)
+        expect(raw_attributes(obj)[:image]).to eql(Base64.strict_encode64(binary_value))
+      end
     end
   end
 end

spec/dynamoid/type_casting_spec.rb

@@ -274,7 +274,7 @@
 
         obj = klass.new(values: Set.new([1, 1.5, '2'.to_d]))
 
-        expect(obj.values).to eql(Set.new([1, 1, 2]))
+        expect(obj.values).to eql(Set.new([1, 2]))
       end
 
       it 'type casts numbers' do
@@ -671,12 +671,21 @@ def settings.to_hash
       expect(obj.image).to eql('string representation')
     end
 
-    it 'dups a string' do
-      value = 'foo'
+    it 'does not convert StringIO objects' do
+      value = StringIO.new('foo')
       obj = klass.new(image: value)
 
-      expect(obj.image).to eql(value)
-      expect(obj.image).not_to equal(value)
+      expect(obj.image).to equal(value)
+    end
+
+    it 'does not convert IO objects' do
+      Tempfile.create('image') do |value|
+        value.write('foo')
+        value.rewind
+
+        obj = klass.new(image: value)
+        expect(obj.image).to equal(value)
+      end
     end
   end