Commit ddd7690e authored by Drew Blessing's avatar Drew Blessing

Preload group root ancestor for Group Projects API

The group projects API relies on the root ancestor namespace when
determining many settings and features. Preload this association
to prevent N+1s.

Changelog: fixed
parent 40b78d62
---
name: group_projects_api_preload_groups
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81838
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/354372
milestone: '14.9'
type: development
group: group::authentication and authorization
default_enabled: false
...@@ -14,6 +14,7 @@ module API ...@@ -14,6 +14,7 @@ module API
Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_relation, options[:current_user]).execute if options[:current_user] Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_relation, options[:current_user]).execute if options[:current_user]
Preloaders::SingleHierarchyProjectGroupPlansPreloader.new(projects_relation).execute if options[:single_hierarchy] Preloaders::SingleHierarchyProjectGroupPlansPreloader.new(projects_relation).execute if options[:single_hierarchy]
preload_groups(projects_relation) if options[:with] == Entities::Project
projects_relation projects_relation
end end
...@@ -40,6 +41,25 @@ module API ...@@ -40,6 +41,25 @@ module API
def repositories_for_preload(projects_relation) def repositories_for_preload(projects_relation)
projects_relation.map(&:repository) projects_relation.map(&:repository)
end end
# For all projects except those in a user namespace, the `namespace`
# and `group` are identical. Preload the group when it's not a user namespace.
def preload_groups(projects_relation)
return unless Feature.enabled?(:group_projects_api_preload_groups)
group_projects = projects_for_group_preload(projects_relation)
groups = group_projects.map(&:namespace)
Preloaders::GroupRootAncestorPreloader.new(groups).execute
group_projects.each do |project|
project.group = project.namespace
end
end
def projects_for_group_preload(projects_relation)
projects_relation.select { |project| project.namespace.type == Group.sti_name }
end
end end
end end
end end
...@@ -1164,17 +1164,47 @@ RSpec.describe API::Groups do ...@@ -1164,17 +1164,47 @@ RSpec.describe API::Groups do
end end
context 'when include_subgroups is true' do context 'when include_subgroups is true' do
it "returns projects including those in subgroups" do before do
subgroup = create(:group, parent: group1) subgroup = create(:group, parent: group1)
subgroup2 = create(:group, parent: subgroup)
create(:project, group: subgroup) create(:project, group: subgroup)
create(:project, group: subgroup) create(:project, group: subgroup)
create(:project, group: subgroup2)
group1.reload
end
it "only looks up root ancestor once and returns projects including those in subgroups" do
expect(Namespace).to receive(:find_by).with(id: group1.id.to_s).once.and_call_original # For the group sent in the API call
expect(Namespace).to receive(:find_by).with(id: group1.traversal_ids.first).once.and_call_original # root_ancestor direct lookup
expect(Namespace).to receive(:joins).with(start_with('INNER JOIN (SELECT id, traversal_ids[1]')).once.and_call_original # All-in-one root_ancestor query
get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers expect(response).to include_pagination_headers
expect(json_response).to be_an(Array) expect(json_response).to be_an(Array)
expect(json_response.length).to eq(5) expect(json_response.length).to eq(6)
end
context 'when group_projects_api_preload_groups feature is disabled' do
before do
stub_feature_flags(group_projects_api_preload_groups: false)
end
it 'looks up the root ancestor multiple times' do
expect(Namespace).to receive(:find_by).with(id: group1.id.to_s).once.and_call_original
expect(Namespace).to receive(:find_by).with(id: group1.traversal_ids.first).at_least(:twice).and_call_original
expect(Namespace).not_to receive(:joins).with(start_with('INNER JOIN (SELECT id, traversal_ids[1]'))
get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true }
expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers
expect(json_response).to be_an(Array)
expect(json_response.length).to eq(6)
end
end 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