Fix N+1 problem in `JobsController#index`fix-jobs-controller-index-n-1

This adds missing preloads, and introduces
additional n+1 matcher to look for
duplicates.

4 files changed, 81 insertions, 5 deletions

diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index 2a4933e7bc2..d7c0039b234 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -31,8 +31,12 @@ class Projects::JobsController < Projects::ApplicationController
         @builds
       end
     @builds = @builds.includes([
-      { pipeline: :project },
+      { pipeline: [:project, :user] },
+      :job_artifacts_archive,
+      :metadata,
+      :trigger_request,
       :project,
+      :user,
       :tags
     ])
     @builds = @builds.page(params[:page]).per(30).without_count
diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb
index 0dabe27977a..901402aa5fd 100644
--- a/spec/controllers/projects/jobs_controller_spec.rb
+++ b/spec/controllers/projects/jobs_controller_spec.rb
@@ -73,21 +73,27 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
     end
 
     context 'number of queries' do
+      render_views
+
       before do
         Ci::Build::AVAILABLE_STATUSES.each do |status|
           create_job(status, status)
         end
+
+        allow(Appearance).to receive(:current_without_cache)
+          .and_return(nil)
       end
 
       it 'verifies number of queries', :request_store do
-        recorded = ActiveRecord::QueryRecorder.new { get_index }
-        expect(recorded.count).to be_within(5).of(7)
+        expect { get_index }.not_to be_n_plus_1_query.with_threshold(3)
       end
 
       def create_job(name, status)
-        pipeline = create(:ci_pipeline, project: project)
+        user = create(:user)
+        pipeline = create(:ci_pipeline, project: project, user: user)
         create(:ci_build, :tags, :triggered, :artifacts,
-               pipeline: pipeline, name: name, status: status)
+               pipeline: pipeline, name: name, status: status,
+               user: user)
       end
     end
 
diff --git a/spec/support/helpers/query_recorder.rb b/spec/support/helpers/query_recorder.rb
index d45377267f3..f77b43391dd 100644
--- a/spec/support/helpers/query_recorder.rb
+++ b/spec/support/helpers/query_recorder.rb
@@ -35,5 +35,9 @@ module ActiveRecord
     def log_message
       @log.join("\n\n")
     end
+
+    def occurrences
+      @log.group_by(&:to_s).transform_values(&:count)
+    end
   end
 end
diff --git a/spec/support/matchers/be_n_plus_1_query.rb b/spec/support/matchers/be_n_plus_1_query.rb
new file mode 100644
index 00000000000..bbfd1897f04
--- /dev/null
+++ b/spec/support/matchers/be_n_plus_1_query.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module Nplus1QueryHelpers
+  DEFAULT_THRESHOLD = 3
+
+  def with_threshold(threshold)
+    @threshold = threshold
+    self
+  end
+
+  def for_query(query)
+    @query = query
+    self
+  end
+
+  def threshold
+    @threshold || DEFAULT_THRESHOLD
+  end
+
+  def occurrences
+    @occurrences ||=
+      if @query
+        recorder.occurrences.select { |recorded, count| recorded =~ @query }
+      else
+        recorder.occurrences
+      end
+  end
+
+  def over_threshold
+    occurrences.select do |recorded, count|
+      count > threshold
+    end
+  end
+
+  def recorder
+    @recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block)
+  end
+
+  def verify_count(&block)
+    @subject_block = block
+    over_threshold.present?
+  end
+
+  def failure_message
+    log_message = over_threshold.to_yaml
+    "The following queries are executed more than #{threshold} time(s):\n#{log_message}"
+  end
+end
+
+RSpec::Matchers.define :be_n_plus_1_query do
+  supports_block_expectations
+
+  include Nplus1QueryHelpers
+
+  match do |block|
+    verify_count(&block)
+  end
+
+  failure_message_when_negated do |actual|
+    failure_message
+  end
+end