diff --git a/Gemfile b/Gemfile index f2e35b613b..e1eaf5bb33 100644 --- a/Gemfile +++ b/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' diff --git a/docs/tutorials/mongoid-relations.txt b/docs/tutorials/mongoid-relations.txt index f2909d17ef..432e660f36 100644 --- a/docs/tutorials/mongoid-relations.txt +++ b/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 `_. + +``: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 `_. The counter_cache Option ------------------------ diff --git a/gemfiles/driver_master.gemfile b/gemfiles/driver_master.gemfile index c472861f73..42e1fdbf0c 100644 --- a/gemfiles/driver_master.gemfile +++ b/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' diff --git a/gemfiles/driver_master_jruby.gemfile b/gemfiles/driver_master_jruby.gemfile index b2337dd509..154078504b 100644 --- a/gemfiles/driver_master_jruby.gemfile +++ b/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' diff --git a/gemfiles/driver_min.gemfile b/gemfiles/driver_min.gemfile index 912c1bd1d8..b44152a1c9 100644 --- a/gemfiles/driver_min.gemfile +++ b/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' diff --git a/gemfiles/driver_min_jruby.gemfile b/gemfiles/driver_min_jruby.gemfile index 3c8b35cc0c..4c92d5d6cf 100644 --- a/gemfiles/driver_min_jruby.gemfile +++ b/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' diff --git a/gemfiles/driver_oldstable.gemfile b/gemfiles/driver_oldstable.gemfile index 82dc669125..dab8c1af77 100644 --- a/gemfiles/driver_oldstable.gemfile +++ b/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' diff --git a/gemfiles/driver_oldstable_jruby.gemfile b/gemfiles/driver_oldstable_jruby.gemfile index f955c46b97..21631b3bd9 100644 --- a/gemfiles/driver_oldstable_jruby.gemfile +++ b/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' diff --git a/gemfiles/driver_stable.gemfile b/gemfiles/driver_stable.gemfile index 6c8db3d2cd..bb62821857 100644 --- a/gemfiles/driver_stable.gemfile +++ b/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' diff --git a/gemfiles/driver_stable_jruby.gemfile b/gemfiles/driver_stable_jruby.gemfile index 783a4efb57..50176d7748 100644 --- a/gemfiles/driver_stable_jruby.gemfile +++ b/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' diff --git a/gemfiles/i18n-1.0.gemfile b/gemfiles/i18n-1.0.gemfile index a38fe7239a..16b73457ea 100644 --- a/gemfiles/i18n-1.0.gemfile +++ b/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' diff --git a/gemfiles/rails_51.gemfile b/gemfiles/rails_51.gemfile index 4dbabf92a2..194c27de11 100644 --- a/gemfiles/rails_51.gemfile +++ b/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' diff --git a/gemfiles/rails_52.gemfile b/gemfiles/rails_52.gemfile index c1da69f839..8bacaaec06 100644 --- a/gemfiles/rails_52.gemfile +++ b/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' diff --git a/gemfiles/rails_master.gemfile b/gemfiles/rails_master.gemfile index f28349f2ce..3b34c1060a 100644 --- a/gemfiles/rails_master.gemfile +++ b/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' diff --git a/lib/mongoid/association/embedded/embedded_in.rb b/lib/mongoid/association/embedded/embedded_in.rb index b7a9a733ac..7db5562b62 100644 --- a/lib/mongoid/association/embedded/embedded_in.rb +++ b/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 diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 8998c56bb4..94c81a9321 100644 --- a/lib/mongoid/touchable.rb +++ b/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 diff --git a/spec/lite_spec_helper.rb b/spec/lite_spec_helper.rb index fc46a00208..4731734b12 100644 --- a/spec/lite_spec_helper.rb +++ b/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" diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 41becb3fb8..0cc6466cb1 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -2,6 +2,7 @@ # encoding: utf-8 require "spec_helper" +require_relative './touchable_spec_models' describe Mongoid::Touchable do @@ -9,7 +10,7 @@ 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 diff --git a/spec/mongoid/touchable_spec_models.rb b/spec/mongoid/touchable_spec_models.rb new file mode 100644 index 0000000000..deef80e88b --- /dev/null +++ b/spec/mongoid/touchable_spec_models.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true +# encoding: utf-8 + +module TouchableSpec + module Embedded + class Building + include Mongoid::Document + include Mongoid::Timestamps + + embeds_many :entrances + embeds_many :floors + end + + class Entrance + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :building + end + + class Floor + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :building, touch: true + end + end + + module Referenced + class Building + include Mongoid::Document + include Mongoid::Timestamps + + has_many :entrances, inverse_of: :building + has_many :floors, inverse_of: :building + end + + class Entrance + include Mongoid::Document + include Mongoid::Timestamps + + belongs_to :building + end + + class Floor + include Mongoid::Document + include Mongoid::Timestamps + + belongs_to :building, touch: true + end + end +end