diff options
Diffstat (limited to 'spec/lib/gitlab/database/migrations')
7 files changed, 1033 insertions, 15 deletions
diff --git a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb index f21f1ac5e52..d4fff947c29 100644 --- a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb @@ -14,9 +14,6 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do shared_examples_for 'helpers that enqueue background migrations' do |worker_class, connection_class, tracking_database| before do allow(model).to receive(:tracking_database).and_return(tracking_database) - - # Due to lib/gitlab/database/load_balancing/configuration.rb:92 requiring RequestStore - # we cannot use stub_feature_flags(force_no_sharing_primary_model: true) allow(connection_class.connection.load_balancer.configuration) .to receive(:use_dedicated_connection?).and_return(true) diff --git a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb index a2f6e6b43ed..3e249b14f2e 100644 --- a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb @@ -425,4 +425,99 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d end end end + + describe '#ensure_batched_background_migration_is_finished' do + let(:job_class_name) { 'CopyColumnUsingBackgroundMigrationJob' } + let(:table) { :events } + let(:column_name) { :id } + let(:job_arguments) { [["id"], ["id_convert_to_bigint"], nil] } + + let(:configuration) do + { + job_class_name: job_class_name, + table_name: table, + column_name: column_name, + job_arguments: job_arguments + } + end + + let(:migration_attributes) do + configuration.merge(gitlab_schema: Gitlab::Database.gitlab_schemas_for_connection(migration.connection).first) + end + + before do + allow(migration).to receive(:transaction_open?).and_return(false) + end + + subject(:ensure_batched_background_migration_is_finished) { migration.ensure_batched_background_migration_is_finished(**configuration) } + + it 'raises an error when migration exists and is not marked as finished' do + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!).twice + + create(:batched_background_migration, :active, migration_attributes) + + allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| + allow(runner).to receive(:finalize).with(job_class_name, table, column_name, job_arguments).and_return(false) + end + + expect { ensure_batched_background_migration_is_finished } + .to raise_error "Expected batched background migration for the given configuration to be marked as 'finished', but it is 'active':" \ + "\t#{configuration}" \ + "\n\n" \ + "Finalize it manually by running the following command in a `bash` or `sh` shell:" \ + "\n\n" \ + "\tsudo gitlab-rake gitlab:background_migrations:finalize[CopyColumnUsingBackgroundMigrationJob,events,id,'[[\"id\"]\\,[\"id_convert_to_bigint\"]\\,null]']" \ + "\n\n" \ + "For more information, check the documentation" \ + "\n\n" \ + "\thttps://docs.gitlab.com/ee/user/admin_area/monitoring/background_migrations.html#database-migrations-failing-because-of-batched-background-migration-not-finished" + end + + it 'does not raise error when migration exists and is marked as finished' do + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!) + + create(:batched_background_migration, :finished, migration_attributes) + + expect { ensure_batched_background_migration_is_finished } + .not_to raise_error + end + + it 'logs a warning when migration does not exist' do + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!) + + create(:batched_background_migration, :active, migration_attributes.merge(gitlab_schema: :gitlab_something_else)) + + expect(Gitlab::AppLogger).to receive(:warn) + .with("Could not find batched background migration for the given configuration: #{configuration}") + + expect { ensure_batched_background_migration_is_finished } + .not_to raise_error + end + + it 'finalizes the migration' do + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!).twice + + migration = create(:batched_background_migration, :active, configuration) + + allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| + expect(runner).to receive(:finalize).with(job_class_name, table, column_name, job_arguments).and_return(migration.finish!) + end + + ensure_batched_background_migration_is_finished + end + + context 'when the flag finalize is false' do + it 'does not finalize the migration' do + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!) + + create(:batched_background_migration, :active, configuration) + + allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| + expect(runner).not_to receive(:finalize).with(job_class_name, table, column_name, job_arguments) + end + + expect { migration.ensure_batched_background_migration_is_finished(**configuration.merge(finalize: false)) }.to raise_error(RuntimeError) + end + end + end end diff --git a/spec/lib/gitlab/database/migrations/constraints_helpers_spec.rb b/spec/lib/gitlab/database/migrations/constraints_helpers_spec.rb new file mode 100644 index 00000000000..6848fc85aa1 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/constraints_helpers_spec.rb @@ -0,0 +1,679 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::ConstraintsHelpers do + let(:model) do + ActiveRecord::Migration.new.extend(described_class) + end + + before do + allow(model).to receive(:puts) + end + + describe '#check_constraint_name' do + it 'returns a valid constraint name' do + name = model.check_constraint_name(:this_is_a_very_long_table_name, + :with_a_very_long_column_name, + :with_a_very_long_type) + + expect(name).to be_an_instance_of(String) + expect(name).to start_with('check_') + expect(name.length).to eq(16) + end + end + + describe '#check_constraint_exists?' do + before do + ActiveRecord::Migration.connection.execute( + 'ALTER TABLE projects ADD CONSTRAINT check_1 CHECK (char_length(path) <= 5) NOT VALID' + ) + + ActiveRecord::Migration.connection.execute( + 'CREATE SCHEMA new_test_schema' + ) + + ActiveRecord::Migration.connection.execute( + 'CREATE TABLE new_test_schema.projects (id integer, name character varying)' + ) + + ActiveRecord::Migration.connection.execute( + 'ALTER TABLE new_test_schema.projects ADD CONSTRAINT check_2 CHECK (char_length(name) <= 5)' + ) + end + + it 'returns true if a constraint exists' do + expect(model) + .to be_check_constraint_exists(:projects, 'check_1') + end + + it 'returns false if a constraint does not exist' do + expect(model) + .not_to be_check_constraint_exists(:projects, 'this_does_not_exist') + end + + it 'returns false if a constraint with the same name exists in another table' do + expect(model) + .not_to be_check_constraint_exists(:users, 'check_1') + end + + it 'returns false if a constraint with the same name exists for the same table in another schema' do + expect(model) + .not_to be_check_constraint_exists(:projects, 'check_2') + end + end + + describe '#add_check_constraint' do + before do + allow(model).to receive(:check_constraint_exists?).and_return(false) + end + + context 'when 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 'when inside a transaction' do + it 'raises an error' do + expect(model).to receive(:transaction_open?).and_return(true) + + expect do + model.add_check_constraint( + :test_table, + 'name IS NOT NULL', + 'check_name_not_null' + ) + end.to raise_error(RuntimeError) + end + end + + context 'when outside a transaction' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + + context 'when the constraint is already defined in the database' do + it 'does not create a constraint' do + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, 'check_name_not_null') + .and_return(true) + + expect(model).not_to receive(:execute).with(/ADD CONSTRAINT/) + + # setting validate: false to only focus on the ADD CONSTRAINT command + model.add_check_constraint( + :test_table, + 'name IS NOT NULL', + 'check_name_not_null', + validate: false + ) + end + end + + context 'when the constraint is not defined in the database' do + it 'creates the constraint' do + expect(model).to receive(:with_lock_retries).and_call_original + expect(model).to receive(:execute).with(/ADD CONSTRAINT check_name_not_null/) + + # setting validate: false to only focus on the ADD CONSTRAINT command + model.add_check_constraint( + :test_table, + 'char_length(name) <= 255', + 'check_name_not_null', + validate: false + ) + end + end + + context 'when validate is not provided' do + it 'performs validation' do + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, 'check_name_not_null') + .and_return(false).exactly(1) + + expect(model).to receive(:disable_statement_timeout).and_call_original + expect(model).to receive(:statement_timeout_disabled?).and_return(false) + expect(model).to receive(:execute).with(/SET statement_timeout TO/) + expect(model).to receive(:with_lock_retries).and_call_original + expect(model).to receive(:execute).with(/ADD CONSTRAINT check_name_not_null/) + + # we need the check constraint to exist so that the validation proceeds + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, 'check_name_not_null') + .and_return(true).exactly(1) + + expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/) + expect(model).to receive(:execute).ordered.with(/RESET statement_timeout/) + + model.add_check_constraint( + :test_table, + 'char_length(name) <= 255', + 'check_name_not_null' + ) + end + end + + context 'when validate is provided with a falsey value' do + it 'skips validation' do + expect(model).not_to receive(:disable_statement_timeout) + expect(model).to receive(:with_lock_retries).and_call_original + expect(model).to receive(:execute).with(/ADD CONSTRAINT/) + expect(model).not_to receive(:execute).with(/VALIDATE CONSTRAINT/) + + model.add_check_constraint( + :test_table, + 'char_length(name) <= 255', + 'check_name_not_null', + validate: false + ) + end + end + + context 'when validate is provided with a truthy value' do + it 'performs validation' do + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, 'check_name_not_null') + .and_return(false).exactly(1) + + expect(model).to receive(:disable_statement_timeout).and_call_original + expect(model).to receive(:statement_timeout_disabled?).and_return(false) + expect(model).to receive(:execute).with(/SET statement_timeout TO/) + expect(model).to receive(:with_lock_retries).and_call_original + expect(model).to receive(:execute).with(/ADD CONSTRAINT check_name_not_null/) + + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, 'check_name_not_null') + .and_return(true).exactly(1) + + expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/) + expect(model).to receive(:execute).ordered.with(/RESET statement_timeout/) + + model.add_check_constraint( + :test_table, + 'char_length(name) <= 255', + 'check_name_not_null', + validate: true + ) + end + end + end + end + + describe '#validate_check_constraint' do + context 'when the constraint does not exist' do + it 'raises an error' do + error_message = /Could not find check constraint "check_1" on table "test_table"/ + + expect(model).to receive(:check_constraint_exists?).and_return(false) + + expect do + model.validate_check_constraint(:test_table, 'check_1') + end.to raise_error(RuntimeError, error_message) + end + end + + context 'when the constraint exists' do + it 'performs validation' do + validate_sql = /ALTER TABLE test_table VALIDATE CONSTRAINT check_name/ + + expect(model).to receive(:check_constraint_exists?).and_return(true) + expect(model).to receive(:disable_statement_timeout).and_call_original + expect(model).to receive(:statement_timeout_disabled?).and_return(false) + expect(model).to receive(:execute).with(/SET statement_timeout TO/) + expect(model).to receive(:execute).ordered.with(validate_sql) + expect(model).to receive(:execute).ordered.with(/RESET statement_timeout/) + + model.validate_check_constraint(:test_table, 'check_name') + end + end + end + + describe '#remove_check_constraint' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + + it 'removes the constraint' do + drop_sql = /ALTER TABLE test_table\s+DROP CONSTRAINT IF EXISTS check_name/ + + expect(model).to receive(:with_lock_retries).and_call_original + expect(model).to receive(:execute).with(drop_sql) + + model.remove_check_constraint(:test_table, 'check_name') + end + end + + describe '#copy_check_constraints' do + context 'when inside a transaction' do + it 'raises an error' do + expect(model).to receive(:transaction_open?).and_return(true) + + expect do + model.copy_check_constraints(:test_table, :old_column, :new_column) + end.to raise_error(RuntimeError) + end + end + + context 'when outside a transaction' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + allow(model).to receive(:column_exists?).and_return(true) + end + + let(:old_column_constraints) do + [ + { + 'schema_name' => 'public', + 'table_name' => 'test_table', + 'column_name' => 'old_column', + 'constraint_name' => 'check_d7d49d475d', + 'constraint_def' => 'CHECK ((old_column IS NOT NULL))' + }, + { + 'schema_name' => 'public', + 'table_name' => 'test_table', + 'column_name' => 'old_column', + 'constraint_name' => 'check_48560e521e', + 'constraint_def' => 'CHECK ((char_length(old_column) <= 255))' + }, + { + 'schema_name' => 'public', + 'table_name' => 'test_table', + 'column_name' => 'old_column', + 'constraint_name' => 'custom_check_constraint', + 'constraint_def' => 'CHECK (((old_column IS NOT NULL) AND (another_column IS NULL)))' + }, + { + 'schema_name' => 'public', + 'table_name' => 'test_table', + 'column_name' => 'old_column', + 'constraint_name' => 'not_valid_check_constraint', + 'constraint_def' => 'CHECK ((old_column IS NOT NULL)) NOT VALID' + } + ] + end + + it 'copies check constraints from one column to another' do + allow(model).to receive(:check_constraints_for) + .with(:test_table, :old_column, schema: nil) + .and_return(old_column_constraints) + + allow(model).to receive(:not_null_constraint_name).with(:test_table, :new_column) + .and_return('check_1') + + allow(model).to receive(:text_limit_name).with(:test_table, :new_column) + .and_return('check_2') + + allow(model).to receive(:check_constraint_name) + .with(:test_table, :new_column, 'copy_check_constraint') + .and_return('check_3') + + expect(model).to receive(:add_check_constraint) + .with( + :test_table, + '(new_column IS NOT NULL)', + 'check_1', + validate: true + ).once + + expect(model).to receive(:add_check_constraint) + .with( + :test_table, + '(char_length(new_column) <= 255)', + 'check_2', + validate: true + ).once + + expect(model).to receive(:add_check_constraint) + .with( + :test_table, + '((new_column IS NOT NULL) AND (another_column IS NULL))', + 'check_3', + validate: true + ).once + + expect(model).to receive(:add_check_constraint) + .with( + :test_table, + '(new_column IS NOT NULL)', + 'check_1', + validate: false + ).once + + model.copy_check_constraints(:test_table, :old_column, :new_column) + end + + it 'does nothing if there are no constraints defined for the old column' do + allow(model).to receive(:check_constraints_for) + .with(:test_table, :old_column, schema: nil) + .and_return([]) + + expect(model).not_to receive(:add_check_constraint) + + model.copy_check_constraints(:test_table, :old_column, :new_column) + end + + it 'raises an error when the orginating column does not exist' do + allow(model).to receive(:column_exists?).with(:test_table, :old_column).and_return(false) + + error_message = /Column old_column does not exist on test_table/ + + expect do + model.copy_check_constraints(:test_table, :old_column, :new_column) + end.to raise_error(RuntimeError, error_message) + end + + it 'raises an error when the target column does not exist' do + allow(model).to receive(:column_exists?).with(:test_table, :new_column).and_return(false) + + error_message = /Column new_column does not exist on test_table/ + + expect do + model.copy_check_constraints(:test_table, :old_column, :new_column) + end.to raise_error(RuntimeError, error_message) + end + end + end + + describe '#add_text_limit' do + context 'when it is called with the default options' do + it 'calls add_check_constraint with an infered constraint name and validate: true' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'max_length') + check = "char_length(name) <= 255" + + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:add_check_constraint) + .with(:test_table, check, constraint_name, validate: true) + + model.add_text_limit(:test_table, :name, 255) + end + end + + context 'when all parameters are provided' do + it 'calls add_check_constraint with the correct parameters' do + constraint_name = 'check_name_limit' + check = "char_length(name) <= 255" + + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:add_check_constraint) + .with(:test_table, check, constraint_name, validate: false) + + model.add_text_limit( + :test_table, + :name, + 255, + constraint_name: constraint_name, + validate: false + ) + end + end + end + + describe '#validate_text_limit' do + context 'when constraint_name is not provided' do + it 'calls validate_check_constraint with an infered constraint name' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'max_length') + + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:validate_check_constraint) + .with(:test_table, constraint_name) + + model.validate_text_limit(:test_table, :name) + end + end + + context 'when constraint_name is provided' do + it 'calls validate_check_constraint with the correct parameters' do + constraint_name = 'check_name_limit' + + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:validate_check_constraint) + .with(:test_table, constraint_name) + + model.validate_text_limit(:test_table, :name, constraint_name: constraint_name) + end + end + end + + describe '#remove_text_limit' do + context 'when constraint_name is not provided' do + it 'calls remove_check_constraint with an infered constraint name' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'max_length') + + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:remove_check_constraint) + .with(:test_table, constraint_name) + + model.remove_text_limit(:test_table, :name) + end + end + + context 'when constraint_name is provided' do + it 'calls remove_check_constraint with the correct parameters' do + constraint_name = 'check_name_limit' + + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:remove_check_constraint) + .with(:test_table, constraint_name) + + model.remove_text_limit(:test_table, :name, constraint_name: constraint_name) + end + end + end + + describe '#check_text_limit_exists?' do + context 'when constraint_name is not provided' do + it 'calls check_constraint_exists? with an infered constraint name' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'max_length') + + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, constraint_name) + + model.check_text_limit_exists?(:test_table, :name) + end + end + + context 'when constraint_name is provided' do + it 'calls check_constraint_exists? with the correct parameters' do + constraint_name = 'check_name_limit' + + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, constraint_name) + + model.check_text_limit_exists?(:test_table, :name, constraint_name: constraint_name) + end + end + end + + describe '#add_not_null_constraint' do + context 'when it is called with the default options' do + it 'calls add_check_constraint with an infered constraint name and validate: true' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'not_null') + check = "name IS NOT NULL" + + expect(model).to receive(:column_is_nullable?).and_return(true) + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:add_check_constraint) + .with(:test_table, check, constraint_name, validate: true) + + model.add_not_null_constraint(:test_table, :name) + end + end + + context 'when all parameters are provided' do + it 'calls add_check_constraint with the correct parameters' do + constraint_name = 'check_name_not_null' + check = "name IS NOT NULL" + + expect(model).to receive(:column_is_nullable?).and_return(true) + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:add_check_constraint) + .with(:test_table, check, constraint_name, validate: false) + + model.add_not_null_constraint( + :test_table, + :name, + constraint_name: constraint_name, + validate: false + ) + end + end + + context 'when the column is defined as NOT NULL' do + it 'does not add a check constraint' do + expect(model).to receive(:column_is_nullable?).and_return(false) + expect(model).not_to receive(:check_constraint_name) + expect(model).not_to receive(:add_check_constraint) + + model.add_not_null_constraint(:test_table, :name) + end + end + end + + describe '#validate_not_null_constraint' do + context 'when constraint_name is not provided' do + it 'calls validate_check_constraint with an infered constraint name' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'not_null') + + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:validate_check_constraint) + .with(:test_table, constraint_name) + + model.validate_not_null_constraint(:test_table, :name) + end + end + + context 'when constraint_name is provided' do + it 'calls validate_check_constraint with the correct parameters' do + constraint_name = 'check_name_not_null' + + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:validate_check_constraint) + .with(:test_table, constraint_name) + + model.validate_not_null_constraint(:test_table, :name, constraint_name: constraint_name) + end + end + end + + describe '#remove_not_null_constraint' do + context 'when constraint_name is not provided' do + it 'calls remove_check_constraint with an infered constraint name' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'not_null') + + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:remove_check_constraint) + .with(:test_table, constraint_name) + + model.remove_not_null_constraint(:test_table, :name) + end + end + + context 'when constraint_name is provided' do + it 'calls remove_check_constraint with the correct parameters' do + constraint_name = 'check_name_not_null' + + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:remove_check_constraint) + .with(:test_table, constraint_name) + + model.remove_not_null_constraint(:test_table, :name, constraint_name: constraint_name) + end + end + end + + describe '#check_not_null_constraint_exists?' do + context 'when constraint_name is not provided' do + it 'calls check_constraint_exists? with an infered constraint name' do + constraint_name = model.check_constraint_name(:test_table, + :name, + 'not_null') + + expect(model).to receive(:check_constraint_name).and_call_original + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, constraint_name) + + model.check_not_null_constraint_exists?(:test_table, :name) + end + end + + context 'when constraint_name is provided' do + it 'calls check_constraint_exists? with the correct parameters' do + constraint_name = 'check_name_not_null' + + expect(model).not_to receive(:check_constraint_name) + expect(model).to receive(:check_constraint_exists?) + .with(:test_table, constraint_name) + + model.check_not_null_constraint_exists?(:test_table, :name, constraint_name: constraint_name) + end + end + end + + describe '#rename_constraint' do + it "executes the statement to rename constraint" do + expect(model).to receive(:execute).with( + /ALTER TABLE "test_table"\nRENAME CONSTRAINT "fk_old_name" TO "fk_new_name"/ + ) + + model.rename_constraint(:test_table, :fk_old_name, :fk_new_name) + end + end + + describe '#drop_constraint' do + it "executes the statement to drop the constraint" do + expect(model).to receive(:execute).with( + "ALTER TABLE \"test_table\" DROP CONSTRAINT \"constraint_name\" CASCADE\n" + ) + + model.drop_constraint(:test_table, :constraint_name, cascade: true) + end + + context 'when cascade option is false' do + it "executes the statement to drop the constraint without cascade" do + expect(model).to receive(:execute).with("ALTER TABLE \"test_table\" DROP CONSTRAINT \"constraint_name\" \n") + + model.drop_constraint(:test_table, :constraint_name, cascade: false) + end + end + end +end diff --git a/spec/lib/gitlab/database/migrations/extension_helpers_spec.rb b/spec/lib/gitlab/database/migrations/extension_helpers_spec.rb new file mode 100644 index 00000000000..fb29e06bc01 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/extension_helpers_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::ExtensionHelpers do + let(:model) do + ActiveRecord::Migration.new.extend(described_class) + end + + before do + allow(model).to receive(:puts) + end + + describe '#create_extension' do + subject { model.create_extension(extension) } + + let(:extension) { :btree_gist } + + it 'executes CREATE EXTENSION statement' do + expect(model).to receive(:execute).with(/CREATE EXTENSION IF NOT EXISTS #{extension}/) + + subject + end + + context 'without proper permissions' do + before do + allow(model).to receive(:execute) + .with(/CREATE EXTENSION IF NOT EXISTS #{extension}/) + .and_raise(ActiveRecord::StatementInvalid, 'InsufficientPrivilege: permission denied') + end + + it 'raises an exception and prints an error message' do + expect { subject } + .to output(/user is not allowed/).to_stderr + .and raise_error(ActiveRecord::StatementInvalid, /InsufficientPrivilege/) + end + end + end + + describe '#drop_extension' do + subject { model.drop_extension(extension) } + + let(:extension) { 'btree_gist' } + + it 'executes CREATE EXTENSION statement' do + expect(model).to receive(:execute).with(/DROP EXTENSION IF EXISTS #{extension}/) + + subject + end + + context 'without proper permissions' do + before do + allow(model).to receive(:execute) + .with(/DROP EXTENSION IF EXISTS #{extension}/) + .and_raise(ActiveRecord::StatementInvalid, 'InsufficientPrivilege: permission denied') + end + + it 'raises an exception and prints an error message' do + expect { subject } + .to output(/user is not allowed/).to_stderr + .and raise_error(ActiveRecord::StatementInvalid, /InsufficientPrivilege/) + end + end + end +end diff --git a/spec/lib/gitlab/database/migrations/lock_retries_helpers_spec.rb b/spec/lib/gitlab/database/migrations/lock_retries_helpers_spec.rb new file mode 100644 index 00000000000..a8739f6758f --- /dev/null +++ b/spec/lib/gitlab/database/migrations/lock_retries_helpers_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::LockRetriesHelpers do + let(:model) do + ActiveRecord::Migration.new.extend(described_class) + end + + describe '#with_lock_retries' do + let(:buffer) { StringIO.new } + let(:in_memory_logger) { Gitlab::JsonLogger.new(buffer) } + let(:env) { { 'DISABLE_LOCK_RETRIES' => 'true' } } + + it 'sets the migration class name in the logs' do + model.with_lock_retries(env: env, logger: in_memory_logger) {} + + buffer.rewind + expect(buffer.read).to include("\"class\":\"#{model.class}\"") + end + + where(raise_on_exhaustion: [true, false]) + + with_them do + it 'sets raise_on_exhaustion as requested' do + with_lock_retries = double + expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries) + expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: raise_on_exhaustion) + + model.with_lock_retries(env: env, logger: in_memory_logger, raise_on_exhaustion: raise_on_exhaustion) {} + end + end + + it 'does not raise on exhaustion by default' do + with_lock_retries = double + expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries) + expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: false) + + model.with_lock_retries(env: env, logger: in_memory_logger) {} + end + + it 'defaults to allowing subtransactions' do + with_lock_retries = double + + expect(Gitlab::Database::WithLockRetries) + .to receive(:new).with(hash_including(allow_savepoints: true)).and_return(with_lock_retries) + expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: false) + + model.with_lock_retries(env: env, logger: in_memory_logger) {} + end + end +end diff --git a/spec/lib/gitlab/database/migrations/runner_spec.rb b/spec/lib/gitlab/database/migrations/runner_spec.rb index f364ebfa522..bd382547689 100644 --- a/spec/lib/gitlab/database/migrations/runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/runner_spec.rb @@ -2,26 +2,65 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::Runner, :reestablished_active_record_base do - include Database::MultipleDatabases - let(:base_result_dir) { Pathname.new(Dir.mktmpdir) } let(:migration_runs) { [] } # This list gets populated as the runner tries to run migrations # Tests depend on all of these lists being sorted in the order migrations would be applied - let(:applied_migrations_other_branches) { [double(ActiveRecord::Migration, version: 1, name: 'migration_complete_other_branch')] } + let(:applied_migrations_other_branches) do + [ + double( + ActiveRecord::Migration, + version: 1, + name: 'migration_complete_other_branch', + filename: 'db/migrate/1_migration_complete_other_branch.rb' + ) + ] + end let(:applied_migrations_this_branch) do [ - double(ActiveRecord::Migration, version: 2, name: 'older_migration_complete_this_branch'), - double(ActiveRecord::Migration, version: 3, name: 'newer_migration_complete_this_branch') + double( + ActiveRecord::Migration, + version: 2, + name: 'older_migration_complete_this_branch', + filename: 'db/migrate/2_older_migration_complete_this_branch.rb' + ), + double( + ActiveRecord::Migration, + version: 3, + name: 'post_migration_complete_this_branch', + filename: 'db/post_migrate/3_post_migration_complete_this_branch.rb' + ), + double( + ActiveRecord::Migration, + version: 4, + name: 'newer_migration_complete_this_branch', + filename: 'db/migrate/4_newer_migration_complete_this_branch.rb' + ) ].sort_by(&:version) end let(:pending_migrations) do [ - double(ActiveRecord::Migration, version: 4, name: 'older_migration_pending'), - double(ActiveRecord::Migration, version: 5, name: 'newer_migration_pending') + double( + ActiveRecord::Migration, + version: 5, + name: 'older_migration_pending', + filename: 'db/migrate/5_older_migration_pending.rb' + ), + double( + ActiveRecord::Migration, + version: 6, + name: 'post_migration_pending', + filename: 'db/post_migrate/6_post_migration_pending.rb' + ), + double( + ActiveRecord::Migration, + version: 7, + name: 'newer_migration_pending', + filename: 'db/migrate/7_newer_migration_pending.rb' + ) ].sort_by(&:version) end @@ -87,11 +126,11 @@ RSpec.describe Gitlab::Database::Migrations::Runner, :reestablished_active_recor context 'running migrations' do subject(:up) { described_class.up(database: database, legacy_mode: legacy_mode) } - it 'runs the unapplied migrations in version order', :aggregate_failures do + it 'runs the unapplied migrations in regular/post order, then version order', :aggregate_failures do up.run - expect(migration_runs.map(&:dir)).to match_array([:up, :up]) - expect(migration_runs.map(&:version_to_migrate)).to eq(pending_migrations.map(&:version)) + expect(migration_runs.map(&:dir)).to match_array([:up, :up, :up]) + expect(migration_runs.map(&:version_to_migrate)).to eq([5, 7, 6]) end it 'writes a metadata file with the current schema version and database name' do @@ -130,8 +169,8 @@ RSpec.describe Gitlab::Database::Migrations::Runner, :reestablished_active_recor it 'runs the applied migrations for the current branch in reverse order', :aggregate_failures do down.run - expect(migration_runs.map(&:dir)).to match_array([:down, :down]) - expect(migration_runs.map(&:version_to_migrate)).to eq(applied_migrations_this_branch.reverse.map(&:version)) + expect(migration_runs.map(&:dir)).to match_array([:down, :down, :down]) + expect(migration_runs.map(&:version_to_migrate)).to eq([3, 4, 2]) end end diff --git a/spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb b/spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb new file mode 100644 index 00000000000..d35211af680 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::TimeoutHelpers do + let(:model) do + ActiveRecord::Migration.new.extend(described_class) + end + + describe '#disable_statement_timeout' do + it 'disables statement timeouts to current transaction only' do + expect(model).to receive(:execute).with('SET LOCAL statement_timeout TO 0') + + model.disable_statement_timeout + end + + # this specs runs without an enclosing transaction (:delete truncation method for db_cleaner) + context 'with real environment', :delete do + before do + model.execute("SET statement_timeout TO '20000'") + end + + after do + model.execute('RESET statement_timeout') + end + + it 'defines statement to 0 only for current transaction' do + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') + + model.connection.transaction do + model.disable_statement_timeout + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') + end + + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') + end + + context 'when passing a blocks' do + it 'disables statement timeouts on session level and executes the block' do + expect(model).to receive(:execute).with('SET statement_timeout TO 0') + expect(model).to receive(:execute).with('RESET statement_timeout').at_least(:once) + + expect { |block| model.disable_statement_timeout(&block) }.to yield_control + end + + # this specs runs without an enclosing transaction (:delete truncation method for db_cleaner) + context 'with real environment', :delete do + before do + model.execute("SET statement_timeout TO '20000'") + end + + after do + model.execute('RESET statement_timeout') + end + + it 'defines statement to 0 for any code run inside the block' do + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('20s') + + model.disable_statement_timeout do + model.connection.transaction do + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') + end + + expect(model.execute('SHOW statement_timeout').first['statement_timeout']).to eq('0') + end + end + end + end + end + + # This spec runs without an enclosing transaction (:delete truncation method for db_cleaner) + context 'when the statement_timeout is already disabled', :delete do + before do + ActiveRecord::Migration.connection.execute('SET statement_timeout TO 0') + end + + after do + # Use ActiveRecord::Migration.connection instead of model.execute + # so that this call is not counted below + ActiveRecord::Migration.connection.execute('RESET statement_timeout') + end + + it 'yields control without disabling the timeout or resetting' do + expect(model).not_to receive(:execute).with('SET statement_timeout TO 0') + expect(model).not_to receive(:execute).with('RESET statement_timeout') + + expect { |block| model.disable_statement_timeout(&block) }.to yield_control + end + end + end +end |