MONGOID-4823 Values for BSON::Binary fields are not converted to Binary (#5317)

* MONGOID-4823 add (de)mongoize for Binary

* MONGOID-4823 add tests for ObjectId

* MONGOID-4823 add release note

* MONGOID-4823 fix comment

* MONGOID-4823 answer comments

* MONGOID-4823 use ::BSON

* Revert "MONGOID-4823 use ::BSON"

This reverts commit 5c893e9.

* MONGOID-4823 revert back to without ::BSON

* MONGOID-4823 fix document test

Author: neilshweky

docs/release-notes/mongoid-8.0.txt

@@ -447,3 +447,28 @@ For example:
 
 As you can see from this example, after pushing ``user2`` to the users array,
 Mongoid 8 correctly updates the number of elements without requiring a reload.
+
+
+Repaired Storing Strings in BSON::Binary fields
+-----------------------------------------------
+
+In Mongoid 8, storing a String in a field of type ``BSON::Binary`` will be
+stored in and returned from the database as a ``BSON::Binary``. Prior to Mongoid 8
+it was stored and returned as a String.
+
+.. code::
+
+  class Registry
+    include Mongoid::Document
+    field :data, type: BSON::Binary
+  end
+
+  registry = Registry.create!(data: "data!")
+  p registry.data
+  # => Mongoid 7: "data!"
+  # => Mongoid 8: <BSON::Binary:0x2580 type=generic data=0x6461746121...>
+
+  registry = Registry.find(registry.id)
+  p registry.data
+  # => Mongoid 7: "data!"
+  # => Mongoid 8: <BSON::Binary:0x2600 type=generic data=0x6461746121...>

lib/mongoid/extensions.rb

@@ -34,6 +34,7 @@ def transform_keys
 
 require "mongoid/extensions/array"
 require "mongoid/extensions/big_decimal"
+require "mongoid/extensions/binary"
 require "mongoid/extensions/boolean"
 require "mongoid/extensions/date"
 require "mongoid/extensions/date_time"

lib/mongoid/extensions/binary.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module Mongoid
+  module Extensions
+    module Binary
+
+      # Turn the object from the ruby type we deal with to a Mongo friendly
+      # type.
+      #
+      # @example Mongoize the object.
+      #   object.mongoize
+      #
+      # @return [ Object ] The object.
+      def mongoize
+        BSON::Binary.mongoize(self)
+      end
+
+      module ClassMethods
+
+        # Mongoize an object of any type to how it's stored in the db.
+        #
+        # @example Mongoize the object.
+        #   BigDecimal.mongoize(123)
+        #
+        # @param [ Object ] object The object to Mongoize
+        #
+        # @return [ String | Symbol | BSON::Binary | nil ] A String or Binary
+        #   representing the object or nil.
+        def mongoize(object)
+          return if object.nil?
+
+          case object
+          when BSON::Binary
+            object
+          when String, Symbol
+            BSON::Binary.new(object.to_s)
+          else
+            # TODO: MONGOID-5222 raise on the setting of feature flag.
+            nil
+          end
+        end
+      end
+    end
+  end
+end
+
+BSON::Binary.__send__(:include, Mongoid::Extensions::Binary)
+BSON::Binary.extend(Mongoid::Extensions::Binary::ClassMethods)

spec/mongoid/document_fields_spec.rb

@@ -40,20 +40,168 @@
       end
 
       it 'assigns as a BSON::Binary object' do
-        pending 'https://jira.mongodb.org/browse/MONGOID-4823'
-
         registry.data.should be_a(BSON::Binary)
       end
 
       it 'persists' do
-        pending 'https://jira.mongodb.org/browse/MONGOID-4823'
-
         registry.save!
 
         _registry = Registry.find(registry.id)
         _registry.data.should == BSON::Binary.new(data)
       end
     end
+
+    context 'when assigned nil' do
+      let(:data) do
+        nil
+      end
+
+      let(:registry) do
+        Registry.new(data: data)
+      end
+
+      it 'assigns nil' do
+        registry.data.should be nil
+      end
+
+      it 'persists' do
+        registry.save!
+
+        _registry = Registry.find(registry.id)
+        _registry.data.should be nil
+      end
+    end
+
+    # TODO: MONGOID-5222 test with uncastable feature flag
+    context 'when assigned an invalid type' do
+      let(:data) do
+        true
+      end
+
+      let(:registry) do
+        Registry.new(data: data)
+      end
+
+      it 'assigns nil' do
+        registry.data.should be nil
+      end
+
+      it 'persists' do
+        registry.save!
+
+        _registry = Registry.find(registry.id)
+        _registry.data.should be nil
+      end
+    end
+  end
+
+  describe 'BSON::ObjectId field' do
+    context 'when assigned a BSON::ObjectId instance' do
+      let(:obj_id) do
+        BSON::ObjectId.new
+      end
+
+      let(:registry) do
+        Registry.new(obj_id: obj_id)
+      end
+
+      it 'does not freeze the specified data' do
+        registry
+
+        obj_id.should_not be_frozen
+      end
+
+      it 'persists' do
+        registry.save!
+
+        _registry = Registry.find(registry.id)
+        _registry.obj_id.should == obj_id
+      end
+    end
+
+    context 'when assigned a valid string' do
+      let(:obj_id) do
+        BSON::ObjectId.new.to_s
+      end
+
+      let(:registry) do
+        Registry.new(obj_id: obj_id)
+      end
+
+      it 'assigns as a BSON::Binary object' do
+        registry.obj_id.should be_a(BSON::ObjectId)
+      end
+
+      it 'persists' do
+        registry.save!
+
+        _registry = Registry.find(registry.id)
+        _registry.obj_id.should == BSON::ObjectId.from_string(obj_id)
+      end
+    end
+
+    context 'when assigned nil' do
+      let(:obj_id) do
+        nil
+      end
+
+      let(:registry) do
+        Registry.new(obj_id: obj_id)
+      end
+
+      it 'assigns nil' do
+        registry.obj_id.should be nil
+      end
+
+      it 'persists' do
+        registry.save!
+
+        _registry = Registry.find(registry.id)
+        _registry.obj_id.should be nil
+      end
+    end
+
+    context 'when assigned an invalid string' do
+      let(:obj_id) do
+        "hello"
+      end
+
+      let(:registry) do
+        Registry.new(obj_id: obj_id)
+      end
+
+      it 'assigns nil' do
+        registry.obj_id.should == "hello"
+      end
+
+      it 'persists' do
+        registry.save!
+
+        _registry = Registry.find(registry.id)
+        _registry.obj_id.should == "hello"
+      end
+    end
+
+    context 'when assigned an invalid type' do
+      let(:obj_id) do
+        :sym
+      end
+
+      let(:registry) do
+        Registry.new(obj_id: obj_id)
+      end
+
+      it 'assigns nil' do
+        registry.obj_id.should == :sym
+      end
+
+      it 'persists' do
+        registry.save!
+
+        _registry = Registry.find(registry.id)
+        _registry.obj_id.should == :sym
+      end
+    end
   end
 
   describe 'Hash field' do

spec/mongoid/extensions/binary_spec.rb

@@ -15,6 +15,54 @@
     end
   end
 
+  describe ".mongoize" do
+
+    let(:binary) do
+      BSON::Binary.new("testing")
+    end
+
+    let(:mongoized) do
+      BSON::Binary.mongoize(value)
+    end
+
+    context "when mongoizing a BSON::Binary" do
+
+      let(:value) { binary }
+
+      it "returns the binary" do
+        expect(mongoized).to eq(binary)
+      end
+    end
+
+    context "when mongoizing a String" do
+
+      let(:value) { "testing" }
+
+      it "returns it as binary" do
+        expect(mongoized).to eq(binary)
+      end
+    end
+
+    context "when mongoizing nil" do
+
+      let(:value) { nil }
+
+      it "returns nil" do
+        expect(mongoized).to eq(nil)
+      end
+    end
+
+    context "when mongoizing an invalid type" do
+
+      let(:value) { true }
+
+      # TODO: MONGOID-5222 test with feature flag on and off
+      it "returns nil" do
+        expect(mongoized).to eq(nil)
+      end
+    end
+  end
+
   describe ".demongoize" do
 
     let(:binary) do

spec/support/models/registry.rb

@@ -3,4 +3,5 @@
 class Registry
   include Mongoid::Document
   field :data, type: BSON::Binary
+  field :obj_id, type: BSON::ObjectId
 end