Fix MONGOID-4874 Values seem to be lost when using add_to_set on a no…

…n-existent attribute on a loaded instance (#4749)

* Retain values added to sets of loaded instances

It seems when an instance is loaded from a database, its `attributes`
method returns an instance of `BSON::Document`. This causes some
surprising behavior when we use `#add_to_set` on that loaded instance.

It seems when a value is assigned to a `BSON::Document` instance, it gets
modified:

https://github.com/mongodb/bson-ruby/blob/master/lib/bson/document.rb#L87-L89

During modification, the attribute loses its reference to the original
value, however the `add_to_set` method relies on this reference:

https://github.com/mongodb/mongoid/blob/136ccbc140b25719f48ff5185bb471f90c063148/lib/mongoid/persistable/pushable.rb#L27-L31

Since that reference is broken, it no longer update the instance's
attribute and the value is lost in memory, though still retained in the
DB.

This ensures that the reference remains intact.

* expand test coverage

* express the requirements more strictly

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

lib/mongoid/persistable/pushable.rb

@@ -22,7 +22,13 @@ module Pushable
       def add_to_set(adds)
         prepare_atomic_operation do |ops|
           process_atomic_operations(adds) do |field, value|
-            existing = send(field) || (attributes[field] ||= [])
+            existing = send(field) || attributes[field]
+            if existing.nil?
+              attributes[field] = []
+              # Read the value out of attributes:
+              # https://jira.mongodb.org/browse/MONGOID-4874
+              existing = attributes[field]
+            end
             values = [ value ].flatten(1)
             values.each do |val|
               existing.push(val) unless existing.include?(val)

spec/mongoid/persistable/pushable_spec.rb

@@ -4,7 +4,7 @@
 
   describe "#add_to_set" do
 
-    context "when the document is a root document" do
+    context "when the document is a top level document" do
 
       shared_examples_for "a unique pushable root document" do
 
@@ -62,6 +62,60 @@
 
         it_behaves_like "a unique pushable root document"
       end
+
+      context 'when the host model is not saved' do
+        context 'when attribute exists' do
+          let(:person) do
+            Person.new(aliases: [2])
+          end
+
+          it 'records the change' do
+            person.add_to_set({aliases: 1})
+
+            expect(person.aliases).to eq([2, 1])
+          end
+        end
+
+        context 'when attribute does not exist' do
+          let(:person) do
+            Person.new
+          end
+
+          it 'records the change' do
+            person.add_to_set({aliases: 1})
+
+            expect(person.aliases).to eq([1])
+          end
+        end
+      end
+
+      context 'when the host model is loaded from database' do
+        context 'when attribute exists' do
+          let(:person) do
+            Person.create!(aliases: [2])
+            person = Person.last
+          end
+
+          it 'records the change' do
+            person.add_to_set({aliases: 1})
+
+            expect(person.aliases).to eq([2, 1])
+          end
+        end
+
+        context 'when attribute does not exist' do
+          let(:person) do
+            Person.create!
+            person = Person.last
+          end
+
+          it 'records the change' do
+            person.add_to_set({aliases: 1})
+
+            expect(person.aliases).to eq([1])
+          end
+        end
+      end
     end
 
     context "when the document is embedded" do