Merge branch 'if-324746-linear_self_and_ancestors' into 'master'

Linear traversal query for Namespace#self_and_ancestors [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!61163

app/models/concerns/cascading_namespace_setting_attribute.rb

@@ -230,7 +230,7 @@ module CascadingNamespaceSettingAttribute
 
   def namespace_ancestor_ids
     strong_memoize(:namespace_ancestor_ids) do
-      namespace.self_and_ancestors(hierarchy_order: :asc).pluck(:id).reject { |id| id == namespace_id }
+      namespace.ancestor_ids(hierarchy_order: :asc)
     end
   end
 

app/models/namespaces/traversal/linear.rb

@@ -64,6 +64,13 @@ module Namespaces
         traversal_ids.present?
       end
 
+      def use_traversal_ids_for_ancestors?
+        return false unless use_traversal_ids?
+        return false unless Feature.enabled?(:use_traversal_ids_for_ancestors, root_ancestor, default_enabled: :yaml)
+
+        traversal_ids.present?
+      end
+
       def root_ancestor
         return super if parent.nil?
         return super unless persisted?
@@ -95,14 +102,33 @@ module Namespaces
       end
 
       def ancestors(hierarchy_order: nil)
-        return super() unless use_traversal_ids?
-        return super() unless Feature.enabled?(:use_traversal_ids_for_ancestors, root_ancestor, default_enabled: :yaml)
+        return super unless use_traversal_ids_for_ancestors?
 
         return self.class.none if parent_id.blank?
 
         lineage(bottom: parent, hierarchy_order: hierarchy_order)
       end
 
+      def ancestor_ids(hierarchy_order: nil)
+        return super unless use_traversal_ids_for_ancestors?
+
+        hierarchy_order == :desc ? traversal_ids[0..-2] : traversal_ids[0..-2].reverse
+      end
+
+      def self_and_ancestors(hierarchy_order: nil)
+        return super unless use_traversal_ids_for_ancestors?
+
+        return self.class.where(id: id) if parent_id.blank?
+
+        lineage(bottom: self, hierarchy_order: hierarchy_order)
+      end
+
+      def self_and_ancestor_ids(hierarchy_order: nil)
+        return super unless use_traversal_ids_for_ancestors?
+
+        hierarchy_order == :desc ? traversal_ids : traversal_ids.reverse
+      end
+
       private
 
       # Update the traversal_ids for the full hierarchy.

app/models/namespaces/traversal/recursive.rb

@@ -10,7 +10,7 @@ module Namespaces
 
         if persisted?
           strong_memoize(:root_ancestor) do
-            self_and_ancestors.reorder(nil).find_by(parent_id: nil)
+            recursive_self_and_ancestors.reorder(nil).find_by(parent_id: nil)
           end
         else
           parent.root_ancestor
@@ -26,14 +26,19 @@ module Namespaces
       alias_method :recursive_self_and_hierarchy, :self_and_hierarchy
 
       # Returns all the ancestors of the current namespaces.
-      def ancestors
+      def ancestors(hierarchy_order: nil)
         return self.class.none unless parent_id
 
         object_hierarchy(self.class.where(id: parent_id))
-          .base_and_ancestors
+          .base_and_ancestors(hierarchy_order: hierarchy_order)
       end
       alias_method :recursive_ancestors, :ancestors
 
+      def ancestor_ids(hierarchy_order: nil)
+        recursive_ancestors(hierarchy_order: hierarchy_order).pluck(:id)
+      end
+      alias_method :recursive_ancestor_ids, :ancestor_ids
+
       # returns all ancestors upto but excluding the given namespace
       # when no namespace is given, all ancestors upto the top are returned
       def ancestors_upto(top = nil, hierarchy_order: nil)
@@ -49,6 +54,11 @@ module Namespaces
       end
       alias_method :recursive_self_and_ancestors, :self_and_ancestors
 
+      def self_and_ancestor_ids(hierarchy_order: nil)
+        recursive_self_and_ancestors(hierarchy_order: hierarchy_order).pluck(:id)
+      end
+      alias_method :recursive_self_and_ancestor_ids, :self_and_ancestor_ids
+
       # Returns all the descendants of the current namespace.
       def descendants
         object_hierarchy(self.class.where(parent_id: id))
@@ -63,7 +73,7 @@ module Namespaces
       alias_method :recursive_self_and_descendants, :self_and_descendants
 
       def self_and_descendant_ids
-        self_and_descendants.select(:id)
+        recursive_self_and_descendants.select(:id)
       end
       alias_method :recursive_self_and_descendant_ids, :self_and_descendant_ids
 

spec/models/namespace_spec.rb

@@ -996,6 +996,36 @@ RSpec.describe Namespace do
     end
   end
 
