From 38933a1694a614c2b07953a9d298bc0d70838bba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= <loic@discourse.org>
Date: Thu, 26 Jan 2023 16:02:54 +0100
Subject: [PATCH] DEV: Add the 'n_plus_one_control' gem to tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This gem provides new RSpec matchers helping us to make sure we’re not
creating N+1 queries. It works differently than some other alternatives
as here we don’t have to provide how many queries we expect. The tool
will vary the number of records in the DB and will raise an error
if the number of queries has changed.

A small example has been added in this patch.

Docs at: https://github.com/palkan/n_plus_one_control
---
 Gemfile                     |  1 +
 Gemfile.lock                |  2 ++
 lib/topic_view.rb           |  2 +-
 spec/lib/topic_view_spec.rb | 13 +++++++++++--
 spec/rails_helper.rb        |  1 +
 5 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Gemfile b/Gemfile
index 4bd110bf4b..2cd3ce149c 100644
--- a/Gemfile
+++ b/Gemfile
@@ -147,6 +147,7 @@ group :test do
   gem "selenium-webdriver", require: false
   gem "test-prof"
   gem "webdrivers", require: false
+  gem "n_plus_one_control"
 end
 
 group :test, :development do
diff --git a/Gemfile.lock b/Gemfile.lock
index 0f22d6f063..1ea2c7bb1f 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -234,6 +234,7 @@ GEM
     multi_json (1.15.0)
     multi_xml (0.6.0)
     mustache (1.1.1)
+    n_plus_one_control (0.6.2)
     net-http (0.3.2)
       uri
     net-imap (0.3.4)
@@ -583,6 +584,7 @@ DEPENDENCIES
   mocha
   multi_json
   mustache
+  n_plus_one_control
   net-http
   net-imap
   net-pop
diff --git a/lib/topic_view.rb b/lib/topic_view.rb
index 576b6d3c5c..53e70346a6 100644
--- a/lib/topic_view.rb
+++ b/lib/topic_view.rb
@@ -820,7 +820,7 @@ class TopicView
   def find_topic(topic_or_topic_id)
     return topic_or_topic_id if topic_or_topic_id.is_a?(Topic)
     # with_deleted covered in #check_and_raise_exceptions
-    Topic.with_deleted.includes(:category, :tags).find_by(id: topic_or_topic_id)
+    Topic.with_deleted.includes(:category).find_by(id: topic_or_topic_id)
   end
 
   def unfiltered_posts
diff --git a/spec/lib/topic_view_spec.rb b/spec/lib/topic_view_spec.rb
index 5032846488..3db6981606 100644
--- a/spec/lib/topic_view_spec.rb
+++ b/spec/lib/topic_view_spec.rb
@@ -1076,13 +1076,22 @@ RSpec.describe TopicView do
   describe "#tags" do
     subject(:topic_view_tags) { topic_view.tags }
 
-    let(:topic_view) { described_class.new(topic, user) }
+    let(:topic_view) { described_class.new(topic) }
     let(:topic) { Fabricate.build(:topic, tags: tags) }
     let(:tags) { Fabricate.build_times(2, :tag) }
-    let(:user) { Fabricate(:user) }
 
     it "returns the tags names" do
       expect(topic_view_tags).to match tags.map(&:name)
     end
+
+    context "with N+1", :n_plus_one do
+      let!(:topic) { Fabricate(:topic) }
+
+      populate { |n| topic.tags = Fabricate.times(n, :tag) }
+
+      specify do
+        expect { described_class.new(topic.id).tags }.to perform_constant_number_of_queries
+      end
+    end
   end
 end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 22009af6fa..37fe250e77 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -62,6 +62,7 @@ require "test_prof/before_all/adapters/active_record"
 require "webdrivers"
 require "selenium-webdriver"
 require "capybara/rails"
+require "n_plus_one_control/rspec"
 
 # The shoulda-matchers gem no longer detects the test framework
 # you're using or mixes itself into that framework automatically.