Merge branch 'remove-nested-groups-checks' into 'master'

Remove code related to object hierarchy and MySQL

Closes #65056 and #65055

See merge request gitlab-org/gitlab-ce!31095

app/controllers/application_controller.rb

@@ -421,7 +421,7 @@ class ApplicationController < ActionController::Base
   end
   def manifest_import_enabled?
-    Group.supports_nested_objects? && Gitlab::CurrentSettings.import_sources.include?('manifest')
+    Gitlab::CurrentSettings.import_sources.include?('manifest')
   end
   def phabricator_import_enabled?

app/controllers/concerns/group_tree.rb

@@ -32,18 +32,14 @@ module GroupTree
   def filtered_groups_with_ancestors(groups)
     filtered_groups = groups.search(params[:filter]).page(params[:page])
-    if Group.supports_nested_objects?
-      # We find the ancestors by ID of the search results here.
-      # Otherwise the ancestors would also have filters applied,
-      # which would cause them not to be preloaded.
-      #
-      # Pagination needs to be applied before loading the ancestors to
-      # make sure ancestors are not cut off by pagination.
-      Gitlab::ObjectHierarchy.new(Group.where(id: filtered_groups.select(:id)))
-        .base_and_ancestors
-    else
-      filtered_groups
-    end
+    # We find the ancestors by ID of the search results here.
+    # Otherwise the ancestors would also have filters applied,
+    # which would cause them not to be preloaded.
+    #
+    # Pagination needs to be applied before loading the ancestors to
+    # make sure ancestors are not cut off by pagination.
+    Gitlab::ObjectHierarchy.new(Group.where(id: filtered_groups.select(:id)))
+      .base_and_ancestors
   end
   # rubocop: enable CodeReuse/ActiveRecord
 end

app/finders/group_descendants_finder.rb

@@ -132,8 +132,6 @@ class GroupDescendantsFinder
   end
   def subgroups
-    return Group.none unless Group.supports_nested_objects?
-
     # When filtering subgroups, we want to find all matches within the tree of
     # descendants to show to the user
     groups = if params[:filter]

app/graphql/types/group_type.rb

@@ -14,10 +14,8 @@ module Types
       group.avatar_url(only_path: false)
     end
-    if ::Group.supports_nested_objects?
-      field :parent, GroupType,
-            null: true,
-            resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Group, obj.parent_id).find }
-    end
+    field :parent, GroupType,
+          null: true,
+          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Group, obj.parent_id).find }
   end
 end

app/helpers/groups_helper.rb

@@ -127,10 +127,6 @@ module GroupsHelper
     groups.to_json
   end
-  def supports_nested_groups?
-    Group.supports_nested_objects?
-  end
-
   private
   def get_group_sidebar_links

app/models/concerns/descendant.rb

-# frozen_string_literal: true
-
-module Descendant
-  extend ActiveSupport::Concern
-
-  class_methods do
-    def supports_nested_objects?
-      Gitlab::Database.postgresql?
-    end
-  end
-end

app/models/group.rb

@@ -10,7 +10,6 @@ class Group < Namespace
   include Referable
   include SelectForProjectAuthorization
   include LoadedInGroupList
-  include Descendant
   include GroupDescendant
   include TokenAuthenticatable
   include WithUploads

app/models/namespace.rb

@@ -332,8 +332,6 @@ class Namespace < ApplicationRecord
   end
   def force_share_with_group_lock_on_descendants
-    return unless Group.supports_nested_objects?
-
     # We can't use `descendants.update_all` since Rails will throw away the WITH
     # RECURSIVE statement. We also can't use WHERE EXISTS since we can't use
     # different table aliases, hence we're just using WHERE IN. Since we have a

app/policies/group_policy.rb

@@ -16,8 +16,6 @@ class GroupPolicy < BasePolicy
   condition(:maintainer) { access_level >= GroupMember::MAINTAINER }
   condition(:reporter) { access_level >= GroupMember::REPORTER }
