From 3f543cd2c93d987723d51d629000b5550eb59636 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Wed, 19 Jun 2019 11:18:04 +0200
Subject: Fix N+1 problem in `JobsController#index`

This adds missing preloads, and introduces
additional n+1 matcher to look for
duplicates.
---
 spec/controllers/projects/jobs_controller_spec.rb | 14 +++--
 spec/support/helpers/query_recorder.rb            |  4 ++
 spec/support/matchers/be_n_plus_1_query.rb        | 62 +++++++++++++++++++++++
 3 files changed, 76 insertions(+), 4 deletions(-)
 create mode 100644 spec/support/matchers/be_n_plus_1_query.rb

(limited to 'spec')

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
-- 
cgit v1.2.1