diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index 16c64f5653593117c94d6ca38295d2eb87763b71..200d07444c917ff842603a33ca8f2e5f17e9f630 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -458,9 +458,6 @@ project_security_statistics: column: project_id on_delete: async_delete projects: - - table: organizations - column: organization_id - on_delete: async_nullify - table: users column: marked_for_deletion_by_user_id on_delete: async_nullify diff --git a/db/post_migrate/20240923055610_add_foreign_key_to_projects_on_organization_id.rb b/db/post_migrate/20240923055610_add_foreign_key_to_projects_on_organization_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..a08cd76a6043244c031887519e9bdc21abb64569 --- /dev/null +++ b/db/post_migrate/20240923055610_add_foreign_key_to_projects_on_organization_id.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddForeignKeyToProjectsOnOrganizationId < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.5' + + def up + add_concurrent_foreign_key( + :projects, + :organizations, + column: :organization_id, + on_delete: :cascade, + reverse_lock_order: true + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists(:projects, column: :organization_id) + end + end +end diff --git a/db/schema_migrations/20240923055610 b/db/schema_migrations/20240923055610 new file mode 100644 index 0000000000000000000000000000000000000000..437b04ef37230c4a05517a31999fe22fc9b54ec0 --- /dev/null +++ b/db/schema_migrations/20240923055610 @@ -0,0 +1 @@ +bc20fac32c1f56b838c3cccc6f780233c1f3c1765c90201d5672ac6892c2cc9a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c70a36970661e19caf8ce7e0478730067cfb65cb..80d87f71566e53a5545923f6a706a0aae37bdbf4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -34426,6 +34426,9 @@ ALTER TABLE ONLY approval_group_rules_users ALTER TABLE ONLY import_failures ADD CONSTRAINT fk_9a9b9ba21c FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY projects + ADD CONSTRAINT fk_9aee26923d FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; + ALTER TABLE ONLY deploy_tokens ADD CONSTRAINT fk_9b0d2e92a6 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; diff --git a/spec/migrations/db/post_migrate/20240923090549_fix_inconsistent_organization_id_spec.rb b/spec/migrations/db/post_migrate/20240923090549_fix_inconsistent_organization_id_spec.rb index 9659b27fac96a826d927d02d0009f89530201736..97c61dc81ead4caaad30ce045eb9992ff05e3535 100644 --- a/spec/migrations/db/post_migrate/20240923090549_fix_inconsistent_organization_id_spec.rb +++ b/spec/migrations/db/post_migrate/20240923090549_fix_inconsistent_organization_id_spec.rb @@ -4,81 +4,90 @@ require_migration! RSpec.describe FixInconsistentOrganizationId, migration: :gitlab_main, feature_category: :cell do + let(:organization_2) { table(:organizations).create!(path: 'organization_2') } + let(:organization_3) { table(:organizations).create!(path: 'organization_3') } + let(:organization_4) { table(:organizations).create!(path: 'organization_4') } + let(:organization_5) { table(:organizations).create!(path: 'organization_5') } + let(:organization_6) { table(:organizations).create!(path: 'organization_6') } + let(:organization_7) { table(:organizations).create!(path: 'organization_7') } + let(:organization_8) { table(:organizations).create!(path: 'organization_8') } + let(:organization_9) { table(:organizations).create!(path: 'organization_9') } + # Inconsistent data: let!(:parent_namespace_1) do - table(:namespaces).create!(type: 'Group', name: 'parent', path: 'parent', organization_id: 2) + table(:namespaces).create!(type: 'Group', name: 'parent', path: 'parent', organization_id: organization_2.id) end let!(:namespace_1) do table(:namespaces).create!(type: 'Group', name: 'child', path: 'child', parent_id: parent_namespace_1.id, - organization_id: 3) + organization_id: organization_3.id) end let!(:namespace_2) do table(:namespaces).create!(type: 'Group', name: 'parent group of project', path: 'parent-group-of-project', - organization_id: 4) + organization_id: organization_4.id) end let!(:project_namespace_1) do table(:namespaces).create!(type: 'Project', name: 'project namespace', path: 'project_namespace', - organization_id: 5) + organization_id: organization_5.id) end let!(:project_1) do table(:projects).create!(namespace_id: namespace_2.id, - project_namespace_id: project_namespace_1.id, organization_id: 5) + project_namespace_id: project_namespace_1.id, organization_id: organization_5.id) end let!(:namespace_2_1) do table(:namespaces).create!(parent_id: namespace_2.id, type: 'Group', name: 'sub-group of project', - path: 'sub-group-of-project', organization_id: 6) + path: 'sub-group-of-project', organization_id: organization_6.id) end let!(:project_namespace_2_1) do table(:namespaces).create!(type: 'Project', name: 'project namespace 2 1', path: 'project_namespace-2-1', - organization_id: 7) + organization_id: organization_7.id) end let!(:project_2_1) do table(:projects).create!(namespace_id: namespace_2_1.id, - project_namespace_id: project_namespace_2_1.id, organization_id: 7) + project_namespace_id: project_namespace_2_1.id, organization_id: organization_7.id) end # Valid data let!(:parent_namespace_2) do - table(:namespaces).create!(type: 'Group', name: 'parent 2', path: 'parent2', organization_id: 8) + table(:namespaces).create!(type: 'Group', name: 'parent 2', path: 'parent2', organization_id: organization_8.id) end let!(:namespace_3) do table(:namespaces).create!(type: 'Group', name: 'child 2', path: 'child2', parent_id: parent_namespace_2.id, - organization_id: 8) + organization_id: organization_8.id) end let!(:namespace_4) do table(:namespaces).create!(type: 'Group', name: 'parent of project 2', path: 'parent-of-project-2 ', - organization_id: 9) + organization_id: organization_9.id) end let!(:project_namespace_2) do table(:namespaces).create!(type: 'Project', name: 'project namespace 2', path: 'project_namespace_2', - organization_id: 9) + organization_id: organization_9.id) end let!(:project_2) do table(:projects).create!(namespace_id: namespace_4.id, - project_namespace_id: project_namespace_2.id, organization_id: 9) + project_namespace_id: project_namespace_2.id, organization_id: organization_9.id) end describe '#up' do it 'sets organization_id to parent organization if organization_ids do not match' do migrate! - expect(namespace_1.reload.organization_id).to eq(2) - expect(project_1.reload.organization_id).to eq(4) - expect(project_2_1.reload.organization_id).to eq(4) + expect(namespace_1.reload.organization_id).to eq(organization_2.id) + expect(project_1.reload.organization_id).to eq(organization_4.id) + expect(project_2_1.reload.organization_id).to eq(organization_4.id) - expect(namespace_3.reload.organization_id).to eq(8) - expect(project_2.reload.organization_id).to eq(9) + expect(namespace_3.reload.organization_id).to eq(organization_8.id) + expect(project_2.reload.organization_id).to eq(organization_9.id) end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 073d73f32149fd94f767162817d1836dadc1c00a..dd3a8ea6a324b467aaaa0a856f6f3ee39f77a90c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -9441,14 +9441,6 @@ def create_hook end end - context 'with loose foreign key on organization_id' do - it_behaves_like 'cleanup by a loose foreign key' do - let_it_be(:parent) { create(:organization) } - let_it_be(:group) { create(:group, organization: parent) } - let_it_be(:model) { create(:project, group: group, organization: parent) } - end - end - describe 'catalog resource process sync events worker' do let_it_be_with_reload(:project) { create(:project, name: 'Test project', description: 'Test description') }