Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/closure_tree/finders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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! do
# shenanigans because children.create is bound to the superclass
# (in the case of polymorphism):
child = self.children.where(attrs).first || begin
Expand Down Expand Up @@ -159,7 +159,7 @@ def find_or_create_by_path(path, attributes = {})
attr_path = _ct.build_ancestry_attr_path(path, attributes)
find_by_path(attr_path) || begin
root_attrs = attr_path.shift
_ct.with_advisory_lock do
_ct.with_advisory_lock! do
# shenanigans because find_or_create can't infer that we want the same class as this:
# Note that roots will already be constrained to this subclass (in the case of polymorphism):
root = roots.where(root_attrs).first || _ct.create!(self, root_attrs)
Expand Down
1 change: 1 addition & 0 deletions lib/closure_tree/has_closure_tree.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def has_closure_tree(options = {})
:numeric_order,
:touch,
:with_advisory_lock,
:advisory_lock_timeout_seconds,
:order_belong_to
)

Expand Down
8 changes: 4 additions & 4 deletions lib/closure_tree/hierarchy_maintenance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def _ct_after_save
end

def _ct_before_destroy
_ct.with_advisory_lock do
_ct.with_advisory_lock! do
delete_hierarchy_references
if _ct.options[:dependent] == :nullify
self.class.find(self.id).children.find_each { |c| c.rebuild! }
Expand All @@ -63,7 +63,7 @@ def _ct_before_destroy
end

def rebuild!(called_by_rebuild = false)
_ct.with_advisory_lock do
_ct.with_advisory_lock! do
delete_hierarchy_references unless (defined? @was_new_record) && @was_new_record
hierarchy_class.create!(:ancestor => self, :descendant => self, :generations => 0)
unless root?
Expand All @@ -89,7 +89,7 @@ def rebuild!(called_by_rebuild = false)
end

def delete_hierarchy_references
_ct.with_advisory_lock do
_ct.with_advisory_lock! 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
Expand All @@ -111,7 +111,7 @@ module ClassMethods
# Rebuilds the hierarchy table based on the parent_id column in the database.
# Note that the hierarchy table will be truncated.
def rebuild!
_ct.with_advisory_lock do
_ct.with_advisory_lock! do
cleanup!
roots.find_each { |n| n.send(:rebuild!) } # roots just uses the parent_id column, so this is safe.
end
Expand Down
2 changes: 1 addition & 1 deletion lib/closure_tree/numeric_deterministic_ordering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,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! do
prior_sibling_parent = sibling.parent
reorder_from_value = if prior_sibling_parent == self.parent
[self.order_value, sibling.order_value].compact.min
Expand Down
8 changes: 6 additions & 2 deletions lib/closure_tree/support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +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,
: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?
raise ArgumentError, "advisory_lock_timeout_seconds cannot be provided when advisory lock is disabled"
end
if order_is_numeric?
extend NumericOrderSupport.adapter_for_connection(connection)
end
Expand Down Expand Up @@ -108,9 +112,9 @@ def where_eq(column_name, value)
end
end

def with_advisory_lock(&block)
def with_advisory_lock!(&block)
if options[:with_advisory_lock]
model_class.with_advisory_lock(advisory_lock_name) do
model_class.with_advisory_lock!(advisory_lock_name, advisory_lock_options) do
transaction { yield }
end
else
Expand Down
4 changes: 4 additions & 0 deletions lib/closure_tree/support_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ def advisory_lock_name
Digest::SHA1.hexdigest("ClosureTree::#{base_class.name}")[0..32]
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
Expand Down
2 changes: 1 addition & 1 deletion spec/closure_tree/parallel_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def run_workers(worker_class = FindOrCreateWorker)

it 'creates dupe roots without advisory locks' do
# disable with_advisory_lock:
allow(Tag).to receive(:with_advisory_lock) { |_lock_name, &block| block.call }
allow(Tag).to receive(:with_advisory_lock!) { |_lock_name, &block| block.call }
run_workers
# duplication from at least one iteration:
expect(Tag.where(name: @names).size).to be > @iterations
Expand Down
28 changes: 26 additions & 2 deletions spec/closure_tree/support_spec.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,38 @@
require 'spec_helper'

RSpec.describe ClosureTree::Support do
let(:sut) { Tag._ct }
let(:model) { Tag }
let(:options) { {} }
let(:sut) { described_class.new(model, options) }

it 'passes through table names without prefix and suffix' do
expected = 'some_random_table_name'
expect(sut.remove_prefix_and_suffix(expected)).to eq(expected)
end

it 'extracts through table name with prefix and suffix' do
expected = 'some_random_table_name'
tn = ActiveRecord::Base.table_name_prefix + expected + ActiveRecord::Base.table_name_suffix
expect(sut.remove_prefix_and_suffix(tn)).to eq(expected)
end
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
end
end
Loading