MONGOID-3468 Always touch parents of embedded documents when embedded documents are touched (#4904)

* perform exact time assertions in existing tests

* tests for child -> parent updated_at updates

* propagate touches through composition hierarchy

* fix the tests to reflect embedded associations always touching their parents

* documentation

* forgot to include

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

Author: p-mongo

Gemfile

@@ -12,6 +12,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

docs/tutorials/mongoid-relations.txt

@@ -1082,19 +1082,54 @@ Mongoid to instantiate a new document when the association is accessed and it is
 Touching
 --------
 
-Any ``belongs_to`` association, no matter where it hangs off from, can take an optional ``:touch``
-option which will call the touch method on it and any parent associations with the option defined
-when the base document calls ``#touch``.
+Any ``belongs_to`` association can take an optional ``:touch`` option which
+will cause the parent document be touched whenever the child document is
+touched:
 
 .. code-block:: ruby
 
-   class Band
-     include Mongoid::Document
-     belongs_to :label, touch: true
-   end
+  class Band
+    include Mongoid::Document
+    belongs_to :label, touch: true
+  end
 
-   band = Band.first
-   band.touch # Calls touch on the parent label.
+  band = Band.first
+  band.touch # Calls touch on the parent label.
+
+``:touch`` can also take a string or symbol argument specifying a field to
+be touched on the parent association in addition to updated_at:
+
+.. code-block:: ruby
+
+  class Label
+    include Mongoid::Document
+    include Mongoid::Timestamps
+    field :bands_updated_at, type: Time
+    has_many :bands
+  end
+
+  class Band
+    include Mongoid::Document
+    belongs_to :label, touch: :bands_updated_at
+  end
+  
+  label = Label.create!
+  band = Band.create!(label: label)
+
+  band.touch # Updates updated_at and bands_updated_at on the label.
+
+When an embedded document is touched, its parents are recursively touched
+through the composition root (because all of the parents are necessarily saved
+when the embedded document is saved). The ``:touch`` attribute therefore is
+unnecessary on ``embedded_in`` associations.
+
+Mongoid currently `does not support specifying an additional field to be
+touched on an embedded_in association <https://jira.mongodb.org/browse/MONGOID-5014>`_.
+
+``:touch`` should not be set to ``false`` on an ``embedded_in`` association,
+since composition hierarchy is always updated upon a touch of an embedded
+document. This is currently not enforced but enforcement is `intended in the
+future <https://jira.mongodb.org/browse/MONGOID-5016>`_.
 
 The counter_cache Option
 ------------------------

gemfiles/driver_master.gemfile

@@ -14,6 +14,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/driver_master_jruby.gemfile

@@ -18,6 +18,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/driver_min.gemfile

@@ -13,6 +13,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/driver_min_jruby.gemfile

@@ -16,6 +16,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/driver_oldstable.gemfile

@@ -14,6 +14,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/driver_oldstable_jruby.gemfile

@@ -16,6 +16,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/driver_stable.gemfile

@@ -14,6 +14,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/driver_stable_jruby.gemfile

@@ -16,6 +16,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/i18n-1.0.gemfile

@@ -12,6 +12,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/rails_51.gemfile

@@ -11,6 +11,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/rails_52.gemfile

@@ -11,6 +11,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

gemfiles/rails_master.gemfile

@@ -11,6 +11,7 @@ group :development do
 end
 
 group :test do
+  gem 'timecop'
   gem 'rspec-retry'
   gem 'benchmark-ips'
   gem 'rspec-core', '~> 3.7'

lib/mongoid/association/embedded/embedded_in.rb

@@ -26,7 +26,7 @@ class EmbeddedIn
             :autobuild,
             :cyclic,
             :polymorphic,
-            :touch
+            :touch,
         ].freeze
 
         # The complete list of valid options for this association, including

lib/mongoid/touchable.rb

@@ -30,11 +30,30 @@ def touch(field = nil)
         write_attribute(:updated_at, current) if respond_to?("updated_at=")
         write_attribute(field, current) if field
 
-        touches = touch_atomic_updates(field)
-        unless touches["$set"].blank?
-          selector = atomic_selector
-          _root.collection.find(selector).update_one(positionally(selector, touches), session: _session)
+        # If the document being touched is embedded, touch its parents
+        # all the way through the composition hierarchy to the root object,
+        # because when an embedded document is changed the write is actually
+        # performed by the composition root. See MONGOID-3468.
+        if _parent
+          # This will persist updated_at on this document as well as parents.
+          # TODO support passing the field name to the parent's touch method;
+          # I believe it should be read out of
+          # _association.inverse_association.options but inverse_association
+          # seems to not always/ever be set here. See MONGOID-5014.
+          _parent.touch
+        else
+          # If the current document is not embedded, it is composition root
+          # and we need to persist the write here.
+          touches = touch_atomic_updates(field)
+          unless touches["$set"].blank?
+            selector = atomic_selector
+            _root.collection.find(selector).update_one(positionally(selector, touches), session: _session)
+          end
         end