+  describe '#use_traversal_ids_for_ancestors?' do
+    let_it_be(:namespace, reload: true) { create(:namespace) }
+
+    subject { namespace.use_traversal_ids_for_ancestors? }
+
+    context 'when use_traversal_ids_for_ancestors? feature flag is true' do
+      before do
+        stub_feature_flags(use_traversal_ids_for_ancestors: true)
+      end
+
+      it { is_expected.to eq true }
+    end
+
+    context 'when use_traversal_ids_for_ancestors? feature flag is false' do
+      before do
+        stub_feature_flags(use_traversal_ids_for_ancestors: false)
+      end
+
+      it { is_expected.to eq false }
+    end
+
+    context 'when use_traversal_ids? feature flag is false' do
+      before do
+        stub_feature_flags(use_traversal_ids: false)
+      end
+
+      it { is_expected.to eq false }
+    end
+  end
+
   describe '#users_with_descendants' do
     let(:user_a) { create(:user) }
     let(:user_b) { create(:user) }

spec/support/shared_examples/features/cascading_settings_shared_examples.rb

@@ -13,10 +13,22 @@ RSpec.shared_examples 'a cascading setting' do
       click_save_button
     end
 
-    it 'disables setting in subgroups' do
-      visit subgroup_path
+    shared_examples 'subgroup settings are disabled' do
+      it 'disables setting in subgroups' do
+        visit subgroup_path
+
+        expect(find("#{setting_field_selector}[disabled]")).to be_checked
+      end
+    end
+
+    include_examples 'subgroup settings are disabled'
+
+    context 'when use_traversal_ids_for_ancestors is disabled' do
+      before do
+        stub_feature_flags(use_traversal_ids_for_ancestors: false)
+      end
 
-      expect(find("#{setting_field_selector}[disabled]")).to be_checked
+      include_examples 'subgroup settings are disabled'
     end
 
     it 'does not show enforcement checkbox in subgroups' do

spec/support/shared_examples/namespaces/traversal_examples.rb

@@ -12,16 +12,18 @@ RSpec.shared_examples 'namespace traversal' do
 
     it "makes a recursive query" do
       groups.each do |group|
-        expect { group.public_send(recursive_method).load }.to make_queries_matching(/WITH RECURSIVE/)
+        expect { group.public_send(recursive_method).try(:load) }.to make_queries_matching(/WITH RECURSIVE/)
       end
     end
   end
 
-  describe '#root_ancestor' do
-    let_it_be(:group) { create(:group) }
-    let_it_be(:nested_group) { create(:group, parent: group) }
-    let_it_be(:deep_nested_group) { create(:group, parent: nested_group) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:nested_group) { create(:group, parent: group) }
+  let_it_be(:deep_nested_group) { create(:group, parent: nested_group) }
+  let_it_be(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
+  let_it_be(:groups) { [group, nested_group, deep_nested_group, very_deep_nested_group] }
 
+  describe '#root_ancestor' do
     it 'returns the correct root ancestor' do
       expect(group.root_ancestor).to eq(group)
       expect(nested_group.root_ancestor).to eq(group)
@@ -29,8 +31,6 @@ RSpec.shared_examples 'namespace traversal' do
     end
 
     describe '#recursive_root_ancestor' do
-      let(:groups) { [group, nested_group, deep_nested_group] }
-
       it "is equivalent to #recursive_root_ancestor" do
         groups.each do |group|
           expect(group.root_ancestor).to eq(group.recursive_root_ancestor)
@@ -40,12 +40,8 @@ RSpec.shared_examples 'namespace traversal' do
   end
 
   describe '#self_and_hierarchy' do
-    let!(:group) { create(:group, path: 'git_lab') }
-    let!(:nested_group) { create(:group, parent: group) }
-    let!(:deep_nested_group) { create(:group, parent: nested_group) }
-    let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
-    let!(:another_group) { create(:group, path: 'gitllab') }
-    let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) }
+    let!(:another_group) { create(:group) }
+    let!(:another_group_nested) { create(:group, parent: another_group) }
 
     it 'returns the correct tree' do
       expect(group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group)
@@ -54,18 +50,11 @@ RSpec.shared_examples 'namespace traversal' do
     end
 
     describe '#recursive_self_and_hierarchy' do
-      let(:groups) { [group, nested_group, very_deep_nested_group] }
-
       it_behaves_like 'recursive version', :self_and_hierarchy
     end
   end
 
   describe '#ancestors' do