-  condition(:nested_groups_supported, scope: :global) { Group.supports_nested_objects? }
-
   condition(:has_parent, scope: :subject) { @subject.has_parent? }
   condition(:share_with_group_locked, scope: :subject) { @subject.share_with_group_lock? }
   condition(:parent_share_with_group_locked, scope: :subject) { @subject.parent&.share_with_group_lock? }
@@ -108,8 +106,8 @@ class GroupPolicy < BasePolicy
     enable :read_nested_project_resources
   end
-  rule { owner & nested_groups_supported }.enable :create_subgroup
-  rule { maintainer & maintainer_can_create_group & nested_groups_supported }.enable :create_subgroup
+  rule { owner }.enable :create_subgroup
+  rule { maintainer & maintainer_can_create_group }.enable :create_subgroup
   rule { public_group | logged_in_viewable }.enable :view_globally

app/services/groups/nested_create_service.rb

@@ -18,10 +18,6 @@ module Groups
         return namespace
       end
-      if group_path.include?('/') && !Group.supports_nested_objects?
-        raise 'Nested groups are not supported on MySQL'
-      end
-
       create_group_path
     end

app/services/groups/transfer_service.rb

@@ -43,7 +43,6 @@ module Groups
     def ensure_allowed_transfer
       raise_transfer_error(:group_is_already_root) if group_is_already_root?
-      raise_transfer_error(:database_not_supported) unless Group.supports_nested_objects?
       raise_transfer_error(:same_parent_as_current) if same_parent?
       raise_transfer_error(:invalid_policies) unless valid_policies?
       raise_transfer_error(:namespace_with_same_path) if namespace_with_same_path?

app/services/members/destroy_service.rb

@@ -31,7 +31,7 @@ module Members
       return unless member.is_a?(GroupMember) && member.user && member.group
       delete_project_members(member)
-      delete_subgroup_members(member) if Group.supports_nested_objects?
+      delete_subgroup_members(member)
     end
     def delete_project_members(member)

app/services/users/refresh_authorized_projects_service.rb

@@ -102,13 +102,7 @@ module Users
     end
     def fresh_authorizations
-      klass = if Group.supports_nested_objects?
-                Gitlab::ProjectAuthorizations::WithNestedGroups
-              else
-                Gitlab::ProjectAuthorizations::WithoutNestedGroups
-              end
-
-      klass.new(user).calculate
+      Gitlab::ProjectAuthorizations.new(user).calculate
     end
   end
 end

app/views/groups/settings/_advanced.html.haml

@@ -23,20 +23,19 @@
     = f.submit 'Change group path', class: 'btn btn-warning'
-- if supports_nested_groups?
-  .sub-section
-    %h4.warning-title Transfer group
-    = form_for @group, url: transfer_group_path(@group), method: :put do |f|
-      .form-group
-        = dropdown_tag('Select parent group', options: { toggle_class: 'js-groups-dropdown', title: 'Parent Group', filter: true, dropdown_class: 'dropdown-open-top dropdown-group-transfer', placeholder: 'Search groups', data: { data: parent_group_options(@group) } })
-        = hidden_field_tag 'new_parent_group_id'
-      %ul
-        %li Be careful. Changing a group's parent can have unintended #{link_to 'side effects', 'https://docs.gitlab.com/ce/user/project/index.html#redirects-when-changing-repository-paths', target: 'blank'}.
-        %li You can only transfer the group to a group you manage.
-        %li You will need to update your local repositories to point to the new location.
-        %li If the parent group's visibility is lower than the group current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility.
-      = f.submit 'Transfer group', class: 'btn btn-warning'
+.sub-section
+  %h4.warning-title Transfer group
+  = form_for @group, url: transfer_group_path(@group), method: :put do |f|
+    .form-group
+      = dropdown_tag('Select parent group', options: { toggle_class: 'js-groups-dropdown', title: 'Parent Group', filter: true, dropdown_class: 'dropdown-open-top dropdown-group-transfer', placeholder: 'Search groups', data: { data: parent_group_options(@group) } })
+      = hidden_field_tag 'new_parent_group_id'
+    %ul
+      %li Be careful. Changing a group's parent can have unintended #{link_to 'side effects', 'https://docs.gitlab.com/ce/user/project/index.html#redirects-when-changing-repository-paths', target: 'blank'}.
+      %li You can only transfer the group to a group you manage.
+      %li You will need to update your local repositories to point to the new location.
+      %li If the parent group's visibility is lower than the group current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility.
+    = f.submit 'Transfer group', class: 'btn btn-warning'
 .sub-section
   %h4.danger-title= _('Remove group')

