Commit 033acc01 authored by Rémy Coutable's avatar Rémy Coutable

Fix some N+1 queries in the GET /projects API

Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 050eae8c
...@@ -503,16 +503,24 @@ module API ...@@ -503,16 +503,24 @@ module API
class ProjectWithAccess < Project class ProjectWithAccess < Project
expose :permissions do expose :permissions do
expose :project_access, using: Entities::ProjectAccess do |project, options| expose :project_access, using: Entities::ProjectAccess do |project, options|
if options.key?(:project_members)
(options[:project_members] || []).find { |member| member.source_id == project.id }
else
project.project_members.find_by(user_id: options[:current_user].id) project.project_members.find_by(user_id: options[:current_user].id)
end end
end
expose :group_access, using: Entities::GroupAccess do |project, options| expose :group_access, using: Entities::GroupAccess do |project, options|
if project.group if project.group
if options.key?(:group_members)
(options[:group_members] || []).find { |member| member.source_id == project.namespace_id }
else
project.group.group_members.find_by(user_id: options[:current_user].id) project.group.group_members.find_by(user_id: options[:current_user].id)
end end
end end
end end
end end
end
class LabelBasic < Grape::Entity class LabelBasic < Grape::Entity
expose :id, :name, :color, :description expose :id, :name, :color, :description
......
...@@ -77,9 +77,17 @@ module API ...@@ -77,9 +77,17 @@ module API
projects = projects.with_issues_enabled if params[:with_issues_enabled] projects = projects.with_issues_enabled if params[:with_issues_enabled]
projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled]
if current_user
projects = projects.includes(:route, :taggings, namespace: :route)
project_members = current_user.project_members
group_members = current_user.group_members
end
options = options.reverse_merge( options = options.reverse_merge(
with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails,
statistics: params[:statistics], statistics: params[:statistics],
project_members: project_members,
group_members: group_members,
current_user: current_user current_user: current_user
) )
options[:with] = Entities::BasicProjectDetails if params[:simple] options[:with] = Entities::BasicProjectDetails if params[:simple]
......
...@@ -52,6 +52,24 @@ describe API::Projects do ...@@ -52,6 +52,24 @@ describe API::Projects do
end end
end end
shared_examples_for 'projects response without N + 1 queries' do
it 'avoids N + 1 queries' do
control_count = ActiveRecord::QueryRecorder.new do
get api('/projects', current_user)
end.count
if defined?(additional_project)
additional_project
else
create(:empty_project, :public)
end
expect do
get api('/projects', current_user)
end.not_to exceed_query_limit(control_count + 8)
end
end
let!(:public_project) { create(:empty_project, :public, name: 'public_project') } let!(:public_project) { create(:empty_project, :public, name: 'public_project') }
before do before do
project project
...@@ -62,9 +80,13 @@ describe API::Projects do ...@@ -62,9 +80,13 @@ describe API::Projects do
context 'when unauthenticated' do context 'when unauthenticated' do
it_behaves_like 'projects response' do it_behaves_like 'projects response' do
let(:filter) { {} } let(:filter) { { search: project.name } }
let(:current_user) { user }
let(:projects) { [project] }
end
it_behaves_like 'projects response without N + 1 queries' do
let(:current_user) { nil } let(:current_user) { nil }
let(:projects) { [public_project] }
end end
end end
...@@ -75,6 +97,21 @@ describe API::Projects do ...@@ -75,6 +97,21 @@ describe API::Projects do
let(:projects) { [public_project, project, project2, project3] } let(:projects) { [public_project, project, project2, project3] }
end end
it_behaves_like 'projects response without N + 1 queries' do
let(:current_user) { user }
end
context 'when some projects are in a group' do
before do
create(:empty_project, :public, group: create(:group))
end
it_behaves_like 'projects response without N + 1 queries' do
let(:current_user) { user }
let(:additional_project) { create(:empty_project, :public, group: create(:group)) }
end
end
it 'includes the project labels as the tag_list' do it 'includes the project labels as the tag_list' do
get api('/projects', user) get api('/projects', user)
......
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