feat: raise error when advisory lock cannot be acquired within config…

README.md

@@ -1,29 +1,4 @@
-# Closure Tree (OpsLevel Fork)
-
-This is the OpsLevel fork of [closure_tree](https://github.com/ClosureTree/closure_tree), based on upstream v9.5.0.
-
-## Fork-Specific Changes
-
-This fork adds one enhancement to the advisory lock behavior:
-
-### Advisory Lock Timeout with Error on Failure
-
-- Adds `advisory_lock_timeout_seconds` option (default: 15 seconds)
-- Raises an error when the advisory lock cannot be acquired within the timeout period
-
-The upstream version has no timeout and hangs forever if lock acquisition fails. This fork uses `with_advisory_lock!` (with bang) which raises `WithAdvisoryLock::FailedToAcquireLock` when the lock cannot be obtained, providing explicit failure behavior for lock contention issues.
-
-```ruby
-class Tag < ApplicationRecord
-  # Use default 15 second timeout
-  has_closure_tree
-
-  # Or customize the timeout
-  has_closure_tree advisory_lock_timeout_seconds: 30
-end
-```
-
----
+# Closure Tree
 
 ### Closure_tree lets your ActiveRecord models act as nodes in a [tree data structure](http://en.wikipedia.org/wiki/Tree_%28data_structure%29)
 
@@ -347,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,7 +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: 15,
+        advisory_lock_timeout_seconds: nil,
         numeric_order: false
       }.merge(options)
       raise ArgumentError, "name_column can't be 'path'" if options[:name_column] == 'path'
@@ -31,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)
@@ -154,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, advisory_lock_options) 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

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