diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1ea0403751..2d02424699 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,7 +36,7 @@ jobs: - id: start-mongodb name: start mongodb - uses: mongodb-labs/drivers-evergreen-tools@b26580ac7b03a7eb282053871da0a050c7075288 + uses: mongodb-labs/drivers-evergreen-tools@18aed4f176dab1ed505c9a2a53466ddf3f4796ec with: version: "${{matrix.mongodb}}" topology: "${{matrix.topology}}" diff --git a/.mod/drivers-evergreen-tools b/.mod/drivers-evergreen-tools index b26580ac7b..18aed4f176 160000 --- a/.mod/drivers-evergreen-tools +++ b/.mod/drivers-evergreen-tools @@ -1 +1 @@ -Subproject commit b26580ac7b03a7eb282053871da0a050c7075288 +Subproject commit 18aed4f176dab1ed505c9a2a53466ddf3f4796ec diff --git a/lib/mongoid/persistable/updatable.rb b/lib/mongoid/persistable/updatable.rb index b9a7b65c68..fa26e9cd9b 100644 --- a/lib/mongoid/persistable/updatable.rb +++ b/lib/mongoid/persistable/updatable.rb @@ -103,7 +103,7 @@ def prepare_update(options = {}) process_flagged_destroys update_children = cascadable_children(:update) - process_touch_option(options, update_children) do + process_touch_option(options) do run_all_callbacks_for_update(update_children) do result = yield(self) self.previously_new_record = false @@ -160,24 +160,22 @@ def update_document(options = {}) end end - # If there is a touch option and it is false, this method will call the - # timeless method so that the updated_at attribute is not updated. It - # will call the timeless method on all of the cascadable children as - # well. Note that timeless is cleared in the before_update callback. + # If there is a touch option and it is false, this method suppresses + # timestamping for the duration of the update using a block-based + # timeless scope, which covers this document and every cascaded child + # (at any nesting depth), and also suppresses touch callbacks. # # @param [ Hash ] options The options. - # @param [ Array ] children The children that the :update - # callbacks will be executed on. # # @option options [ true | false ] :touch Whether or not the updated_at # attribute will be updated with the current time. - def process_touch_option(options, children, &block) + def process_touch_option(options, &block) if options.fetch(:touch, true) yield else - timeless - children.each(&:timeless) - suppress_touch_callbacks(&block) + Mongoid::Timestamps::Timeless.with_timeless do + suppress_touch_callbacks(&block) + end end end diff --git a/lib/mongoid/timestamps/timeless.rb b/lib/mongoid/timestamps/timeless.rb index f116b6bd36..c3d3ece3b1 100644 --- a/lib/mongoid/timestamps/timeless.rb +++ b/lib/mongoid/timestamps/timeless.rb @@ -7,6 +7,10 @@ module Timestamps module Timeless extend ActiveSupport::Concern + # Deprecator for the block-less form of the timeless API. Its removal + # horizon is computed automatically as (current major + 1).0. + DEPRECATION = Mongoid::Deprecation.new + # Clears out the timeless option. # # @example Clear the timeless option. @@ -22,13 +26,20 @@ def clear_timeless_option true end - # Begin an execution that should skip timestamping. + # Skip timestamping for the duration of the given block, or (in the + # deprecated, block-less form) for the next persistence operation. + # + # @example Save a document but don't timestamp (block form). + # person.timeless { person.save } # - # @example Save a document but don't timestamp. + # @example Save a document but don't timestamp (deprecated chained form). # person.timeless.save # - # @return [ Document ] The document this was called on. - def timeless + # @return [ Object | Document ] The return value of the block, or (in the + # block-less form) the document this was called on. + def timeless(&block) + return Timeless.with_timeless(&block) if block + self.class.timeless self end @@ -47,6 +58,9 @@ class << self # The key to use to store the timeless table TIMELESS_TABLE_KEY = '[mongoid]:timeless' + # The key to use to store the block-based timeless flag. + TIMELESS_FLAG_KEY = '[mongoid]:timeless-flag' + # Returns the in-memory thread cache of classes # for which to skip timestamping. # @@ -58,16 +72,70 @@ def timeless_table end def_delegators :timeless_table, :[]=, :[] + + # Skip timestamping for the duration of the given block, on the + # current thread or fiber. This applies to every document persisted + # while the block is executing, regardless of class, including + # cascaded embedded children at any nesting depth. + # + # @example Skip timestamping for a block. + # Mongoid::Timestamps::Timeless.with_timeless do + # person.save + # end + # + # @return [ Object ] The return value of the block. + def with_timeless + # Only the outermost block owns the flag: if we are already inside a + # timeless scope, we leave the suppression in place when this block + # ends. This avoids tracking a nesting depth that could drift out of + # sync. + already_timeless = suppressing_timestamps? + set_suppressing_timestamps(true) unless already_timeless + yield + ensure + set_suppressing_timestamps(false) unless already_timeless + end + + # Whether a block-based timeless scope is currently active on this + # thread/fiber. + # + # @return [ true | false ] Whether timestamps are being suppressed. + # + # @api private + def suppressing_timestamps? + !!Threaded.get(TIMELESS_FLAG_KEY) { false } + end + + # Set whether a block-based timeless scope is active on this + # thread/fiber. + # + # @param [ true | false ] value Whether to suppress timestamps. + # + # @api private + def set_suppressing_timestamps(value) + Threaded.set(TIMELESS_FLAG_KEY, value) + end end module ClassMethods - # Begin an execution that should skip timestamping. + # Skip timestamping for the duration of the given block, or (in the + # deprecated, block-less form) for the next persistence operation. # - # @example Create a document but don't timestamp. + # @example Create a document but don't timestamp (block form). + # Person.timeless { Person.create(title: "Sir") } + # + # @example Create a document but don't timestamp (deprecated form). # Person.timeless.create(:title => "Sir") # - # @return [ Class ] The class this was called on. - def timeless + # @return [ Object | Class ] The return value of the block, or (in the + # block-less form) the class this was called on. + def timeless(&block) + return Timeless.with_timeless(&block) if block + + DEPRECATION.warn( + 'Calling #timeless without a block is deprecated; pass a block ' \ + 'instead, e.g. `record.timeless { record.save }`.' + ) counter = 0 counter += 1 if self < Mongoid::Timestamps::Created counter += 1 if self < Mongoid::Timestamps::Updated @@ -109,12 +177,14 @@ def set_timeless_counter(counter) Timeless[name] = (counter == 0) ? nil : counter end - # Returns whether the current class should skip timestamping. + # Returns whether the current class should skip timestamping. This is + # true when either a block-based timeless scope is active on the + # current thread/fiber, or the deprecated per-class counter is set. # # @return [ true | false ] Whether the current class should # skip timestamping. def timeless? - !!Timeless[name] + Timeless.suppressing_timestamps? || !!Timeless[name] end end end diff --git a/spec/mongoid/timestamps/timeless_spec.rb b/spec/mongoid/timestamps/timeless_spec.rb index ab6bc57127..f410c33c1f 100644 --- a/spec/mongoid/timestamps/timeless_spec.rb +++ b/spec/mongoid/timestamps/timeless_spec.rb @@ -135,4 +135,194 @@ class Egg end end end + + describe '#timeless with a block' do + before(:all) do + class TimelessOther + include Mongoid::Document + include Mongoid::Timestamps + end + end + + after(:all) do + Object.send(:remove_const, :TimelessOther) + end + + context 'when called on an instance' do + let(:document) { Dokument.new } + + it 'executes the block and persists the document' do + document.timeless { document.save! } + expect(document).to be_persisted + end + + it 'does not set the created timestamp' do + document.timeless { document.save! } + expect(document.created_at).to be_nil + end + + it 'does not set the updated timestamp' do + document.timeless { document.save! } + expect(document.updated_at).to be_nil + end + + it 'returns the value of the block' do + expect(document.timeless { 42 }).to eq(42) + end + + it 'resumes timestamping after the block' do + document.timeless { document.save! } + document.update_attribute(:title, 'Sir') + expect(document.updated_at).not_to be_nil + end + + it 'is not timeless outside the block' do + document.timeless { document.save! } + expect(document).not_to be_timeless + end + + it 'restores state even when the block raises' do + expect do + document.timeless { raise 'boom' } + end.to raise_error('boom') + expect(document).not_to be_timeless + end + end + + context 'when called on the class' do + it 'does not set timestamps for documents created in the block' do + document = Dokument.timeless { Dokument.create! } + expect(document.created_at).to be_nil + expect(document.updated_at).to be_nil + end + end + + context 'when nested' do + let(:document) { Dokument.new } + + it 'remains timeless until the outermost block exits' do + Dokument.timeless do + Dokument.timeless { document.save! } + # inner block has exited, but we are still inside the outer block + expect(document).to be_timeless + end + expect(document).not_to be_timeless + expect(document.created_at).to be_nil + end + end + + context 'when other documents are persisted in the block' do + it 'suppresses timestamps globally on the thread for the block duration' do + other = nil + Dokument.timeless { other = TimelessOther.create! } + expect(other.created_at).to be_nil + end + end + end + + # Regression for MONGOID-5782: saving a parent timeless must not bump the + # updated_at of embedded children, at any nesting depth, when the + # associations cascade callbacks. + describe 'MONGOID-5782 cascaded embedded timestamps' do + before(:all) do + class TimelessBaz + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :timeless_bar + field :val, type: String + end + + class TimelessBar + include Mongoid::Document + include Mongoid::Timestamps + + embedded_in :timeless_foo + embeds_many :timeless_bazs, cascade_callbacks: true + field :val, type: String + end + + class TimelessFoo + include Mongoid::Document + include Mongoid::Timestamps + + embeds_many :timeless_bars, cascade_callbacks: true + field :val, type: String + end + end + + after(:all) do + Object.send(:remove_const, :TimelessBaz) + Object.send(:remove_const, :TimelessBar) + Object.send(:remove_const, :TimelessFoo) + end + + let!(:start_time) { Timecop.freeze(Time.at(Time.now.to_i)) } + + let!(:foo) do + TimelessFoo.create!(timeless_bars: [ { val: 'a', timeless_bazs: [ { val: 'x' } ] } ]) + end + + let(:bar) { foo.timeless_bars.first } + let(:baz) { bar.timeless_bazs.first } + + after do + Timecop.return + end + + it 'does not bump the embedded child updated_at' do + original = bar.updated_at + bar.val = 'b' + Timecop.freeze(Time.at(Time.now.to_i) + 2) + foo.timeless { foo.save! } + # the change was actually persisted (the block ran)... + expect(foo.reload.timeless_bars.first.val).to eq('b') + # ...but the timestamp was suppressed. + expect(bar.updated_at).to eq(original) + end + + it 'does not bump the nested embedded child updated_at' do + original = baz.updated_at + baz.val = 'y' + Timecop.freeze(Time.at(Time.now.to_i) + 2) + foo.timeless { foo.save! } + expect(foo.reload.timeless_bars.first.timeless_bazs.first.val).to eq('y') + expect(baz.updated_at).to eq(original) + end + + it 'still preserves the parent updated_at' do + original = foo.updated_at + foo.val = 'c' + Timecop.freeze(Time.at(Time.now.to_i) + 2) + foo.timeless { foo.save! } + expect(TimelessFoo.find(foo.id).val).to eq('c') + expect(foo.updated_at).to eq(original) + end + end + + describe 'deprecation of the block-less form' do + let(:document) { Dokument.new } + + it 'warns when called on an instance without a block' do + expect(Mongoid.logger).to receive(:warn).with(/timeless/).and_call_original + document.timeless.save! + end + + it 'warns when called on the class without a block' do + expect(Mongoid.logger).to receive(:warn).with(/timeless/).and_call_original + Dokument.timeless.create! + end + + it 'does not warn when called with a block' do + expect(Mongoid.logger).not_to receive(:warn) + document.timeless { document.save! } + end + + it 'does not warn from the internal touch: false path' do + document.save! + document.title = 'changed' + expect(Mongoid.logger).not_to receive(:warn).with(/timeless/) + document.save!(touch: false) + end + end end