Add specs specific to project namespace creation

Check that project namespace is created when project is
created and also that there is no project namespace created
if the project is not created for any reason.

app/models/project.rb

@@ -2920,18 +2920,20 @@ class Project < ApplicationRecord
   end
 
   def ensure_project_namespace_in_sync
-    if self.namespace && self.root_namespace.project_namespace_creation_enabled?
-      if new_record? && !project_namespace
-        build_project_namespace
-        sync_attributes(project_namespace)
-      end
+    # create project_namespace when project is created if create_project_namespace_on_project_create FF is enabled
+    build_project_namespace if project_namespace_creation_enabled?
 
+    # regardless of create_project_namespace_on_project_create FF we need
+    # to keep project and project namespace in sync if there is one
     sync_attributes(project_namespace) if sync_project_namespace?
   end
+
+  def project_namespace_creation_enabled?
+    new_record? && !project_namespace && self.namespace && self.root_namespace.project_namespace_creation_enabled?
   end
 
   def sync_project_namespace?
-    (changes.keys & %w(name path namespace_id namespace visibility_level)).any? && project_namespace.present?
+    (changes.keys & %w(name path namespace_id namespace visibility_level shared_runners_enabled)).any? && project_namespace.present?
   end
 
   def sync_attributes(project_namespace)

config/feature_flags/development/create_project_namespace_on_project_create.yml

 ---
 name: create_project_namespace_on_project_create
-introduced_by_url:
-rollout_issue_url:
-milestone: '14.4'
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70972
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344954
+milestone: '14.5'
 type: development
 group: group::workspace
 default_enabled: false

spec/lib/gitlab/path_regex_spec.rb

@@ -418,16 +418,16 @@ RSpec.describe Gitlab::PathRegex do
     it { is_expected.to match('_underscore.js') }
     it { is_expected.to match('100px.com') }
     it { is_expected.to match('gitlab.org') }
-    it { is_expected.to match('gitlab.org-') }
-    it { is_expected.to match('gitlab.org_') }
-    it { is_expected.not_to match('gitlab.org.') }
-    it { is_expected.not_to match('gitlab.org/') }
-    it { is_expected.not_to match('/gitlab.org') }
     it { is_expected.not_to match('?gitlab') }
-    it { is_expected.not_to match('gitlab?') }
     it { is_expected.not_to match('git lab') }
     it { is_expected.not_to match('gitlab.git') }
+    it { is_expected.not_to match('gitlab.org.') }
+    it { is_expected.not_to match('gitlab.org/') }
+    it { is_expected.not_to match('/gitlab.org') }
     it { is_expected.not_to match('gitlab git') }
+    it { is_expected.not_to match('gitlab?') }
+    it { is_expected.to match('gitlab.org-') }
+    it { is_expected.to match('gitlab.org_') }
   end
 
   describe '.project_path_format_regex' do
@@ -437,17 +437,17 @@ RSpec.describe Gitlab::PathRegex do
     it { is_expected.to match('gitlab_git') }
     it { is_expected.to match('_underscore.js') }
     it { is_expected.to match('100px.com') }
+    it { is_expected.not_to match('?gitlab') }
+    it { is_expected.not_to match('git lab') }
+    it { is_expected.not_to match('gitlab.git') }
+    it { is_expected.not_to match('gitlab?') }
+    it { is_expected.not_to match('gitlab git') }
     it { is_expected.to match('gitlab.org') }
     it { is_expected.to match('gitlab.org-') }
     it { is_expected.to match('gitlab.org_') }
     it { is_expected.to match('gitlab.org.') }
     it { is_expected.not_to match('gitlab.org/') }
     it { is_expected.not_to match('/gitlab.org') }
-    it { is_expected.not_to match('?gitlab') }
-    it { is_expected.not_to match('gitlab?') }
-    it { is_expected.not_to match('git lab') }
-    it { is_expected.not_to match('gitlab.git') }
-    it { is_expected.not_to match('gitlab git') }
   end
 
   context 'repository routes' do

spec/models/namespace_spec.rb

@@ -243,7 +243,7 @@ RSpec.describe Namespace do
           project = create(:project)
           namespace = project.project_namespace
 
-          namespace.update(path: 'j')
+          namespace.update!(path: 'j')
 
           expect(namespace).to be_valid
         end
@@ -569,7 +569,7 @@ RSpec.describe Namespace do
 
     context 'with project namespaces' do
       let_it_be(:project) { create(:project, namespace: parent_group, path: 'some-new-path') }
-      let_it_be(:project_namespace) { create(:project_namespace, project: project) }
+      let_it_be(:project_namespace) { project.project_namespace }
 
       it 'does not return project namespace' do
         search_result = described_class.search('path')

