Merge branch 'dz-limit-nested-groups' into 'master'

Limit level of nesting for groups

See merge request !9000

app/models/namespace.rb

@@ -7,6 +7,11 @@ class Namespace < ActiveRecord::Base
   include Gitlab::CurrentSettings
   include Routable
 
+  # Prevent users from creating unreasonably deep level of nesting.
+  # The number 20 was taken based on maximum nesting level of
+  # Android repo (15) + some extra backup.
+  NUMBER_OF_ANCESTORS_ALLOWED = 20
+
   cache_markdown_field :description, pipeline: :description
 
   has_many :projects, dependent: :destroy
@@ -29,6 +34,8 @@ class Namespace < ActiveRecord::Base
     length: { maximum: 255 },
     namespace: true
 
+  validate :nesting_level_allowed
+
   delegate :name, to: :owner, allow_nil: true, prefix: true
 
   after_update :move_dir, if: :path_changed?
@@ -194,7 +201,7 @@ class Namespace < ActiveRecord::Base
   # Scopes the model on ancestors of the record
   def ancestors
     if parent_id
-      path = route.path
+      path = route ? route.path : full_path
       paths = []
 
       until path.blank?
@@ -274,4 +281,10 @@ class Namespace < ActiveRecord::Base
       path_was
     end
   end
+
+  def nesting_level_allowed
+    if ancestors.count > Group::NUMBER_OF_ANCESTORS_ALLOWED
+      errors.add(:parent_id, "has too deep level of nesting")
+    end
+  end
 end

spec/models/namespace_spec.rb

@@ -3,21 +3,32 @@ require 'spec_helper'
 describe Namespace, models: true do
   let!(:namespace) { create(:namespace) }
 
-  it { is_expected.to have_many :projects }
-  it { is_expected.to have_many :project_statistics }
-  it { is_expected.to belong_to :parent }
-  it { is_expected.to have_many :children }
+  describe 'associations' do
+    it { is_expected.to have_many :projects }
+    it { is_expected.to have_many :project_statistics }
+    it { is_expected.to belong_to :parent }
+    it { is_expected.to have_many :children }
+  end
 
-  it { is_expected.to validate_presence_of(:name) }
-  it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) }
-  it { is_expected.to validate_length_of(:name).is_at_most(255) }
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:name) }
+    it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) }
+    it { is_expected.to validate_length_of(:name).is_at_most(255) }
+    it { is_expected.to validate_length_of(:description).is_at_most(255) }
+    it { is_expected.to validate_presence_of(:path) }
+    it { is_expected.to validate_length_of(:path).is_at_most(255) }
+    it { is_expected.to validate_presence_of(:owner) }
 
-  it { is_expected.to validate_length_of(:description).is_at_most(255) }
+    it 'does not allow too deep nesting' do
+      ancestors = (1..21).to_a
+      nested = build(:namespace, parent: namespace)
 
-  it { is_expected.to validate_presence_of(:path) }
-  it { is_expected.to validate_length_of(:path).is_at_most(255) }
+      allow(nested).to receive(:ancestors).and_return(ancestors)
 
-  it { is_expected.to validate_presence_of(:owner) }
+      expect(nested).not_to be_valid
+      expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting')
+    end
+  end
 
   describe "Respond to" do
     it { is_expected.to respond_to(:human_name) }