Merge branch 'add_not_null_constraint_on_project_namespace_id' into 'master'

Add not null and delete cascade constraints on project_namespace_id

See merge request gitlab-org/gitlab!82309

db/post_migrate/20220326161803_add_cascade_delete_fk_on_project_namespace_id.rb

+# frozen_string_literal: true
+
+class AddCascadeDeleteFkOnProjectNamespaceId < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  TARGET_COLUMN = :project_namespace_id
+
+  def up
+    # add the new FK before removing the old one
+    add_concurrent_foreign_key(
+      :projects,
+      :namespaces,
+      column: TARGET_COLUMN,
+      name: fk_name("#{TARGET_COLUMN}_new"),
+      on_delete: :cascade
+    )
+
+    with_lock_retries do
+      remove_foreign_key_if_exists(:projects, column: TARGET_COLUMN, name: fk_name(TARGET_COLUMN))
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(
+      :projects,
+      :namespaces,
+      column: TARGET_COLUMN,
+      name: fk_name(TARGET_COLUMN),
+      on_delete: :nullify
+    )
+
+    with_lock_retries do
+      remove_foreign_key_if_exists(:projects, column: TARGET_COLUMN, name: fk_name("#{TARGET_COLUMN}_new"))
+    end
+  end
+
+  def fk_name(column_name)
+    # generate a FK name
+    concurrent_foreign_key_name(:projects, column_name)
+  end
+end

db/post_migrate/20220326163653_add_not_null_contraint_to_project_namespace_id.rb

+# frozen_string_literal: true
+
+class AddNotNullContraintToProjectNamespaceId < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  def up
+    add_not_null_constraint :projects, :project_namespace_id, validate: false
+  end
+
+  def down
+    remove_not_null_constraint :projects, :project_namespace_id
+  end
+end

db/schema_migrations/20220326161803

+c051ced08a5879ac1dc35d06509df91bdb0d26026cde127dc5ca4f905cb6a9fa
\ No newline at end of file

db/schema_migrations/20220326163653

+112902ff530b1a4e34362845d9a521d320f4049bbd7992e8b3c2b2418d35100c
\ No newline at end of file

db/structure.sql

@@ -24033,6 +24033,9 @@ ALTER TABLE group_import_states
 ALTER TABLE sprints
     ADD CONSTRAINT check_df3816aed7 CHECK ((due_date IS NOT NULL)) NOT VALID;
 
+ALTER TABLE projects
+    ADD CONSTRAINT check_fa75869cb1 CHECK ((project_namespace_id IS NOT NULL)) NOT VALID;
+
 ALTER TABLE ONLY ci_build_needs
     ADD CONSTRAINT ci_build_needs_pkey PRIMARY KEY (id);
 
