From df9045fddfb740142aa93df0555aac12749412a2 Mon Sep 17 00:00:00 2001
From: Heinrich Lee Yu <heinrich@gitlab.com>
Date: Wed, 4 Aug 2021 16:03:32 +0000
Subject: [PATCH] Revert "Merge branch '335733-recursive-ci-runners-query' into
'master'"
This reverts merge request !65891
---
app/models/namespaces/traversal/linear.rb | 19 +++++-
.../namespaces/traversal/linear_scopes.rb | 59 -------------------
app/models/namespaces/traversal/recursive.rb | 1 -
.../namespaces/traversal/recursive_scopes.rb | 25 --------
spec/models/namespace_spec.rb | 16 +----
.../namespaces/linear_traversal_examples.rb | 23 ++++++++
.../namespaces/traversal_scope_examples.rb | 51 ----------------
7 files changed, 40 insertions(+), 154 deletions(-)
delete mode 100644 app/models/namespaces/traversal/linear_scopes.rb
delete mode 100644 app/models/namespaces/traversal/recursive_scopes.rb
create mode 100644 spec/support/shared_examples/namespaces/linear_traversal_examples.rb
delete mode 100644 spec/support/shared_examples/namespaces/traversal_scope_examples.rb
diff --git a/app/models/namespaces/traversal/linear.rb b/app/models/namespaces/traversal/linear.rb
index 79df466fd641b..3d78f3846348e 100644
--- a/app/models/namespaces/traversal/linear.rb
+++ b/app/models/namespaces/traversal/linear.rb
@@ -37,7 +37,6 @@ module Namespaces
module Traversal
module Linear
extend ActiveSupport::Concern
- include LinearScopes
UnboundedSearch = Class.new(StandardError)
@@ -45,6 +44,14 @@ module Linear
before_update :lock_both_roots, if: -> { sync_traversal_ids? && parent_id_changed? }
after_create :sync_traversal_ids, if: -> { sync_traversal_ids? }
after_update :sync_traversal_ids, if: -> { sync_traversal_ids? && saved_change_to_parent_id? }
+
+ scope :traversal_ids_contains, ->(ids) { where("traversal_ids @> (?)", ids) }
+ # When filtering namespaces by the traversal_ids column to compile a
+ # list of namespace IDs, it's much faster to reference the ID in
+ # traversal_ids than the primary key ID column.
+ # WARNING This scope must be used behind a linear query feature flag
+ # such as `use_traversal_ids`.
+ scope :as_ids, -> { select('traversal_ids[array_length(traversal_ids, 1)] AS id') }
end
def sync_traversal_ids?
@@ -157,14 +164,20 @@ def lock_both_roots
Namespace.lock.select(:id).where(id: roots).order(id: :asc).load
end
+ # Make sure we drop the STI `type = 'Group'` condition for better performance.
+ # Logically equivalent so long as hierarchies remain homogeneous.
+ def without_sti_condition
+ self.class.unscope(where: :type)
+ end
+
# Search this namespace's lineage. Bound inclusively by top node.
def lineage(top: nil, bottom: nil, hierarchy_order: nil)
raise UnboundedSearch, 'Must bound search by either top or bottom' unless top || bottom
- skope = self.class.without_sti_condition
+ skope = without_sti_condition
if top
- skope = skope.where("traversal_ids @> ('{?}')", top.id)
+ skope = skope.traversal_ids_contains("{#{top.id}}")
end
if bottom
diff --git a/app/models/namespaces/traversal/linear_scopes.rb b/app/models/namespaces/traversal/linear_scopes.rb
deleted file mode 100644
index f352497e6deca..0000000000000
--- a/app/models/namespaces/traversal/linear_scopes.rb
+++ /dev/null
@@ -1,59 +0,0 @@
-# frozen_string_literal: true
-
-module Namespaces
- module Traversal
- module LinearScopes
- extend ActiveSupport::Concern
-
- class_methods do
- # When filtering namespaces by the traversal_ids column to compile a
- # list of namespace IDs, it can be faster to reference the ID in
- # traversal_ids than the primary key ID column.
- def as_ids
- return super unless use_traversal_ids?
-
- select('namespaces.traversal_ids[array_length(namespaces.traversal_ids, 1)] AS id')
- end
-
- def self_and_descendants
- return super unless use_traversal_ids?
-
- without_dups = self_and_descendants_with_duplicates
- .select('DISTINCT on(namespaces.id) namespaces.*')
-
- # Wrap the `SELECT DISTINCT on(....)` with a normal query so we
- # retain expected Rails behavior. Otherwise count and other
- # aggregates won't work.
- unscoped.without_sti_condition.from(without_dups, :namespaces)
- end
-
- def self_and_descendant_ids
- return super unless use_traversal_ids?
-
- self_and_descendants_with_duplicates.select('DISTINCT namespaces.id')
- end
-
- # Make sure we drop the STI `type = 'Group'` condition for better performance.
- # Logically equivalent so long as hierarchies remain homogeneous.
- def without_sti_condition
- unscope(where: :type)
- end
-
- private
-
- def use_traversal_ids?
- Feature.enabled?(:use_traversal_ids, default_enabled: :yaml)
- end
-
- def self_and_descendants_with_duplicates
- base_ids = select(:id)
-
- unscoped
- .without_sti_condition
- .from("namespaces, (#{base_ids.to_sql}) base")
- .where('namespaces.traversal_ids @> ARRAY[base.id]')
- end
- end
- end
- end
-end
diff --git a/app/models/namespaces/traversal/recursive.rb b/app/models/namespaces/traversal/recursive.rb
index c1ada715d6dcb..d9e8743aa50a3 100644
--- a/app/models/namespaces/traversal/recursive.rb
+++ b/app/models/namespaces/traversal/recursive.rb
@@ -4,7 +4,6 @@ module Namespaces
module Traversal
module Recursive
extend ActiveSupport::Concern
- include RecursiveScopes
def root_ancestor
return self if parent.nil?
diff --git a/app/models/namespaces/traversal/recursive_scopes.rb b/app/models/namespaces/traversal/recursive_scopes.rb
deleted file mode 100644
index 0dcb23b656743..0000000000000
--- a/app/models/namespaces/traversal/recursive_scopes.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-# frozen_string_literal: true
-
-module Namespaces
- module Traversal
- module RecursiveScopes
- extend ActiveSupport::Concern
-
- class_methods do
- def as_ids
- select('id')
- end
-
- def self_and_descendants
- Gitlab::ObjectHierarchy.new(all).base_and_descendants
- end
- alias_method :recursive_self_and_descendants, :self_and_descendants
-
- def self_and_descendant_ids
- self_and_descendants.as_ids
- end
- alias_method :recursive_self_and_descendant_ids, :self_and_descendant_ids
- end
- end
- end
-end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index e2700378f5f6a..b945fa34ebb4a 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -208,23 +208,9 @@
it { is_expected.to include_module(Gitlab::VisibilityLevel) }
it { is_expected.to include_module(Namespaces::Traversal::Recursive) }
it { is_expected.to include_module(Namespaces::Traversal::Linear) }
- it { is_expected.to include_module(Namespaces::Traversal::RecursiveScopes) }
- it { is_expected.to include_module(Namespaces::Traversal::LinearScopes) }
end
- context 'traversal scopes' do
- context 'recursive' do
- before do
- stub_feature_flags(use_traversal_ids: false)
- end
-
- it_behaves_like 'namespace traversal scopes'
- end
-
- context 'linear' do
- it_behaves_like 'namespace traversal scopes'
- end
- end
+ it_behaves_like 'linear namespace traversal'
context 'traversal_ids on create' do
context 'default traversal_ids' do
diff --git a/spec/support/shared_examples/namespaces/linear_traversal_examples.rb b/spec/support/shared_examples/namespaces/linear_traversal_examples.rb
new file mode 100644
index 0000000000000..2fd90c36953b1
--- /dev/null
+++ b/spec/support/shared_examples/namespaces/linear_traversal_examples.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+# Traversal examples common to linear and recursive methods are in
+# spec/support/shared_examples/namespaces/traversal_examples.rb
+
+RSpec.shared_examples 'linear namespace traversal' do
+ context 'when use_traversal_ids feature flag is enabled' do
+ before do
+ stub_feature_flags(use_traversal_ids: true)
+ end
+
+ context 'scopes' do
+ describe '.as_ids' do
+ let_it_be(:namespace1) { create(:group) }
+ let_it_be(:namespace2) { create(:group) }
+
+ subject { Namespace.where(id: [namespace1, namespace2]).as_ids.pluck(:id) }
+
+ it { is_expected.to contain_exactly(namespace1.id, namespace2.id) }
+ end
+ end
+ end
+end
diff --git a/spec/support/shared_examples/namespaces/traversal_scope_examples.rb b/spec/support/shared_examples/namespaces/traversal_scope_examples.rb
deleted file mode 100644
index bc17d104034fc..0000000000000
--- a/spec/support/shared_examples/namespaces/traversal_scope_examples.rb
+++ /dev/null
@@ -1,51 +0,0 @@
-# frozen_string_literal: true
-
-RSpec.shared_examples 'namespace traversal scopes' do
- # Hierarchy 1
- let_it_be(:group_1) { create(:group) }
- let_it_be(:nested_group_1) { create(:group, parent: group_1) }
- let_it_be(:deep_nested_group_1) { create(:group, parent: nested_group_1) }
-
- # Hierarchy 2
- let_it_be(:group_2) { create(:group) }
- let_it_be(:nested_group_2) { create(:group, parent: group_2) }
- let_it_be(:deep_nested_group_2) { create(:group, parent: nested_group_2) }
-
- # All groups
- let_it_be(:groups) do
- [
- group_1, nested_group_1, deep_nested_group_1,
- group_2, nested_group_2, deep_nested_group_2
- ]
- end
-
- describe '.as_ids' do
- subject { described_class.where(id: [group_1, group_2]).as_ids.pluck(:id) }
-
- it { is_expected.to contain_exactly(group_1.id, group_2.id) }
- end
-
- describe '.without_sti_condition' do
- subject { described_class.without_sti_condition }
-
- it { expect(subject.where_values_hash).not_to have_key(:type) }
- end
-
- describe '.self_and_descendants' do
- subject { described_class.where(id: [nested_group_1, nested_group_2]).self_and_descendants }
-
- it { is_expected.to contain_exactly(nested_group_1, deep_nested_group_1, nested_group_2, deep_nested_group_2) }
-
- context 'with duplicate descendants' do
- subject { described_class.where(id: [group_1, group_2, nested_group_1]).self_and_descendants }
-
- it { is_expected.to match_array(groups) }
- end
- end
-
- describe '.self_and_descendant_ids' do
- subject { described_class.where(id: [nested_group_1, nested_group_2]).self_and_descendant_ids.pluck(:id) }
-
- it { is_expected.to contain_exactly(nested_group_1.id, deep_nested_group_1.id, nested_group_2.id, deep_nested_group_2.id) }
- end
-end
--
GitLab