MONGOID-5782 Add block-based Timeless API to fix cascaded timestamp leak (#6165)

* MONGOID-5782 Add block-based Timeless API to fix cascaded timestamp leak

The counter-based timeless mechanism was consumed by the first timestamp
callback in a save, so cascaded embedded children (and nested children,
whose callbacks run more than once) lost suppression and had their
updated_at bumped.

Introduce a block-based API, timeless { ... }, backed by a thread/fiber
nesting depth that is only cleared when the block exits. This suppresses
timestamping for everything persisted in the block, at any cascade depth,
and is easier to reason about than the implicit next-operation scope.

The block-less chained form still works but is deprecated (removal in
Mongoid 10.0) via Mongoid::Deprecation. process_touch_option now uses the
block form internally so touch: false updates no longer leak on nested
embeds and no longer emit an internal deprecation warning.

* bump drivers-evergreen-tools to get NPM fix

* simplify timeless tracking -- it only needs to be true/false

* bump DEG in the test workflow

Author: jamis

.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}}"

.mod/drivers-evergreen-tools

@@ -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<Document> ] 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
 

lib/mongoid/persistable/updatable.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

lib/mongoid/timestamps/timeless.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