diff options
author | Jan Provaznik <jprovaznik@gitlab.com> | 2019-07-19 10:16:41 +0200 |
---|---|---|
committer | Heinrich Lee Yu <heinrich@gitlab.com> | 2019-08-05 17:49:04 +0800 |
commit | afbe0b616b1e17f88bacc283a7a8fbe0bece580a (patch) | |
tree | 69c7da09a7d2e36e5f1fb7f73421b4a736c22356 /spec | |
parent | d126df55fde21d2dc8eb9d5f72841a9792bca105 (diff) | |
download | gitlab-ce-afbe0b616b1e17f88bacc283a7a8fbe0bece580a.tar.gz |
Optimize rebalancing of relative positioning
Moving of neighbour items was done recursively - this
was extremely expensive when multiple items had to be moved.
This change optimizes the code to find nearest possible gap where
items can be moved and moves all of them with single update query.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/support/shared_examples/relative_positioning_shared_examples.rb | 76 |
1 files changed, 76 insertions, 0 deletions
diff --git a/spec/support/shared_examples/relative_positioning_shared_examples.rb b/spec/support/shared_examples/relative_positioning_shared_examples.rb index 1c53e2602eb..417ef4f315b 100644 --- a/spec/support/shared_examples/relative_positioning_shared_examples.rb +++ b/spec/support/shared_examples/relative_positioning_shared_examples.rb @@ -9,6 +9,12 @@ RSpec.shared_examples 'a class that supports relative positioning' do create(factory, params.merge(default_params)) end + def create_items_with_positions(positions) + positions.map do |position| + create_item(relative_position: position) + end + end + describe '.move_nulls_to_end' do it 'moves items with null relative_position to the end' do skip("#{item1} has a default relative position") if item1.relative_position @@ -257,5 +263,75 @@ RSpec.shared_examples 'a class that supports relative positioning' do expect(new_item.relative_position).to be(100) end + + it 'avoids N+1 queries when rebalancing other items' do + items = create_items_with_positions([100, 101, 102]) + + count = ActiveRecord::QueryRecorder.new do + new_item.move_between(items[-2], items[-1]) + end + + items = create_items_with_positions([150, 151, 152, 153, 154]) + + expect { new_item.move_between(items[-2], items[-1]) }.not_to exceed_query_limit(count) + end + end + + describe '#move_sequence_before' do + it 'moves the whole sequence of items to the middle of the nearest gap' do + items = create_items_with_positions([90, 100, 101, 102]) + + items.last.move_sequence_before + items.last.save! + + positions = items.map { |item| item.reload.relative_position } + expect(positions).to eq([90, 95, 96, 102]) + end + + it 'finds a gap if there are unused positions' do + items = create_items_with_positions([100, 101, 102]) + + items.last.move_sequence_before + items.last.save! + + positions = items.map { |item| item.reload.relative_position } + expect(positions).to eq([50, 51, 102]) + end + + it 'if raises an exception if gap is not found' do + stub_const('RelativePositioning::MAX_SEQUENCE_LIMIT', 2) + items = create_items_with_positions([100, 101, 102]) + + expect { items.last.move_sequence_before }.to raise_error(RelativePositioning::GapNotFound) + end + end + + describe '#move_sequence_after' do + it 'moves the whole sequence of items to the middle of the nearest gap' do + items = create_items_with_positions([100, 101, 102, 110]) + + items.first.move_sequence_after + items.first.save! + + positions = items.map { |item| item.reload.relative_position } + expect(positions).to eq([100, 105, 106, 110]) + end + + it 'finds a gap if there are unused positions' do + items = create_items_with_positions([100, 101, 102]) + + items.first.move_sequence_after + items.first.save! + + positions = items.map { |item| item.reload.relative_position } + expect(positions).to eq([100, 601, 602]) + end + + it 'if raises an exception if gap is not found' do + stub_const('RelativePositioning::MAX_SEQUENCE_LIMIT', 2) + items = create_items_with_positions([100, 101, 102]) + + expect { items.first.move_sequence_after }.to raise_error(RelativePositioning::GapNotFound) + end end end |