diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-18 11:18:50 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-18 11:18:50 +0000 |
commit | 8c7f4e9d5f36cff46365a7f8c4b9c21578c1e781 (patch) | |
tree | a77e7fe7a93de11213032ed4ab1f33a3db51b738 /spec/lib/gitlab/database | |
parent | 00b35af3db1abfe813a778f643dad221aad51fca (diff) | |
download | gitlab-ce-8c7f4e9d5f36cff46365a7f8c4b9c21578c1e781.tar.gz |
Add latest changes from gitlab-org/gitlab@13-1-stable-ee
Diffstat (limited to 'spec/lib/gitlab/database')
-rw-r--r-- | spec/lib/gitlab/database/custom_structure_spec.rb | 65 | ||||
-rw-r--r-- | spec/lib/gitlab/database/migration_helpers_spec.rb | 151 | ||||
-rw-r--r-- | spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb | 55 | ||||
-rw-r--r-- | spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb (renamed from spec/lib/gitlab/database/partitioning_migration_helpers_spec.rb) | 79 | ||||
-rw-r--r-- | spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb | 289 | ||||
-rw-r--r-- | spec/lib/gitlab/database/schema_cleaner_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/database/with_lock_retries_spec.rb | 5 |
7 files changed, 503 insertions, 145 deletions
diff --git a/spec/lib/gitlab/database/custom_structure_spec.rb b/spec/lib/gitlab/database/custom_structure_spec.rb new file mode 100644 index 00000000000..f03b5ed0a7f --- /dev/null +++ b/spec/lib/gitlab/database/custom_structure_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Database::CustomStructure do + let_it_be(:structure) { described_class.new } + let_it_be(:filepath) { Rails.root.join(described_class::CUSTOM_DUMP_FILE) } + let_it_be(:file_header) do + <<~DATA + -- this file tracks custom GitLab data, such as foreign keys referencing partitioned tables + -- more details can be found in the issue: https://gitlab.com/gitlab-org/gitlab/-/issues/201872 + SET search_path=public; + DATA + end + + let(:io) { StringIO.new } + + before do + allow(File).to receive(:open).with(filepath, anything).and_yield(io) + end + + context 'when there are no partitioned_foreign_keys' do + it 'dumps a valid structure file' do + structure.dump + + expect(io.string).to eq("#{file_header}\n") + end + end + + context 'when there are partitioned_foreign_keys' do + let!(:first_fk) do + Gitlab::Database::PartitioningMigrationHelpers::PartitionedForeignKey.create( + cascade_delete: true, from_table: 'issues', from_column: 'project_id', to_table: 'projects', to_column: 'id') + end + let!(:second_fk) do + Gitlab::Database::PartitioningMigrationHelpers::PartitionedForeignKey.create( + cascade_delete: false, from_table: 'issues', from_column: 'moved_to_id', to_table: 'issues', to_column: 'id') + end + + it 'dumps a file with the command to restore the current keys' do + structure.dump + + expect(io.string).to eq(<<~DATA) + #{file_header} + COPY partitioned_foreign_keys (id, cascade_delete, from_table, from_column, to_table, to_column) FROM STDIN; + #{first_fk.id}\ttrue\tissues\tproject_id\tprojects\tid + #{second_fk.id}\tfalse\tissues\tmoved_to_id\tissues\tid + \\. + DATA + + first_fk.destroy + io.truncate(0) + io.rewind + + structure.dump + + expect(io.string).to eq(<<~DATA) + #{file_header} + COPY partitioned_foreign_keys (id, cascade_delete, from_table, from_column, to_table, to_column) FROM STDIN; + #{second_fk.id}\tfalse\tissues\tmoved_to_id\tissues\tid + \\. + DATA + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 203d39be22b..bed444ee7c7 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1539,12 +1539,17 @@ describe Gitlab::Database::MigrationHelpers do end describe '#create_or_update_plan_limit' do - class self::Plan < ActiveRecord::Base - self.table_name = 'plans' - end + before do + stub_const('Plan', Class.new(ActiveRecord::Base)) + stub_const('PlanLimits', Class.new(ActiveRecord::Base)) + + Plan.class_eval do + self.table_name = 'plans' + end - class self::PlanLimits < ActiveRecord::Base - self.table_name = 'plan_limits' + PlanLimits.class_eval do + self.table_name = 'plan_limits' + end end it 'properly escapes names' do @@ -1560,28 +1565,28 @@ describe Gitlab::Database::MigrationHelpers do context 'when plan does not exist' do it 'does not create any plan limits' do expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) } - .not_to change { self.class::PlanLimits.count } + .not_to change { PlanLimits.count } end end context 'when plan does exist' do - let!(:plan) { self.class::Plan.create!(name: 'plan_name') } + let!(:plan) { Plan.create!(name: 'plan_name') } context 'when limit does not exist' do it 'inserts a new plan limits' do expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) } - .to change { self.class::PlanLimits.count }.by(1) + .to change { PlanLimits.count }.by(1) - expect(self.class::PlanLimits.pluck(:project_hooks)).to contain_exactly(10) + expect(PlanLimits.pluck(:project_hooks)).to contain_exactly(10) end end context 'when limit does exist' do - let!(:plan_limit) { self.class::PlanLimits.create!(plan_id: plan.id) } + let!(:plan_limit) { PlanLimits.create!(plan_id: plan.id) } it 'updates an existing plan limits' do expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 999) } - .not_to change { self.class::PlanLimits.count } + .not_to change { PlanLimits.count } expect(plan_limit.reload.project_hooks).to eq(999) end @@ -1605,19 +1610,23 @@ describe Gitlab::Database::MigrationHelpers do describe '#backfill_iids' do include MigrationsHelpers - class self::Issue < ActiveRecord::Base - include AtomicInternalId + before do + stub_const('Issue', Class.new(ActiveRecord::Base)) + + Issue.class_eval do + include AtomicInternalId - self.table_name = 'issues' - self.inheritance_column = :_type_disabled + self.table_name = 'issues' + self.inheritance_column = :_type_disabled - belongs_to :project, class_name: "::Project" + belongs_to :project, class_name: "::Project" - has_internal_id :iid, - scope: :project, - init: ->(s) { s&.project&.issues&.maximum(:iid) }, - backfill: true, - presence: false + has_internal_id :iid, + scope: :project, + init: ->(s) { s&.project&.issues&.maximum(:iid) }, + backfill: true, + presence: false + end end let(:namespaces) { table(:namespaces) } @@ -1636,7 +1645,7 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue = self.class::Issue.create!(project_id: project.id) + issue = Issue.create!(project_id: project.id) expect(issue.iid).to eq(1) end @@ -1647,7 +1656,7 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue_b = self.class::Issue.create!(project_id: project.id) + issue_b = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.iid).to eq(2) @@ -1662,8 +1671,8 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue_a = self.class::Issue.create!(project_id: project_a.id) - issue_b = self.class::Issue.create!(project_id: project_b.id) + issue_a = Issue.create!(project_id: project_a.id) + issue_b = Issue.create!(project_id: project_b.id) expect(issue_a.iid).to eq(2) expect(issue_b.iid).to eq(3) @@ -1672,7 +1681,7 @@ describe Gitlab::Database::MigrationHelpers do context 'when the new code creates a row post deploy but before the migration runs' do it 'does not change the row iid' do project = setup - issue = self.class::Issue.create!(project_id: project.id) + issue = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1683,7 +1692,7 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1697,8 +1706,8 @@ describe Gitlab::Database::MigrationHelpers do project_b = setup issue_a = issues.create!(project_id: project_a.id) issue_b = issues.create!(project_id: project_b.id) - issue_c = self.class::Issue.create!(project_id: project_a.id) - issue_d = self.class::Issue.create!(project_id: project_b.id) + issue_c = Issue.create!(project_id: project_a.id) + issue_d = Issue.create!(project_id: project_b.id) model.backfill_iids('issues') @@ -1712,12 +1721,12 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) model.backfill_iids('issues') - issue_d = self.class::Issue.create!(project_id: project.id) - issue_e = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1731,14 +1740,14 @@ describe Gitlab::Database::MigrationHelpers do project_b = setup issue_a = issues.create!(project_id: project_a.id) issue_b = issues.create!(project_id: project_b.id) - issue_c = self.class::Issue.create!(project_id: project_a.id) - issue_d = self.class::Issue.create!(project_id: project_b.id) + issue_c = Issue.create!(project_id: project_a.id) + issue_d = Issue.create!(project_id: project_b.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project_a.id) - issue_f = self.class::Issue.create!(project_id: project_b.id) - issue_g = self.class::Issue.create!(project_id: project_a.id) + issue_e = Issue.create!(project_id: project_a.id) + issue_f = Issue.create!(project_id: project_b.id) + issue_g = Issue.create!(project_id: project_a.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(1) @@ -1754,7 +1763,7 @@ describe Gitlab::Database::MigrationHelpers do it 'backfills iids' do project = setup issue_a = issues.create!(project_id: project.id) - issue_b = self.class::Issue.create!(project_id: project.id) + issue_b = Issue.create!(project_id: project.id) issue_c = issues.create!(project_id: project.id) model.backfill_iids('issues') @@ -1768,12 +1777,12 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_d = issues.create!(project_id: project.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1787,9 +1796,9 @@ describe Gitlab::Database::MigrationHelpers do it 'backfills iids' do project = setup issue_a = issues.create!(project_id: project.id) - issue_b = self.class::Issue.create!(project_id: project.id) + issue_b = Issue.create!(project_id: project.id) issue_c = issues.create!(project_id: project.id) - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1803,13 +1812,13 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_d = issues.create!(project_id: project.id) - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) model.backfill_iids('issues') - issue_f = self.class::Issue.create!(project_id: project.id) + issue_f = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1825,7 +1834,7 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete model.backfill_iids('issues') @@ -1838,12 +1847,12 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete model.backfill_iids('issues') - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1856,7 +1865,7 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete issue_d = issues.create!(project_id: project.id) @@ -1871,13 +1880,13 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete issue_d = issues.create!(project_id: project.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1891,9 +1900,9 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1906,13 +1915,13 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1929,7 +1938,7 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue_b = self.class::Issue.create!(project_id: project_b.id) + issue_b = Issue.create!(project_id: project_b.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(1) @@ -2066,6 +2075,34 @@ describe Gitlab::Database::MigrationHelpers do allow(model).to receive(:check_constraint_exists?).and_return(false) end + context 'constraint name validation' do + it 'raises an error when too long' do + expect do + model.add_check_constraint( + :test_table, + 'name IS NOT NULL', + 'a' * (Gitlab::Database::MigrationHelpers::MAX_IDENTIFIER_NAME_LENGTH + 1) + ) + end.to raise_error(RuntimeError) + end + + it 'does not raise error when the length is acceptable' do + constraint_name = 'a' * Gitlab::Database::MigrationHelpers::MAX_IDENTIFIER_NAME_LENGTH + + expect(model).to receive(:transaction_open?).and_return(false) + expect(model).to receive(:check_constraint_exists?).and_return(false) + expect(model).to receive(:with_lock_retries).and_call_original + expect(model).to receive(:execute).with(/ADD CONSTRAINT/) + + model.add_check_constraint( + :test_table, + 'name IS NOT NULL', + constraint_name, + validate: false + ) + end + end + context 'inside a transaction' do it 'raises an error' do expect(model).to receive(:transaction_open?).and_return(true) diff --git a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb index 0f68201a153..dee1d7df1a9 100644 --- a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb +++ b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb @@ -3,39 +3,48 @@ require 'spec_helper' describe Gitlab::Database::ObsoleteIgnoredColumns do - module Testing + before do + stub_const('Testing', Module.new) + stub_const('Testing::MyBase', Class.new(ActiveRecord::Base)) + stub_const('SomeAbstract', Class.new(Testing::MyBase)) + stub_const('Testing::B', Class.new(Testing::MyBase)) + stub_const('Testing::A', Class.new(SomeAbstract)) + stub_const('Testing::C', Class.new(Testing::MyBase)) + # Used a fixed date to prevent tests failing across date boundaries - REMOVE_DATE = Date.new(2019, 12, 16) + stub_const('REMOVE_DATE', Date.new(2019, 12, 16)) - class MyBase < ApplicationRecord - end + Testing.module_eval do + Testing::MyBase.class_eval do + end - class SomeAbstract < MyBase - include IgnorableColumns + SomeAbstract.class_eval do + include IgnorableColumns - self.abstract_class = true + self.abstract_class = true - self.table_name = 'projects' + self.table_name = 'projects' - ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0' - end + ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0' + end - class B < MyBase - include IgnorableColumns + Testing::B.class_eval do + include IgnorableColumns - self.table_name = 'issues' + self.table_name = 'issues' - ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0' - ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' - end + ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0' + ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' + end - class A < SomeAbstract - ignore_column :also_unused, remove_after: '2019-02-01', remove_with: '12.1' - ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' - end + Testing::A.class_eval do + ignore_column :also_unused, remove_after: '2019-02-01', remove_with: '12.1' + ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' + end - class C < MyBase - self.table_name = 'users' + Testing::C.class_eval do + self.table_name = 'users' + end end end @@ -43,7 +52,7 @@ describe Gitlab::Database::ObsoleteIgnoredColumns do describe '#execute' do it 'returns a list of class names and columns pairs' do - Timecop.freeze(Testing::REMOVE_DATE) do + Timecop.freeze(REMOVE_DATE) do expect(subject.execute).to eq([ ['Testing::A', { 'unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0'), diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb index 0e2fb047469..9cec77b434d 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb @@ -2,17 +2,21 @@ require 'spec_helper' -describe Gitlab::Database::PartitioningMigrationHelpers do +describe Gitlab::Database::PartitioningMigrationHelpers::ForeignKeyHelpers do + include TriggerHelpers + let(:model) do ActiveRecord::Migration.new.extend(described_class) end let_it_be(:connection) { ActiveRecord::Base.connection } let(:referenced_table) { :issues } - let(:function_name) { model.fk_function_name(referenced_table) } - let(:trigger_name) { model.fk_trigger_name(referenced_table) } + let(:function_name) { '_test_partitioned_foreign_keys_function' } + let(:trigger_name) { '_test_partitioned_foreign_keys_trigger' } before do allow(model).to receive(:puts) + allow(model).to receive(:fk_function_name).and_return(function_name) + allow(model).to receive(:fk_trigger_name).and_return(trigger_name) end describe 'adding a foreign key' do @@ -25,7 +29,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers do model.add_partitioned_foreign_key :issue_assignees, referenced_table expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -41,7 +45,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers do expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id', 'delete from epic_issues where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -57,7 +61,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers do expect_function_to_contain(function_name, 'delete from issues where moved_to_id = old.id', 'delete from issues where duplicated_to_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -66,7 +70,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers do model.add_partitioned_foreign_key :issues, referenced_table, column: :moved_to_id expect_function_to_contain(function_name, 'delete from issues where moved_to_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end end @@ -77,7 +81,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers do model.add_partitioned_foreign_key :issue_assignees, referenced_table, on_delete: :nullify expect_function_to_contain(function_name, 'update issue_assignees set issue_id = null where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -86,7 +90,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers do model.add_partitioned_foreign_key :issues, referenced_table, column: :duplicated_to_id expect_function_to_contain(function_name, 'delete from issues where duplicated_to_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -97,7 +101,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers do model.add_partitioned_foreign_key :user_preferences, referenced_table, column: :user_id, primary_key: :user_id expect_function_to_contain(function_name, 'delete from user_preferences where user_id = old.user_id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -135,12 +139,12 @@ describe Gitlab::Database::PartitioningMigrationHelpers do expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id', 'delete from epic_issues where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') model.remove_partitioned_foreign_key :issue_assignees, referenced_table expect_function_to_contain(function_name, 'delete from epic_issues where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -151,12 +155,12 @@ describe Gitlab::Database::PartitioningMigrationHelpers do it 'removes the trigger function altogether' do expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') model.remove_partitioned_foreign_key :issue_assignees, referenced_table - expect(find_function_def(function_name)).to be_nil - expect(find_trigger_def(trigger_name)).to be_nil + expect_function_not_to_exist(function_name) + expect_trigger_not_to_exist(referenced_table, trigger_name) end end @@ -167,12 +171,12 @@ describe Gitlab::Database::PartitioningMigrationHelpers do it 'ignores the invalid key and properly recreates the trigger function' do expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') model.remove_partitioned_foreign_key :issues, referenced_table, column: :moved_to_id expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id') - expect_valid_function_trigger(trigger_name, function_name) + expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete') end end @@ -186,45 +190,4 @@ describe Gitlab::Database::PartitioningMigrationHelpers do end end end - - def expect_function_to_contain(name, *statements) - return_stmt, *body_stmts = parsed_function_statements(name).reverse - - expect(return_stmt).to eq('return old') - expect(body_stmts).to contain_exactly(*statements) - end - - def expect_valid_function_trigger(name, fn_name) - event, activation, definition = cleaned_trigger_def(name) - - expect(event).to eq('delete') - expect(activation).to eq('after') - expect(definition).to eq("execute procedure #{fn_name}()") - end - - def parsed_function_statements(name) - cleaned_definition = find_function_def(name)['fn_body'].downcase.gsub(/\s+/, ' ') - statements = cleaned_definition.sub(/\A\s*begin\s*(.*)\s*end\s*\Z/, "\\1") - statements.split(';').map! { |stmt| stmt.strip.presence }.compact! - end - - def find_function_def(name) - connection.execute("select prosrc as fn_body from pg_proc where proname = '#{name}';").first - end - - def cleaned_trigger_def(name) - find_trigger_def(name).values_at('event', 'activation', 'definition').map!(&:downcase) - end - - def find_trigger_def(name) - connection.execute(<<~SQL).first - select - string_agg(event_manipulation, ',') as event, - action_timing as activation, - action_statement as definition - from information_schema.triggers - where trigger_name = '#{name}' - group by 2, 3 - SQL - end end diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb new file mode 100644 index 00000000000..586b57d2002 --- /dev/null +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb @@ -0,0 +1,289 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers do + include PartitioningHelpers + include TriggerHelpers + + let(:migration) do + ActiveRecord::Migration.new.extend(described_class) + end + + let_it_be(:connection) { ActiveRecord::Base.connection } + let(:template_table) { :audit_events } + let(:partitioned_table) { '_test_migration_partitioned_table' } + let(:function_name) { '_test_migration_function_name' } + let(:trigger_name) { '_test_migration_trigger_name' } + let(:partition_column) { 'created_at' } + let(:min_date) { Date.new(2019, 12) } + let(:max_date) { Date.new(2020, 3) } + + before do + allow(migration).to receive(:puts) + allow(migration).to receive(:transaction_open?).and_return(false) + allow(migration).to receive(:partitioned_table_name).and_return(partitioned_table) + allow(migration).to receive(:sync_function_name).and_return(function_name) + allow(migration).to receive(:sync_trigger_name).and_return(trigger_name) + allow(migration).to receive(:assert_table_is_whitelisted) + end + + describe '#partition_table_by_date' do + let(:partition_column) { 'created_at' } + let(:old_primary_key) { 'id' } + let(:new_primary_key) { [old_primary_key, partition_column] } + + context 'when the table is not whitelisted' do + let(:template_table) { :this_table_is_not_whitelisted } + + it 'raises an error' do + expect(migration).to receive(:assert_table_is_whitelisted).with(template_table).and_call_original + + expect do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/#{template_table} is not whitelisted for use/) + end + end + + context 'when run inside a transaction block' do + it 'raises an error' do + expect(migration).to receive(:transaction_open?).and_return(true) + + expect do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/can not be run inside a transaction/) + end + end + + context 'when the the max_date is less than the min_date' do + let(:max_date) { Time.utc(2019, 6) } + + it 'raises an error' do + expect do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/max_date #{max_date} must be greater than min_date #{min_date}/) + end + end + + context 'when the max_date is equal to the min_date' do + let(:max_date) { min_date } + + it 'raises an error' do + expect do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/max_date #{max_date} must be greater than min_date #{min_date}/) + end + end + + context 'when the given table does not have a primary key' do + let(:template_table) { :_partitioning_migration_helper_test_table } + let(:partition_column) { :some_field } + + it 'raises an error' do + migration.create_table template_table, id: false do |t| + t.integer :id + t.datetime partition_column + end + + expect do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/primary key not defined for #{template_table}/) + end + end + + context 'when an invalid partition column is given' do + let(:partition_column) { :_this_is_not_real } + + it 'raises an error' do + expect do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/partition column #{partition_column} does not exist/) + end + end + + describe 'constructing the partitioned table' do + it 'creates a table partitioned by the proper column' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + expect(connection.table_exists?(partitioned_table)).to be(true) + expect(connection.primary_key(partitioned_table)).to eq(new_primary_key) + + expect_table_partitioned_by(partitioned_table, [partition_column]) + end + + it 'changes the primary key datatype to bigint' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } + + expect(pk_column.sql_type).to eq('bigint') + end + + context 'with a non-integer primary key datatype' do + before do + connection.create_table :another_example, id: false do |t| + t.string :identifier, primary_key: true + t.timestamp :created_at + end + end + + let(:template_table) { :another_example } + let(:old_primary_key) { 'identifier' } + + it 'does not change the primary key datatype' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + original_pk_column = connection.columns(template_table).find { |c| c.name == old_primary_key } + pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } + + expect(pk_column).not_to be_nil + expect(pk_column).to eq(original_pk_column) + end + end + + it 'removes the default from the primary key column' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } + + expect(pk_column.default_function).to be_nil + end + + it 'creates the partitioned table with the same non-key columns' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + copied_columns = filter_columns_by_name(connection.columns(partitioned_table), new_primary_key) + original_columns = filter_columns_by_name(connection.columns(template_table), new_primary_key) + + expect(copied_columns).to match_array(original_columns) + end + + it 'creates a partition spanning over each month in the range given' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + expect_range_partition_of("#{partitioned_table}_000000", partitioned_table, 'MINVALUE', "'2019-12-01 00:00:00'") + expect_range_partition_of("#{partitioned_table}_201912", partitioned_table, "'2019-12-01 00:00:00'", "'2020-01-01 00:00:00'") + expect_range_partition_of("#{partitioned_table}_202001", partitioned_table, "'2020-01-01 00:00:00'", "'2020-02-01 00:00:00'") + expect_range_partition_of("#{partitioned_table}_202002", partitioned_table, "'2020-02-01 00:00:00'", "'2020-03-01 00:00:00'") + end + end + + describe 'keeping data in sync with the partitioned table' do + let(:template_table) { :todos } + let(:model) { Class.new(ActiveRecord::Base) } + let(:timestamp) { Time.utc(2019, 12, 1, 12).round } + + before do + model.primary_key = :id + model.table_name = partitioned_table + end + + it 'creates a trigger function on the original table' do + expect_function_not_to_exist(function_name) + expect_trigger_not_to_exist(template_table, trigger_name) + + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + expect_function_to_exist(function_name) + expect_valid_function_trigger(template_table, trigger_name, function_name, after: %w[delete insert update]) + end + + it 'syncs inserts to the partitioned tables' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + expect(model.count).to eq(0) + + first_todo = create(:todo, created_at: timestamp, updated_at: timestamp) + second_todo = create(:todo, created_at: timestamp, updated_at: timestamp) + + expect(model.count).to eq(2) + expect(model.find(first_todo.id).attributes).to eq(first_todo.attributes) + expect(model.find(second_todo.id).attributes).to eq(second_todo.attributes) + end + + it 'syncs updates to the partitioned tables' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + first_todo = create(:todo, :pending, commit_id: nil, created_at: timestamp, updated_at: timestamp) + second_todo = create(:todo, created_at: timestamp, updated_at: timestamp) + + expect(model.count).to eq(2) + + first_copy = model.find(first_todo.id) + second_copy = model.find(second_todo.id) + + expect(first_copy.attributes).to eq(first_todo.attributes) + expect(second_copy.attributes).to eq(second_todo.attributes) + + first_todo.update(state_event: 'done', commit_id: 'abc123', updated_at: timestamp + 1.second) + + expect(model.count).to eq(2) + expect(first_copy.reload.attributes).to eq(first_todo.attributes) + expect(second_copy.reload.attributes).to eq(second_todo.attributes) + end + + it 'syncs deletes to the partitioned tables' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + first_todo = create(:todo, created_at: timestamp, updated_at: timestamp) + second_todo = create(:todo, created_at: timestamp, updated_at: timestamp) + + expect(model.count).to eq(2) + + first_todo.destroy + + expect(model.count).to eq(1) + expect(model.find_by_id(first_todo.id)).to be_nil + expect(model.find(second_todo.id).attributes).to eq(second_todo.attributes) + end + end + end + + describe '#drop_partitioned_table_for' do + let(:expected_tables) do + %w[000000 201912 202001 202002].map { |suffix| "#{partitioned_table}_#{suffix}" }.unshift(partitioned_table) + end + + context 'when the table is not whitelisted' do + let(:template_table) { :this_table_is_not_whitelisted } + + it 'raises an error' do + expect(migration).to receive(:assert_table_is_whitelisted).with(template_table).and_call_original + + expect do + migration.drop_partitioned_table_for template_table + end.to raise_error(/#{template_table} is not whitelisted for use/) + end + end + + it 'drops the trigger syncing to the partitioned table' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + expect_function_to_exist(function_name) + expect_valid_function_trigger(template_table, trigger_name, function_name, after: %w[delete insert update]) + + migration.drop_partitioned_table_for template_table + + expect_function_not_to_exist(function_name) + expect_trigger_not_to_exist(template_table, trigger_name) + end + + it 'drops the partitioned copy and all partitions' do + migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + + expected_tables.each do |table| + expect(connection.table_exists?(table)).to be(true) + end + + migration.drop_partitioned_table_for template_table + + expected_tables.each do |table| + expect(connection.table_exists?(table)).to be(false) + end + end + end + + def filter_columns_by_name(columns, names) + columns.reject { |c| names.include?(c.name) } + end +end diff --git a/spec/lib/gitlab/database/schema_cleaner_spec.rb b/spec/lib/gitlab/database/schema_cleaner_spec.rb index ee9477156fb..adaeb85d52d 100644 --- a/spec/lib/gitlab/database/schema_cleaner_spec.rb +++ b/spec/lib/gitlab/database/schema_cleaner_spec.rb @@ -15,10 +15,6 @@ describe Gitlab::Database::SchemaCleaner do expect(subject).not_to include('COMMENT ON EXTENSION') end - it 'includes the plpgsql extension' do - expect(subject).to include('CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;') - end - it 'sets the search_path' do expect(subject.split("\n").first).to eq('SET search_path=public;') end diff --git a/spec/lib/gitlab/database/with_lock_retries_spec.rb b/spec/lib/gitlab/database/with_lock_retries_spec.rb index 9c8c9749125..d7eee594631 100644 --- a/spec/lib/gitlab/database/with_lock_retries_spec.rb +++ b/spec/lib/gitlab/database/with_lock_retries_spec.rb @@ -35,9 +35,6 @@ describe Gitlab::Database::WithLockRetries do end context 'when lock retry is enabled' do - class ActiveRecordSecond < ActiveRecord::Base - end - let(:lock_fiber) do Fiber.new do # Initiating a second DB connection for the lock @@ -52,6 +49,8 @@ describe Gitlab::Database::WithLockRetries do end before do + stub_const('ActiveRecordSecond', Class.new(ActiveRecord::Base)) + lock_fiber.resume # start the transaction and lock the table end |