lib/api/entities.rb

@@ -366,10 +366,7 @@ module API
       end
       expose :request_access_enabled
       expose :full_name, :full_path
-
-      if ::Group.supports_nested_objects?
-        expose :parent_id
-      end
+      expose :parent_id
       expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes

lib/api/groups.rb

@@ -114,10 +114,7 @@ module API
       params do
         requires :name, type: String, desc: 'The name of the group'
         requires :path, type: String, desc: 'The path of the group'
-
-        if ::Group.supports_nested_objects?
-          optional :parent_id, type: Integer, desc: 'The parent group id for creating nested group'
-        end
+        optional :parent_id, type: Integer, desc: 'The parent group id for creating nested group'
         use :optional_params
       end

lib/gitlab/object_hierarchy.rb

@@ -32,11 +32,6 @@ module Gitlab
     # Returns the maximum depth starting from the base
     # A base object with no children has a maximum depth of `1`
     def max_descendants_depth
-      unless hierarchy_supported?
-        # This makes the return value consistent with the case where hierarchy is supported
-        return descendants_base.exists? ? 1 : nil
-      end
-
       base_and_descendants(with_depth: true).maximum(DEPTH_COLUMN)
     end
@@ -66,8 +61,6 @@ module Gitlab
     # each parent.
     # rubocop: disable CodeReuse/ActiveRecord
     def base_and_ancestors(upto: nil, hierarchy_order: nil)
-      return ancestors_base unless hierarchy_supported?
-
       recursive_query = base_and_ancestors_cte(upto, hierarchy_order).apply_to(model.all)
       recursive_query = recursive_query.order(depth: hierarchy_order) if hierarchy_order
@@ -81,10 +74,6 @@ module Gitlab
     # When `with_depth` is `true`, a `depth` column is included where it starts with `1` for the base objects
     # and incremented as we go down the descendant tree
     def base_and_descendants(with_depth: false)
-      unless hierarchy_supported?
-        return with_depth ? descendants_base.select("1 as #{DEPTH_COLUMN}", objects_table[Arel.star]) : descendants_base
-      end
-
       read_only(base_and_descendants_cte(with_depth: with_depth).apply_to(model.all))
     end
@@ -112,8 +101,6 @@ module Gitlab
     # If nested objects are not supported, ancestors_base is returned.
     # rubocop: disable CodeReuse/ActiveRecord
     def all_objects
-      return ancestors_base unless hierarchy_supported?
-
       ancestors = base_and_ancestors_cte
       descendants = base_and_descendants_cte
@@ -135,10 +122,6 @@ module Gitlab
     private
-    def hierarchy_supported?
-      Gitlab::Database.postgresql?
-    end
-
     # rubocop: disable CodeReuse/ActiveRecord
     def base_and_ancestors_cte(stop_id = nil, hierarchy_order = nil)
       cte = SQL::RecursiveCTE.new(:base_and_ancestors)

lib/gitlab/project_authorizations.rb

