diff --git a/README.md b/README.md index 45dd33f..e4017b5 100644 --- a/README.md +++ b/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 diff --git a/lib/closure_tree/has_closure_tree.rb b/lib/closure_tree/has_closure_tree.rb index 24dc138..8c2ed61 100644 --- a/lib/closure_tree/has_closure_tree.rb +++ b/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 ) diff --git a/lib/closure_tree/support.rb b/lib/closure_tree/support.rb index b27b7c6..04f73ac 100644 --- a/lib/closure_tree/support.rb +++ b/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 diff --git a/lib/closure_tree/support_attributes.rb b/lib/closure_tree/support_attributes.rb index fd30493..a40ed57 100644 --- a/lib/closure_tree/support_attributes.rb +++ b/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 diff --git a/test/closure_tree/parallel_test.rb b/test/closure_tree/parallel_test.rb index 6dc5cf9..dc528f4 100644 --- a/test/closure_tree/parallel_test.rb +++ b/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 diff --git a/test/closure_tree/support_test.rb b/test/closure_tree/support_test.rb index 3770d95..9e353e6 100644 --- a/test/closure_tree/support_test.rb +++ b/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