Create ProjectNamespace when a Project is created

Changelog: changed

app/models/concerns/loaded_in_group_list.rb

@@ -69,7 +69,7 @@ module LoadedInGroupList
   end
 
   def subgroup_count
-    @subgroup_count ||= try(:preloaded_subgroup_count) || children.count
+    @subgroup_count ||= try(:preloaded_subgroup_count) || children.without_project_namespaces.count
   end
 
   def member_count

app/models/namespace.rb

@@ -497,6 +497,10 @@ class Namespace < ApplicationRecord
     Feature.enabled?(:block_issue_repositioning, self, type: :ops, default_enabled: :yaml)
   end
 
+  def project_namespace_creation_enabled?
+    Feature.enabled?(:create_project_namespace_on_project_create, self, default_enabled: :yaml)
+  end
+
   private
 
   def expire_child_caches

app/models/project.rb

@@ -98,7 +98,7 @@ class Project < ApplicationRecord
   before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? }
 
   before_save :ensure_runners_token
-  before_save :ensure_project_namespace_in_sync
+  before_validation :ensure_project_namespace_in_sync
 
   after_save :update_project_statistics, if: :saved_change_to_namespace_id?
 
@@ -476,6 +476,7 @@ class Project < ApplicationRecord
   validates :project_feature, presence: true
 
   validates :namespace, presence: true
+  validates :project_namespace, presence: true, on: :create, if: -> { self.namespace.blank? || self.root_namespace.project_namespace_creation_enabled? }
   validates :name, uniqueness: { scope: :namespace_id }
   validates :import_url, public_url: { schemes: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
                                        ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
@@ -2918,14 +2919,32 @@ class Project < ApplicationRecord
     @online_runners_with_tags ||= active_runners.with_tags.online
   end
 
+  def create_project_namespace
+    project_namespace = Namespaces::ProjectNamespace.new(project: self)
+    sync_attributes(project_namespace)
+
+    self.project_namespace = project_namespace
+  end
+
   def ensure_project_namespace_in_sync
-    if changes.keys & [:name, :path, :namespace_id, :visibility_level] && project_namespace.present?
-      project_namespace.name = name
-      project_namespace.path = path
-      project_namespace.parent = namespace
-      project_namespace.visibility_level = visibility_level
+    if new_record? && !project_namespace
+      create_project_namespace if !self.namespace || self.root_namespace.project_namespace_creation_enabled?
+    else
+      sync_attributes(project_namespace) if sync_project_namespace?
     end
   end
+
+  def sync_project_namespace?
+    changes.keys & [:name, :path, :namespace_id, :namespace, :visibility_level] && project_namespace.present?
+  end
+
+  def sync_attributes(project_namespace)
+    project_namespace.name = name
+    project_namespace.path = path
+    project_namespace.parent = namespace
+    project_namespace.shared_runners_enabled = shared_runners_enabled
+    project_namespace.visibility_level = visibility_level
+  end
 end
 
 Project.prepend_mod_with('Project')

app/services/groups/transfer_service.rb

@@ -96,7 +96,9 @@ module Groups
 
     # rubocop: disable CodeReuse/ActiveRecord
     def namespace_with_same_path?
-      Namespace.exists?(path: @group.path, parent: @new_parent_group)
+      # Only check user namespace and Group, the ProjectNamespace will fail at project level
+      # TODO 70972: review the exclusion of ProjectNamespace
+      Namespace.without_project_namespaces.exists?(path: @group.path, parent: @new_parent_group)
     end
     # rubocop: enable CodeReuse/ActiveRecord
 

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'
+type: development
+group: group::workspace
+default_enabled: false

db/fixtures/development/17_cycle_analytics.rb

@@ -157,16 +157,17 @@ class Gitlab::Seeder::CycleAnalytics
   end
 
   def create_new_vsm_project
+    namespace = FactoryBot.create(
+      :group,
+      name: "Value Stream Management Group #{suffix}",
+      path: "vsmg-#{suffix}"
+    )
     project = FactoryBot.create(
       :project,
       name: "Value Stream Management Project #{suffix}",
       path: "vsmp-#{suffix}",
       creator: admin,
-      namespace: FactoryBot.create(
-        :group,
-        name: "Value Stream Management Group #{suffix}",
-        path: "vsmg-#{suffix}"
-      )
+      namespace: namespace
     )
 
     project.create_repository

ee/spec/services/projects/destroy_service_spec.rb

@@ -98,7 +98,7 @@ RSpec.describe Projects::DestroyService do
   end
 
   context 'when project has an associated ProjectNamespace' do
-    let!(:project_namespace) { create(:project_namespace, project: project) }
+    let!(:project_namespace) { project.project_namespace }
 
     it 'destroys the associated ProjectNamespace also' do
       subject.execute

spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb

@@ -38,7 +38,8 @@ RSpec.describe Gitlab::Database::Count::ReltuplesCountStrategy do
       it 'returns nil counts for inherited tables' do
         models.each { |model| expect(model).not_to receive(:count) }
 
-        expect(subject).to eq({ Namespace => 3 })
+        # 3 Namespaces as parents for each Project and 3 ProjectNamespaces(for each Project)
+        expect(subject).to eq({ Namespace => 6 })
       end
     end
 

spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb

@@ -47,7 +47,8 @@ RSpec.describe Gitlab::Database::Count::TablesampleCountStrategy do
         result = subject
         expect(result[Project]).to eq(3)
         expect(result[Group]).to eq(1)
-        expect(result[Namespace]).to eq(4)
+        # 1-Group, 3 namespaces for each project and 3 project namespaces for each project
+        expect(result[Namespace]).to eq(7)
       end
     end
 

spec/lib/gitlab/path_regex_spec.rb

@@ -418,12 +418,15 @@ 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.not_to match('?gitlab') }
-    it { is_expected.not_to match('git lab') }
-    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.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 git') }
   end
 
