From 047e2cb941068bf2d76a64e6a7dbc500625438fa Mon Sep 17 00:00:00 2001 From: shields Date: Fri, 30 Sep 2022 21:39:11 +0900 Subject: [PATCH 01/18] MONGOID-5142 - Optimize #touch method: - Perform all touch operations on the document in a single write operation - Synchronize the timestamp of all touches on the document (parent+child will have same timestamp) - Remove touches from the atomic set operations, so that they are no longer there when saving the next time. - Cleanup/refactor touch method to be more readable --- lib/mongoid/association/options.rb | 7 +- lib/mongoid/atomic.rb | 46 ---- lib/mongoid/touchable.rb | 101 +++++--- spec/mongoid/touchable_spec.rb | 359 +++++++++++++++++++++++--- spec/mongoid/touchable_spec_models.rb | 48 +++- 5 files changed, 431 insertions(+), 130 deletions(-) diff --git a/lib/mongoid/association/options.rb b/lib/mongoid/association/options.rb index 6bdda5f804..7f2f4e1787 100644 --- a/lib/mongoid/association/options.rb +++ b/lib/mongoid/association/options.rb @@ -112,8 +112,11 @@ def touch_field @touch_field ||= options[:touch] if (options[:touch].is_a?(String) || options[:touch].is_a?(Symbol)) end - private - + # Whether the association object should be automatically touched + # when its inverse object is updated. + # + # @return [ true | false ] returns true if this association is + # automatically touched, false otherwise. The default is false. def touchable? !!@options[:touch] end diff --git a/lib/mongoid/atomic.rb b/lib/mongoid/atomic.rb index 3b1751c15b..7eb55d9d7a 100644 --- a/lib/mongoid/atomic.rb +++ b/lib/mongoid/atomic.rb @@ -328,51 +328,5 @@ def generate_atomic_updates(mods, doc) mods.add_to_set(doc.atomic_array_add_to_sets) mods.pull_all(doc.atomic_array_pulls) end - - # Get the atomic updates for a touch operation. Should only include the - # updated_at field and the optional extra field. - # - # @api private - # - # @example Get the touch atomic updates. - # document.touch_atomic_updates - # - # @param [ Symbol ] field The optional field. - # - # @return [ Hash ] The atomic updates. - def touch_atomic_updates(field = nil) - updates = atomic_updates - return {} unless atomic_updates.key?("$set") - touches = {} - wanted_keys = %w(updated_at u_at) - # TODO this permits field to be passed as an empty string in which case - # it is ignored, get rid of this behavior. - if field.present? - wanted_keys << field.to_s - end - updates["$set"].each_pair do |key, value| - if wanted_keys.include?(key.split('.').last) - touches.update(key => value) - end - end - { "$set" => touches } - end - - # Returns the $set atomic updates affecting the specified field. - # - # @param [ String ] field The field name. - # - # @api private - def set_field_atomic_updates(field) - updates = atomic_updates - return {} unless atomic_updates.key?("$set") - sets = {} - updates["$set"].each_pair do |key, value| - if key.split('.').last == field - sets.update(key => value) - end - end - { "$set" => sets } - end end end diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 6fd8aeeb7e..5eed0ecb78 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -22,46 +22,68 @@ module InstanceMethods # @return [ true/false ] false if document is new_record otherwise true. def touch(field = nil) return false if _root.new_record? - current = Time.configured.now + + touches = __gather_touch_updates(Time.configured.now, field) + _root.send(:persist_atomic_operations, '$set' => touches) if touches.present? + + __run_touch_callbacks_from_root + true + end + + # Recursively sets touchable fields on the current document and each of its + # parents, including the root node. Returns the combined atomic $set + # operations to be performed on the root document. + # + # @param [ Time ] now The timestamp used for synchronizing the touched time. + # @param [ Symbol ] field The name of an additional field to update. + # + # @return [ Hash ] The touch operations to perform as an atomic $set. + # + # @api private + def __gather_touch_updates(now, field = nil) field = database_field_name(field) - write_attribute(:updated_at, current) if respond_to?("updated_at=") - write_attribute(field, current) if field - - # 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 - - if field - # If we are told to also touch a field, perform a separate write - # for that field. See MONGOID-5136. - # In theory we should combine the writes, which would require - # passing the fields to be updated to the parents - MONGOID-5142. - sets = set_field_atomic_updates(field) - selector = atomic_selector - _root.collection.find(selector).update_one(positionally(selector, sets), session: _session) - end - 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 + write_attribute(:updated_at, now) if respond_to?("updated_at=") + write_attribute(field, now) if field + + touch_parent = _parent && _association&.inverse_association&.touchable? + + touches = __extract_touches_from_atomic_sets(field) || {} + touches.merge!(_parent.__gather_touch_updates(now) || {}) if touch_parent + touches + end - # Callbacks are invoked on the composition root first and on the - # leaf-most embedded document last. + # Recursively runs :touch callbacks for the document and its parents, + # beginning with the root document and cascading through each successive + # child document. + # + # @api private + def __run_touch_callbacks_from_root + _parent.__run_touch_callbacks_from_root if _parent run_callbacks(:touch) - true + end + + private + + # Extract and remove the atomic updates for the touch operation(s) + # from the currently enqueued atomic $set operations. + # + # @param [ Symbol ] field The optional field. + # + # @return [ Hash ] The field-value pairs to update atomically. + # + # @api private + def __extract_touches_from_atomic_sets(field = nil) + updates = atomic_updates['$set'] + return {} unless updates + + touchable_keys = %w(updated_at u_at) + touchable_keys << field.to_s if field.present? + + updates.keys.each_with_object({}) do |key, touches| + if touchable_keys.include?(key.split('.').last) + touches[key] = updates.delete(key) + end + end end end @@ -82,7 +104,10 @@ def define_touchable!(association) association.inverse_class.tap do |klass| klass.after_save method_name klass.after_destroy method_name - klass.after_touch method_name + + # Embedded docs handle touch updates recursively within + # the #touch method itself + klass.after_touch method_name unless association.embedded? end end diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 7360a4be93..97f6595c23 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -58,7 +58,7 @@ update_time entrance.touch - entrance.updated_at.should == update_time + expect(entrance.updated_at).to eq update_time end it "persists the changes" do @@ -66,7 +66,7 @@ update_time entrance.touch - entrance.reload.updated_at.should == update_time + expect(entrance.reload.updated_at).to eq update_time end end @@ -77,7 +77,7 @@ update_time floor.touch - building.updated_at.should == update_time + expect(building.updated_at).to eq update_time end it 'persists updated updated_at on parent' do @@ -85,17 +85,17 @@ update_time floor.touch - building.reload.updated_at.should == update_time + expect(building.reload.updated_at).to eq update_time end end - shared_examples 'updates the parent when :touch is false' do + shared_examples 'does not update the parent when :touch is false' do it 'does not update updated_at on parent' do entrance update_time entrance.touch - building.updated_at.should == update_time + expect(building.updated_at).to eq start_time end it 'does not persist updated updated_at on parent' do @@ -103,26 +103,12 @@ update_time entrance.touch - building.reload.updated_at.should == update_time + expect(building.reload.updated_at).to eq start_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 + it_behaves_like 'does not update the parent when :touch is false' end context "when the document is embedded" do @@ -130,9 +116,9 @@ include_examples 'updates the child' include_examples 'updates the parent when :touch is true' - include_examples 'updates the parent when :touch is false' + include_examples 'does not update the parent when :touch is not set' - context 'when also updating an additional field' do + context 'when also updating an additional field when :touch is true' do it 'persists the update to the additional field' do entrance update_time @@ -142,11 +128,29 @@ building.reload # This is the assertion we want. - entrance.last_used_at.should == update_time + expect(entrance.last_used_at).to eq update_time # Check other timestamps for good measure. - entrance.updated_at.should == update_time - building.updated_at.should == update_time + expect(entrance.updated_at).to eq update_time + expect(building.updated_at).to eq start_time + end + end + + context 'when also updating an additional field when :touch is not set' do + it 'persists the update to the additional field' do + floor + update_time + floor.touch(:last_used_at) + + floor.reload + building.reload + + # This is the assertion we want. + expect(floor.last_used_at).to eq update_time + + # Check other timestamps for good measure. + expect(floor.updated_at).to eq update_time + expect(building.updated_at).to eq update_time end end end @@ -156,7 +160,7 @@ 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' + include_examples 'does not update the parent when :touch is false' end end @@ -619,8 +623,8 @@ end it "updates the parent's timestamp" do - building.updated_at.should == update_time - building.reload.updated_at.should == update_time + expect(building.updated_at).to eq update_time + expect(building.reload.updated_at).to eq update_time end end @@ -644,12 +648,8 @@ context "when the touch option is false" do shared_examples "does not update the parent" do - 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 + let(:update_time) { Timecop.freeze(Time.at(Time.now.to_i) + 2) } after do Timecop.return @@ -670,13 +670,13 @@ end it "updates the child's timestamp" do - entrance.updated_at.should == update_time - entrance.reload.updated_at.should == update_time + expect(entrance.updated_at).to eq update_time + expect(entrance.reload.updated_at).to eq update_time end it "does not update the parent's timestamp" do - building.updated_at.should == start_time - building.reload.updated_at.should == start_time + expect(building.updated_at).to eq start_time + expect(building.reload.updated_at).to eq start_time end end @@ -692,13 +692,288 @@ let(:meth) { meth } let(:parent_cls) { TouchableSpec::Embedded::Building } - before do - skip "MONGOID-5274" + include_examples "does not update the parent" + end + end + end + + describe 'multi-level' do + + let(:child_name) do + child_cls.name.demodulize.underscore + end + + let(:grandchild_name) do + grandchild_cls.name.demodulize.underscore + end + + let(:parent) do + parent_cls.create! + end + + let(:child) do + parent.send(child_name.pluralize).create! + end + + let(:grandchild) do + grandchild = child.send(grandchild_name.pluralize).create! + grandchild.created_at = Time.now + 1.day # arbitrary change so save! works + grandchild + end + + shared_examples "updates the parent" do + it "updates the parent's timestamp" do + expect(parent.updated_at).to eq update_time + expect(parent.reload.updated_at).to eq update_time + end + end + + shared_examples "does not update the parent" do + it "does not update the parent's timestamp" do + expect(parent.updated_at).to eq start_time + expect(parent.reload.updated_at).to eq start_time + end + end + + shared_examples "updates the child" do + it "updates the child's timestamp" do + expect(child.updated_at).to eq update_time + expect(child.reload.updated_at).to eq update_time + end + end + + shared_examples "does not update the child" do + it "does not update the child's timestamp" do + expect(child.updated_at).to eq start_time + expect(child.reload.updated_at).to eq start_time + end + end + + shared_examples "updates the grandchild" do + it "updates the grandchild's timestamp" do + if grandchild.destroyed? + expect(grandchild.updated_at).to eq start_time + else + expect(grandchild.updated_at).to eq update_time + expect(grandchild.reload.updated_at).to eq update_time end + end + end - include_examples "does not update the parent" + shared_examples "does not update the grandchild" do + it "does not update the grandchild's timestamp" do + expect(grandchild.updated_at).to eq start_time + expect(grandchild.reload.updated_at).to eq start_time unless grandchild.destroyed? end end + + let!(:start_time) { Timecop.freeze(Time.at(Time.now.to_i)) } + let(:update_time) { Timecop.freeze(Time.at(Time.now.to_i) + 2) } + + before do + grandchild + update_time + puts meth + puts "start_time: #{update_time}" + puts "update_time: #{update_time}" + grandchild.send(meth) + puts "--------" + puts "p #{parent.updated_at}" + puts "p reload #{parent.reload.updated_at}" + puts "c #{child.updated_at}" + puts "c reload #{child.reload.updated_at}" + puts "g #{grandchild.updated_at}" + puts "g reload #{grandchild.reload.updated_at unless grandchild.destroyed?}" + end + + after do + Timecop.return + end + + context 'parent > embedded child > embedded grandchild' do + + let(:parent_cls) { TouchableSpec::Embedded::Building } + + context 'child touch: true' do + + let(:child_cls) do + TouchableSpec::Embedded::Floor + end + + context 'grandchild touch: true' do + + let(:grandchild_cls) do + TouchableSpec::Embedded::Sofa + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "updates the parent" + it_behaves_like "updates the child" + it_behaves_like "updates the grandchild" + end + end + end + + context 'grandchild touch: false' do + + let(:grandchild_cls) do + TouchableSpec::Embedded::Chair + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "does not update the child" + it_behaves_like "updates the grandchild" + end + end + end + end + + context 'child touch: false' do + + let(:child_cls) do + TouchableSpec::Embedded::Entrance + end + + context 'grandchild touch: true' do + + let(:grandchild_cls) do + TouchableSpec::Embedded::Sofa + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "updates the child" + it_behaves_like "updates the grandchild" + end + end + end + + context 'grandchild touch: false' do + + let(:grandchild_cls) do + TouchableSpec::Embedded::Chair + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "does not update the child" + it_behaves_like "updates the grandchild" + end + end + end + end + end + + # describe 'parent > referenced child > embedded grandchild' do + # + # context 'child touch: true' do + # + # let(:child_cls) do + # TouchableSpec::Referenced::Floor + # end + # + # context 'grandchild touch: true' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Sofa + # end + # + # end + # + # context 'grandchild touch: false' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Chair + # end + # + # end + # end + # + # context 'child touch: false' do + # + # let(:child_cls) do + # TouchableSpec::Referenced::Entrance + # end + # + # context 'grandchild touch: true' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Sofa + # end + # + # end + # + # context 'grandchild touch: false' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Chair + # end + # + # end + # end + # end + # + # describe 'parent > referenced child > referenced grandchild' do + # + # context 'child touch: true' do + # + # let(:child_cls) do + # TouchableSpec::Referenced::Floor + # end + # + # context 'grandchild touch: true' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Window + # end + # + # end + # + # context 'grandchild touch: false' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Plant + # end + # + # end + # end + # + # context 'child touch: false' do + # + # let(:child_cls) do + # TouchableSpec::Referenced::Entrance + # end + # + # context 'grandchild touch: true' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Window + # end + # + # end + # + # context 'grandchild touch: false' do + # + # let(:grandchild_cls) do + # TouchableSpec::Embedded::Plant + # end + # + # end + # end + # end end end diff --git a/spec/mongoid/touchable_spec_models.rb b/spec/mongoid/touchable_spec_models.rb index f34d122c9f..4216b3cd8a 100644 --- a/spec/mongoid/touchable_spec_models.rb +++ b/spec/mongoid/touchable_spec_models.rb @@ -16,9 +16,12 @@ class Entrance include Mongoid::Document include Mongoid::Timestamps - embedded_in :building, touch: false, class_name: "TouchableSpec::Embedded::Building" - field :last_used_at, type: Time + + embedded_in :building, class_name: "TouchableSpec::Embedded::Building" + + embeds_many :chairs, class_name: "TouchableSpec::Embedded::Chair" + embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" end class Floor @@ -26,8 +29,26 @@ class Floor include Mongoid::Timestamps field :level, type: Integer + field :last_used_at, type: Time embedded_in :building, touch: true, class_name: "TouchableSpec::Embedded::Building" + + embeds_many :chairs, class_name: "TouchableSpec::Embedded::Chair" + embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" + end + + class Chair + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :chairable, polymorphic: true + end + + class Sofa + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :sofable, touch: true, polymorphic: true end end @@ -45,6 +66,9 @@ class Entrance include Mongoid::Timestamps belongs_to :building, touch: false, class_name: "TouchableSpec::Referenced::Building" + + embeds_many :chairs, class_name: "TouchableSpec::Embedded::Chair" + embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" end class Floor @@ -54,6 +78,26 @@ class Floor field :level, type: Integer belongs_to :building, touch: true, class_name: "TouchableSpec::Referenced::Building" + + embeds_many :chairs, class_name: "TouchableSpec::Embedded::Chair" + embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" + + has_many :plants, class_name: "TouchableSpec::Referenced::Plant" + has_many :windows, class_name: "TouchableSpec::Referenced::Window" + end + + class Plant + include Mongoid::Document + include Mongoid::Timestamps + + belongs_to :floor, touch: false, class_name: "TouchableSpec::Referenced::Floor" + end + + class Window + include Mongoid::Document + include Mongoid::Timestamps + + belongs_to :floor, touch: true, class_name: "TouchableSpec::Referenced::Floor" end end end From f2e92c0cc22d926708c3c90cbe0344f678e46e22 Mon Sep 17 00:00:00 2001 From: shields Date: Sat, 1 Oct 2022 00:26:45 +0900 Subject: [PATCH 02/18] typo --- spec/mongoid/touchable_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 97f6595c23..8148bcf3fc 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -1085,7 +1085,7 @@ end end - context "when saving embedded associations with cascadable callbacks" do + context "when saving embedded associations with cascading callbacks" do shared_examples "timeless is cleared" do it "clears the timeless option" do From d29943f2ad7c988485420d239b2545869a8c1926 Mon Sep 17 00:00:00 2001 From: shields Date: Sat, 1 Oct 2022 00:40:32 +0900 Subject: [PATCH 03/18] WIP on fixes --- spec/mongoid/touchable_spec.rb | 36 ++++++++--------- spec/mongoid/touchable_spec_models.rb | 58 +++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 1e30b52656..e86770bd89 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -736,7 +736,7 @@ end end - describe 'multi-level' do + context 'multi-level' do let(:child_name) do child_cls.name.demodulize.underscore @@ -809,21 +809,21 @@ let!(:start_time) { Timecop.freeze(Time.at(Time.now.to_i)) } let(:update_time) { Timecop.freeze(Time.at(Time.now.to_i) + 2) } - before do - grandchild - update_time - puts meth - puts "start_time: #{update_time}" - puts "update_time: #{update_time}" - grandchild.send(meth) - puts "--------" - puts "p #{parent.updated_at}" - puts "p reload #{parent.reload.updated_at}" - puts "c #{child.updated_at}" - puts "c reload #{child.reload.updated_at}" - puts "g #{grandchild.updated_at}" - puts "g reload #{grandchild.reload.updated_at unless grandchild.destroyed?}" - end + # before do + # grandchild + # update_time + # puts meth + # puts "start_time: #{update_time}" + # puts "update_time: #{update_time}" + # grandchild.send(meth) + # puts "--------" + # puts "p #{parent.updated_at}" + # puts "p reload #{parent.reload.updated_at}" + # puts "c #{child.updated_at}" + # puts "c reload #{child.reload.updated_at}" + # puts "g #{grandchild.updated_at}" + # puts "g reload #{grandchild.reload.updated_at unless grandchild.destroyed?}" + # end after do Timecop.return @@ -883,7 +883,7 @@ context 'grandchild touch: true' do let(:grandchild_cls) do - TouchableSpec::Embedded::Sofa + TouchableSpec::Embedded::Camera end [ :save!, :destroy, :touch ].each do |meth| @@ -900,7 +900,7 @@ context 'grandchild touch: false' do let(:grandchild_cls) do - TouchableSpec::Embedded::Chair + TouchableSpec::Embedded::Keypad end [ :save!, :destroy, :touch ].each do |meth| diff --git a/spec/mongoid/touchable_spec_models.rb b/spec/mongoid/touchable_spec_models.rb index 4216b3cd8a..77d60f3b36 100644 --- a/spec/mongoid/touchable_spec_models.rb +++ b/spec/mongoid/touchable_spec_models.rb @@ -20,8 +20,8 @@ class Entrance embedded_in :building, class_name: "TouchableSpec::Embedded::Building" - embeds_many :chairs, class_name: "TouchableSpec::Embedded::Chair" - embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" + embeds_many :keypads, class_name: "TouchableSpec::Embedded::Keypad" + embeds_many :cameras, class_name: "TouchableSpec::Embedded::Camera" end class Floor @@ -37,18 +37,32 @@ class Floor embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" end + class Keypad + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :entrance, class_name: "TouchableSpec::Embedded::Entrance" + end + + class Camera + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :entrance, touch: true, class_name: "TouchableSpec::Embedded::Entrance" + end + class Chair include Mongoid::Document include Mongoid::Timestamps - embedded_in :chairable, polymorphic: true + embedded_in :floor, class_name: "TouchableSpec::Embedded::Floor" end class Sofa include Mongoid::Document include Mongoid::Timestamps - embedded_in :sofable, touch: true, polymorphic: true + embedded_in :floor, touch: true, class_name: "TouchableSpec::Embedded::Floor" end end @@ -67,8 +81,8 @@ class Entrance belongs_to :building, touch: false, class_name: "TouchableSpec::Referenced::Building" - embeds_many :chairs, class_name: "TouchableSpec::Embedded::Chair" - embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" + embeds_many :keypads, class_name: "TouchableSpec::Referenced::Keypad" + embeds_many :cameras, class_name: "TouchableSpec::Referenced::Camera" end class Floor @@ -79,8 +93,8 @@ class Floor belongs_to :building, touch: true, class_name: "TouchableSpec::Referenced::Building" - embeds_many :chairs, class_name: "TouchableSpec::Embedded::Chair" - embeds_many :sofas, class_name: "TouchableSpec::Embedded::Sofa" + embeds_many :chairs, class_name: "TouchableSpec::Referenced::Chair" + embeds_many :sofas, class_name: "TouchableSpec::Referenced::Sofa" has_many :plants, class_name: "TouchableSpec::Referenced::Plant" has_many :windows, class_name: "TouchableSpec::Referenced::Window" @@ -99,5 +113,33 @@ class Window belongs_to :floor, touch: true, class_name: "TouchableSpec::Referenced::Floor" end + + class Keypad + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :entrance, class_name: "TouchableSpec::Referenced::Entrance" + end + + class Camera + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :entrance, touch: true, class_name: "TouchableSpec::Referenced::Entrance" + end + + class Chair + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :floor, class_name: "TouchableSpec::Referenced::Floor" + end + + class Sofa + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :floor, touch: true, class_name: "TouchableSpec::Referenced::Floor" + end end end From ee7f640f1852b206f545060349b8223cdf9ef9f5 Mon Sep 17 00:00:00 2001 From: shields Date: Sat, 1 Oct 2022 00:49:16 +0900 Subject: [PATCH 04/18] all but one spec passing... --- spec/mongoid/touchable_spec.rb | 256 ++++++++++++++++++--------------- 1 file changed, 144 insertions(+), 112 deletions(-) diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index e86770bd89..208ea211e1 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -809,21 +809,21 @@ let!(:start_time) { Timecop.freeze(Time.at(Time.now.to_i)) } let(:update_time) { Timecop.freeze(Time.at(Time.now.to_i) + 2) } - # before do - # grandchild - # update_time - # puts meth - # puts "start_time: #{update_time}" - # puts "update_time: #{update_time}" - # grandchild.send(meth) - # puts "--------" - # puts "p #{parent.updated_at}" - # puts "p reload #{parent.reload.updated_at}" - # puts "c #{child.updated_at}" - # puts "c reload #{child.reload.updated_at}" - # puts "g #{grandchild.updated_at}" - # puts "g reload #{grandchild.reload.updated_at unless grandchild.destroyed?}" - # end + before do + grandchild + update_time + puts meth + puts "start_time: #{update_time}" + puts "update_time: #{update_time}" + grandchild.send(meth) + puts "--------" + puts "p #{parent.updated_at}" + puts "p reload #{parent.reload.updated_at}" + puts "c #{child.updated_at}" + puts "c reload #{child.reload.updated_at}" + puts "g #{grandchild.updated_at}" + puts "g reload #{grandchild.reload.updated_at unless grandchild.destroyed?}" + end after do Timecop.return @@ -916,103 +916,135 @@ end end - # describe 'parent > referenced child > embedded grandchild' do - # - # context 'child touch: true' do - # - # let(:child_cls) do - # TouchableSpec::Referenced::Floor - # end - # - # context 'grandchild touch: true' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Sofa - # end - # - # end - # - # context 'grandchild touch: false' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Chair - # end - # - # end - # end - # - # context 'child touch: false' do - # - # let(:child_cls) do - # TouchableSpec::Referenced::Entrance - # end - # - # context 'grandchild touch: true' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Sofa - # end - # - # end - # - # context 'grandchild touch: false' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Chair - # end - # - # end - # end - # end - # - # describe 'parent > referenced child > referenced grandchild' do - # - # context 'child touch: true' do - # - # let(:child_cls) do - # TouchableSpec::Referenced::Floor - # end - # - # context 'grandchild touch: true' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Window - # end - # - # end - # - # context 'grandchild touch: false' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Plant - # end - # - # end - # end - # - # context 'child touch: false' do - # - # let(:child_cls) do - # TouchableSpec::Referenced::Entrance - # end - # - # context 'grandchild touch: true' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Window - # end - # - # end - # - # context 'grandchild touch: false' do - # - # let(:grandchild_cls) do - # TouchableSpec::Embedded::Plant - # end - # - # end - # end - # end + context 'parent > referenced child > embedded grandchild' do + + let(:parent_cls) { TouchableSpec::Referenced::Building } + + context 'child touch: true' do + + let(:child_cls) do + TouchableSpec::Referenced::Floor + end + + context 'grandchild touch: true' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Sofa + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "updates the parent" + it_behaves_like "updates the child" + it_behaves_like "updates the grandchild" + end + end + end + + context 'grandchild touch: false' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Chair + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "does not update the child" + it_behaves_like "updates the grandchild" + end + end + end + end + + context 'child touch: false' do + + let(:child_cls) do + TouchableSpec::Referenced::Entrance + end + + context 'grandchild touch: true' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Camera + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "updates the child" + it_behaves_like "updates the grandchild" + end + end + end + + context 'grandchild touch: false' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Keypad + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "does not update the child" + it_behaves_like "updates the grandchild" + end + end + end + end + end + + context 'parent > referenced child > referenced grandchild' do + + let(:parent_cls) { TouchableSpec::Referenced::Building } + + let(:child_cls) do + TouchableSpec::Referenced::Floor + end + + context 'grandchild touch: true' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Window + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "updates the parent" + it_behaves_like "updates the child" + it_behaves_like "updates the grandchild" + end + end + end + + context 'grandchild touch: false' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Plant + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "does not update the child" + it_behaves_like "updates the grandchild" + end + end + end + end end end From a1ad6d9d51b320843fb95ebc8cd3723d07440914 Mon Sep 17 00:00:00 2001 From: shields Date: Sat, 1 Oct 2022 01:13:22 +0900 Subject: [PATCH 05/18] Fix final spec --- lib/mongoid/touchable.rb | 12 +++++++----- spec/mongoid/touchable_spec.rb | 17 ++++------------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 5eed0ecb78..547b5ccf48 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -45,10 +45,8 @@ def __gather_touch_updates(now, field = nil) write_attribute(:updated_at, now) if respond_to?("updated_at=") write_attribute(field, now) if field - touch_parent = _parent && _association&.inverse_association&.touchable? - touches = __extract_touches_from_atomic_sets(field) || {} - touches.merge!(_parent.__gather_touch_updates(now) || {}) if touch_parent + touches.merge!(_parent.__gather_touch_updates(now) || {}) if __touchable_parent? touches end @@ -58,10 +56,14 @@ def __gather_touch_updates(now, field = nil) # # @api private def __run_touch_callbacks_from_root - _parent.__run_touch_callbacks_from_root if _parent + _parent.__run_touch_callbacks_from_root if __touchable_parent? run_callbacks(:touch) end + def __touchable_parent? + _parent && _association&.inverse_association&.touchable? + end + private # Extract and remove the atomic updates for the touch operation(s) @@ -142,7 +144,7 @@ def define_relation_touch_method(name, association) if association.touch_field # Note that this looks up touch_field at runtime, rather than # at method definition time. - relation.touch association.touch_field + relation.touch(association.touch_field) else relation.touch end diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 208ea211e1..2fc6a3e5c8 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -738,6 +738,10 @@ context 'multi-level' do + let!(:start_time) { Timecop.freeze(Time.at(Time.now.to_i)) } + + let(:update_time) { Timecop.freeze(Time.at(Time.now.to_i) + 2) } + let(:child_name) do child_cls.name.demodulize.underscore end @@ -806,23 +810,10 @@ end end - let!(:start_time) { Timecop.freeze(Time.at(Time.now.to_i)) } - let(:update_time) { Timecop.freeze(Time.at(Time.now.to_i) + 2) } - before do grandchild update_time - puts meth - puts "start_time: #{update_time}" - puts "update_time: #{update_time}" grandchild.send(meth) - puts "--------" - puts "p #{parent.updated_at}" - puts "p reload #{parent.reload.updated_at}" - puts "c #{child.updated_at}" - puts "c reload #{child.reload.updated_at}" - puts "g #{grandchild.updated_at}" - puts "g reload #{grandchild.reload.updated_at unless grandchild.destroyed?}" end after do From 286e109832839d6a45b41ace4500ef003dfe6ae4 Mon Sep 17 00:00:00 2001 From: shields Date: Sat, 1 Oct 2022 01:17:36 +0900 Subject: [PATCH 06/18] Add missing code doc --- lib/mongoid/touchable.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 547b5ccf48..698f8bd55e 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -60,6 +60,9 @@ def __run_touch_callbacks_from_root run_callbacks(:touch) end + # Indicates whether the parent exists and is touchable. + # + # @api private def __touchable_parent? _parent && _association&.inverse_association&.touchable? end From e9b1d38c3bdae806db24970f03208ebef741f71f Mon Sep 17 00:00:00 2001 From: shields Date: Sat, 1 Oct 2022 02:40:45 +0900 Subject: [PATCH 07/18] Fix failing spec --- spec/integration/callbacks_models.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/integration/callbacks_models.rb b/spec/integration/callbacks_models.rb index 8e6f59d655..2d16141c7f 100644 --- a/spec/integration/callbacks_models.rb +++ b/spec/integration/callbacks_models.rb @@ -23,7 +23,7 @@ class Star include Mongoid::Document include Mongoid::Timestamps - embedded_in :galaxy + embedded_in :galaxy, touch: true field :age, type: Integer field :was_touched_after_parent, type: Mongoid::Boolean, default: false @@ -47,7 +47,7 @@ class Planet include Mongoid::Document include Mongoid::Timestamps - embedded_in :star + embedded_in :star, touch: true field :age, type: Integer field :was_touched_after_parent, type: Mongoid::Boolean, default: false From 29a54cccf1950214f5b2529240735df124e34614 Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Tue, 4 Oct 2022 00:34:47 +0900 Subject: [PATCH 08/18] Update options.rb --- lib/mongoid/association/options.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/mongoid/association/options.rb b/lib/mongoid/association/options.rb index 7f2f4e1787..884ef4f8b5 100644 --- a/lib/mongoid/association/options.rb +++ b/lib/mongoid/association/options.rb @@ -117,6 +117,8 @@ def touch_field # # @return [ true | false ] returns true if this association is # automatically touched, false otherwise. The default is false. + # + # @api private def touchable? !!@options[:touch] end From a5fc2030d029a0839a898c71c4071c4cb282a28f Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Tue, 4 Oct 2022 00:41:49 +0900 Subject: [PATCH 09/18] Use Set --- lib/mongoid/touchable.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 698f8bd55e..6ffe734677 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -81,8 +81,8 @@ def __extract_touches_from_atomic_sets(field = nil) updates = atomic_updates['$set'] return {} unless updates - touchable_keys = %w(updated_at u_at) - touchable_keys << field.to_s if field.present? + touchable_keys = Set.new(%w(updated_at u_at)) + touchable_keys << field.to_s updates.keys.each_with_object({}) do |key, touches| if touchable_keys.include?(key.split('.').last) From 1c8ebe9953ec1ee091bf6be4ad3d837fc382eb2f Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Tue, 4 Oct 2022 00:46:00 +0900 Subject: [PATCH 10/18] Fix last commit --- lib/mongoid/touchable.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 6ffe734677..ac99fd1656 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -81,8 +81,8 @@ def __extract_touches_from_atomic_sets(field = nil) updates = atomic_updates['$set'] return {} unless updates - touchable_keys = Set.new(%w(updated_at u_at)) - touchable_keys << field.to_s + touchable_keys = Set['updated_at', 'u_at'] + touchable_keys << field.to_s if field.present? updates.keys.each_with_object({}) do |key, touches| if touchable_keys.include?(key.split('.').last) From 8fe4c58759e94ed19505439e9ee2940a056b786f Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Tue, 4 Oct 2022 00:51:38 +0900 Subject: [PATCH 11/18] Remove unnecessary relation touch conditional --- lib/mongoid/touchable.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index ac99fd1656..13f5e2b158 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -144,13 +144,9 @@ def define_relation_touch_method(name, association) define_method(method_name) do without_autobuild do if relation = __send__(name) - if association.touch_field - # Note that this looks up touch_field at runtime, rather than - # at method definition time. - relation.touch(association.touch_field) - else - relation.touch - end + # This looks up touch_field at runtime, rather than at method definition time. + # If touch_field is nil, it will only touch the default field (updated_at) + relation.touch(association.touch_field) end end end From e647af79927a5cc9c825a89782533c2bc8a405d5 Mon Sep 17 00:00:00 2001 From: shields Date: Tue, 4 Oct 2022 00:55:35 +0900 Subject: [PATCH 12/18] Use single underscores --- lib/mongoid/touchable.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 13f5e2b158..e0c22ddefb 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -23,10 +23,10 @@ module InstanceMethods def touch(field = nil) return false if _root.new_record? - touches = __gather_touch_updates(Time.configured.now, field) + touches = _gather_touch_updates(Time.configured.now, field) _root.send(:persist_atomic_operations, '$set' => touches) if touches.present? - __run_touch_callbacks_from_root + _run_touch_callbacks_from_root true end @@ -40,13 +40,13 @@ def touch(field = nil) # @return [ Hash ] The touch operations to perform as an atomic $set. # # @api private - def __gather_touch_updates(now, field = nil) + def _gather_touch_updates(now, field = nil) field = database_field_name(field) write_attribute(:updated_at, now) if respond_to?("updated_at=") write_attribute(field, now) if field - touches = __extract_touches_from_atomic_sets(field) || {} - touches.merge!(_parent.__gather_touch_updates(now) || {}) if __touchable_parent? + touches = _extract_touches_from_atomic_sets(field) || {} + touches.merge!(_parent._gather_touch_updates(now) || {}) if _touchable_parent? touches end @@ -55,15 +55,15 @@ def __gather_touch_updates(now, field = nil) # child document. # # @api private - def __run_touch_callbacks_from_root - _parent.__run_touch_callbacks_from_root if __touchable_parent? + def _run_touch_callbacks_from_root + _parent._run_touch_callbacks_from_root if _touchable_parent? run_callbacks(:touch) end # Indicates whether the parent exists and is touchable. # # @api private - def __touchable_parent? + def _touchable_parent? _parent && _association&.inverse_association&.touchable? end @@ -77,7 +77,7 @@ def __touchable_parent? # @return [ Hash ] The field-value pairs to update atomically. # # @api private - def __extract_touches_from_atomic_sets(field = nil) + def _extract_touches_from_atomic_sets(field = nil) updates = atomic_updates['$set'] return {} unless updates From 84357fb7594f01ed2ccd9f13ba8271e89788c713 Mon Sep 17 00:00:00 2001 From: shields Date: Tue, 4 Oct 2022 01:03:23 +0900 Subject: [PATCH 13/18] Add additional multi-level specs --- spec/mongoid/touchable_spec.rb | 86 ++++++++++++++++++++------- spec/mongoid/touchable_spec_models.rb | 5 ++ 2 files changed, 70 insertions(+), 21 deletions(-) diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 2fc6a3e5c8..1b5857614f 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -998,40 +998,84 @@ let(:parent_cls) { TouchableSpec::Referenced::Building } - let(:child_cls) do - TouchableSpec::Referenced::Floor - end + context 'child touch: true' do + + let(:child_cls) do + TouchableSpec::Referenced::Floor + end + + context 'grandchild touch: true' do - context 'grandchild touch: true' do + let(:grandchild_cls) do + TouchableSpec::Referenced::Window + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } - let(:grandchild_cls) do - TouchableSpec::Referenced::Window + it_behaves_like "updates the parent" + it_behaves_like "updates the child" + it_behaves_like "updates the grandchild" + end + end end - [ :save!, :destroy, :touch ].each do |meth| - context "when calling #{meth} method" do - let(:meth) { meth } + context 'grandchild touch: false' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Plant + end - it_behaves_like "updates the parent" - it_behaves_like "updates the child" - it_behaves_like "updates the grandchild" + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "does not update the child" + it_behaves_like "updates the grandchild" + end end end end - context 'grandchild touch: false' do + context 'child touch: false' do - let(:grandchild_cls) do - TouchableSpec::Referenced::Plant + let(:child_cls) do + TouchableSpec::Referenced::Entrance end - [ :save!, :destroy, :touch ].each do |meth| - context "when calling #{meth} method" do - let(:meth) { meth } + context 'grandchild touch: true' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Window + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } - it_behaves_like "does not update the parent" - it_behaves_like "does not update the child" - it_behaves_like "updates the grandchild" + it_behaves_like "does not update the parent" + it_behaves_like "updates the child" + it_behaves_like "updates the grandchild" + end + end + end + + context 'grandchild touch: false' do + + let(:grandchild_cls) do + TouchableSpec::Referenced::Plant + end + + [ :save!, :destroy, :touch ].each do |meth| + context "when calling #{meth} method" do + let(:meth) { meth } + + it_behaves_like "does not update the parent" + it_behaves_like "does not update the child" + it_behaves_like "updates the grandchild" + end end end end diff --git a/spec/mongoid/touchable_spec_models.rb b/spec/mongoid/touchable_spec_models.rb index 77d60f3b36..cc26f17864 100644 --- a/spec/mongoid/touchable_spec_models.rb +++ b/spec/mongoid/touchable_spec_models.rb @@ -83,6 +83,9 @@ class Entrance embeds_many :keypads, class_name: "TouchableSpec::Referenced::Keypad" embeds_many :cameras, class_name: "TouchableSpec::Referenced::Camera" + + has_many :plants, class_name: "TouchableSpec::Referenced::Plant" + has_many :windows, class_name: "TouchableSpec::Referenced::Window" end class Floor @@ -105,6 +108,7 @@ class Plant include Mongoid::Timestamps belongs_to :floor, touch: false, class_name: "TouchableSpec::Referenced::Floor" + belongs_to :entrance, touch: false, class_name: "TouchableSpec::Referenced::Entrance" end class Window @@ -112,6 +116,7 @@ class Window include Mongoid::Timestamps belongs_to :floor, touch: true, class_name: "TouchableSpec::Referenced::Floor" + belongs_to :entrance, touch: true, class_name: "TouchableSpec::Referenced::Entrance" end class Keypad From 00b76a828772d445f92b0ae9a0428586c510910f Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Tue, 4 Oct 2022 01:04:04 +0900 Subject: [PATCH 14/18] Update lib/mongoid/touchable.rb Co-authored-by: Neil Shweky --- lib/mongoid/touchable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index e0c22ddefb..a9242ccf36 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -145,7 +145,7 @@ def define_relation_touch_method(name, association) without_autobuild do if relation = __send__(name) # This looks up touch_field at runtime, rather than at method definition time. - # If touch_field is nil, it will only touch the default field (updated_at) + # If touch_field is nil, it will only touch the default field (updated_at). relation.touch(association.touch_field) end end From 9b24ccffcc09bd863c23fc08912289c685660103 Mon Sep 17 00:00:00 2001 From: Neil Shweky Date: Tue, 4 Oct 2022 12:36:10 -0400 Subject: [PATCH 15/18] MONGOID-5136 explicitly specify touch in models, and fix test bug --- spec/mongoid/touchable_spec.rb | 3 ++- spec/mongoid/touchable_spec_models.rb | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 1b5857614f..92a3a0a074 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -705,7 +705,8 @@ before do entrance update_time - entrance.touch + entrance.reload # TODO: remove this after MONGOID-5504 + entrance.send(meth) end it "updates the child's timestamp" do diff --git a/spec/mongoid/touchable_spec_models.rb b/spec/mongoid/touchable_spec_models.rb index cc26f17864..58590ae4d5 100644 --- a/spec/mongoid/touchable_spec_models.rb +++ b/spec/mongoid/touchable_spec_models.rb @@ -18,7 +18,7 @@ class Entrance field :last_used_at, type: Time - embedded_in :building, class_name: "TouchableSpec::Embedded::Building" + embedded_in :building, touch: false, class_name: "TouchableSpec::Embedded::Building" embeds_many :keypads, class_name: "TouchableSpec::Embedded::Keypad" embeds_many :cameras, class_name: "TouchableSpec::Embedded::Camera" @@ -41,7 +41,7 @@ class Keypad include Mongoid::Document include Mongoid::Timestamps - embedded_in :entrance, class_name: "TouchableSpec::Embedded::Entrance" + embedded_in :entrance, touch: false, class_name: "TouchableSpec::Embedded::Entrance" end class Camera @@ -55,7 +55,7 @@ class Chair include Mongoid::Document include Mongoid::Timestamps - embedded_in :floor, class_name: "TouchableSpec::Embedded::Floor" + embedded_in :floor, touch: false, class_name: "TouchableSpec::Embedded::Floor" end class Sofa @@ -123,7 +123,7 @@ class Keypad include Mongoid::Document include Mongoid::Timestamps - embedded_in :entrance, class_name: "TouchableSpec::Referenced::Entrance" + embedded_in :entrance, touch: false, class_name: "TouchableSpec::Referenced::Entrance" end class Camera @@ -137,7 +137,7 @@ class Chair include Mongoid::Document include Mongoid::Timestamps - embedded_in :floor, class_name: "TouchableSpec::Referenced::Floor" + embedded_in :floor, touch: false, class_name: "TouchableSpec::Referenced::Floor" end class Sofa From a39d072ff1f8412556c0c3347c33aa405e873e4a Mon Sep 17 00:00:00 2001 From: shields Date: Wed, 5 Oct 2022 02:08:14 +0900 Subject: [PATCH 16/18] Add release note --- docs/release-notes/mongoid-9.0.txt | 34 ++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt index 8d9fb0fb5a..de0d874c4c 100644 --- a/docs/release-notes/mongoid-9.0.txt +++ b/docs/release-notes/mongoid-9.0.txt @@ -73,3 +73,37 @@ the type conversion. Note that in prior Mongoid versions, typecasting Date to Time during persistence operations was already correctly using the ``Mongoid.use_activesupport_time_zone`` setting. + + +```#touch`` method on embedded documents correctly handles ``touch: false`` option +---------------------------------------------------------------------------------- + +When the ``touch: false`` option is set on an ``embedded_in`` relation, +calling the ``#touch`` method on an embedded child document will not +invoke ``#touch`` on its parent document. + +.. code-block:: ruby + + class Address + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :mall, touch: false + end + + class Mall + include Mongoid::Document + include Mongoid::Timestamps + + embeds_many :addresses + end + + mall = Mall.create! + address = mall.addresses.create! + + address.touch + #=> updates address.updated_at but not mall.updated_at + +In addition, the ``#touch`` method has been optimized perform one +persistence operation per parent document, even when using multiple +levels of nested embedded documents. From 39e9dee930873f423ef298d6061cd652193d3c4a Mon Sep 17 00:00:00 2001 From: Neil Shweky Date: Tue, 4 Oct 2022 13:10:34 -0400 Subject: [PATCH 17/18] Update docs/release-notes/mongoid-9.0.txt --- docs/release-notes/mongoid-9.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt index 53fb1242e5..6e9ab7ca24 100644 --- a/docs/release-notes/mongoid-9.0.txt +++ b/docs/release-notes/mongoid-9.0.txt @@ -104,7 +104,7 @@ invoke ``#touch`` on its parent document. address.touch #=> updates address.updated_at but not mall.updated_at -In addition, the ``#touch`` method has been optimized perform one +In addition, the ``#touch`` method has been optimized to perform one persistence operation per parent document, even when using multiple levels of nested embedded documents. From dd34b8f702d0b32b1800ddc95688e278d97b1cfe Mon Sep 17 00:00:00 2001 From: Neil Shweky Date: Tue, 4 Oct 2022 13:26:09 -0400 Subject: [PATCH 18/18] MONGOID-5136 fix and format specs --- spec/mongoid/touchable_spec.rb | 68 ++++++++++++++------------- spec/mongoid/touchable_spec_models.rb | 3 ++ 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/spec/mongoid/touchable_spec.rb b/spec/mongoid/touchable_spec.rb index 92a3a0a074..bbdd3788ba 100644 --- a/spec/mongoid/touchable_spec.rb +++ b/spec/mongoid/touchable_spec.rb @@ -58,7 +58,7 @@ update_time entrance.touch - expect(entrance.updated_at).to eq update_time + expect(entrance.updated_at).to eq(update_time) end it "persists the changes" do @@ -66,7 +66,7 @@ update_time entrance.touch - expect(entrance.reload.updated_at).to eq update_time + expect(entrance.reload.updated_at).to eq(update_time) end end @@ -77,7 +77,7 @@ update_time floor.touch - expect(building.updated_at).to eq update_time + expect(building.updated_at).to eq(update_time) end it 'persists updated updated_at on parent' do @@ -85,7 +85,7 @@ update_time floor.touch - expect(building.reload.updated_at).to eq update_time + expect(building.reload.updated_at).to eq(update_time) end end @@ -95,7 +95,7 @@ update_time entrance.touch - expect(building.updated_at).to eq start_time + expect(building.updated_at).to eq(start_time) end it 'does not persist updated updated_at on parent' do @@ -103,7 +103,7 @@ update_time entrance.touch - expect(building.reload.updated_at).to eq start_time + expect(building.reload.updated_at).to eq(start_time) end end @@ -128,11 +128,11 @@ building.reload # This is the assertion we want. - expect(entrance.last_used_at).to eq update_time + expect(entrance.last_used_at).to eq(update_time) # Check other timestamps for good measure. - expect(entrance.updated_at).to eq update_time - expect(building.updated_at).to eq start_time + expect(entrance.updated_at).to eq(update_time) + expect(building.updated_at).to eq(start_time) end end @@ -146,11 +146,11 @@ building.reload # This is the assertion we want. - expect(floor.last_used_at).to eq update_time + expect(floor.last_used_at).to eq(update_time) # Check other timestamps for good measure. - expect(floor.updated_at).to eq update_time - expect(building.updated_at).to eq update_time + expect(floor.updated_at).to eq(update_time) + expect(building.updated_at).to eq(update_time) end end end @@ -623,8 +623,8 @@ end it "updates the parent's timestamp" do - expect(building.updated_at).to eq update_time - expect(building.reload.updated_at).to eq update_time + expect(building.updated_at).to eq(update_time) + expect(building.reload.updated_at).to eq(update_time) end end @@ -705,18 +705,22 @@ before do entrance update_time - entrance.reload # TODO: remove this after MONGOID-5504 + entrance.level = 1 entrance.send(meth) end it "updates the child's timestamp" do - expect(entrance.updated_at).to eq update_time - expect(entrance.reload.updated_at).to eq update_time + if entrance.destroyed? + expect(entrance.updated_at).to eq(start_time) + else + expect(entrance.updated_at).to eq(update_time) + expect(entrance.reload.updated_at).to eq(update_time) + end end it "does not update the parent's timestamp" do - expect(building.updated_at).to eq start_time - expect(building.reload.updated_at).to eq start_time + expect(building.updated_at).to eq(start_time) + expect(building.reload.updated_at).to eq(start_time) end end @@ -767,47 +771,47 @@ shared_examples "updates the parent" do it "updates the parent's timestamp" do - expect(parent.updated_at).to eq update_time - expect(parent.reload.updated_at).to eq update_time + expect(parent.updated_at).to eq(update_time) + expect(parent.reload.updated_at).to eq(update_time) end end shared_examples "does not update the parent" do it "does not update the parent's timestamp" do - expect(parent.updated_at).to eq start_time - expect(parent.reload.updated_at).to eq start_time + expect(parent.updated_at).to eq(start_time) + expect(parent.reload.updated_at).to eq(start_time) end end shared_examples "updates the child" do it "updates the child's timestamp" do - expect(child.updated_at).to eq update_time - expect(child.reload.updated_at).to eq update_time + expect(child.updated_at).to eq(update_time) + expect(child.reload.updated_at).to eq(update_time) end end shared_examples "does not update the child" do it "does not update the child's timestamp" do - expect(child.updated_at).to eq start_time - expect(child.reload.updated_at).to eq start_time + expect(child.updated_at).to eq(start_time) + expect(child.reload.updated_at).to eq(start_time) end end shared_examples "updates the grandchild" do it "updates the grandchild's timestamp" do if grandchild.destroyed? - expect(grandchild.updated_at).to eq start_time + expect(grandchild.updated_at).to eq(start_time) else - expect(grandchild.updated_at).to eq update_time - expect(grandchild.reload.updated_at).to eq update_time + expect(grandchild.updated_at).to eq(update_time) + expect(grandchild.reload.updated_at).to eq(update_time) end end end shared_examples "does not update the grandchild" do it "does not update the grandchild's timestamp" do - expect(grandchild.updated_at).to eq start_time - expect(grandchild.reload.updated_at).to eq start_time unless grandchild.destroyed? + expect(grandchild.updated_at).to eq(start_time) + expect(grandchild.reload.updated_at).to eq(start_time) unless grandchild.destroyed? end end diff --git a/spec/mongoid/touchable_spec_models.rb b/spec/mongoid/touchable_spec_models.rb index 58590ae4d5..f74a612127 100644 --- a/spec/mongoid/touchable_spec_models.rb +++ b/spec/mongoid/touchable_spec_models.rb @@ -17,6 +17,7 @@ class Entrance include Mongoid::Timestamps field :last_used_at, type: Time + field :level, type: Integer embedded_in :building, touch: false, class_name: "TouchableSpec::Embedded::Building" @@ -79,6 +80,8 @@ class Entrance include Mongoid::Document include Mongoid::Timestamps + field :level, type: Integer + belongs_to :building, touch: false, class_name: "TouchableSpec::Referenced::Building" embeds_many :keypads, class_name: "TouchableSpec::Referenced::Keypad"