Merge branch '29492-useless-queries' into 'master'

remove useless queries with false conditions (e.g 1=0)

Closes #29492

See merge request !10141

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 =

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

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

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

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

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