Commit fde4980f authored by Jan Provaznik's avatar Jan Provaznik

Avoid N+1 queries in epic reference

* We can avoid N+1 in epic autocomplete by preloading epic groups.
* We can also optimize epic cross reference by using just group id instead
of using epic's group (and group's projects).
parent 81cbfaf5
...@@ -387,13 +387,22 @@ module EE ...@@ -387,13 +387,22 @@ module EE
def to_reference(from = nil, full: false) def to_reference(from = nil, full: false)
reference = "#{self.class.reference_prefix}#{iid}" reference = "#{self.class.reference_prefix}#{iid}"
return reference unless (cross_referenced?(from) && !group.projects.include?(from)) || full return reference unless full || cross_referenced?(from)
"#{group.full_path}#{reference}" "#{group.full_path}#{reference}"
end end
def cross_referenced?(from) def cross_referenced?(from)
from && from != group return false unless from
case from
when ::Group
from.id != group_id
when ::Project
from.namespace_id != group_id
else
true
end
end end
def ancestors def ancestors
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
module EE module EE
module Groups module Groups
module AutocompleteService module AutocompleteService
# rubocop: disable CodeReuse/ActiveRecord
def epics(confidential_only: false) def epics(confidential_only: false)
finder_params = { group_id: group.id } finder_params = { group_id: group.id }
finder_params[:confidential] = true if confidential_only.present? finder_params[:confidential] = true if confidential_only.present?
...@@ -10,8 +11,10 @@ module EE ...@@ -10,8 +11,10 @@ module EE
# See https://gitlab.com/gitlab-org/gitlab/issues/6837 # See https://gitlab.com/gitlab-org/gitlab/issues/6837
EpicsFinder.new(current_user, finder_params) EpicsFinder.new(current_user, finder_params)
.execute .execute
.preload(:group)
.select(:iid, :title, :group_id) .select(:iid, :title, :group_id)
end end
# rubocop: enable CodeReuse/ActiveRecord
def vulnerabilities def vulnerabilities
::Autocomplete::VulnerabilitiesAutocompleteFinder ::Autocomplete::VulnerabilitiesAutocompleteFinder
......
---
title: Minimize number of SQL queries on epic autocompletion.
merge_request: 61885
author:
type: performance
...@@ -568,6 +568,14 @@ RSpec.describe Epic do ...@@ -568,6 +568,14 @@ RSpec.describe Epic do
expect(epic.to_reference(group_project, full: true)).to eq('group-a&1') expect(epic.to_reference(group_project, full: true)).to eq('group-a&1')
end end
end end
it 'avoids additional SQL queries' do
epic # pre-create the epic
recorder = ActiveRecord::QueryRecorder.new { epic.to_reference(project) }
expect(recorder.count).to be_zero
end
end end
describe '#has_children?' do describe '#has_children?' do
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Groups::AutocompleteSourcesController do RSpec.describe 'groups autocomplete' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be_with_reload(:group) { create(:group, :private) } let_it_be_with_reload(:group) { create(:group, :private) }
let_it_be(:epic) { create(:epic, group: group) } let_it_be(:epic) { create(:epic, group: group) }
...@@ -32,9 +32,8 @@ RSpec.describe Groups::AutocompleteSourcesController do ...@@ -32,9 +32,8 @@ RSpec.describe Groups::AutocompleteSourcesController do
with_them do with_them do
it 'returns the correct response', :aggregate_failures do it 'returns the correct response', :aggregate_failures do
issues = Array(expected).flat_map { |sym| public_send(sym) } issues = Array(expected).flat_map { |sym| public_send(sym) }
params = { group_id: group, issue_types: issue_types }.compact
get :issues, params: params get issues_group_autocomplete_sources_path(group, issue_types: issue_types)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_an(Array) expect(json_response).to be_an(Array)
...@@ -47,19 +46,29 @@ RSpec.describe Groups::AutocompleteSourcesController do ...@@ -47,19 +46,29 @@ RSpec.describe Groups::AutocompleteSourcesController do
describe '#epics' do describe '#epics' do
it 'returns 200 status' do it 'returns 200 status' do
get :epics, params: { group_id: group } get epics_group_autocomplete_sources_path(group)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end
it 'returns the correct response' do it 'returns the correct response' do
get :epics, params: { group_id: group } get epics_group_autocomplete_sources_path(group)
expect(json_response).to be_an(Array) expect(json_response).to be_an(Array)
expect(json_response.first).to include( expect(json_response.first).to include(
'iid' => epic.iid, 'title' => epic.title, 'reference' => epic.to_reference(epic.group) 'iid' => epic.iid, 'title' => epic.title, 'reference' => epic.to_reference(epic.group)
) )
end end
it 'avoids N+1 queries' do
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { get epics_group_autocomplete_sources_path(group) }
create_list(:epic, 3, group: group)
expect do
get epics_group_autocomplete_sources_path(group)
end.not_to exceed_all_query_limit(control)
end
end end
describe '#vulnerabilities' do describe '#vulnerabilities' do
...@@ -72,13 +81,13 @@ RSpec.describe Groups::AutocompleteSourcesController do ...@@ -72,13 +81,13 @@ RSpec.describe Groups::AutocompleteSourcesController do
end end
it 'returns 200 status' do it 'returns 200 status' do
get :vulnerabilities, params: { group_id: group } get vulnerabilities_group_autocomplete_sources_path(group)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end
it 'returns the correct response', :aggregate_failures do it 'returns the correct response', :aggregate_failures do
get :vulnerabilities, params: { group_id: group } get vulnerabilities_group_autocomplete_sources_path(group)
expect(json_response).to be_an(Array) expect(json_response).to be_an(Array)
expect(json_response.first).to include( expect(json_response.first).to include(
...@@ -89,13 +98,13 @@ RSpec.describe Groups::AutocompleteSourcesController do ...@@ -89,13 +98,13 @@ RSpec.describe Groups::AutocompleteSourcesController do
describe '#commands' do describe '#commands' do
it 'returns 200 status' do it 'returns 200 status' do
get :commands, params: { group_id: group, type: 'Epic', type_id: epic.iid } get commands_group_autocomplete_sources_path(group, type: 'Epic', type_id: epic.iid)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end
it 'returns the correct response' do it 'returns the correct response' do
get :commands, params: { group_id: group, type: 'Epic', type_id: epic.iid } get commands_group_autocomplete_sources_path(group, type: 'Epic', type_id: epic.iid)
expect(json_response).to be_an(Array) expect(json_response).to be_an(Array)
expect(json_response).to include( expect(json_response).to include(
...@@ -107,7 +116,7 @@ RSpec.describe Groups::AutocompleteSourcesController do ...@@ -107,7 +116,7 @@ RSpec.describe Groups::AutocompleteSourcesController do
end end
it 'handles new epics' do it 'handles new epics' do
get :commands, params: { group_id: group, type: 'Epic', type_id: nil } get commands_group_autocomplete_sources_path(group, type: 'Epic', type_id: nil)
expect(json_response).to be_an(Array) expect(json_response).to be_an(Array)
expect(json_response).to include( expect(json_response).to include(
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Groups::AutocompleteSourcesController do RSpec.describe 'groups autocomplete' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be_with_reload(:group) { create(:group, :private) } let_it_be_with_reload(:group) { create(:group, :private) }
...@@ -35,9 +35,8 @@ RSpec.describe Groups::AutocompleteSourcesController do ...@@ -35,9 +35,8 @@ RSpec.describe Groups::AutocompleteSourcesController do
with_them do with_them do
it 'returns the correct response', :aggregate_failures do it 'returns the correct response', :aggregate_failures do
issues = Array(expected).flat_map { |sym| public_send(sym) } issues = Array(expected).flat_map { |sym| public_send(sym) }
params = { group_id: group, issue_types: issue_types }.compact
get :issues, params: params get issues_group_autocomplete_sources_path(group, issue_types: issue_types)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_an(Array) expect(json_response).to be_an(Array)
...@@ -57,7 +56,7 @@ RSpec.describe Groups::AutocompleteSourcesController do ...@@ -57,7 +56,7 @@ RSpec.describe Groups::AutocompleteSourcesController do
create(:milestone, group: sub_group) create(:milestone, group: sub_group)
group_milestone = create(:milestone, group: group) group_milestone = create(:milestone, group: group)
get :milestones, params: { group_id: group } get milestones_group_autocomplete_sources_path(group)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.count).to eq(1) expect(json_response.count).to eq(1)
......
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