make advisory_lock_timeout_seconds required when advisory_lock_timeout_seconds is true

Author: farjaad-opslevel

lib/closure_tree/support.rb

@@ -23,11 +23,13 @@ def initialize(model_class, options)
         :dependent => :nullify, # or :destroy or :delete_all -- see the README
         :name_column => 'name',
         :with_advisory_lock => true,
-        :advisory_lock_timeout_seconds => nil,
+        :advisory_lock_timeout_seconds => 0,
         :numeric_order => false
       }.merge(options)
       raise ArgumentError, "name_column can't be 'path'" if options[:name_column] == 'path'
-      if !options[:with_advisory_lock] && options[:advisory_lock_timeout_seconds].present?
+      if options[:with_advisory_lock] && options[:advisory_lock_timeout_seconds].nil?
+        raise ArgumentError, "advisory_lock_timeout_seconds must be provided when advisory lock is enabled"
+      elsif !options[:with_advisory_lock] && options[:advisory_lock_timeout_seconds].present?
         raise ArgumentError, "advisory_lock_timeout_seconds cannot be provided when advisory lock is disabled"
       end
       if order_is_numeric?
@@ -114,8 +116,7 @@ def where_eq(column_name, value)
 
     def with_advisory_lock!(&block)
       if options[:with_advisory_lock]
-        lock_options = { timeout_seconds: options[:advisory_lock_timeout_seconds] }.compact
-        model_class.with_advisory_lock!(advisory_lock_name, lock_options) do
+        model_class.with_advisory_lock!(advisory_lock_name, timeout_seconds: options[:advisory_lock_timeout_seconds]) do
           transaction { yield }
         end
       else

spec/closure_tree/support_spec.rb

@@ -2,7 +2,7 @@
 
 RSpec.describe ClosureTree::Support do
   let(:model) { Tag }
-  let(:options) { {} }
+  let(:options) { { with_advisory_lock: true, advisory_lock_timeout_seconds: 10 } }
   let(:sut) { described_class.new(model, options) }
 
   it 'passes through table names without prefix and suffix' do
@@ -16,23 +16,8 @@
     expect(sut.remove_prefix_and_suffix(tn)).to eq(expected)
   end
 
-  [
-    [true, 10, { timeout_seconds: 10 }],
-    [true, nil, {}],
-    [false, nil, {}]
-  ].each do |with_lock, timeout, expected_options|
-    context "when with_advisory_lock is #{with_lock} and timeout is #{timeout}" do
-      let(:options) { { with_advisory_lock: with_lock, advisory_lock_timeout_seconds: timeout } }
-
-      it "uses advisory lock with timeout options: #{expected_options}" do
-        if with_lock
-          expect(model).to receive(:with_advisory_lock!).with(anything, expected_options).and_yield
-        else
-          expect(model).not_to receive(:with_advisory_lock!)
-        end
-
-        expect { |b| sut.with_advisory_lock!(&b) }.to yield_control
-      end
-    end
+  it 'calls with_advisory_lock! with timeout options' do
+    expect(model).to receive(:with_advisory_lock!).with(anything, { timeout_seconds: 10 }).and_yield
+    expect { |b| sut.with_advisory_lock!(&b) }.to yield_control
   end
 end