Commit 52e59891 authored by Alex Pooley's avatar Alex Pooley Committed by Patrick Bair

Use common namespace ancestor queries

Changelog: performance
parent b1d905c2
......@@ -274,7 +274,7 @@ class Integration < ApplicationRecord
end
def self.closest_group_integration(type, scope)
group_ids = scope.ancestors.select(:id)
group_ids = scope.ancestors(hierarchy_order: :asc).select(:id)
array = group_ids.to_sql.present? ? "array(#{group_ids.to_sql})" : 'ARRAY[]'
where(type: type, group_id: group_ids, inherit_from_id: nil)
......
......@@ -177,7 +177,13 @@ module Namespaces
if hierarchy_order
depth_sql = "ABS(#{traversal_ids.count} - array_length(traversal_ids, 1))"
skope = skope.select(skope.arel_table[Arel.star], "#{depth_sql} as depth")
.order(depth: hierarchy_order)
# The SELECT includes an extra depth attribute. We wrap the SQL in a
# standard SELECT to avoid mismatched attribute errors when trying to
# chain future ActiveRelation commands, and retain the ordering.
skope = self.class
.without_sti_condition
.from(skope, self.class.table_name)
.order(depth: hierarchy_order)
end
skope
......
......@@ -914,7 +914,13 @@ class Project < ApplicationRecord
.base_and_ancestors(upto: top, hierarchy_order: hierarchy_order)
end
alias_method :ancestors, :ancestors_upto
def ancestors(hierarchy_order: nil)
if Feature.enabled?(:linear_project_ancestors, self, default_enabled: :yaml)
group&.self_and_ancestors(hierarchy_order: hierarchy_order) || Group.none
else
ancestors_upto(hierarchy_order: hierarchy_order)
end
end
def ancestors_upto_ids(...)
ancestors_upto(...).pluck(:id)
......
---
name: linear_project_ancestors
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68072
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338403
milestone: '14.2'
type: development
group: group::access
default_enabled: false
......@@ -33,7 +33,7 @@ RSpec.describe Projects::ProjectMembersHelper do
expect(project.members.count).to eq(3)
expect { call_project_members_app_data_json }.not_to exceed_query_limit(control_count).with_threshold(6) # existing n+1
expect { call_project_members_app_data_json }.not_to exceed_query_limit(control_count).with_threshold(7) # existing n+1
end
end
......
......@@ -6,6 +6,7 @@ RSpec.describe Project, factory_default: :keep do
include ProjectForksHelper
include GitHelpers
include ExternalAuthorizationServiceHelpers
include ReloadHelpers
using RSpec::Parameterized::TableSyntax
let_it_be(:namespace) { create_default(:namespace).freeze }
......@@ -3021,29 +3022,106 @@ RSpec.describe Project, factory_default: :keep do
end
end
describe '#ancestors_upto' do
let_it_be(:parent) { create(:group) }
let_it_be(:child) { create(:group, parent: parent) }
let_it_be(:child2) { create(:group, parent: child) }
let_it_be(:project) { create(:project, namespace: child2) }
shared_context 'project with group ancestry' do
let(:parent) { create(:group) }
let(:child) { create(:group, parent: parent) }
let(:child2) { create(:group, parent: child) }
let(:project) { create(:project, namespace: child2) }
before do
reload_models(parent, child, child2)
end
end
shared_context 'project with namespace ancestry' do
let(:namespace) { create :namespace }
let(:project) { create :project, namespace: namespace }
end
it 'returns all ancestors when no namespace is given' do
expect(project.ancestors_upto).to contain_exactly(child2, child, parent)
shared_examples 'project with group ancestors' do
it 'returns all ancestors' do
is_expected.to contain_exactly(child2, child, parent)
end
end
shared_examples 'project with ordered group ancestors' do
let(:hierarchy_order) { :desc }
it 'includes ancestors upto but excluding the given ancestor' do
expect(project.ancestors_upto(parent)).to contain_exactly(child2, child)
it 'returns ancestors ordered by descending hierarchy' do
is_expected.to eq([parent, child, child2])
end
end
shared_examples '#ancestors' do
context 'group ancestory' do
include_context 'project with group ancestry'
describe 'with hierarchy_order' do
it 'returns ancestors ordered by descending hierarchy' do
expect(project.ancestors_upto(hierarchy_order: :desc)).to eq([parent, child, child2])
it_behaves_like 'project with group ancestors' do
subject { project.ancestors }
end
it 'can be used with upto option' do
expect(project.ancestors_upto(parent, hierarchy_order: :desc)).to eq([child, child2])
it_behaves_like 'project with ordered group ancestors' do
subject { project.ancestors(hierarchy_order: hierarchy_order) }
end
end
context 'namespace ancestry' do
include_context 'project with namespace ancestry'
subject { project.ancestors }
it { is_expected.to be_empty }
end
end
describe '#ancestors' do
context 'with linear_project_ancestors feature flag enabled' do
before do
stub_feature_flags(linear_project_ancestors: true)
end
include_examples '#ancestors'
end
context 'with linear_project_ancestors feature flag disabled' do
before do
stub_feature_flags(linear_project_ancestors: false)
end
include_examples '#ancestors'
end
end
describe '#ancestors_upto' do
context 'group ancestry' do
include_context 'project with group ancestry'
it_behaves_like 'project with group ancestors' do
subject { project.ancestors_upto }
end
it_behaves_like 'project with ordered group ancestors' do
subject { project.ancestors_upto(hierarchy_order: hierarchy_order) }
end
it 'includes ancestors upto but excluding the given ancestor' do
expect(project.ancestors_upto(parent)).to contain_exactly(child2, child)
end
describe 'with hierarchy_order' do
it 'can be used with upto option' do
expect(project.ancestors_upto(parent, hierarchy_order: :desc)).to eq([child, child2])
end
end
end
context 'namespace ancestry' do
include_context 'project with namespace ancestry'
subject { project.ancestors_upto }
it { is_expected.to be_empty }
end
end
describe '#root_ancestor' 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