MONGOID-5452 add legacy_attributes to 7.5 (#5424)

* MONGOID-5365 Change #attributes return value to Hash with a flag (#5301)

* MONGOID-5365 Change #attributes return value to Hash with a flag

* MONGOID-5365 add config_override to tests

* MONGOID-5365 rename flag and flip default

* MONGOID-5365 fix old flag ref

* Update lib/mongoid/config.rb

* Update docs/reference/configuration.txt

* Update docs/reference/configuration.txt

* MONGOID-5365 fix config test

Author: neilshweky

docs/reference/configuration.txt

@@ -369,6 +369,12 @@ for details on driver options.
       # to parent contexts by default. (default: false)
       join_contexts: false
 
+      # When this flag is true, the attributes method on a document will return
+      # a BSON::Document when that document is retrieved from the database, and
+      # a Hash otherwise. When this flag is false, the attributes method will
+      # always return a Hash. (default: true)
+      legacy_attributes: true
+
       # Maintain legacy behavior of pluck and distinct, which does not demongoize
       # values on returning them. Setting this option to false will cause
       # pluck and distinct to return demongoized values. Setting this option to

docs/release-notes/mongoid-7.5.txt

@@ -206,7 +206,7 @@ example:
 .. code-block:: ruby
 
   Band.all.map(:name)
-  
+
   # Equivalent to:
   Band.pluck(:name)
 
@@ -244,3 +244,42 @@ The ``Document#to_a`` method is deprecated in Mongoid 7.5.
 Mongoid 7.5 fixes incorrect usage of the driver's ``update_one`` method from
 Mongoid's ``upsert`` method. Mongoid's ``upsert`` actually performs a
 replacing upsert, and Mongoid 7.5 correctly calls ``replace_one``.
+
+
+Force the ``attributes`` Method to Always Return a ``Hash``
+-----------------------------------------------------------
+
+Mongoid 7.5 with the ``Mongoid.legacy_attributes`` option set to ``false``
+will always return a ``Hash`` when calling the ``attributes`` method.
+For example:
+
+.. code-block:: ruby
+
+  class Band
+    include Mongoid::Document
+
+    field :name
+  end
+
+  band = Band.create!(name: "The Rolling Stones")
+  p band.attributes.class
+  # => Hash
+
+  band = Band.first
+  p band.attributes.class
+  # => Hash
+
+In Mongoid 7.4 and earlier, and in 7.5 with the ``Mongoid.legacy_attributes``
+option set to ``true``, the ``attributes`` method on a document will return a
+``BSON::Document`` when retrieving that document from the database, but will
+return a ``Hash`` when instantiating a new document:
+
+.. code-block:: ruby
+
+  band = Band.create!(name: "The Rolling Stones")
+  p band.attributes.class
+  # => Hash
+
+  band = Band.first
+  p band.attributes.class
+  # => BSON::Document

lib/mongoid/config.rb

@@ -116,6 +116,12 @@ module Config
     # using and's instead of overwriting them.
     option :overwrite_chained_operators, default: true
 
+    # When this flag is true, the attributes method on a document will return
+    # a BSON::Document when that document is retrieved from the database, and
+    # a Hash otherwise. When this flag is false, the attributes method will
+    # always return a Hash.
+    option :legacy_attributes, default: true
+
     # Has Mongoid been configured? This is checking that at least a valid
     # client config exists.
     #

lib/mongoid/document.rb

@@ -277,8 +277,15 @@ module ClassMethods
       #   criteria.
       #
       # @return [ Document ] A new document.
+      #
+      # @api private
       def instantiate(attrs = nil, selected_fields = nil)
-        attributes = attrs || {}
+        attributes = if Mongoid.legacy_attributes
+          attrs
+        else
+          attrs&.to_h
+        end || {}
+
         doc = allocate
         doc.__selected_fields = selected_fields
         doc.instance_variable_set(:@attributes, attributes)

spec/mongoid/association/referenced/has_and_belongs_to_many/proxy_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
+require_relative "../has_and_belongs_to_many_models.rb"
 
 describe Mongoid::Association::Referenced::HasAndBelongsToMany::Proxy do
 
@@ -3770,4 +3771,33 @@ class Distributor
       expect(p2.d_ids).to match_array([d2.id])
     end
   end
+
+  # This test is for MONGOID-5344 which tests that the initial call to
+  # signature_ids refers to the same array as subsequent calls to signature_ids.
+  # Prior to the change in that ticket, this test broke because the array
+  # returned from write_attribute (which is triggered the first time the
+  # foreign key array is referenced, to set the default), refers to a different
+  # array to the one stored in the attributes hash. This happened because,
+  # when retrieving a document from the database, the attributes hash is actually
+  # a BSON::Document, which applies a transformation to the array before
+  # storing it.
+  context "when executing concat on foreign key array from the db" do
+    config_override :legacy_attributes, false
+
+    before do
+      HabtmmContract.create!
+      HabtmmSignature.create!
+    end
+
+    let!(:contract) { HabtmmContract.first }
+    let!(:signature) { HabtmmSignature.first }
+
+    before do
+      contract.signature_ids.concat([signature.id])
+    end
+
+    it "works on the first attempt" do
+      expect(contract.signature_ids).to eq([signature.id])
+    end
+  end
 end

spec/mongoid/association/referenced/has_many/proxy_spec.rb

@@ -4089,4 +4089,24 @@
       expect(band.same_name).to eq([agent])
     end
   end
+
+  context "when executing concat on foreign key array from the db" do
+    config_override :legacy_attributes, false
+
+    before do
+      Agent.create!
+      Basic.create!
+    end
+
+    let!(:agent) { Agent.first }
+    let!(:basic) { Basic.first }
+
+    before do
+      agent.basic_ids.concat([basic.id])
+    end
+
+    it "works on the first attempt" do
+      expect(agent.basic_ids).to eq([basic.id])
+    end
+  end
 end

spec/mongoid/attributes_spec.rb

@@ -1675,6 +1675,50 @@
         end
       end
     end
+
+    context "when comparing the object_ids of the written value" do
+      config_override :legacy_attributes, false
+
+      before do
+        Person.create!
+      end
+
+      let(:person) do
+        Person.first
+      end
+
+      context "when the field is not resizable" do
+        let(:test) do
+          person.write_attribute(:test, "aliased field to test")
+        end
+
+        it "has the same object_id as the attributes hash value" do
+          expect(test.object_id).to eq(person.test.object_id)
+        end
+      end
+
+      context "when the field is resizable" do
+
+        let(:arrays) do
+          person.write_attribute(:arrays, [])
+        end
+
+        it "has the same object_id as the attributes hash value" do
+          expect(arrays.object_id).to eq(person.arrays.object_id)
+        end
+      end
+
+      context "when the field is a HABTM foreign key array" do
+
+        let(:preference_ids) do
+          person.write_attribute(:preference_ids, [])
+        end
+
+        it "has the same object_id as the attributes hash value" do
+          expect(preference_ids.object_id).to eq(person.preference_ids.object_id)
+        end
+      end
+    end
   end
 
   describe "#typed_value_for" do

spec/mongoid/config_spec.rb

@@ -335,6 +335,13 @@
     it_behaves_like "a config option"
   end
 
+  context 'when setting the legacy_attributes option in the config' do
+    let(:option) { :legacy_attributes }
+    let(:default) { true }
+
+    it_behaves_like "a config option"
+  end
+
   describe "#load!" do
 
     before(:all) do