feat: raise error when advisory lock cannot be acquired within configured timeout

Author: farjaad-opslevel

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``` Raises an error when the advisory lock cannot be acquired within the timeout period
 
 ## 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)
@@ -155,7 +159,9 @@ def build_scope_where_clause(scope_conditions)
 
     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
+
+        model_class.public_send(lock_method, advisory_lock_name, advisory_lock_options) do
           transaction(&block)
         end
       else

test/closure_tree/support_test.rb

@@ -3,7 +3,9 @@
 require 'test_helper'
 
 describe ClosureTree::Support do
-  let(:sut) { Tag._ct }
+  let(:model) { Tag }
+  let(:options) { {} }
+  let(:sut) { ClosureTree::Support.new(model, options) }
 
   it 'passes through table names without prefix and suffix' do
     expected = 'some_random_table_name'
@@ -15,4 +17,29 @@
     tn = ActiveRecord::Base.table_name_prefix + expected + ActiveRecord::Base.table_name_suffix
     assert_equal expected, sut.remove_prefix_and_suffix(tn)
   end
+
+  it 'calls :with_advisory_lock! when with_advisory_lock is true and timeout is 10' do
+    sut = ClosureTree::Support.new(model, { with_advisory_lock: true, advisory_lock_timeout_seconds: 10 })
+    called = false
+    model.stub(:with_advisory_lock!, ->(_lock_name, _options, &block) { block.call }) do
+      sut.with_advisory_lock { called = true }
+    end
+    assert called, 'block should have been called'
+  end
+
+  it 'calls :with_advisory_lock when with_advisory_lock is true and timeout is nil' do
+    sut = ClosureTree::Support.new(model, { with_advisory_lock: true, advisory_lock_timeout_seconds: nil })
+    called = false
+    model.stub(:with_advisory_lock, ->(_lock_name, _options, &block) { block.call }) do
+      sut.with_advisory_lock { called = true }
+    end
+    assert called, 'block should have been called'
+  end
+
+  it 'does not call advisory lock methods when with_advisory_lock is false' do
+    sut = ClosureTree::Support.new(model, { with_advisory_lock: false, advisory_lock_timeout_seconds: nil })
+    called = false
+    sut.with_advisory_lock { called = true }
+    assert called, 'block should have been called'
+  end
 end