diff --git a/lib/closure_tree/finders.rb b/lib/closure_tree/finders.rb index eb7a696..c9e57fe 100644 --- a/lib/closure_tree/finders.rb +++ b/lib/closure_tree/finders.rb @@ -20,7 +20,7 @@ def find_or_create_by_path(path, attributes = {}) return found if found attrs = subpath.shift - _ct.with_advisory_lock do + _ct.with_advisory_lock(self) do # shenanigans because children.create is bound to the superclass # (in the case of polymorphism): child = children.where(attrs).first || begin diff --git a/lib/closure_tree/hierarchy_maintenance.rb b/lib/closure_tree/hierarchy_maintenance.rb index 951fc14..caa43ac 100644 --- a/lib/closure_tree/hierarchy_maintenance.rb +++ b/lib/closure_tree/hierarchy_maintenance.rb @@ -64,7 +64,7 @@ def _ct_after_save end def _ct_before_destroy - _ct.with_advisory_lock do + _ct.with_advisory_lock(self) do _ct_adopt_children_to_grandparent if _ct.options[:dependent] == :adopt delete_hierarchy_references self.class.find(id).children.find_each(&:rebuild!) if _ct.options[:dependent] == :nullify @@ -86,7 +86,7 @@ def _ct_before_destroy end def rebuild!(called_by_rebuild = false) - _ct.with_advisory_lock do + _ct.with_advisory_lock(self) do delete_hierarchy_references unless (defined? @was_new_record) && @was_new_record hierarchy_class.create!(ancestor: self, descendant: self, generations: 0) unless root? @@ -112,7 +112,7 @@ def rebuild!(called_by_rebuild = false) end def delete_hierarchy_references - _ct.with_advisory_lock do + _ct.with_advisory_lock(self) do # The crazy double-wrapped sub-subselect works around MySQL's limitation of subselects on the same table that is being mutated. # It shouldn't affect performance of postgresql. # See http://dev.mysql.com/doc/refman/5.0/en/subquery-errors.html diff --git a/lib/closure_tree/numeric_deterministic_ordering.rb b/lib/closure_tree/numeric_deterministic_ordering.rb index e0e1644..6573769 100644 --- a/lib/closure_tree/numeric_deterministic_ordering.rb +++ b/lib/closure_tree/numeric_deterministic_ordering.rb @@ -162,7 +162,7 @@ def add_sibling(sibling, add_after = true) # Make sure self isn't dirty, because we're going to call reload: save - _ct.with_advisory_lock do + _ct.with_advisory_lock(self) do prior_sibling_parent = sibling.parent sibling.order_value = order_value diff --git a/lib/closure_tree/support.rb b/lib/closure_tree/support.rb index b8fa91d..ea74c88 100644 --- a/lib/closure_tree/support.rb +++ b/lib/closure_tree/support.rb @@ -157,10 +157,10 @@ def build_scope_where_clause(scope_conditions) " AND #{conditions.join(' AND ')}" end - def with_advisory_lock(&block) + def with_advisory_lock(instance = nil, &block) 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 + model_class.public_send(lock_method, advisory_lock_name_for(instance), 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 a40ed57..a44538e 100644 --- a/lib/closure_tree/support_attributes.rb +++ b/lib/closure_tree/support_attributes.rb @@ -34,6 +34,17 @@ def advisory_lock_name end end + def advisory_lock_name_for(instance = nil) + base = advisory_lock_name + return base unless instance && options[:scope] + + scope_values = scope_values_from_instance(instance) + return base if scope_values.empty? + + suffix = scope_values.values.map(&:to_s).join('_') + "#{base}_#{suffix}" + end + def advisory_lock_options { timeout_seconds: options[:advisory_lock_timeout_seconds] }.compact end diff --git a/test/closure_tree/scoped_advisory_lock_test.rb b/test/closure_tree/scoped_advisory_lock_test.rb new file mode 100644 index 0000000..4eabbe6 --- /dev/null +++ b/test/closure_tree/scoped_advisory_lock_test.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'test_helper' + +# Test that advisory lock names include scope values when scope option is configured +class ScopedAdvisoryLockTest < ActiveSupport::TestCase + def test_advisory_lock_name_for_with_scope_includes_scope_value + item = ScopedItem.new(user_id: 42) + base = item._ct.advisory_lock_name + assert_equal "#{base}_42", item._ct.advisory_lock_name_for(item) + end + + def test_advisory_lock_name_for_different_scope_values_differ + item_a = ScopedItem.new(user_id: 1) + item_b = ScopedItem.new(user_id: 2) + assert_not_equal item_a._ct.advisory_lock_name_for(item_a), + item_b._ct.advisory_lock_name_for(item_b) + end + + def test_advisory_lock_name_for_same_scope_values_match + item_a = ScopedItem.new(user_id: 7) + item_b = ScopedItem.new(user_id: 7) + assert_equal item_a._ct.advisory_lock_name_for(item_a), + item_b._ct.advisory_lock_name_for(item_b) + end + + def test_advisory_lock_name_for_without_scope_returns_base + tag = Tag.new + base = tag._ct.advisory_lock_name + assert_equal base, tag._ct.advisory_lock_name_for(tag) + end + + def test_advisory_lock_name_for_nil_instance_returns_base + item = ScopedItem.new(user_id: 1) + assert_equal item._ct.advisory_lock_name, item._ct.advisory_lock_name_for(nil) + end + + def test_advisory_lock_name_for_multi_scope + item = MultiScopedItem.new(user_id: 10, group_id: 20) + base = item._ct.advisory_lock_name + assert_equal "#{base}_10_20", item._ct.advisory_lock_name_for(item) + end +end