+
+        # Callbacks are invoked on the composition root first and on the
+        # leaf-most embedded document last.
+        # TODO add tests, see MONGOID-5015.
         run_callbacks(:touch)
         true
       end

spec/lite_spec_helper.rb

@@ -15,6 +15,8 @@
 # https://github.com/jruby/jruby/issues/5599
 require 'pp'
 
+autoload :Timecop, 'timecop'
+
 require 'support/spec_config'
 require 'mrss/lite_constraints'
 require "support/session_registry"

spec/mongoid/touchable_spec.rb

@@ -2,14 +2,15 @@
 # encoding: utf-8
 
 require "spec_helper"
+require_relative './touchable_spec_models'
 
 describe Mongoid::Touchable do
 
   describe "#touch" do
 
     context "when the document has no associations" do
       let(:updatable) do
-        Updatable.create
+        Updatable.create!
       end
 
       it "responds to #touch" do
@@ -28,30 +29,117 @@
       end
     end
 
-    context "when the document is embedded" do
+    context 'when the document has a parent association' do
 
-      before do
-        Label.send(:include, Mongoid::Touchable::InstanceMethods)
+      let(:building) do
+        parent_cls.create!
       end
 
-      let(:band) do
-        Band.create(name: "Placebo")
+      let(:entrance) do
+        building.entrances.create!
       end
 
-      let(:label) do
-        band.create_label(name: "Mute", updated_at: 10.days.ago)
+      let(:floor) do
+        building.floors.create!
       end
 
-      before do
-        label.touch
+      let!(:start_time) { Timecop.freeze(Time.at(Time.now.to_i)) }
+
+      let(:update_time) do
+        Timecop.freeze(Time.at(Time.now.to_i) + 2)
       end
 
-      it "updates the updated_at timestamp" do
-        expect(label.updated_at).to be_within(1).of(Time.now)
+      after do
+        Timecop.return
+      end
+
+      shared_examples 'updates the child' do
+        it "updates the updated_at timestamp" do
+          entrance
+          update_time
+          entrance.touch
+
+          entrance.updated_at.should == update_time
+        end
+
+        it "persists the changes" do
+          entrance
+          update_time
+          entrance.touch
+
+          entrance.reload.updated_at.should == update_time
+        end
       end
 
-      it "persists the changes" do
-        expect(label.reload.updated_at).to be_within(1).of(Time.now)
+      shared_examples 'updates the parent when :touch is true' do
+
+        it 'updates updated_at on parent' do
+          floor
+          update_time
+          floor.touch
+
+          building.updated_at.should == update_time
+        end
+
+        it 'persists updated updated_at on parent' do
+          floor
+          update_time
+          floor.touch
+
+          building.reload.updated_at.should == update_time
+        end
+      end
+
+      shared_examples 'updates the parent when :touch is not set' do
+        it 'does not update updated_at on parent' do
+          entrance
+          update_time
+          entrance.touch
+
+          building.updated_at.should == update_time
+        end
+
+        it 'does not persist updated updated_at on parent' do
+          entrance
+          update_time
+          entrance.touch
+
+          building.reload.updated_at.should == update_time
+        end
+      end
+
+      shared_examples 'does not update the parent when :touch is not set' do
+        it 'does not update updated_at on parent' do
+          entrance
+          update_time
+          entrance.touch
+
+          building.updated_at.should == start_time
+        end
+
+        it 'does not persist updated updated_at on parent' do
+          entrance
+          update_time
+          entrance.touch
+
+          building.reload.updated_at.should == start_time
+        end
+      end
+
+      context "when the document is embedded" do
+        let(:parent_cls) { TouchableSpec::Embedded::Building }
+
+        include_examples 'updates the child'
+        include_examples 'updates the parent when :touch is true'
+        include_examples 'updates the parent when :touch is not set'
+      end
+
+      context "when the document is referenced" do
+        let(:parent_cls) { TouchableSpec::Referenced::Building }
+
+        include_examples 'updates the child'
+        include_examples 'updates the parent when :touch is true'
+        include_examples 'does not update the parent when :touch is not set'
       end
     end
 
@@ -415,11 +503,11 @@
       context "when modifying the child" do
 
         let!(:agency) do
-          Agency.create
+          Agency.create!
         end
 
         let!(:agent) do
-          agency.agents.create(number: '1')
+          agency.agents.create!(number: '1')
         end
 
         it "updates the parent's updated at" do