-    let_it_be(:group) { create(:group) }
-    let_it_be(:nested_group) { create(:group, parent: group) }
-    let_it_be(:deep_nested_group) { create(:group, parent: nested_group) }
-    let_it_be(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
-
     it 'returns the correct ancestors' do
       # #reload is called to make sure traversal_ids are reloaded
       expect(very_deep_nested_group.reload.ancestors).to contain_exactly(group, nested_group, deep_nested_group)
@@ -75,18 +64,28 @@ RSpec.shared_examples 'namespace traversal' do
     end
 
     describe '#recursive_ancestors' do
-      let(:groups) { [nested_group, deep_nested_group, very_deep_nested_group] }
+      let_it_be(:groups) { [nested_group, deep_nested_group, very_deep_nested_group] }
 
       it_behaves_like 'recursive version', :ancestors
     end
   end
 
-  describe '#self_and_ancestors' do
-    let(:group) { create(:group) }
-    let(:nested_group) { create(:group, parent: group) }
-    let(:deep_nested_group) { create(:group, parent: nested_group) }
-    let(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
+  describe '#ancestor_ids' do
+    it 'returns the correct ancestor ids' do
+      expect(very_deep_nested_group.ancestor_ids).to contain_exactly(group.id, nested_group.id, deep_nested_group.id)
+      expect(deep_nested_group.ancestor_ids).to contain_exactly(group.id, nested_group.id)
+      expect(nested_group.ancestor_ids).to contain_exactly(group.id)
+      expect(group.ancestor_ids).to be_empty
+    end
+
+    describe '#recursive_ancestor_ids' do
+      let_it_be(:groups) { [nested_group, deep_nested_group, very_deep_nested_group] }
+
+      it_behaves_like 'recursive version', :ancestor_ids
+    end
+  end
 
+  describe '#self_and_ancestors' do
     it 'returns the correct ancestors' do
       expect(very_deep_nested_group.self_and_ancestors).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group)
       expect(deep_nested_group.self_and_ancestors).to contain_exactly(group, nested_group, deep_nested_group)
@@ -95,19 +94,30 @@ RSpec.shared_examples 'namespace traversal' do
     end
 
     describe '#recursive_self_and_ancestors' do
-      let(:groups) { [nested_group, deep_nested_group, very_deep_nested_group] }
+      let_it_be(:groups) { [nested_group, deep_nested_group, very_deep_nested_group] }
 
       it_behaves_like 'recursive version', :self_and_ancestors
     end
   end
 
+  describe '#self_and_ancestor_ids' do
+    it 'returns the correct ancestor ids' do
+      expect(very_deep_nested_group.self_and_ancestor_ids).to contain_exactly(group.id, nested_group.id, deep_nested_group.id, very_deep_nested_group.id)
+      expect(deep_nested_group.self_and_ancestor_ids).to contain_exactly(group.id, nested_group.id, deep_nested_group.id)
+      expect(nested_group.self_and_ancestor_ids).to contain_exactly(group.id, nested_group.id)
+      expect(group.self_and_ancestor_ids).to contain_exactly(group.id)
+    end
+
+    describe '#recursive_self_and_ancestor_ids' do
+      let_it_be(:groups) { [nested_group, deep_nested_group, very_deep_nested_group] }
+
+      it_behaves_like 'recursive version', :self_and_ancestor_ids
+    end
+  end
+
   describe '#descendants' do
-    let!(:group) { create(:group, path: 'git_lab') }
-    let!(:nested_group) { create(:group, parent: group) }
-    let!(:deep_nested_group) { create(:group, parent: nested_group) }
-    let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
-    let!(:another_group) { create(:group, path: 'gitllab') }
-    let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) }
+    let!(:another_group) { create(:group) }
+    let!(:another_group_nested) { create(:group, parent: another_group) }
 
     it 'returns the correct descendants' do
       expect(very_deep_nested_group.descendants.to_a).to eq([])
@@ -117,19 +127,13 @@ RSpec.shared_examples 'namespace traversal' do
     end
 
     describe '#recursive_descendants' do
-      let(:groups) { [group, nested_group, deep_nested_group, very_deep_nested_group] }
-
       it_behaves_like 'recursive version', :descendants
     end
   end
 
   describe '#self_and_descendants' do
-    let!(:group) { create(:group, path: 'git_lab') }
-    let!(:nested_group) { create(:group, parent: group) }
-    let!(:deep_nested_group) { create(:group, parent: nested_group) }
-    let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
-    let!(:another_group) { create(:group, path: 'gitllab') }
-    let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) }
+    let!(:another_group) { create(:group) }
+    let!(:another_group_nested) { create(:group, parent: another_group) }
 
     it 'returns the correct descendants' do
       expect(very_deep_nested_group.self_and_descendants).to contain_exactly(very_deep_nested_group)
@@ -139,24 +143,18 @@ RSpec.shared_examples 'namespace traversal' do
     end
 
     describe '#recursive_self_and_descendants' do
-      let(:groups) { [group, nested_group, deep_nested_group, very_deep_nested_group] }
+      let_it_be(:groups) { [group, nested_group, deep_nested_group] }
 
       it_behaves_like 'recursive version', :self_and_descendants
     end
   end
 
   describe '#self_and_descendant_ids' do
-    let!(:group) { create(:group, path: 'git_lab') }
-    let!(:nested_group) { create(:group, parent: group) }
-    let!(:deep_nested_group) { create(:group, parent: nested_group) }
-
     subject { group.self_and_descendant_ids.pluck(:id) }
 
-    it { is_expected.to contain_exactly(group.id, nested_group.id, deep_nested_group.id) }
+    it { is_expected.to contain_exactly(group.id, nested_group.id, deep_nested_group.id, very_deep_nested_group.id) }
 
     describe '#recursive_self_and_descendant_ids' do
-      let(:groups) { [group, nested_group, deep_nested_group] }
-
       it_behaves_like 'recursive version', :self_and_descendant_ids
     end
   end