From 1bf4fece121298260663c6ca73d39716d3548a03 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 10 Apr 2020 00:10:04 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- spec/models/concerns/where_composite_spec.rb | 184 +++++++++++++++++++++ spec/models/issue_spec.rb | 16 ++ spec/support/matchers/exceed_query_limit.rb | 36 ++++ .../concerns/composite_id_shared_examples.rb | 47 ++++++ 4 files changed, 283 insertions(+) create mode 100644 spec/models/concerns/where_composite_spec.rb create mode 100644 spec/support/shared_examples/models/concerns/composite_id_shared_examples.rb (limited to 'spec') diff --git a/spec/models/concerns/where_composite_spec.rb b/spec/models/concerns/where_composite_spec.rb new file mode 100644 index 00000000000..1c0951d90d0 --- /dev/null +++ b/spec/models/concerns/where_composite_spec.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe WhereComposite do + describe '.where_composite' do + let_it_be(:test_table_name) { "test_table_#{SecureRandom.hex(10)}" } + + let(:model) do + tbl_name = test_table_name + Class.new(ActiveRecord::Base) do + self.table_name = tbl_name + + include WhereComposite + end + end + + def connection + ActiveRecord::Base.connection + end + + before_all do + connection.create_table(test_table_name) do |t| + t.integer :foo + t.integer :bar + t.string :wibble + end + end + + it 'requires at least one permitted key' do + expect { model.where_composite([], nil) } + .to raise_error(ArgumentError) + end + + it 'requires all arguments to match the permitted_keys' do + expect { model.where_composite([:foo], [{ foo: 1 }, { bar: 2 }]) } + .to raise_error(ArgumentError) + end + + it 'attaches a key error as cause if a key is missing' do + expect { model.where_composite([:foo], [{ foo: 1 }, { bar: 2 }]) } + .to raise_error(have_attributes(cause: KeyError)) + end + + it 'returns an empty relation if there are no arguments' do + expect(model.where_composite([:foo, :bar], nil)) + .to be_empty + + expect(model.where_composite([:foo, :bar], [])) + .to be_empty + end + + it 'permits extra arguments' do + a = model.where_composite([:foo, :bar], { foo: 1, bar: 2 }) + b = model.where_composite([:foo, :bar], { foo: 1, bar: 2, baz: 3 }) + + expect(a.to_sql).to eq(b.to_sql) + end + + it 'can handle multiple fields' do + fields = [:foo, :bar, :wibble] + args = { foo: 1, bar: 2, wibble: 'wobble' } + pattern = %r{ + WHERE \s+ + \(? + \s* "#{test_table_name}"\."foo" \s* = \s* 1 + \s+ AND + \s+ "#{test_table_name}"\."bar" \s* = \s* 2 + \s+ AND + \s+ "#{test_table_name}"\."wibble" \s* = \s* 'wobble' + \s* + \)? + }x + + expect(model.where_composite(fields, args).to_sql).to match(pattern) + end + + it 'is equivalent to ids.map { |attrs| model.find_by(attrs) }' do + 10.times do |i| + 10.times do |j| + model.create!(foo: i, bar: j, wibble: generate(:filename)) + end + end + + ids = [{ foo: 1, bar: 9 }, { foo: 9, bar: 1 }] + + found = model.where_composite(%i[foo bar], ids) + + expect(found).to match_array(ids.map { |attrs| model.find_by!(attrs) }) + end + + it 'constructs (A&B) for one argument' do + fields = [:foo, :bar] + args = [ + { foo: 1, bar: 2 } + ] + pattern = %r{ + WHERE \s+ + \(? + \s* "#{test_table_name}"\."foo" \s* = \s* 1 + \s+ AND + \s+ "#{test_table_name}"\."bar" \s* = \s* 2 + \s* + \)? + }x + + expect(model.where_composite(fields, args).to_sql).to match(pattern) + expect(model.where_composite(fields, args[0]).to_sql).to match(pattern) + end + + it 'constructs (A&B) OR (C&D) for two arguments' do + args = [ + { foo: 1, bar: 2 }, + { foo: 3, bar: 4 } + ] + pattern = %r{ + WHERE \s+ + \( \s* "#{test_table_name}"\."foo" \s* = \s* 1 + \s+ AND + \s+ "#{test_table_name}"\."bar" \s* = \s* 2 + \s* \)? + \s* OR \s* + \(? \s* "#{test_table_name}"\."foo" \s* = \s* 3 + \s+ AND + \s+ "#{test_table_name}"\."bar" \s* = \s* 4 + \s* \) + }x + + q = model.where_composite([:foo, :bar], args) + + expect(q.to_sql).to match(pattern) + end + + it 'constructs (A&B) OR (C&D) OR (E&F) for three arguments' do + args = [ + { foo: 1, bar: 2 }, + { foo: 3, bar: 4 }, + { foo: 5, bar: 6 } + ] + pattern = %r{ + WHERE \s+ + \({2} + \s* "#{test_table_name}"\."foo" \s* = \s* 1 + \s+ AND + \s+ "#{test_table_name}"\."bar" \s* = \s* 2 + \s* \)? + \s* OR \s* + \(? \s* "#{test_table_name}"\."foo" \s* = \s* 3 + \s+ AND + \s+ "#{test_table_name}"\."bar" \s* = \s* 4 + \s* \)? + \s* OR \s* + \(? \s* "#{test_table_name}"\."foo" \s* = \s* 5 + \s+ AND + \s+ "#{test_table_name}"\."bar" \s* = \s* 6 + \s* \) + }x + + q = model.where_composite([:foo, :bar], args) + + expect(q.to_sql).to match(pattern) + end + + describe 'large sets of IDs' do + def query(size) + args = (0..).lazy.take(size).map { |n| { foo: n, bar: n * n, wibble: 'x' * n } }.to_a + model.where_composite([:foo, :bar, :wibble], args) + end + + it 'constructs correct trees of constraints' do + n = described_class::TooManyIds::LIMIT + q = query(n) + sql = q.to_sql + + expect(sql.scan(/OR/).count).to eq(n - 1) + expect(sql.scan(/AND/).count).to eq(2 * n) + end + + it 'raises errors if too many IDs are passed' do + expect { query(described_class::TooManyIds::LIMIT + 1) }.to raise_error(described_class::TooManyIds) + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 9797e0a0472..e8103be0682 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -900,6 +900,22 @@ describe Issue do end end + describe '.by_project_id_and_iid' do + let_it_be(:issue_a) { create(:issue) } + let_it_be(:issue_b) { create(:issue, iid: issue_a.iid) } + let_it_be(:issue_c) { create(:issue, project: issue_a.project) } + let_it_be(:issue_d) { create(:issue, project: issue_a.project) } + + it_behaves_like 'a where_composite scope', :by_project_id_and_iid do + let(:all_results) { [issue_a, issue_b, issue_c, issue_d] } + let(:first_result) { issue_a } + + let(:composite_ids) do + all_results.map { |issue| { project_id: issue.project_id, iid: issue.iid } } + end + end + end + it_behaves_like 'throttled touch' do subject { create(:issue, updated_at: 1.hour.ago) } end diff --git a/spec/support/matchers/exceed_query_limit.rb b/spec/support/matchers/exceed_query_limit.rb index f38ae44a577..b9630b00038 100644 --- a/spec/support/matchers/exceed_query_limit.rb +++ b/spec/support/matchers/exceed_query_limit.rb @@ -73,6 +73,42 @@ module ExceedQueryLimitHelpers end end +RSpec::Matchers.define :issue_same_number_of_queries_as do + supports_block_expectations + + include ExceedQueryLimitHelpers + + def control + block_arg + end + + def control_recorder + @control_recorder ||= ActiveRecord::QueryRecorder.new(&control) + end + + def expected_count + @expected_count ||= control_recorder.count + end + + def verify_count(&block) + @subject_block = block + + (expected_count - actual_count).abs <= threshold + end + + match do |block| + verify_count(&block) + end + + failure_message_when_negated do |actual| + failure_message + end + + def skip_cached + false + end +end + RSpec::Matchers.define :exceed_all_query_limit do |expected| supports_block_expectations diff --git a/spec/support/shared_examples/models/concerns/composite_id_shared_examples.rb b/spec/support/shared_examples/models/concerns/composite_id_shared_examples.rb new file mode 100644 index 00000000000..cd925659311 --- /dev/null +++ b/spec/support/shared_examples/models/concerns/composite_id_shared_examples.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a where_composite scope' do |scope_name| + let(:result) { described_class.public_send(scope_name, ids) } + + context 'we pass an empty array' do + let(:ids) { [] } + + it 'returns a null relation' do + expect(result).to be_empty + end + end + + context 'we pass nil' do + let(:ids) { nil } + + it 'returns a null relation' do + expect(result).to be_empty + end + end + + context 'we pass a singleton composite id' do + let(:ids) { composite_ids.first } + + it 'finds the first result' do + expect(result).to contain_exactly(first_result) + end + end + + context 'we pass group of ids' do + let(:ids) { composite_ids } + + it 'finds all the results' do + expect(result).to contain_exactly(*all_results) + end + end + + describe 'performance' do + it 'is not O(N)' do + all_ids = composite_ids + one_id = composite_ids.first + + expect { described_class.public_send(scope_name, all_ids) } + .to issue_same_number_of_queries_as { described_class.public_send(scope_name, one_id) } + end + end +end -- cgit v1.2.1