Commit be4a1dac authored by Arturo Herrero's avatar Arturo Herrero

Remove unnecessary validation avoiding N+1 queries

Remove unnecessary validation avoiding N+1 queries when building
integrations. This is a follow-up to fix
https://gitlab.com/gitlab-org/gitlab/-/issues/326209 where find or
initialize services causes N+1 queries.

Find or initialize the services has the following precedence:
- Find existing records.
- Build services from instance-level record or service template record.
  => This commit fixes the N+1 here.
- Build services from scratch. Fixed
  https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58879.

In this case, when building one integration from an instance-level
integration or service template we check for service.invalid? which
triggers a query. service is an in-memory object but it checks the
uniqueness by type and project.  We can remove this because it's
redundant:
- All the integration records should be valid, we worked to verified
  that https://gitlab.com/groups/gitlab-org/-/epics/3366.
- build_from_integration copy the active value from the instance-level
  or service template. So if that integration is inactive,
  service = integration.dup will copy active attribute as false.
parent e4be7cb6
......@@ -241,7 +241,6 @@ class Service < ApplicationRecord
service.project_id = project_id
service.group_id = group_id
service.inherit_from_id = integration.id if integration.instance? || integration.group
service.active = false if service.invalid?
service
end
......
---
title: Remove unnecessary validation avoiding N+1 queries when building integrations
merge_request: 59635
author:
type: performance
......@@ -273,16 +273,6 @@ RSpec.describe Projects::CreateService, '#execute' do
opts[:default_branch] = 'master'
expect(create_project(user, opts)).to eq(nil)
end
it 'sets invalid service as inactive' do
create(:service, type: 'JiraService', project: nil, template: true, active: true)
project = create_project(user, opts)
service = project.services.first
expect(project).to be_persisted
expect(service.active).to be false
end
end
context 'wiki_enabled creates repository directory' do
......@@ -633,17 +623,6 @@ RSpec.describe Projects::CreateService, '#execute' do
end
end
end
context 'when there is an invalid integration' do
before do
create(:service, :template, type: 'DroneCiService', active: true)
end
it 'creates an inactive service' do
expect(project).to be_persisted
expect(project.services.first.active).to be false
end
end
end
context 'when skip_disk_validation is used' do
......
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