spec/models/project_spec.rb

@@ -233,6 +233,58 @@ RSpec.describe Project, factory_default: :keep do
         expect(project.project_setting).to be_an_instance_of(ProjectSetting)
         expect(project.project_setting).to be_new_record
       end
+
+      context 'with project namespaces' do
+        it 'automatically creates a project namespace' do
+          project = build(:project, path: 'hopefully-valid-path1')
+          project.save!
+
+          expect(project).to be_persisted
+          expect(project.project_namespace).to be_persisted
+          expect(project.project_namespace).to be_in_sync_with_project(project)
+        end
+
+        context 'with FF disabled' do
+          before do
+            stub_feature_flags(create_project_namespace_on_project_create: false)
+          end
+
+          it 'does not create a project namespace' do
+            project = build(:project, path: 'hopefully-valid-path2')
+            project.save!
+
+            expect(project).to be_persisted
+            expect(project.project_namespace).to be_nil
+          end
+        end
+      end
+    end
+
+    context 'updating a project' do
+      context 'with project namespaces' do
+        it 'keeps project namespace in sync with project' do
+          project = create(:project)
+          project.update!(path: 'hopefully-valid-path1')
+
+          expect(project).to be_persisted
+          expect(project.project_namespace).to be_persisted
+          expect(project.project_namespace).to be_in_sync_with_project(project)
+        end
+
+        context 'with FF disabled' do
+          before do
+            stub_feature_flags(create_project_namespace_on_project_create: false)
+          end
+
+          it 'does not create a project namespace when project is updated' do
+            project = create(:project)
+            project.update!(path: 'hopefully-valid-path1')
+
+            expect(project).to be_persisted
+            expect(project.project_namespace).to be_nil
+          end
+        end
+      end
     end
 
     context 'updating cd_cd_settings' do
@@ -322,6 +374,18 @@ RSpec.describe Project, factory_default: :keep do
       create(:project)
     end
 
+    context 'validates project namespace creation' do
+      it 'does not create project namespace if project is not created' do
+        project = build(:project, path: 'tree')
+
+        project.valid?
+
+        expect(project).not_to be_valid
+        expect(project).to be_new_record
+        expect(project.project_namespace).to be_new_record
+      end
+    end
+
     context 'repository storages inclusion' do
       let(:project2) { build(:project, repository_storage: 'missing') }
 

spec/services/projects/create_service_spec.rb

@@ -49,6 +49,7 @@ RSpec.describe Projects::CreateService, '#execute' do
       it 'keeps them as specified' do
         expect(project.name).to eq('one')
         expect(project.path).to eq('two')
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
     end
 
@@ -58,6 +59,7 @@ RSpec.describe Projects::CreateService, '#execute' do
       it 'sets name == path' do
         expect(project.path).to eq('one.two_three-four')
         expect(project.name).to eq(project.path)
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
     end
 
@@ -67,6 +69,7 @@ RSpec.describe Projects::CreateService, '#execute' do
       it 'sets path == name' do
         expect(project.name).to eq('one.two_three-four')
         expect(project.path).to eq(project.name)
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
     end
 
@@ -78,6 +81,7 @@ RSpec.describe Projects::CreateService, '#execute' do
       it 'parameterizes the name' do
         expect(project.name).to eq('one.two_three-four and five')
         expect(project.path).to eq('one-two_three-four-and-five')
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
     end
   end
@@ -111,13 +115,14 @@ RSpec.describe Projects::CreateService, '#execute' do
   end
 
   context 'user namespace' do
-    it do
+    it 'creates a project in user namespace' do
       project = create_project(user, opts)
 
       expect(project).to be_valid
       expect(project.owner).to eq(user)
       expect(project.team.maintainers).to include(user)
       expect(project.namespace).to eq(user.namespace)
+      expect(project.project_namespace).to be_in_sync_with_project(project)
     end
   end
 
@@ -151,6 +156,7 @@ RSpec.describe Projects::CreateService, '#execute' do
         expect(project.owner).to eq(user)
         expect(project.team.maintainers).to contain_exactly(user)
         expect(project.namespace).to eq(user.namespace)
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
     end
 
@@ -160,6 +166,7 @@ RSpec.describe Projects::CreateService, '#execute' do
         project = create_project(admin, opts)
 
         expect(project).not_to be_persisted
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
     end
   end
@@ -183,6 +190,7 @@ RSpec.describe Projects::CreateService, '#execute' do
       expect(project.namespace).to eq(group)
       expect(project.team.owners).to include(user)
       expect(user.authorized_projects).to include(project)