+# frozen_string_literal: true
+
+# This class relies on Common Table Expressions to efficiently get all data,
+# including data for nested groups.
+module Gitlab
+  class ProjectAuthorizations
+    attr_reader :user
+
+    # user - The User object for which to calculate the authorizations.
+    def initialize(user)
+      @user = user
+    end
+
+    def calculate
+      cte = recursive_cte
+      cte_alias = cte.table.alias(Group.table_name)
+      projects = Project.arel_table
+      links = ProjectGroupLink.arel_table
+
+      relations = [
+        # The project a user has direct access to.
+        user.projects.select_for_project_authorization,
+
+        # The personal projects of the user.
+        user.personal_projects.select_as_maintainer_for_project_authorization,
+
+        # Projects that belong directly to any of the groups the user has
+        # access to.
+        Namespace
+          .unscoped
+          .select([alias_as_column(projects[:id], 'project_id'),
+                   cte_alias[:access_level]])
+          .from(cte_alias)
+          .joins(:projects),
+
+        # Projects shared with any of the namespaces the user has access to.
+        Namespace
+          .unscoped
+          .select([
+            links[:project_id],
+            least(cte_alias[:access_level], links[:group_access], 'access_level')
+          ])
+          .from(cte_alias)
+          .joins('INNER JOIN project_group_links ON project_group_links.group_id = namespaces.id')
+          .joins('INNER JOIN projects ON projects.id = project_group_links.project_id')
+          .joins('INNER JOIN namespaces p_ns ON p_ns.id = projects.namespace_id')
+          .where('p_ns.share_with_group_lock IS FALSE')
+      ]
+
+      ProjectAuthorization
+        .unscoped
+        .with
+        .recursive(cte.to_arel)
+        .select_from_union(relations)
+    end
+
+    private
+
+    # Builds a recursive CTE that gets all the groups the current user has
+    # access to, including any nested groups.
+    def recursive_cte
+      cte = Gitlab::SQL::RecursiveCTE.new(:namespaces_cte)
+      members = Member.arel_table
+      namespaces = Namespace.arel_table
+
+      # Namespaces the user is a member of.
+      cte << user.groups
+        .select([namespaces[:id], members[:access_level]])
+        .except(:order)
+
+      # Sub groups of any groups the user is a member of.
+      cte << Group.select([
+          namespaces[:id],
+          greatest(members[:access_level], cte.table[:access_level], 'access_level')
+        ])
+        .joins(join_cte(cte))
+        .joins(join_members)
+        .except(:order)
+
+      cte
+    end
+
+    # Builds a LEFT JOIN to join optional memberships onto the CTE.
+    def join_members
+      members = Member.arel_table
+      namespaces = Namespace.arel_table
+
+      cond = members[:source_id]
+        .eq(namespaces[:id])
+        .and(members[:source_type].eq('Namespace'))
+        .and(members[:requested_at].eq(nil))
+        .and(members[:user_id].eq(user.id))
+
+      Arel::Nodes::OuterJoin.new(members, Arel::Nodes::On.new(cond))
+    end
+
+    # Builds an INNER JOIN to join namespaces onto the CTE.
+    def join_cte(cte)
+      namespaces = Namespace.arel_table
+      cond = cte.table[:id].eq(namespaces[:parent_id])
+
+      Arel::Nodes::InnerJoin.new(cte.table, Arel::Nodes::On.new(cond))
+    end
+
+    def greatest(left, right, column_alias)
+      sql_function('GREATEST', [left, right], column_alias)
+    end
+
+    def least(left, right, column_alias)
+      sql_function('LEAST', [left, right], column_alias)
+    end
+
+    def sql_function(name, args, column_alias)
+      alias_as_column(Arel::Nodes::NamedFunction.new(name, args), column_alias)
+    end
+
+    def alias_as_column(value, alias_to)
+      Arel::Nodes::As.new(value, Arel::Nodes::SqlLiteral.new(alias_to))
+    end
+  end
+end

