Commit 785c95ba authored by Mario de la Ossa's avatar Mario de la Ossa

Graphql Issues - Fix N+1 for Labels

Adds a Lookahead preload for labels in IssueType
parent d7d65763
......@@ -46,7 +46,7 @@ module LooksAhead
if lookahead.selects?(:nodes)
lookahead.selection(:nodes)
elsif lookahead.selects?(:edges)
lookahead.selection(:edges).selection(:nodes)
lookahead.selection(:edges).selection(:node)
end
end
end
......@@ -34,7 +34,8 @@ module Resolvers
def preloads
{
alert_management_alert: [:alert_management_alert]
alert_management_alert: [:alert_management_alert],
labels: [:labels]
}
end
......
......@@ -42,8 +42,7 @@ module Types
field :assignees, Types::UserType.connection_type, null: true, complexity: 5,
description: 'Assignees of the issue'
# Remove complexity when BatchLoader is used
field :labels, Types::LabelType.connection_type, null: true, complexity: 5,
field :labels, Types::LabelType.connection_type, null: true,
description: 'Labels of the issue'
field :milestone, Types::MilestoneType, null: true,
description: 'Milestone of the issue',
......
......@@ -292,4 +292,65 @@ RSpec.describe 'getting an issue list for a project' do
expect(alert_titles).to contain_exactly(*expected_titles)
end
end
context 'fetching labels' do
let(:fields) do
<<~QUERY
edges {
node {
id
labels {
nodes {
id
}
}
}
}
QUERY
end
let(:query) do
graphql_query_for(
'project',
{ 'fullPath' => project.full_path },
query_graphql_field('issues', {}, fields)
)
end
let_it_be(:project) { create(:project, :public) }
let_it_be(:label1) { create(:label, project: project) }
let_it_be(:label2) { create(:label, project: project) }
let_it_be(:issue1) { create(:issue, project: project, labels: [label1]) }
let_it_be(:issue2) { create(:issue, project: project, labels: [label2]) }
let_it_be(:issues) { [issue1, issue2] }
before do
stub_feature_flags(graphql_lookahead_support: true)
end
def response_label_ids(response_data)
response_data.map do |edge|
edge['node']['labels']['nodes'].map { |u| u['id'] }
end.flatten
end
def labels_as_global_ids(issues)
issues.map(&:labels).flatten.map(&:to_global_id).map(&:to_s)
end
it 'avoids N+1 queries', :aggregate_failures do
control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) }
expect(issues_data.count).to eq(2)
expect(response_label_ids(issues_data)).to match_array(labels_as_global_ids(issues))
issues.append create(:issue, project: project, labels: [create(:label, project: project)])
expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control)
# graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb:271)
# so we have to parse the body ourselves the second time
response_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges']
expect(response_data.count).to eq(3)
expect(response_label_ids(response_data)).to match_array(labels_as_global_ids(issues))
end
end
end
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