@@ -31351,6 +31354,9 @@ ALTER TABLE ONLY namespaces
 ALTER TABLE ONLY geo_event_log
     ADD CONSTRAINT fk_6ada82d42a FOREIGN KEY (container_repository_updated_event_id) REFERENCES geo_container_repository_updated_events(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY projects
+    ADD CONSTRAINT fk_6ca23af0a3 FOREIGN KEY (project_namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY dast_profile_schedules
     ADD CONSTRAINT fk_6cca0d8800 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
@@ -31369,9 +31375,6 @@ ALTER TABLE ONLY deploy_tokens
 ALTER TABLE ONLY protected_branch_push_access_levels
     ADD CONSTRAINT fk_7111b68cdb FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
 
-ALTER TABLE ONLY projects
-    ADD CONSTRAINT fk_71625606ac FOREIGN KEY (project_namespace_id) REFERENCES namespaces(id) ON DELETE SET NULL;
-
 ALTER TABLE ONLY integrations
     ADD CONSTRAINT fk_71cce407f9 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

ee/spec/lib/ee/gitlab/background_migration/create_security_setting_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::CreateSecuritySetting do
+RSpec.describe Gitlab::BackgroundMigration::CreateSecuritySetting, :migration, schema: 20220326161803 do
   let(:projects) { table(:projects) }
   let(:settings) { table(:project_security_settings) }
   let(:namespaces) { table(:namespaces) }

ee/spec/lib/ee/gitlab/background_migration/populate_latest_pipeline_ids_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::PopulateLatestPipelineIds do
+RSpec.describe Gitlab::BackgroundMigration::PopulateLatestPipelineIds, :migration, schema: 20220326161803 do
   let(:migrator) { described_class.new }
 
   let(:namespaces) { table(:namespaces) }

ee/spec/lib/ee/gitlab/background_migration/populate_resolved_on_default_branch_column_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe ::Gitlab::BackgroundMigration::PopulateResolvedOnDefaultBranchColumn do
+RSpec.describe ::Gitlab::BackgroundMigration::PopulateResolvedOnDefaultBranchColumn, :migration, schema: 20220326161803 do
   let(:users) { table(:users) }
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }

ee/spec/lib/ee/gitlab/background_migration/recalculate_vulnerability_finding_signatures_for_findings_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::RecalculateVulnerabilityFindingSignaturesForFindings do
+RSpec.describe Gitlab::BackgroundMigration::RecalculateVulnerabilityFindingSignaturesForFindings, :migration, schema: 20220326161803 do
   let(:namespaces) { table(:namespaces) }
   let(:group) { namespaces.create!(name: 'foo', path: 'foo') }
   let(:projects) { table(:projects) }

spec/factories/projects.rb

@@ -59,7 +59,7 @@ FactoryBot.define do
       builds_access_level = [evaluator.builds_access_level, evaluator.repository_access_level].min
       merge_requests_access_level = [evaluator.merge_requests_access_level, evaluator.repository_access_level].min
 
-      hash = {
+      project_feature_hash = {
         wiki_access_level: evaluator.wiki_access_level,
         builds_access_level: builds_access_level,
         snippets_access_level: evaluator.snippets_access_level,
@@ -75,7 +75,16 @@ FactoryBot.define do
         security_and_compliance_access_level: evaluator.security_and_compliance_access_level
       }
 
-      project.build_project_feature(hash)
+      project_namespace_hash = {
+        name: evaluator.name,
+        path: evaluator.path,
+        parent: evaluator.namespace,
+        shared_runners_enabled: evaluator.shared_runners_enabled,
+        visibility_level: evaluator.visibility_level
+      }
+
+      project.build_project_namespace(project_namespace_hash)
+      project.build_project_feature(project_feature_hash)
     end
 
     after(:create) do |project, evaluator|

spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::BackfillDraftStatusOnMergeRequests do
+RSpec.describe Gitlab::BackgroundMigration::BackfillDraftStatusOnMergeRequests, :migration, schema: 20220326161803 do
   let(:namespaces)     { table(:namespaces) }
   let(:projects)       { table(:projects) }
   let(:merge_requests) { table(:merge_requests) }

spec/lib/gitlab/background_migration/backfill_issue_search_data_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::BackfillIssueSearchData do
+RSpec.describe Gitlab::BackgroundMigration::BackfillIssueSearchData, :migration, schema: 20220326161803 do
   let(:namespaces_table) { table(:namespaces) }
   let(:projects_table) { table(:projects) }
   let(:issue_search_data_table) { table(:issue_search_data) }

spec/lib/gitlab/background_migration/backfill_work_item_type_id_for_issues_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::BackfillWorkItemTypeIdForIssues do
+RSpec.describe Gitlab::BackgroundMigration::BackfillWorkItemTypeIdForIssues, :migration, schema: 20220326161803 do
   subject(:migrate) { migration.perform(start_id, end_id, batch_table, batch_column, sub_batch_size, pause_ms, issue_type_enum[:issue], issue_type.id) }
 
   let(:migration) { described_class.new }

spec/lib/gitlab/background_migration/batching_strategies/backfill_issue_work_item_type_batching_strategy_spec.rb

@@ -2,8 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::BackfillIssueWorkItemTypeBatchingStrategy,
-               '#next_batch' do
+RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::BackfillIssueWorkItemTypeBatchingStrategy, '#next_batch', schema: 20220326161803 do # rubocop:disable Layout/LineLength
   # let! can't be used in migration specs because all tables but `work_item_types` are deleted after each spec
   let!(:issue_type_enum) { { issue: 0, incident: 1, test_case: 2, requirement: 3, task: 4 } }
   let!(:namespace) { table(:namespaces).create!(name: 'namespace', path: 'namespace') }

spec/lib/gitlab/background_migration/batching_strategies/backfill_project_namespace_per_group_batching_strategy_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::BackfillProjectNamespacePerGroupBatchingStrategy, '#next_batch' do
+RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::BackfillProjectNamespacePerGroupBatchingStrategy, '#next_batch', :migration, schema: 20220326161803 do
   let!(:namespaces) { table(:namespaces) }
   let!(:projects) { table(:projects) }
   let!(:background_migrations) { table(:batched_background_migrations) }

spec/lib/gitlab/background_migration/cleanup_draft_data_from_faulty_regex_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::CleanupDraftDataFromFaultyRegex do
+RSpec.describe Gitlab::BackgroundMigration::CleanupDraftDataFromFaultyRegex, :migration, schema: 20220326161803 do
   let(:namespaces)     { table(:namespaces) }
   let(:projects)       { table(:projects) }
   let(:merge_requests) { table(:merge_requests) }

spec/lib/gitlab/background_migration/disable_expiration_policies_linked_to_no_container_images_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::DisableExpirationPoliciesLinkedToNoContainerImages do
+RSpec.describe Gitlab::BackgroundMigration::DisableExpirationPoliciesLinkedToNoContainerImages, :migration, schema: 20220326161803 do # rubocop:disable Layout/LineLength
   let_it_be(:projects) { table(:projects) }
   let_it_be(:container_expiration_policies) { table(:container_expiration_policies) }
   let_it_be(:container_repositories) { table(:container_repositories) }

spec/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::MigrateShimoConfluenceIntegrationCategory do
+RSpec.describe Gitlab::BackgroundMigration::MigrateShimoConfluenceIntegrationCategory, schema: 20220326161803 do
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }
   let(:integrations) { table(:integrations) }

spec/lib/gitlab/background_migration/populate_vulnerability_reads_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::PopulateVulnerabilityReads do
+RSpec.describe Gitlab::BackgroundMigration::PopulateVulnerabilityReads, :migration, schema: 20220326161803 do
   let(:vulnerabilities) { table(:vulnerabilities) }
   let(:vulnerability_reads) { table(:vulnerability_reads) }
   let(:vulnerabilities_findings) { table(:vulnerability_occurrences) }

spec/lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::ProjectNamespaces::BackfillProjectNamespaces, :migration do
+RSpec.describe Gitlab::BackgroundMigration::ProjectNamespaces::BackfillProjectNamespaces, :migration, schema: 20220326161803 do
   include MigrationsHelpers
 
   context 'when migrating data', :aggregate_failures do

spec/lib/gitlab/background_migration/remove_duplicate_vulnerabilities_findings_spec.rb

 # frozen_string_literal: true
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::RemoveDuplicateVulnerabilitiesFindings do
+RSpec.describe Gitlab::BackgroundMigration::RemoveDuplicateVulnerabilitiesFindings, :migration, schema: 20220326161803 do
   let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') }
   let(:users) { table(:users) }
   let(:user) { create_user! }

spec/lib/gitlab/background_migration/remove_occurrence_pipelines_and_duplicate_vulnerabilities_findings_spec.rb

 # frozen_string_literal: true
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::RemoveOccurrencePipelinesAndDuplicateVulnerabilitiesFindings do
+RSpec.describe Gitlab::BackgroundMigration::RemoveOccurrencePipelinesAndDuplicateVulnerabilitiesFindings, :migration, schema: 20220326161803 do
   let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') }
   let(:users) { table(:users) }
   let(:user) { create_user! }

spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_on_projects_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenEncryptedValuesOnProjects do
+RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenEncryptedValuesOnProjects, :migration, schema: 20220326161803 do # rubocop:disable Layout/LineLength
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }
 

spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_on_projects_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenValuesOnProjects do
+RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenValuesOnProjects, :migration, schema: 20220326161803 do # rubocop:disable Layout/LineLength
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }
 

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -2422,7 +2422,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
 
     def setup
       namespace = namespaces.create!(name: 'foo', path: 'foo', type: Namespaces::UserNamespace.sti_name)
-      projects.create!(namespace_id: namespace.id)
+      project_namespace = namespaces.create!(name: 'project-foo', path: 'project-foo', type: 'Project', parent_id: namespace.id, visibility_level: 20)
+      projects.create!(namespace_id: namespace.id, project_namespace_id: project_namespace.id)
     end
 
     it 'generates iids properly for models created after the migration' do

spec/lib/gitlab/database/schema_cache_with_renamed_table_spec.rb

@@ -68,8 +68,8 @@ RSpec.describe Gitlab::Database::SchemaCacheWithRenamedTable do
 
     describe 'when the table behind a model is actually a view' do
       let(:group) { create(:group) }
-      let(:project_attributes) { attributes_for(:project, namespace_id: group.id).except(:creator) }
-      let(:record) { old_model.create!(project_attributes) }
+      let(:attrs) { attributes_for(:project, namespace_id: group.id, project_namespace_id: group.id).except(:creator) }
+      let(:record) { old_model.create!(attrs) }
 
       it 'can persist records' do
         expect(record.reload.attributes).to eq(new_model.find(record.id).attributes)

spec/models/namespaces/project_namespace_spec.rb

@@ -17,11 +17,11 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do
     let_it_be(:project) { create(:project) }
     let_it_be(:project_namespace) { project.project_namespace }
 
-    it 'keeps the associated project' do
+    it 'also deletes associated project' do
       project_namespace.delete
 
       expect { project_namespace.reload }.to raise_error(ActiveRecord::RecordNotFound)
-      expect(project.reload.project_namespace).to be_nil
+      expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
 end

spec/services/groups/transfer_service_spec.rb

@@ -252,23 +252,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
           expect(transfer_service.execute(new_parent_group)).to be_falsy
           expect(transfer_service.error).to eq('Transfer failed: The parent group already has a subgroup or a project with the same path.')
         end
-
-        # currently when a project is created it gets a corresponding project namespace
-        # so we test the case where a project without a project namespace is transferred
-        # for backward compatibility
-        context 'without project namespace' do
-          before do
-            project_namespace = project.project_namespace
-            project.update_column(:project_namespace_id, nil)
-            project_namespace.delete
-          end
-
-          it 'adds an error on group' do
-            expect(project.reload.project_namespace).to be_nil
-            expect(transfer_service.execute(new_parent_group)).to be_falsy
-            expect(transfer_service.error).to eq('Transfer failed: Validation failed: Group URL has already been taken')
-          end
-        end
       end
 
       context 'when projects have project namespaces' do