lib/gitlab/project_authorizations/with_nested_groups.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module ProjectAuthorizations
-    # Calculating new project authorizations when supporting nested groups.
-    #
-    # This class relies on Common Table Expressions to efficiently get all data,
-    # including data for nested groups. As a result this class can only be used
-    # on PostgreSQL.
-    class WithNestedGroups
-      attr_reader :user
-
-      # user - The User object for which to calculate the authorizations.
-      def initialize(user)
-        @user = user
-      end
-
-      def calculate
-        cte = recursive_cte
-        cte_alias = cte.table.alias(Group.table_name)
-        projects = Project.arel_table
-        links = ProjectGroupLink.arel_table
-
-        relations = [
-          # The project a user has direct access to.
-          user.projects.select_for_project_authorization,
-
-          # The personal projects of the user.
-          user.personal_projects.select_as_maintainer_for_project_authorization,
-
-          # Projects that belong directly to any of the groups the user has
-          # access to.
-          Namespace
-            .unscoped
-            .select([alias_as_column(projects[:id], 'project_id'),
-                     cte_alias[:access_level]])
-            .from(cte_alias)
-            .joins(:projects),
-
-          # Projects shared with any of the namespaces the user has access to.
-          Namespace
-            .unscoped
-            .select([links[:project_id],
-                     least(cte_alias[:access_level],
-                           links[:group_access],
-                           'access_level')])
-            .from(cte_alias)
-            .joins('INNER JOIN project_group_links ON project_group_links.group_id = namespaces.id')
-            .joins('INNER JOIN projects ON projects.id = project_group_links.project_id')
-            .joins('INNER JOIN namespaces p_ns ON p_ns.id = projects.namespace_id')
-            .where('p_ns.share_with_group_lock IS FALSE')
-        ]
-
-        ProjectAuthorization
-          .unscoped
-          .with
-          .recursive(cte.to_arel)
-          .select_from_union(relations)
-      end
-
-      private
-
-      # Builds a recursive CTE that gets all the groups the current user has
-      # access to, including any nested groups.
-      def recursive_cte
-        cte = Gitlab::SQL::RecursiveCTE.new(:namespaces_cte)
-        members = Member.arel_table
-        namespaces = Namespace.arel_table
-
-        # Namespaces the user is a member of.
-        cte << user.groups
-          .select([namespaces[:id], members[:access_level]])
-          .except(:order)
-
-        # Sub groups of any groups the user is a member of.
-        cte << Group.select([namespaces[:id],
-                             greatest(members[:access_level],
-                                      cte.table[:access_level], 'access_level')])
-          .joins(join_cte(cte))
-          .joins(join_members)
-          .except(:order)
-
-        cte
-      end
-
-      # Builds a LEFT JOIN to join optional memberships onto the CTE.
-      def join_members
-        members = Member.arel_table
-        namespaces = Namespace.arel_table
-
-        cond = members[:source_id]
-          .eq(namespaces[:id])
-          .and(members[:source_type].eq('Namespace'))
-          .and(members[:requested_at].eq(nil))
-          .and(members[:user_id].eq(user.id))
-
-        Arel::Nodes::OuterJoin.new(members, Arel::Nodes::On.new(cond))
-      end
-
-      # Builds an INNER JOIN to join namespaces onto the CTE.
-      def join_cte(cte)
-        namespaces = Namespace.arel_table
-        cond = cte.table[:id].eq(namespaces[:parent_id])
-
-        Arel::Nodes::InnerJoin.new(cte.table, Arel::Nodes::On.new(cond))
-      end
-
-      def greatest(left, right, column_alias)
-        sql_function('GREATEST', [left, right], column_alias)
-      end
-
-      def least(left, right, column_alias)
-        sql_function('LEAST', [left, right], column_alias)
-      end
-
-      def sql_function(name, args, column_alias)
-        alias_as_column(Arel::Nodes::NamedFunction.new(name, args), column_alias)
-      end
-
-      def alias_as_column(value, alias_to)
-        Arel::Nodes::As.new(value, Arel::Nodes::SqlLiteral.new(alias_to))
-      end
-    end
-  end
-end

lib/gitlab/project_authorizations/without_nested_groups.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module ProjectAuthorizations
-    # Calculating new project authorizations when not supporting nested groups.
-    class WithoutNestedGroups
-      attr_reader :user
-
-      # user - The User object for which to calculate the authorizations.
-      def initialize(user)
-        @user = user
-      end
-
-      def calculate
-        relations = [
-          # Projects the user is a direct member of
-          user.projects.select_for_project_authorization,
-
-          # Personal projects
-          user.personal_projects.select_as_maintainer_for_project_authorization,
-
-          # Projects of groups the user is a member of
-          user.groups_projects.select_for_project_authorization,
-
-          # Projects shared with groups the user is a member of
-          user.groups.joins(:shared_projects).select_for_project_authorization
-        ]
-
-        ProjectAuthorization
-          .unscoped
-          .select_from_union(relations)
-      end
-    end
-  end
-end