summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2017-07-06 15:09:35 +0200
committerRémy Coutable <remy@rymai.me>2017-07-06 19:08:02 +0200
commit1c0e5e8165fd5ff63792cb42009919f4bca4de9a (patch)
tree571e9aa10c3b0a2cc86256d333458b4775550989
parent050eae8c4dff87fef63e79eb60d927d0171b5f7b (diff)
downloadgitlab-ce-33748-fix-n-plus-1-query-in-the-projects-api.tar.gz
Fix some N+1 queries in the GET /projects API33748-fix-n-plus-1-query-in-the-projects-api
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--lib/api/entities.rb12
-rw-r--r--lib/api/projects.rb8
-rw-r--r--spec/requests/api/projects_spec.rb42
3 files changed, 58 insertions, 4 deletions
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 99eda3b0c4b..94168fa4ebc 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -503,12 +503,20 @@ module API
class ProjectWithAccess < Project
expose :permissions do
expose :project_access, using: Entities::ProjectAccess do |project, options|
- project.project_members.find_by(user_id: options[:current_user].id)
+ 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)
+ end
end
expose :group_access, using: Entities::GroupAccess do |project, options|
if project.group
- project.group.group_members.find_by(user_id: options[:current_user].id)
+ 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)
+ end
end
end
end
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 27d49eae844..c459257158d 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -77,9 +77,17 @@ module API
projects = projects.with_issues_enabled if params[:with_issues_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(
with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails,
statistics: params[:statistics],
+ project_members: project_members,
+ group_members: group_members,
current_user: current_user
)
options[:with] = Entities::BasicProjectDetails if params[:simple]
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index ee25bd1deb1..87de8e4fddf 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -52,6 +52,25 @@ describe API::Projects do
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)
+ # TODO: We still have some N+1 queries problem that will need to be fixed
+ end.not_to exceed_query_limit(control_count + 8)
+ end
+ end
+
let!(:public_project) { create(:empty_project, :public, name: 'public_project') }
before do
project
@@ -62,9 +81,13 @@ describe API::Projects do
context 'when unauthenticated' 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(:projects) { [public_project] }
end
end
@@ -75,6 +98,21 @@ describe API::Projects do
let(:projects) { [public_project, project, project2, project3] }
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
get api('/projects', user)