RUBY-2857 Reorder DBRef fields to have $ref/$id/$db first (#261)

* Revert "from_bson override is no longer needed"

This reverts commit cf2d99b.

* RUBY-2857 Reorder DBRef fields to have $ref/$id/$db first

* RUBY-2857 handle dup and symbol cases

* RUBY-2857 move the instantiation of bson document

* get rid of from_bson again

* RUBY-2857 remove argument from super

Co-authored-by: Oleg Pudeyev <[email protected]>

lib/bson/dbref.rb

@@ -66,8 +66,9 @@ def as_json(*args)
     # @example Create the DBRef.
     #   BSON::DBRef.new({'$ref' => 'users', '$id' => id, '$db' => 'database'})
     #
-    # @param [ Hash ] hash the DBRef hash. It must contain $collection and $id.
+    # @param [ Hash ] hash the DBRef hash. It must contain $ref and $id.
     def initialize(hash)
+      hash = reorder_fields(hash)
       %w($ref $id).each do |key|
         unless hash[key]
           raise ArgumentError, "DBRef must have #{key}: #{hash}"
@@ -84,7 +85,7 @@ def initialize(hash)
         end
       end
 
-      super(hash)
+      super
     end
 
     # Converts the DBRef to raw BSON.
@@ -99,5 +100,26 @@ def initialize(hash)
     def to_bson(buffer = ByteBuffer.new, validating_keys = Config.validating_keys?)
       as_json.to_bson(buffer, validating_keys)
     end
+
+    private
+
+    # Reorder the fields of the given Hash to have $ref first, $id second,
+    # and $db third. The rest of the fields in the hash can come in any
+    # order after that.
+    #
+    # @param [ Hash ] hash The input hash. Must be a valid dbref.
+    #
+    # @return [ Hash ] The hash with it's fields reordered.
+    def reorder_fields(hash)
+      hash = BSON::Document.new(hash)
+      reordered = {}
+      reordered['$ref'] = hash.delete('$ref')
+      reordered['$id'] = hash.delete('$id')
+      if db = hash.delete('$db')
+        reordered['$db'] = db
+      end
+
+      reordered.update(hash)
+    end
   end
 end

spec/bson/dbref_spec.rb

@@ -131,6 +131,70 @@
         end.to raise_error(ArgumentError, /The value for key \$db must be a string/)
       end
     end
+
+    context 'when providing the fieds as symbols' do
+      let(:hash) do
+        { :$ref => 'users', :$id => object_id, :$db => 'db' }
+      end
+
+      it 'does not raise an error' do
+        expect do
+          dbref
+        end.to_not raise_error
+      end
+    end
+
+    context 'when testing the ordering of the fields' do
+      context 'when the fields are in order' do
+        let(:hash) do
+          { '$ref' => 'users', '$id' => object_id, '$db' => 'db' }
+        end
+
+        it 'has the correct order' do
+          expect(dbref.keys).to eq(['$ref', '$id', '$db'])
+        end
+      end
+
+      context 'when the fields are out of order' do
+        let(:hash) do
+          { '$db' => 'db', '$id' => object_id, '$ref' => 'users' }
+        end
+
+        it 'has the correct order' do
+          expect(dbref.keys).to eq(['$ref', '$id', '$db'])
+        end
+      end
+
+      context 'when there is no db' do
+        let(:hash) do
+          { '$id' => object_id, '$ref' => 'users' }
+        end
+
+        it 'has the correct order' do
+          expect(dbref.keys).to eq(['$ref', '$id'])
+        end
+      end
+
+      context 'when the there are other fields in order' do
+        let(:hash) do
+          { '$ref' => 'users', '$id' => object_id, '$db' => 'db', 'x' => 'y', 'y' => 'z' }
+        end
+
+        it 'has the correct order' do
+          expect(dbref.keys).to eq(['$ref', '$id', '$db', 'x', 'y'])
+        end
+      end
+
+      context 'when the there are other fields out of order' do
+        let(:hash) do
+          { 'y' => 'z', '$db' => 'db', '$id' => object_id, 'x' => 'y', '$ref' => 'users' }
+        end
+
+        it 'has the correct order' do
+          expect(dbref.keys).to eq(['$ref', '$id', '$db', 'y', 'x'])
+        end
+      end
+    end
   end
 
   describe '#to_bson' do