@@ -434,9 +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.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

@@ -32,9 +32,10 @@ RSpec.describe Namespace do
     describe '#children' do
       let_it_be(:group) { create(:group) }
       let_it_be(:subgroup) { create(:group, parent: group) }
-      let_it_be(:project_namespace) { create(:project_namespace, parent: group) }
+      let_it_be(:project_with_namespace) { create(:project, namespace: group) }
 
       it 'excludes project namespaces' do
+        expect(project_with_namespace.project_namespace.parent).to eq(group)
         expect(group.children).to match_array([subgroup])
       end
     end
@@ -239,8 +240,10 @@ RSpec.describe Namespace do
         let(:namespace) { build(:project_namespace) }
 
         it 'allows to update path to single char' do
-          namespace = create(:project_namespace)
-          namespace.update!(path: 'j')
+          project = create(:project)
+          namespace = project.project_namespace
+
+          namespace.update(path: 'j')
 
           expect(namespace).to be_valid
         end
@@ -342,9 +345,13 @@ RSpec.describe Namespace do
 
     describe '.without_project_namespaces' do
       let_it_be(:user_namespace) { create(:user_namespace) }
-      let_it_be(:project_namespace) { create(:project_namespace) }
+      let_it_be(:project) { create(:project) }
+      let_it_be(:project_namespace) { project.project_namespace }
 
       it 'excludes project namespaces' do
+        expect(project_namespace).not_to be_nil
+        expect(project_namespace.parent).not_to be_nil
+        expect(described_class.all).to include(project_namespace)
         expect(described_class.without_project_namespaces).to match_array([namespace, namespace1, namespace2, namespace1sub, namespace2sub, user_namespace, project_namespace.parent])
       end
     end

spec/models/namespaces/project_namespace_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do
     # using delete rather than destroy due to `delete` skipping AR hooks/callbacks
     # so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key
     let_it_be(:project) { create(:project) }
-    let_it_be(:project_namespace) { create(:project_namespace, project: project) }
+    let_it_be(:project_namespace) { project.project_namespace }
 
     it 'also deletes the associated project' do
       project_namespace.delete

spec/models/project_spec.rb

