Skip to content

Commit a7a554c

Browse files
seuroszakky21
andauthored
feat: allow advisory_lock_name proc to receive instance for per-tenant lock granularity (#491)
When advisory_lock_name is a 2-arity proc, it receives (base_class, instance) so callers can derive per-instance lock names (e.g. by tenant/scope column). 1-arity procs continue to receive only base_class - fully backward compatible. Instance-method call sites pass self; class-method sites pass nil, falling back to the model-wide lock name as before. Co-authored-by: Masanori OKAZAKI <masanori-okazaki@freee.co.jp>
1 parent 66bde3e commit a7a554c

7 files changed

Lines changed: 78 additions & 19 deletions

File tree

README.md

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -560,24 +560,44 @@ class Tag < ApplicationRecord
560560
has_closure_tree advisory_lock_name: 'custom_tag_lock'
561561
end
562562

563-
# Dynamic via Proc
563+
# Dynamic via Proc (1-arity: receives model class only)
564564
class Tag < ApplicationRecord
565565
has_closure_tree advisory_lock_name: ->(model_class) { "#{Rails.env}_#{model_class.name.underscore}" }
566566
end
567567

568568
# Delegate to model method
569569
class Tag < ApplicationRecord
570570
has_closure_tree advisory_lock_name: :custom_lock_name
571-
571+
572572
def self.custom_lock_name
573573
"tag_lock_#{current_tenant_id}"
574574
end
575575
end
576576
```
577577

578+
#### Per-instance lock names (multi-tenant / scoped models)
579+
580+
Pass a 2-arity proc to receive both the model class and the current record instance.
581+
This is the recommended approach for scoped models where each tenant should have its own lock,
582+
avoiding unnecessary serialization across tenants.
583+
584+
```ruby
585+
class Node < ApplicationRecord
586+
has_closure_tree scope: :company_id,
587+
advisory_lock_name: ->(klass, instance) {
588+
company = instance&.company_id
589+
company ? "ct_#{klass.name}_#{company}" : "ct_#{klass.name}"
590+
}
591+
end
592+
```
593+
594+
When `instance` is `nil` (class-level operations like `Node.rebuild!`), the proc should
595+
fall back to a model-wide name. Instance-level operations (`save`, `destroy`, `add_sibling`,
596+
`find_or_create_by_path`) pass the record itself so the lock is scoped to that tenant.
597+
578598
This is particularly useful when:
579599
* You need environment-specific lock names
580-
* You're using multi-tenancy and need tenant-specific locks
600+
* You're using multi-tenancy and need per-tenant locks (avoiding cross-tenant contention)
581601
* You want to avoid lock name collisions between similar model names
582602

583603
## Multi-Database Support

lib/closure_tree/finders.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def find_or_create_by_path(path, attributes = {})
2020
return found if found
2121

2222
attrs = subpath.shift
23-
_ct.with_advisory_lock do
23+
_ct.with_advisory_lock(self) do
2424
# shenanigans because children.create is bound to the superclass
2525
# (in the case of polymorphism):
2626
child = children.where(attrs).first || begin

lib/closure_tree/hierarchy_maintenance.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def _ct_after_save
6464
end
6565

6666
def _ct_before_destroy
67-
_ct.with_advisory_lock do
67+
_ct.with_advisory_lock(self) do
6868
_ct_adopt_children_to_grandparent if _ct.options[:dependent] == :adopt
6969
delete_hierarchy_references
7070
self.class.find(id).children.find_each(&:rebuild!) if _ct.options[:dependent] == :nullify
@@ -86,7 +86,7 @@ def _ct_before_destroy
8686
end
8787

8888
def rebuild!(called_by_rebuild = false)
89-
_ct.with_advisory_lock do
89+
_ct.with_advisory_lock(self) do
9090
delete_hierarchy_references unless (defined? @was_new_record) && @was_new_record
9191
hierarchy_class.create!(ancestor: self, descendant: self, generations: 0)
9292
unless root?
@@ -112,7 +112,7 @@ def rebuild!(called_by_rebuild = false)
112112
end
113113

114114
def delete_hierarchy_references
115-
_ct.with_advisory_lock do
115+
_ct.with_advisory_lock(self) do
116116
# The crazy double-wrapped sub-subselect works around MySQL's limitation of subselects on the same table that is being mutated.
117117
# It shouldn't affect performance of postgresql.
118118
# See http://dev.mysql.com/doc/refman/5.0/en/subquery-errors.html

lib/closure_tree/numeric_deterministic_ordering.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def add_sibling(sibling, add_after = true)
162162
# Make sure self isn't dirty, because we're going to call reload:
163163
save
164164

165-
_ct.with_advisory_lock do
165+
_ct.with_advisory_lock(self) do
166166
prior_sibling_parent = sibling.parent
167167

168168
sibling.order_value = order_value

lib/closure_tree/support.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,10 @@ def build_scope_where_clause(scope_conditions)
157157
" AND #{conditions.join(' AND ')}"
158158
end
159159

160-
def with_advisory_lock(&block)
160+
def with_advisory_lock(instance = nil, &block)
161161
lock_method = options[:advisory_lock_timeout_seconds].present? ? :with_advisory_lock! : :with_advisory_lock
162162
if options[:with_advisory_lock] && connection.supports_advisory_locks? && model_class.respond_to?(lock_method)
163-
model_class.public_send(lock_method, advisory_lock_name, advisory_lock_options) do
163+
model_class.public_send(lock_method, advisory_lock_name(instance), advisory_lock_options) do
164164
transaction(&block)
165165
end
166166
else

lib/closure_tree/support_attributes.rb

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,22 @@ module SupportAttributes
88
extend Forwardable
99
def_delegators :model_class, :connection, :transaction, :table_name, :base_class, :inheritance_column, :column_names
1010

11-
def advisory_lock_name
12-
# Allow customization via options or instance method
11+
def advisory_lock_name(instance = nil)
1312
if options[:advisory_lock_name]
1413
case options[:advisory_lock_name]
1514
when Proc
16-
# Allow dynamic generation via proc
17-
options[:advisory_lock_name].call(base_class)
15+
proc = options[:advisory_lock_name]
16+
proc.arity == 1 ? proc.call(base_class) : proc.call(base_class, instance)
1817
when Symbol
19-
# Allow delegation to a model method
2018
if model_class.respond_to?(options[:advisory_lock_name])
2119
model_class.send(options[:advisory_lock_name])
2220
else
2321
raise ArgumentError, "Model #{model_class} does not respond to #{options[:advisory_lock_name]}"
2422
end
2523
else
26-
# Use static string value
2724
options[:advisory_lock_name].to_s
2825
end
2926
else
30-
# Default: Use CRC32 for a shorter, consistent hash
31-
# This gives us 8 hex characters which is plenty for uniqueness
32-
# and leaves room for prefixes
3327
"ct_#{Zlib.crc32(base_class.name.to_s).to_s(16)}"
3428
end
3529
end

test/closure_tree/advisory_lock_test.rb

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,51 @@ def test_proc_advisory_lock_name
3333
assert_equal "lock_for_#{@model_class.name.underscore}", instance._ct.advisory_lock_name
3434
end
3535

36+
def test_proc_advisory_lock_name_with_instance_arity
37+
with_temporary_model do
38+
has_closure_tree advisory_lock_name: ->(klass, instance) {
39+
tenant = instance&.name
40+
tenant ? "#{klass.name.underscore}_#{tenant}" : "#{klass.name.underscore}_global"
41+
}
42+
end
43+
44+
instance = @model_class.new
45+
instance.name = "acme"
46+
47+
assert_equal "#{@model_class.name.underscore}_global", @model_class._ct.advisory_lock_name(nil)
48+
assert_equal "#{@model_class.name.underscore}_acme", @model_class._ct.advisory_lock_name(instance)
49+
end
50+
51+
def test_proc_arity_1_backward_compat
52+
with_temporary_model do
53+
has_closure_tree advisory_lock_name: ->(model) { "compat_#{model.name}" }
54+
end
55+
56+
instance = @model_class.new
57+
assert_equal "compat_#{@model_class.name}", @model_class._ct.advisory_lock_name(instance)
58+
assert_equal "compat_#{@model_class.name}", @model_class._ct.advisory_lock_name(nil)
59+
end
60+
61+
def test_different_instances_produce_different_lock_names
62+
with_temporary_model do
63+
has_closure_tree advisory_lock_name: ->(klass, instance) {
64+
tenant = instance&.name
65+
tenant ? "ct_#{klass.name}_#{tenant}" : "ct_#{klass.name}"
66+
}
67+
end
68+
69+
instance_a = @model_class.new.tap { |i| i.name = "tenant_a" }
70+
instance_b = @model_class.new.tap { |i| i.name = "tenant_b" }
71+
72+
lock_a = @model_class._ct.advisory_lock_name(instance_a)
73+
lock_b = @model_class._ct.advisory_lock_name(instance_b)
74+
lock_global = @model_class._ct.advisory_lock_name(nil)
75+
76+
assert_not_equal lock_a, lock_b, "Different tenants must not share a lock"
77+
assert_not_equal lock_a, lock_global, "Instance lock must differ from class-level lock"
78+
assert_equal lock_a, @model_class._ct.advisory_lock_name(@model_class.new.tap { |i| i.name = "tenant_a" })
79+
end
80+
3681
def test_symbol_advisory_lock_name
3782
with_temporary_model do
3883
has_closure_tree advisory_lock_name: :custom_lock_method

0 commit comments

Comments
 (0)