diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-04-04 16:33:42 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-04-04 16:33:42 +0300 |
commit | 796d65f1feeb13c5bbdedee3f7290bd962c4c421 (patch) | |
tree | fee4fcfd24818c3bef0bde6faf1b6c1b51aeb887 | |
parent | 7f8f927aafb0f3a618ccce76e802e30a16b04b42 (diff) | |
parent | e52b1df1d5391f0fa0b9ec7eb2d3492b05f64ba4 (diff) |
Merge branch '29492-useless-queries' into 'master'
remove useless queries with false conditions (e.g 1=0)
Closes #29492
See merge request !10141
-rw-r--r-- | app/controllers/concerns/issuable_collections.rb | 3 | ||||
-rw-r--r-- | app/workers/process_commit_worker.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/29492-useless-queries.yml | 4 | ||||
-rw-r--r-- | spec/support/issuables_list_metadata_shared_examples.rb | 15 | ||||
-rw-r--r-- | spec/support/matchers/query_matcher.rb | 33 | ||||
-rw-r--r-- | spec/workers/process_commit_worker_spec.rb | 7 |
6 files changed, 64 insertions, 0 deletions
diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 85ae4985e58..c8a501d7319 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -15,6 +15,9 @@ module IssuableCollections # a new order into the collection. # We cannot use reorder to not mess up the paginated collection. issuable_ids = issuable_collection.map(&:id) + + return {} if issuable_ids.empty? + issuable_note_count = Note.count_for_collection(issuable_ids, @collection_type) issuable_votes_count = AwardEmoji.votes_for_collection(issuable_ids, @collection_type) issuable_merge_requests_count = diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb index e9a5bd7f24e..2f7967cf531 100644 --- a/app/workers/process_commit_worker.rb +++ b/app/workers/process_commit_worker.rb @@ -53,6 +53,8 @@ class ProcessCommitWorker def update_issue_metrics(commit, author) mentioned_issues = commit.all_references(author).issues + return if mentioned_issues.empty? + Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil). update_all(first_mentioned_in_commit_at: commit.committed_date) end diff --git a/changelogs/unreleased/29492-useless-queries.yml b/changelogs/unreleased/29492-useless-queries.yml new file mode 100644 index 00000000000..266a04be352 --- /dev/null +++ b/changelogs/unreleased/29492-useless-queries.yml @@ -0,0 +1,4 @@ +--- +title: Remove useless queries with false conditions (e.g 1=0) +merge_request: 10141 +author: mhasbini diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb index 7ea4073ef2b..3406e4c3161 100644 --- a/spec/support/issuables_list_metadata_shared_examples.rb +++ b/spec/support/issuables_list_metadata_shared_examples.rb @@ -33,4 +33,19 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| expect(meta_data[id].upvotes).to eq(id + 2) end end + + describe "when given empty collection" do + let(:project2) { create(:empty_project, :public) } + + it "doesn't execute any queries with false conditions" do + get_action = + if action + proc { get action } + else + proc { get :index, namespace_id: project2.namespace, project_id: project2 } + end + + expect(&get_action).not_to make_queries_matching(/WHERE (?:1=0|0=1)/) + end + end end diff --git a/spec/support/matchers/query_matcher.rb b/spec/support/matchers/query_matcher.rb new file mode 100644 index 00000000000..ac8c4ab91d9 --- /dev/null +++ b/spec/support/matchers/query_matcher.rb @@ -0,0 +1,33 @@ +RSpec::Matchers.define :make_queries_matching do |matcher, expected_count = nil| + supports_block_expectations + + match do |block| + @counter = query_count(matcher, &block) + if expected_count + @counter.count == expected_count + else + @counter.count > 0 + end + end + + failure_message_when_negated do |_| + if expected_count + "expected #{matcher} not to match #{expected_count} queries, got #{@counter.count} matches:\n\n#{@counter.inspect}" + else + "expected #{matcher} not to match any query, got #{@counter.count} matches:\n\n#{@counter.inspect}" + end + end + + failure_message do |_| + if expected_count + "expected #{matcher} to match #{expected_count} queries, got #{@counter.count} matches:\n\n#{@counter.inspect}" + else + "expected #{matcher} to match at least one query, got #{@counter.count} matches:\n\n#{@counter.inspect}" + end + end + + def query_count(regex, &block) + @recorder = ActiveRecord::QueryRecorder.new(&block).log + @recorder.select{ |q| q.match(regex) } + end +end diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb index 1c383d0514d..9afe2e610b9 100644 --- a/spec/workers/process_commit_worker_spec.rb +++ b/spec/workers/process_commit_worker_spec.rb @@ -99,6 +99,13 @@ describe ProcessCommitWorker do expect(metric.first_mentioned_in_commit_at).to eq(commit.committed_date) end + + it "doesn't execute any queries with false conditions" do + allow(commit).to receive(:safe_message). + and_return("Lorem Ipsum") + + expect { worker.update_issue_metrics(commit, user) }.not_to make_queries_matching(/WHERE (?:1=0|0=1)/) + end end describe '#build_commit' do |