+      expect(project.project_namespace).to be_in_sync_with_project(project)
     end
   end
 
@@ -339,6 +347,7 @@ RSpec.describe Projects::CreateService, '#execute' do
       end
 
       imported_project
+      expect(imported_project.project_namespace).to be_in_sync_with_project(imported_project)
     end
 
     it 'stores import data and URL' do
@@ -406,6 +415,7 @@ RSpec.describe Projects::CreateService, '#execute' do
         expect(project.visibility_level).to eq(project_level)
         expect(project).to be_saved
         expect(project).to be_valid
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
     end
   end
@@ -424,6 +434,7 @@ RSpec.describe Projects::CreateService, '#execute' do
         expect(project.errors.messages[:visibility_level].first).to(
           match('restricted by your GitLab administrator')
         )
+        expect(project.project_namespace).to be_in_sync_with_project(project)
       end
 
       it 'does not allow a restricted visibility level for admins when admin mode is disabled' do
@@ -493,6 +504,7 @@ RSpec.describe Projects::CreateService, '#execute' do
       expect(project).to be_valid
       expect(project.owner).to eq(user)
       expect(project.namespace).to eq(user.namespace)
+      expect(project.project_namespace).to be_in_sync_with_project(project)
     end
 
     context 'when another repository already exists on disk' do
@@ -522,6 +534,7 @@ RSpec.describe Projects::CreateService, '#execute' do
           expect(project).to respond_to(:errors)
           expect(project.errors.messages).to have_key(:base)
           expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
+          expect(project.project_namespace).to be_in_sync_with_project(project)
         end
 
         it 'does not allow to import project when path matches existing repository on disk' do
@@ -531,6 +544,7 @@ RSpec.describe Projects::CreateService, '#execute' do
           expect(project).to respond_to(:errors)
           expect(project.errors.messages).to have_key(:base)
           expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
+          expect(project.project_namespace).to be_in_sync_with_project(project)
         end
       end
 
@@ -555,6 +569,7 @@ RSpec.describe Projects::CreateService, '#execute' do
           expect(project).to respond_to(:errors)
           expect(project.errors.messages).to have_key(:base)
           expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
+          expect(project.project_namespace).to be_in_sync_with_project(project)
         end
       end
     end
@@ -870,6 +885,7 @@ RSpec.describe Projects::CreateService, '#execute' do
 
             expect(project).to be_valid
             expect(project.shared_runners_enabled).to eq(expected_result_for_project)
+            expect(project.project_namespace).to be_in_sync_with_project(project)
           end
         end
       end
@@ -890,6 +906,7 @@ RSpec.describe Projects::CreateService, '#execute' do
 
             expect(project).to be_valid
             expect(project.shared_runners_enabled).to eq(expected_result_for_project)
+            expect(project.project_namespace).to be_in_sync_with_project(project)
           end
         end
       end
@@ -907,6 +924,7 @@ RSpec.describe Projects::CreateService, '#execute' do
             expect(project.persisted?).to eq(false)
             expect(project).to be_invalid
             expect(project.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group does not allow it')
+            expect(project.project_namespace).to be_in_sync_with_project(project)
           end
         end
       end
@@ -926,6 +944,7 @@ RSpec.describe Projects::CreateService, '#execute' do
 
           expect(project).to be_valid
           expect(project.shared_runners_enabled).to eq(expected_result)
+          expect(project.project_namespace).to be_in_sync_with_project(project)
         end
       end
     end

spec/support/matchers/project_namespace_matcher.rb

+# frozen_string_literal: true
+
+RSpec::Matchers.define :be_in_sync_with_project do |project|
+  match do |project_namespace|
+    # if project is not persisted make sure we do not have a persisted project_namespace for it
+    break false if project.new_record? && project_namespace&.persisted?
+    # don't really care if project is not in sync if the project was never persisted.
+    break true if project.new_record? && !project_namespace.present?
+
+    project_namespace.present? &&
+      project.name == project_namespace.name &&
+      project.path == project_namespace.path &&
+      project.namespace == project_namespace.parent &&
+      project.visibility_level == project_namespace.visibility_level &&
+      project.shared_runners_enabled == project_namespace.shared_runners_enabled
+  end
+
+  failure_message_when_negated do |project_namespace|
+    if project.new_record? && project_namespace&.persisted?
+      "expected that a non persisted project #{project} does not have a persisted project namespace #{project_namespace}"
+    else
+      <<-MSG
+        expected that the project's attributes name, path, namespace_id, visibility_level, shared_runners_enabled
+        are in sync with the corresponding project namespace attributes
+      MSG
+    end
+  end
+end