Commit c6bc7f71 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Preload user project access in group API

This fixes an N+1 in the /group/:id and /group/:id/projects endpoints.

Changelog: performance
parent a2375c4c
...@@ -16,7 +16,7 @@ module API ...@@ -16,7 +16,7 @@ module API
options: { only_owned: true, limit: projects_limit } options: { only_owned: true, limit: projects_limit }
).execute ).execute
Entities::Project.prepare_relation(projects) Entities::Project.prepare_relation(projects, options)
end end
expose :shared_projects, using: Entities::Project do |group, options| expose :shared_projects, using: Entities::Project do |group, options|
...@@ -26,7 +26,7 @@ module API ...@@ -26,7 +26,7 @@ module API
options: { only_shared: true, limit: projects_limit } options: { only_shared: true, limit: projects_limit }
).execute ).execute
Entities::Project.prepare_relation(projects) Entities::Project.prepare_relation(projects, options)
end end
def projects_limit def projects_limit
......
...@@ -92,7 +92,7 @@ module API ...@@ -92,7 +92,7 @@ module API
projects, options = with_custom_attributes(projects, options) projects, options = with_custom_attributes(projects, options)
present options[:with].prepare_relation(projects), options present options[:with].prepare_relation(projects, options), options
end end
def present_groups(params, groups) def present_groups(params, groups)
......
...@@ -182,8 +182,6 @@ module API ...@@ -182,8 +182,6 @@ module API
[options[:with].prepare_relation(projects, options), options] [options[:with].prepare_relation(projects, options), options]
end end
Preloaders::UserMaxAccessLevelInProjectsPreloader.new(records, current_user).execute if current_user
present records, options present records, options
end end
......
...@@ -12,6 +12,8 @@ module API ...@@ -12,6 +12,8 @@ module API
preload_repository_cache(projects_relation) preload_repository_cache(projects_relation)
Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_relation, options[:current_user]).execute if options[:current_user]
projects_relation projects_relation
end end
......
...@@ -728,16 +728,16 @@ RSpec.describe API::Groups do ...@@ -728,16 +728,16 @@ RSpec.describe API::Groups do
end end
it 'avoids N+1 queries with project links' do it 'avoids N+1 queries with project links' do
get api("/groups/#{group1.id}", admin) get api("/groups/#{group1.id}", user1)
control_count = ActiveRecord::QueryRecorder.new do control_count = ActiveRecord::QueryRecorder.new do
get api("/groups/#{group1.id}", admin) get api("/groups/#{group1.id}", user1)
end.count end.count
create(:project, namespace: group1) create(:project, namespace: group1)
expect do expect do
get api("/groups/#{group1.id}", admin) get api("/groups/#{group1.id}", user1)
end.not_to exceed_query_limit(control_count) end.not_to exceed_query_limit(control_count)
end end
...@@ -746,7 +746,7 @@ RSpec.describe API::Groups do ...@@ -746,7 +746,7 @@ RSpec.describe API::Groups do
create(:group_group_link, shared_group: group1, shared_with_group: create(:group)) create(:group_group_link, shared_group: group1, shared_with_group: create(:group))
control_count = ActiveRecord::QueryRecorder.new do control_count = ActiveRecord::QueryRecorder.new do
get api("/groups/#{group1.id}", admin) get api("/groups/#{group1.id}", user1)
end.count end.count
# setup "n" more shared groups # setup "n" more shared groups
...@@ -755,7 +755,7 @@ RSpec.describe API::Groups do ...@@ -755,7 +755,7 @@ RSpec.describe API::Groups do
# test that no of queries for 1 shared group is same as for n shared groups # test that no of queries for 1 shared group is same as for n shared groups
expect do expect do
get api("/groups/#{group1.id}", admin) get api("/groups/#{group1.id}", user1)
end.not_to exceed_query_limit(control_count) end.not_to exceed_query_limit(control_count)
end end
end end
...@@ -1179,6 +1179,20 @@ RSpec.describe API::Groups do ...@@ -1179,6 +1179,20 @@ RSpec.describe API::Groups do
expect(json_response.length).to eq(1) expect(json_response.length).to eq(1)
expect(json_response.first['name']).to eq(project1.name) expect(json_response.first['name']).to eq(project1.name)
end end
it 'avoids N+1 queries' do
get api("/groups/#{group1.id}/projects", user1)
control_count = ActiveRecord::QueryRecorder.new do
get api("/groups/#{group1.id}/projects", user1)
end.count
create(:project, namespace: group1)
expect do
get api("/groups/#{group1.id}/projects", user1)
end.not_to exceed_query_limit(control_count)
end
end end
context "when authenticated as admin" do context "when authenticated as admin" do
...@@ -1196,20 +1210,6 @@ RSpec.describe API::Groups do ...@@ -1196,20 +1210,6 @@ RSpec.describe API::Groups do
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
it 'avoids N+1 queries' do
get api("/groups/#{group1.id}/projects", admin)
control_count = ActiveRecord::QueryRecorder.new do
get api("/groups/#{group1.id}/projects", admin)
end.count
create(:project, namespace: group1)
expect do
get api("/groups/#{group1.id}/projects", admin)
end.not_to exceed_query_limit(control_count)
end
end end
context 'when using group path in URL' do context 'when using group path in URL' do
......
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