From cbc20d2b7f8c73e2892c0c458619df2a9fe0c9ab Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sun, 3 Jun 2018 03:31:41 -0700
Subject: Remove N+1 query for author in issues API

This was being masked by the statement cache because only one author was used
per issue in the test..

Also adds support for an Rspec matcher `exceed_all_query_limit`.
---
 spec/requests/api/issues_spec.rb            |  8 ++--
 spec/support/helpers/query_recorder.rb      |  7 ++--
 spec/support/matchers/exceed_query_limit.rb | 65 ++++++++++++++++++++++-------
 3 files changed, 59 insertions(+), 21 deletions(-)

(limited to 'spec')

diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 4181f4ebbbe..a15d60aafe0 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -630,15 +630,17 @@ describe API::Issues do
     end
 
     it 'avoids N+1 queries' do
-      control_count = ActiveRecord::QueryRecorder.new do
+      get api("/projects/#{project.id}/issues", user)
+
+      control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
         get api("/projects/#{project.id}/issues", user)
       end.count
 
-      create(:issue, author: user, project: project)
+      create_list(:issue, 3, project: project)
 
       expect do
         get api("/projects/#{project.id}/issues", user)
-      end.not_to exceed_query_limit(control_count)
+      end.not_to exceed_all_query_limit(control_count)
     end
 
     it 'returns 404 when project does not exist' do
diff --git a/spec/support/helpers/query_recorder.rb b/spec/support/helpers/query_recorder.rb
index 28536bbef5e..7ce63375d34 100644
--- a/spec/support/helpers/query_recorder.rb
+++ b/spec/support/helpers/query_recorder.rb
@@ -1,10 +1,11 @@
 module ActiveRecord
   class QueryRecorder
-    attr_reader :log, :cached
+    attr_reader :log, :skip_cached, :cached
 
-    def initialize(&block)
+    def initialize(skip_cached: true, &block)
       @log = []
       @cached = []
+      @skip_cached = skip_cached
       ActiveSupport::Notifications.subscribed(method(:callback), 'sql.active_record', &block)
     end
 
@@ -16,7 +17,7 @@ module ActiveRecord
     def callback(name, start, finish, message_id, values)
       show_backtrace(values) if ENV['QUERY_RECORDER_DEBUG']
 
-      if values[:name]&.include?("CACHE")
+      if values[:name]&.include?("CACHE") && skip_cached
         @cached << values[:sql]
       elsif !values[:name]&.include?("SCHEMA")
         @log << values[:sql]
diff --git a/spec/support/matchers/exceed_query_limit.rb b/spec/support/matchers/exceed_query_limit.rb
index 88d22a3ddd9..cd042401f3a 100644
--- a/spec/support/matchers/exceed_query_limit.rb
+++ b/spec/support/matchers/exceed_query_limit.rb
@@ -1,17 +1,4 @@
-RSpec::Matchers.define :exceed_query_limit do |expected|
-  supports_block_expectations
-
-  match do |block|
-    @subject_block = block
-    actual_count > expected_count + threshold
-  end
-
-  failure_message_when_negated do |actual|
-    threshold_message = threshold > 0 ? " (+#{@threshold})" : ''
-    counts = "#{expected_count}#{threshold_message}"
-    "Expected a maximum of #{counts} queries, got #{actual_count}:\n\n#{log_message}"
-  end
-
+module ExceedQueryLimitHelpers
   def with_threshold(threshold)
     @threshold = threshold
     self
@@ -43,7 +30,7 @@ RSpec::Matchers.define :exceed_query_limit do |expected|
   end
 
   def recorder
-    @recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block)
+    @recorder ||= ActiveRecord::QueryRecorder.new(skip_cached: skip_cached, &@subject_block)
   end
 
   def count_queries(queries)
@@ -61,4 +48,52 @@ RSpec::Matchers.define :exceed_query_limit do |expected|
       @recorder.log_message
     end
   end
+
+  def skip_cached
+    true
+  end
+
+  def verify_count(&block)
+    @subject_block = block
+    actual_count > expected_count + threshold
+  end
+
+  def failure_message
+    threshold_message = threshold > 0 ? " (+#{@threshold})" : ''
+    counts = "#{expected_count}#{threshold_message}"
+    "Expected a maximum of #{counts} queries, got #{actual_count}:\n\n#{log_message}"
+  end
+end
+
+RSpec::Matchers.define :exceed_all_query_limit do |expected|
+  supports_block_expectations
+
+  include ExceedQueryLimitHelpers
+
+  match do |block|
+    verify_count(&block)
+  end
+
+  failure_message_when_negated do |actual|
+    failure_message
+  end
+
+  def skip_cached
+    false
+  end
+end
+
+# Excludes cached methods from the query count
+RSpec::Matchers.define :exceed_query_limit do |expected|
+  supports_block_expectations
+
+  include ExceedQueryLimitHelpers
+
+  match do |block|
+    verify_count(&block)
+  end
+
+  failure_message_when_negated do |actual|
+    failure_message
+  end
 end
-- 
cgit v1.2.1