Streamline the path validation in groups & projects

`Project` uses `ProjectPathValidator` which is now a
`NamespaceValidator` that skips the format validation.

That way we're sure we are using the same collection of reserved
paths.

I updated the path constraints to reflect the changes: We now allow
some values that are only used on a top level namespace as a name for
a nested group/project.

4 files changed, 74 insertions, 0 deletions

diff --git a/spec/lib/constraints/group_url_constrainer_spec.rb b/spec/lib/constraints/group_url_constrainer_spec.rb
index 96dacdc5cd2..f95adf3a84b 100644
--- a/spec/lib/constraints/group_url_constrainer_spec.rb
+++ b/spec/lib/constraints/group_url_constrainer_spec.rb
@@ -17,6 +17,13 @@ describe GroupUrlConstrainer, lib: true do
       it { expect(subject.matches?(request)).to be_truthy }
     end
 
+    context 'valid request for nested group with reserved top level name' do
+      let!(:nested_group) { create(:group, path: 'api', parent: group) }
+      let!(:request) { build_request('gitlab/api') }
+
+      it { expect(subject.matches?(request)).to be_truthy }
+    end
+
     context 'invalid request' do
       let(:request) { build_request('foo') }
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 8ffde6f7fbb..7b076d09585 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -57,6 +57,26 @@ describe Group, models: true do
     it { is_expected.not_to validate_presence_of :owner }
     it { is_expected.to validate_presence_of :two_factor_grace_period }
     it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) }
+
+    describe 'path validation' do
+      it 'rejects paths reserved on the root namespace when the group has no parent' do
+        group = build(:group, path: 'api')
+
+        expect(group).not_to be_valid
+      end
+
+      it 'allows root paths when the group has a parent' do
+        group = build(:group, path: 'api', parent: create(:group))
+
+        expect(group).to be_valid
+      end
+
+      it 'rejects any wildcard paths when not a top level group' do
+        group = build(:group, path: 'tree', parent: create(:group))
+
+        expect(group).not_to be_valid
+      end
+    end
   end
 
   describe '.visible_to_user' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 92d420337f9..726dade93f6 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -253,6 +253,20 @@ describe Project, models: true do
         expect(new_project.errors.full_messages.first).to eq('The project is still being deleted. Please try again later.')
       end
     end
+
+    describe 'path validation' do
+      it 'allows paths reserved on the root namespace' do
+        project = build(:project, path: 'api')
+
+        expect(project).to be_valid
+      end
+
+      it 'rejects paths reserved on another level' do
+        project = build(:project, path: 'tree')
+
+        expect(project).not_to be_valid
+      end
+    end
   end
 
   describe 'default_scope' do
diff --git a/spec/validators/namespace_validator_spec.rb b/spec/validators/namespace_validator_spec.rb
index e21b8ef5abd..4b5dd59e048 100644
--- a/spec/validators/namespace_validator_spec.rb
+++ b/spec/validators/namespace_validator_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe NamespaceValidator do
+  let(:validator) { described_class.new(attributes: [:path]) }
   describe 'RESERVED' do
     it 'includes all the top level namespaces' do
       all_top_level_routes = Rails.application.routes.routes.routes.
@@ -26,4 +27,36 @@ describe NamespaceValidator do
       expect(described_class::WILDCARD_ROUTES).to include(*all_wildcard_paths)
     end
   end
+
+  describe '#validation_type' do
+    it 'uses top level validation for groups without parent' do
+      group = build(:group)
+
+      type = validator.validation_type(group)
+
+      expect(type).to eq(:top_level)
+    end
+
+    it 'uses wildcard validation for groups with a parent' do
+      group = build(:group, parent: create(:group))
+
+      type = validator.validation_type(group)
+
+      expect(type).to eq(:wildcard)
+    end
+
+    it 'uses wildcard validation for a project' do
+      project = build(:project)
+
+      type = validator.validation_type(project)
+
+      expect(type).to eq(:wildcard)
+    end
+
+    it 'uses strict validation for everything else' do
+      type = validator.validation_type(double)
+
+      expect(type).to eq(:strict)
+    end
+  end
 end