diff --git a/README.md b/README.md index e4017b5..1935dd9 100644 --- a/README.md +++ b/README.md @@ -560,7 +560,7 @@ class Tag < ApplicationRecord has_closure_tree advisory_lock_name: 'custom_tag_lock' end -# Dynamic via Proc +# Dynamic via Proc (1-arity: receives model class only) class Tag < ApplicationRecord has_closure_tree advisory_lock_name: ->(model_class) { "#{Rails.env}_#{model_class.name.underscore}" } end @@ -568,16 +568,36 @@ end # Delegate to model method class Tag < ApplicationRecord has_closure_tree advisory_lock_name: :custom_lock_name - + def self.custom_lock_name "tag_lock_#{current_tenant_id}" end end ``` +#### Per-instance lock names (multi-tenant / scoped models) + +Pass a 2-arity proc to receive both the model class and the current record instance. +This is the recommended approach for scoped models where each tenant should have its own lock, +avoiding unnecessary serialization across tenants. + +```ruby +class Node < ApplicationRecord + has_closure_tree scope: :company_id, + advisory_lock_name: ->(klass, instance) { + company = instance&.company_id + company ? "ct_#{klass.name}_#{company}" : "ct_#{klass.name}" + } +end +``` + +When `instance` is `nil` (class-level operations like `Node.rebuild!`), the proc should +fall back to a model-wide name. Instance-level operations (`save`, `destroy`, `add_sibling`, +`find_or_create_by_path`) pass the record itself so the lock is scoped to that tenant. + This is particularly useful when: * You need environment-specific lock names -* You're using multi-tenancy and need tenant-specific locks +* You're using multi-tenancy and need per-tenant locks (avoiding cross-tenant contention) * You want to avoid lock name collisions between similar model names ## Multi-Database Support 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..d5ed261 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(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..0e9940b 100644 --- a/lib/closure_tree/support_attributes.rb +++ b/lib/closure_tree/support_attributes.rb @@ -8,28 +8,22 @@ module SupportAttributes extend Forwardable def_delegators :model_class, :connection, :transaction, :table_name, :base_class, :inheritance_column, :column_names - def advisory_lock_name - # Allow customization via options or instance method + def advisory_lock_name(instance = nil) if options[:advisory_lock_name] case options[:advisory_lock_name] when Proc - # Allow dynamic generation via proc - options[:advisory_lock_name].call(base_class) + proc = options[:advisory_lock_name] + proc.arity == 1 ? proc.call(base_class) : proc.call(base_class, instance) when Symbol - # Allow delegation to a model method if model_class.respond_to?(options[:advisory_lock_name]) model_class.send(options[:advisory_lock_name]) else raise ArgumentError, "Model #{model_class} does not respond to #{options[:advisory_lock_name]}" end else - # Use static string value options[:advisory_lock_name].to_s end else - # Default: Use CRC32 for a shorter, consistent hash - # This gives us 8 hex characters which is plenty for uniqueness - # and leaves room for prefixes "ct_#{Zlib.crc32(base_class.name.to_s).to_s(16)}" end end diff --git a/test/closure_tree/advisory_lock_test.rb b/test/closure_tree/advisory_lock_test.rb index 553faaf..589b6ec 100644 --- a/test/closure_tree/advisory_lock_test.rb +++ b/test/closure_tree/advisory_lock_test.rb @@ -33,6 +33,51 @@ def test_proc_advisory_lock_name assert_equal "lock_for_#{@model_class.name.underscore}", instance._ct.advisory_lock_name end + def test_proc_advisory_lock_name_with_instance_arity + with_temporary_model do + has_closure_tree advisory_lock_name: ->(klass, instance) { + tenant = instance&.name + tenant ? "#{klass.name.underscore}_#{tenant}" : "#{klass.name.underscore}_global" + } + end + + instance = @model_class.new + instance.name = "acme" + + assert_equal "#{@model_class.name.underscore}_global", @model_class._ct.advisory_lock_name(nil) + assert_equal "#{@model_class.name.underscore}_acme", @model_class._ct.advisory_lock_name(instance) + end + + def test_proc_arity_1_backward_compat + with_temporary_model do + has_closure_tree advisory_lock_name: ->(model) { "compat_#{model.name}" } + end + + instance = @model_class.new + assert_equal "compat_#{@model_class.name}", @model_class._ct.advisory_lock_name(instance) + assert_equal "compat_#{@model_class.name}", @model_class._ct.advisory_lock_name(nil) + end + + def test_different_instances_produce_different_lock_names + with_temporary_model do + has_closure_tree advisory_lock_name: ->(klass, instance) { + tenant = instance&.name + tenant ? "ct_#{klass.name}_#{tenant}" : "ct_#{klass.name}" + } + end + + instance_a = @model_class.new.tap { |i| i.name = "tenant_a" } + instance_b = @model_class.new.tap { |i| i.name = "tenant_b" } + + lock_a = @model_class._ct.advisory_lock_name(instance_a) + lock_b = @model_class._ct.advisory_lock_name(instance_b) + lock_global = @model_class._ct.advisory_lock_name(nil) + + assert_not_equal lock_a, lock_b, "Different tenants must not share a lock" + assert_not_equal lock_a, lock_global, "Instance lock must differ from class-level lock" + assert_equal lock_a, @model_class._ct.advisory_lock_name(@model_class.new.tap { |i| i.name = "tenant_a" }) + end + def test_symbol_advisory_lock_name with_temporary_model do has_closure_tree advisory_lock_name: :custom_lock_method