feat: raise error when advisory lock cannot be acquired within configured timeout (#480)

------

Co-authored-by: Evan Huus <[email protected]>
Co-authored-by: Farjaad Rawasia <[email protected]>

Author: 3 people

README.md

@@ -322,6 +322,7 @@ When you include ```has_closure_tree``` in your model, you can provide a hash to
 * ```:order``` used to set up [deterministic ordering](#deterministic-ordering)
 * ```:scope``` restricts root nodes and sibling ordering to specific columns. Can be a single symbol or an array of symbols. Example: ```scope: :user_id``` or ```scope: [:user_id, :group_id]```. This ensures that root nodes and siblings are scoped correctly when reordering. See [Ordering Roots](#ordering-roots) for more details.
 * ```:touch``` delegates to the `belongs_to` annotation for the parent, so `touch`ing cascades to all children (the performance of this for deep trees isn't currently optimal).
+* ```:advisory_lock_timeout_seconds``` When set, the advisory lock will raise ```WithAdvisoryLock::FailedToAcquireLock``` if the lock cannot be acquired within the timeout period. This helps callers handle timeout scenarios (e.g. retry or fail fast). If the option is not specified, the lock is waited for indefinitely until it is acquired. See [Lock wait timeouts](https://github.com/ClosureTree/with_advisory_lock?tab=readme-ov-file#lock-wait-timeouts) in the with_advisory_lock gem for details.
 
 ## Accessing Data
 

lib/closure_tree/has_closure_tree.rb

@@ -15,6 +15,7 @@ def has_closure_tree(options = {})
         :touch,
         :with_advisory_lock,
         :advisory_lock_name,
+        :advisory_lock_timeout_seconds,
         :scope
       )
 

lib/closure_tree/support.rb

@@ -19,6 +19,7 @@ def initialize(model_class, options)
         dependent: :nullify, # or :destroy, :delete_all, or :adopt -- see the README
         name_column: 'name',
         with_advisory_lock: true, # This will be overridden by adapter support
+        advisory_lock_timeout_seconds: nil,
         numeric_order: false
       }.merge(options)
       raise ArgumentError, "name_column can't be 'path'" if options[:name_column] == 'path'
@@ -30,6 +31,10 @@ def initialize(model_class, options)
         end
       end
 
+      if !@options[:with_advisory_lock] && @options[:advisory_lock_timeout_seconds].present?
+        raise ArgumentError, "advisory_lock_timeout_seconds can't be specified when advisory_lock is disabled"
+      end
+
       return unless order_is_numeric?
 
       extend NumericOrderSupport.adapter_for_connection(connection)
@@ -153,8 +158,9 @@ def build_scope_where_clause(scope_conditions)
     end
 
     def with_advisory_lock(&block)
-      if options[:with_advisory_lock] && connection.supports_advisory_locks? && model_class.respond_to?(:with_advisory_lock)
-        model_class.with_advisory_lock(advisory_lock_name) do
+      lock_method = options[:advisory_lock_timeout_seconds].present? ? :with_advisory_lock! : :with_advisory_lock
+      if options[:with_advisory_lock] && connection.supports_advisory_locks? && model_class.respond_to?(lock_method)
+        model_class.public_send(lock_method, advisory_lock_name, advisory_lock_options) do
           transaction(&block)
         end
       else

lib/closure_tree/support_attributes.rb

@@ -34,6 +34,10 @@ def advisory_lock_name
       end
     end
 
+    def advisory_lock_options
+      { timeout_seconds: options[:advisory_lock_timeout_seconds] }.compact
+    end
+
     def quoted_table_name
       connection.quote_table_name(table_name)
     end

test/closure_tree/parallel_test.rb

@@ -137,7 +137,7 @@ def run_workers(worker_class = FindOrCreateWorker)
     skip('unsupported') unless run_parallel_tests?
 
     # disable with_advisory_lock:
-    Tag.stub(:with_advisory_lock, ->(_lock_name, &block) { block.call }) do
+    Tag.stub(:with_advisory_lock, ->(_lock_name, _lock_options, &block) { block.call }) do
       run_workers
       # duplication from at least one iteration:
       assert Tag.where(name: @names).size > @iterations

test/closure_tree/support_test.rb

@@ -3,7 +3,8 @@
 require 'test_helper'
 
 describe ClosureTree::Support do
-  let(:sut) { Tag._ct }
+  let(:model) { Tag }
+  let(:sut) { model._ct }
 
   it 'passes through table names without prefix and suffix' do
     expected = 'some_random_table_name'
@@ -15,4 +16,86 @@
     tn = ActiveRecord::Base.table_name_prefix + expected + ActiveRecord::Base.table_name_suffix
     assert_equal expected, sut.remove_prefix_and_suffix(tn)
   end
+
+  it 'initializes without error when with_advisory_lock is false' do
+    assert ClosureTree::Support.new(model, { with_advisory_lock: false })
+  end
+
+  it 'initializes without error when with_advisory_lock is true and advisory_lock_timeout_seconds is set' do
+    assert ClosureTree::Support.new(model, { with_advisory_lock: true, advisory_lock_timeout_seconds: 10 })
+  end
+
+  it 'raises ArgumentError when advisory_lock_timeout_seconds is set but with_advisory_lock is false' do
+    error = assert_raises(ArgumentError) do
+      ClosureTree::Support.new(model, with_advisory_lock: false, advisory_lock_timeout_seconds: 10)
+    end
+    assert_match(/advisory_lock_timeout_seconds can't be specified when advisory_lock is disabled/, error.message)
+  end
+
+  it 'calls :with_advisory_lock! when with_advisory_lock is true and timeout is 10' do
+    options = sut.options.merge(with_advisory_lock: true, advisory_lock_timeout_seconds: 10)
+    received_lock_name = nil
+    received_options = nil
+    called = false
+    sut.stub(:options, options) do
+      model.stub(:with_advisory_lock!, ->(lock_name, opts, &block) {
+        received_lock_name = lock_name
+        received_options = opts
+        block.call
+      }) do
+        sut.with_advisory_lock { called = true }
+      end
+    end
+    assert called, 'block should have been called'
+    assert_equal sut.advisory_lock_name, received_lock_name, 'lock name should be passed to with_advisory_lock!'
+    assert_equal({ timeout_seconds: 10 }, received_options, 'options should include timeout_seconds when timeout is configured')
+  end
+
+  it 'calls :with_advisory_lock when with_advisory_lock is true and timeout is nil' do
+    received_options = nil
+    called = false
+    model.stub(:with_advisory_lock, ->(_lock_name, opts, &block) {
+      received_options = opts
+      block.call
+    }) do
+      sut.with_advisory_lock { called = true }
+    end
+    assert called, 'block should have been called'
+    assert_equal({}, received_options, 'options should be empty when timeout is not configured')
+  end
+
+  it 'does not call advisory lock methods when with_advisory_lock is false' do
+    options = sut.options.merge(with_advisory_lock: false, advisory_lock_timeout_seconds: nil)
+    called = false
+    sut.stub(:options, options) do
+      sut.with_advisory_lock { called = true }
+    end
+    assert called, 'block should have been called'
+  end
+
+  it 'raises WithAdvisoryLock::FailedToAcquireLock when lock cannot be acquired within timeout' do
+    lock_held = false
+    holder_thread = Thread.new do
+      model.connection_pool.with_connection do
+        model.with_advisory_lock(sut.advisory_lock_name) do
+          lock_held = true
+          sleep 2
+        end
+      end
+    end
+
+    # Wait for holder to acquire the lock
+    sleep 0.2 until lock_held
+
+    support_with_timeout = ClosureTree::Support.new(
+      model,
+      sut.options.merge(with_advisory_lock: true, advisory_lock_timeout_seconds: 1)
+    )
+
+    assert_raises(WithAdvisoryLock::FailedToAcquireLock) do
+      support_with_timeout.with_advisory_lock { nil }
+    end
+
+    holder_thread.join
+  end
 end