Reduce number of SQL queries for service templates

- Move logic from Admin::ServicesController to Service.
- Improve performance avoiding N+1 queries:
  - Retrieve service templates reduced from 39 to 6 query counts.
  - Create service templates reduced from 178 to 80 query counts.

app/controllers/admin/services_controller.rb

@@ -3,11 +3,10 @@
 class Admin::ServicesController < Admin::ApplicationController
   include ServiceParams
 
-  before_action :whitelist_query_limiting, only: [:index]
   before_action :service, only: [:edit, :update]
 
   def index
-    @services = services_templates
+    @services = Service.find_or_create_templates
   end
 
   def edit
@@ -30,22 +29,9 @@ class Admin::ServicesController < Admin::ApplicationController
 
   private
 
-  # rubocop: disable CodeReuse/ActiveRecord
-  def services_templates
-    Service.available_services_names.map do |service_name|
-      service_template = "#{service_name}_service".camelize.constantize
-      service_template.where(template: true).first_or_create
-    end
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-
   # rubocop: disable CodeReuse/ActiveRecord
   def service
     @service ||= Service.find_by(id: params[:id], template: true)
   end
   # rubocop: enable CodeReuse/ActiveRecord
-
-  def whitelist_query_limiting
-    Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42430')
-  end
 end

app/models/service.rb

@@ -46,6 +46,7 @@ class Service < ApplicationRecord
   scope :active, -> { where(active: true) }
   scope :without_defaults, -> { where(default: false) }
   scope :by_type, -> (type) { where(type: type) }
+  scope :templates, -> { where(template: true) }
 
   scope :push_hooks, -> { where(push_events: true, active: true) }
   scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) }
@@ -259,6 +260,27 @@ class Service < ApplicationRecord
     self.category == :issue_tracker
   end
 
+  # Find all service templates; if some of them do not exist, create them
+  # within a transaction to perform the lowest possible SQL queries.
+  def self.find_or_create_templates
+    if templates.size == available_services_types.size
+      templates
+    else
+      create_nonexistent_templates
+      templates.reset
+    end
+  end
+
+  private_class_method def self.create_nonexistent_templates
+    nonexistent_services = available_services_types - templates.map(&:type)
+
+    transaction do
+      nonexistent_services.each do |service_type|
+        service_type.constantize.create(template: true)
+      end
+    end
+  end
+
   def self.available_services_names
     service_names = %w[
       alerts
@@ -301,6 +323,10 @@ class Service < ApplicationRecord
     service_names.sort_by(&:downcase)
   end
 
+  def self.available_services_types
+    available_services_names.map { |service_name| "#{service_name}_service".camelize }
+  end
+
   def self.build_from_template(project_id, template)
     service = template.dup
 

changelogs/unreleased/20820-service-templates-performance.yml

+---
+title: Reduce number of SQL queries for service templates
+merge_request: 27396
+author:
+type: performance

spec/controllers/admin/services_controller_spec.rb

@@ -9,25 +9,40 @@ describe Admin::ServicesController do
     sign_in(admin)
   end
 
-  describe 'GET #edit' do
-    let!(:project) { create(:project) }
+  describe 'GET #index' do
+    it 'avoids N+1 queries' do
+      query_count = ActiveRecord::QueryRecorder.new { get :index }.count
+
+      expect(query_count).to be <= 80
+    end
 
-    Service.available_services_names.each do |service_name|
-      context "#{service_name}" do
-        let!(:service) do
-          service_template = "#{service_name}_service".camelize.constantize
-          service_template.where(template: true).first_or_create
-        end
+    context 'with all existing templates' do
+      before do
+        Service.insert_all(
+          Service.available_services_types.map { |type| { template: true, type: type } }
+        )
+      end
 
-        it 'successfully displays the template' do
-          get :edit, params: { id: service.id }
+      it 'avoids N+1 queries' do
+        query_count = ActiveRecord::QueryRecorder.new { get :index }.count
 
-          expect(response).to have_gitlab_http_status(:ok)
-        end
+        expect(query_count).to eq(6)
       end
     end
   end
 
+  describe 'GET #edit' do
+    let!(:service) do
+      create(:jira_service, :template)
+    end
+
+    it 'successfully displays the template' do
+      get :edit, params: { id: service.id }
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+  end
+
   describe "#update" do
     let(:project) { create(:project) }
     let!(:service_template) do

spec/models/service_spec.rb

@@ -149,9 +149,37 @@ describe Service do
     end
   end
 
-  describe "Template" do
+  describe 'template' do
     let(:project) { create(:project) }
 
+    describe '.find_or_create_templates' do
+      it 'creates service templates' do
+        expect { Service.find_or_create_templates }.to change { Service.count }.from(0).to(Service.available_services_names.size)
+      end
+
+      context 'with all existing templates' do
+        before do
+          Service.insert_all(
+            Service.available_services_types.map { |type| { template: true, type: type } }
+          )
+        end
+
+        it 'does not create service templates' do
+          expect { Service.find_or_create_templates }.to change { Service.count }.by(0)
+        end
+      end
+
+      context 'with a few existing templates' do
+        before do
+          JiraService.create(template: true)
+        end
+
+        it 'creates the rest of the service templates' do
+          expect { Service.find_or_create_templates }.to change { Service.count }.from(1).to(Service.available_services_names.size)
+        end
+      end
+    end
+
     describe '.build_from_template' do
       context 'when template is invalid' do
         it 'sets service template to inactive when template is invalid' do