Commit c6652bda authored by Alex Pooley's avatar Alex Pooley

Use common namespace ancestor queries

Changelog: performance
parent 64f3d893
......@@ -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)
......
......@@ -178,6 +178,10 @@ module Namespaces
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 then wrap the SQL
# in a standard SELECT to avoid mismatched attribute errors when
# trying to chain future ActiveRelation commands.
skope = self.class.without_sti_condition.from(skope, self.class.table_name)
end
skope
......
......@@ -914,7 +914,9 @@ class Project < ApplicationRecord
.base_and_ancestors(upto: top, hierarchy_order: hierarchy_order)
end
alias_method :ancestors, :ancestors_upto
def ancestors(hierarchy_order: nil)
namespace&.self_and_ancestors(hierarchy_order: hierarchy_order)
end
def ancestors_upto_ids(...)
ancestors_upto(...).pluck(:id)
......
......@@ -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 }
......@@ -3027,31 +3028,72 @@ RSpec.describe Project, factory_default: :keep do
end
end
describe '#ancestors_upto' do
shared_context 'project with ancestors' 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) }
end
it 'returns all ancestors when no namespace is given' do
expect(project.ancestors_upto).to contain_exactly(child2, child, parent)
shared_examples '#ancestors' do
before do
reload_models(parent, child, child2)
end
it 'includes ancestors upto but excluding the given ancestor' do
expect(project.ancestors_upto(parent)).to contain_exactly(child2, child)
it 'returns all ancestors' do
expect(project.ancestors).to contain_exactly(child2, child, parent)
end
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])
expect(project.ancestors(hierarchy_order: :desc).to_a).to eq([parent, child, child2])
end
end
end
describe '#ancestors' do
include_context 'project with ancestors'
include_examples '#ancestors'
end
describe '#ancestors_upto' do
include_context 'project with ancestors'
include_examples '#ancestors'
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
describe '#ancestors' 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) }
before do
reload_models(parent, child, child2)
end
it 'returns all ancestors' do
expect(project.ancestors).to contain_exactly(child2, child, parent)
end
describe 'with hierarchy_order' do
it 'returns ancestors ordered by descending hierarchy' do
expect(project.ancestors(hierarchy_order: :desc).to_a).to eq([parent, child, child2])
end
end
end
describe '#root_ancestor' do
let(:project) { create(:project) }
......
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