diff options
| -rw-r--r-- | changelogs/unreleased/33748-fix-n-plus-1-query-in-the-projects-api.yml | 4 | ||||
| -rw-r--r-- | lib/api/entities.rb | 12 | ||||
| -rw-r--r-- | lib/api/projects.rb | 8 | ||||
| -rw-r--r-- | spec/requests/api/projects_spec.rb | 41 |
4 files changed, 61 insertions, 4 deletions
diff --git a/changelogs/unreleased/33748-fix-n-plus-1-query-in-the-projects-api.yml b/changelogs/unreleased/33748-fix-n-plus-1-query-in-the-projects-api.yml new file mode 100644 index 00000000000..7402c33c5c6 --- /dev/null +++ b/changelogs/unreleased/33748-fix-n-plus-1-query-in-the-projects-api.yml @@ -0,0 +1,4 @@ +--- +title: Improve the performance of the project list API +merge_request: 12679 +author: 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..fa704f23857 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -52,6 +52,24 @@ 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) + 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 +80,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 +97,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) |