@@ -191,7 +191,7 @@ RSpec.describe Project, factory_default: :keep do
       # using delete rather than destroy due to `delete` skipping AR hooks/callbacks
       # so it's ensured to work at the DB level. Uses AFTER DELETE trigger.
       let_it_be(:project) { create(:project) }
-      let_it_be(:project_namespace) { create(:project_namespace, project: project) }
+      let_it_be(:project_namespace) { project.project_namespace }
 
       it 'also deletes the associated ProjectNamespace' do
         project.delete
@@ -316,8 +316,8 @@ RSpec.describe Project, factory_default: :keep do
     end
 
     it 'validates the visibility' do
-      expect_any_instance_of(described_class).to receive(:visibility_level_allowed_as_fork).and_call_original
-      expect_any_instance_of(described_class).to receive(:visibility_level_allowed_by_group).and_call_original
+      expect_any_instance_of(described_class).to receive(:visibility_level_allowed_as_fork).twice.and_call_original
+      expect_any_instance_of(described_class).to receive(:visibility_level_allowed_by_group).twice.and_call_original
 
       create(:project)
     end

spec/services/groups/transfer_service_spec.rb

@@ -185,9 +185,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
 
         context 'when projects have project namespaces' do
           let_it_be(:project1) { create(:project, :private, namespace: group) }
-          let_it_be(:project_namespace1) { create(:project_namespace, project: project1) }
           let_it_be(:project2) { create(:project, :private, namespace: group) }
-          let_it_be(:project_namespace2) { create(:project_namespace, project: project2) }
 
           it_behaves_like 'project namespace path is in sync with project path' do
             let(:group_full_path) { "#{group.path}" }
@@ -264,8 +262,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
         end
 
         context 'when projects have project namespaces' do
-          let!(:project_namespace) { create(:project_namespace, project: project) }
-
           before do
             transfer_service.execute(new_parent_group)
           end
@@ -445,8 +441,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
       context 'when transferring a group with project descendants' do
         let!(:project1) { create(:project, :repository, :private, namespace: group) }
         let!(:project2) { create(:project, :repository, :internal, namespace: group) }
-        let!(:project_namespace1) { create(:project_namespace, project: project1) }
-        let!(:project_namespace2) { create(:project_namespace, project: project2) }
 
         before do
           TestEnv.clean_test_path
@@ -483,8 +477,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
           let!(:project1) { create(:project, :repository, :public, namespace: group) }
           let!(:project2) { create(:project, :repository, :public, namespace: group) }
           let!(:new_parent_group) { create(:group, :private) }
-          let!(:project_namespace1) { create(:project_namespace, project: project1) }
-          let!(:project_namespace2) { create(:project_namespace, project: project2) }
 
           it 'updates projects visibility to match the new parent' do
             group.projects.each do |project|
@@ -504,8 +496,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
         let!(:project2) { create(:project, :repository, :internal, namespace: group) }
         let!(:subgroup1) { create(:group, :private, parent: group) }
         let!(:subgroup2) { create(:group, :internal, parent: group) }
-        let!(:project_namespace1) { create(:project_namespace, project: project1) }
-        let!(:project_namespace2) { create(:project_namespace, project: project2) }
 
         before do
           TestEnv.clean_test_path

spec/services/projects/transfer_service_spec.rb

@@ -66,8 +66,6 @@ RSpec.describe Projects::TransferService do
     end
 
     context 'when project has an associated project namespace' do
-      let!(:project_namespace) { create(:project_namespace, project: project) }
-
       it 'keeps project namespace in sync with project' do
         transfer_result = execute_transfer
 
@@ -272,8 +270,6 @@ RSpec.describe Projects::TransferService do
     end
 
     context 'when project has an associated project namespace' do
-      let!(:project_namespace) { create(:project_namespace, project: project) }
-
       it 'keeps project namespace in sync with project' do
         attempt_project_transfer
 
@@ -294,8 +290,6 @@ RSpec.describe Projects::TransferService do
     end
 
     context 'when project has an associated project namespace' do
-      let!(:project_namespace) { create(:project_namespace, project: project) }
-
       it 'keeps project namespace in sync with project' do
         transfer_result = execute_transfer