Commit 7266c032 authored by Arturo Herrero's avatar Arturo Herrero Committed by Bob Van Landuyt

Fix N+1 queries to find or initialize services

parent 9965db91
......@@ -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
......@@ -2591,6 +2591,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
......
---
title: Fix N+1 queries to find or initialize services
merge_request: 58879
author:
type: performance
......@@ -5673,16 +5673,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
......
......@@ -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)
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment