Merge branch '326209-fix-find-or-initialize-service-n+1' into 'master'

Fix N+1 queries to find or initialize services

See merge request gitlab-org/gitlab!58879

app/models/project.rb

@@ -1378,7 +1378,7 @@ class Project < ApplicationRecord
   def find_or_initialize_service(name)
     return if disabled_services.include?(name)
 
-    find_service(services, name) || build_from_instance_or_template(name) || public_send("build_#{name}_service") # rubocop:disable GitlabSecurity/PublicSend
+    find_service(services, name) || build_from_instance_or_template(name) || build_service(name)
   end
 
   # rubocop: disable CodeReuse/ServiceClass
@@ -2596,6 +2596,10 @@ class Project < ApplicationRecord
     return Service.build_from_integration(template, project_id: id) if template
   end
 
+  def build_service(name)
+    "#{name}_service".classify.constantize.new(project_id: id)
+  end
+
   def services_templates
     @services_templates ||= Service.for_template
   end

changelogs/unreleased/326209-fix-find-or-initialize-service-n-1.yml

+---
+title: Fix N+1 queries to find or initialize services
+merge_request: 58879
+author:
+type: performance

spec/models/project_spec.rb

@@ -5795,16 +5795,34 @@ RSpec.describe Project, factory_default: :keep do
   end
 
   describe '#find_or_initialize_services' do
-    before do
-      allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover teamcity])
-      allow(subject).to receive(:disabled_services).and_return(%w[prometheus])
+    let_it_be(:subject) { create(:project) }
+
+    it 'avoids N+1 database queries' do
+      control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_services }.count
+
+      expect(control_count).to be <= 4
     end
 
-    it 'returns only enabled services' do
-      services = subject.find_or_initialize_services
+    it 'avoids N+1 database queries with more available services' do
+      allow(Service).to receive(:available_services_names).and_return(%w[pushover])
+      control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_services }
 
-      expect(services.count).to eq(2)
-      expect(services.map(&:title)).to eq(['JetBrains TeamCity CI', 'Pushover'])
+      allow(Service).to receive(:available_services_names).and_call_original
+      expect { subject.find_or_initialize_services }.not_to exceed_query_limit(control_count)
+    end
+
+    context 'with disabled services' do
+      before do
+        allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover teamcity])
+        allow(subject).to receive(:disabled_services).and_return(%w[prometheus])
+      end
+
+      it 'returns only enabled services sorted' do
+        services = subject.find_or_initialize_services
+
+        expect(services.size).to eq(2)
+        expect(services.map(&:title)).to eq(['JetBrains TeamCity CI', 'Pushover'])
+      end
     end
   end
 

spec/services/projects/create_service_spec.rb

@@ -724,9 +724,7 @@ RSpec.describe Projects::CreateService, '#execute' do
 
       it 'cleans invalid record and logs warning', :aggregate_failures do
         invalid_service_record = build(:prometheus_service, properties: { api_url: nil, manual_configuration: true }.to_json)
-        allow_next_instance_of(Project) do |instance|
-          allow(instance).to receive(:build_prometheus_service).and_return(invalid_service_record)
-        end
+        allow(PrometheusService).to receive(:new).and_return(invalid_service_record)
 
         expect(Gitlab::ErrorTracking).to receive(:track_exception).with(an_instance_of(ActiveRecord::RecordInvalid), include(extra: { project_id: a_kind_of(Integer) }))
         project = create_project(user, opts)

spec/workers/projects/post_creation_worker_spec.rb

@@ -64,10 +64,7 @@ RSpec.describe Projects::PostCreationWorker do
 
           it 'cleans invalid record and logs warning', :aggregate_failures do
             invalid_service_record = build(:prometheus_service, properties: { api_url: nil, manual_configuration: true }.to_json)
-
-            allow_next_found_instance_of(Project) do |instance|
-              allow(instance).to receive(:build_prometheus_service).and_return(invalid_service_record)
-            end
+            allow(PrometheusService).to receive(:new).and_return(invalid_service_record)
 
             expect(Gitlab::ErrorTracking).to receive(:track_exception).with(an_instance_of(ActiveRecord::RecordInvalid), include(extra: { project_id: a_kind_of(Integer) })).twice
             subject