diff options
Diffstat (limited to 'spec/lib')
6 files changed, 219 insertions, 47 deletions
diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb index f147e8204e6..97459d4a7be 100644 --- a/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb @@ -311,6 +311,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do let(:table_name) { :_test_batched_migrations_test_table } let(:column_name) { :some_id } let(:job_arguments) { [:some_id, :some_id_convert_to_bigint] } + let(:gitlab_schemas) { Gitlab::Database.gitlab_schemas_for_connection(connection) } let(:migration_status) { :active } @@ -358,7 +359,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do it 'completes the migration' do expect(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:find_for_configuration) - .with('CopyColumnUsingBackgroundMigrationJob', table_name, column_name, job_arguments) + .with(gitlab_schemas, 'CopyColumnUsingBackgroundMigrationJob', table_name, column_name, job_arguments) .and_return(batched_migration) expect(batched_migration).to receive(:finalize!).and_call_original @@ -399,7 +400,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do it 'is a no-op' do expect(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:find_for_configuration) - .with('CopyColumnUsingBackgroundMigrationJob', table_name, column_name, job_arguments) + .with(gitlab_schemas, 'CopyColumnUsingBackgroundMigrationJob', table_name, column_name, job_arguments) .and_return(batched_migration) configuration = { @@ -426,7 +427,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do context 'when the migration does not exist' do it 'is a no-op' do expect(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:find_for_configuration) - .with('CopyColumnUsingBackgroundMigrationJob', table_name, column_name, [:some, :other, :arguments]) + .with(gitlab_schemas, 'CopyColumnUsingBackgroundMigrationJob', table_name, column_name, [:some, :other, :arguments]) .and_return(nil) configuration = { diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb index a1c979bba50..8819171cfd0 100644 --- a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb @@ -78,23 +78,41 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m end describe '.active_migration' do + let(:connection) { Gitlab::Database.database_base_models[:main].connection } let!(:migration1) { create(:batched_background_migration, :finished) } - context 'without migrations on hold' do + subject(:active_migration) { described_class.active_migration(connection: connection) } + + around do |example| + Gitlab::Database::SharedModel.using_connection(connection) do + example.run + end + end + + context 'when there are no migrations on hold' do let!(:migration2) { create(:batched_background_migration, :active) } let!(:migration3) { create(:batched_background_migration, :active) } it 'returns the first active migration according to queue order' do - expect(described_class.active_migration).to eq(migration2) + expect(active_migration).to eq(migration2) end end - context 'with migrations are on hold' do + context 'when there are migrations on hold' do let!(:migration2) { create(:batched_background_migration, :active, on_hold_until: 10.minutes.from_now) } let!(:migration3) { create(:batched_background_migration, :active, on_hold_until: 2.minutes.ago) } it 'returns the first active migration that is not on hold according to queue order' do - expect(described_class.active_migration).to eq(migration3) + expect(active_migration).to eq(migration3) + end + end + + context 'when there are migrations not available for the current connection' do + let!(:migration2) { create(:batched_background_migration, :active, gitlab_schema: :gitlab_not_existing) } + let!(:migration3) { create(:batched_background_migration, :active, gitlab_schema: :gitlab_main) } + + it 'returns the first active migration that is available for the current connection' do + expect(active_migration).to eq(migration3) end end end @@ -553,25 +571,43 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m end describe '.for_configuration' do - let!(:migration) do - create( - :batched_background_migration, + let!(:attributes) do + { job_class_name: 'MyJobClass', table_name: :projects, column_name: :id, - job_arguments: [[:id], [:id_convert_to_bigint]] - ) + job_arguments: [[:id], [:id_convert_to_bigint]], + gitlab_schema: :gitlab_main + } end + let!(:migration) { create(:batched_background_migration, attributes) } + before do - create(:batched_background_migration, job_class_name: 'OtherClass') - create(:batched_background_migration, table_name: 'other_table') - create(:batched_background_migration, column_name: 'other_column') - create(:batched_background_migration, job_arguments: %w[other arguments]) + create(:batched_background_migration, attributes.merge(job_class_name: 'OtherClass')) + create(:batched_background_migration, attributes.merge(table_name: 'other_table')) + create(:batched_background_migration, attributes.merge(column_name: 'other_column')) + create(:batched_background_migration, attributes.merge(job_arguments: %w[other arguments])) end it 'finds the migration matching the given configuration parameters' do - actual = described_class.for_configuration('MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]]) + actual = described_class.for_configuration(:gitlab_main, 'MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]]) + + expect(actual).to contain_exactly(migration) + end + + it 'filters by gitlab schemas available for the connection' do + actual = described_class.for_configuration(:gitlab_ci, 'MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]]) + + expect(actual).to be_empty + end + + it 'doesn not filter by gitlab schemas available for the connection if the column is nor present' do + skip_if_multiple_databases_not_setup + + expect(described_class).to receive(:gitlab_schema_column_exists?).and_return(false) + + actual = described_class.for_configuration(:gitlab_main, 'MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]]) expect(actual).to contain_exactly(migration) end @@ -579,7 +615,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m describe '.find_for_configuration' do it 'returns nill if such migration does not exists' do - expect(described_class.find_for_configuration('MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]])).to be_nil + expect(described_class.find_for_configuration(:gitlab_main, 'MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]])).to be_nil end it 'returns the migration when it exists' do @@ -588,10 +624,25 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m job_class_name: 'MyJobClass', table_name: :projects, column_name: :id, - job_arguments: [[:id], [:id_convert_to_bigint]] + job_arguments: [[:id], [:id_convert_to_bigint]], + gitlab_schema: :gitlab_main ) - expect(described_class.find_for_configuration('MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]])).to eq(migration) + expect(described_class.find_for_configuration(:gitlab_main, 'MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]])).to eq(migration) + end + end + + describe '.for_gitlab_schema' do + let!(:migration) { create(:batched_background_migration, gitlab_schema: :gitlab_main) } + + before do + create(:batched_background_migration, gitlab_schema: :gitlab_not_existing) + end + + it 'finds the migrations matching the given gitlab schema' do + actual = described_class.for_gitlab_schema(:gitlab_main) + + expect(actual).to contain_exactly(migration) end end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 04fe1fad10e..5d9963bf3ea 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2242,10 +2242,16 @@ RSpec.describe Gitlab::Database::MigrationHelpers do } end + let(:migration_attributes) do + configuration.merge(gitlab_schema: Gitlab::Database.gitlab_schemas_for_connection(model.connection).first) + end + subject(:ensure_batched_background_migration_is_finished) { model.ensure_batched_background_migration_is_finished(**configuration) } it 'raises an error when migration exists and is not marked as finished' do - create(:batched_background_migration, :active, configuration) + 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) @@ -2265,13 +2271,19 @@ RSpec.describe Gitlab::Database::MigrationHelpers do end it 'does not raise error when migration exists and is marked as finished' do - create(:batched_background_migration, :finished, configuration) + 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}") @@ -2280,6 +2292,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do 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| @@ -2291,6 +2305,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do 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| 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 d1a66036149..87b6c65bf7e 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 @@ -12,6 +12,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d before do allow(Gitlab::Database::PgClass).to receive(:for_table).and_call_original + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!) end context 'when such migration already exists' do @@ -27,7 +28,8 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d batch_class_name: 'MyBatchClass', batch_size: 200, sub_batch_size: 20, - job_arguments: [[:id], [:id_convert_to_bigint]] + job_arguments: [[:id], [:id_convert_to_bigint]], + gitlab_schema: :gitlab_ci ) expect do @@ -41,7 +43,8 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d batch_max_value: 1000, batch_class_name: 'MyBatchClass', batch_size: 100, - sub_batch_size: 10) + sub_batch_size: 10, + gitlab_schema: :gitlab_ci) end.not_to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count } end end @@ -60,7 +63,8 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d batch_class_name: 'MyBatchClass', batch_size: 100, max_batch_size: 10000, - sub_batch_size: 10) + sub_batch_size: 10, + gitlab_schema: :gitlab_ci) end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) expect(Gitlab::Database::BackgroundMigration::BatchedMigration.last).to have_attributes( @@ -76,7 +80,8 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d sub_batch_size: 10, job_arguments: %w[], status_name: :active, - total_tuple_count: pgclass_info.cardinality_estimate) + total_tuple_count: pgclass_info.cardinality_estimate, + gitlab_schema: 'gitlab_ci') end context 'when the job interval is lower than the minimum' do @@ -162,11 +167,29 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d end end end + + context 'when gitlab_schema is not given' do + it 'fetches gitlab_schema from the migration context' do + expect(migration).to receive(:gitlab_schema_from_context).and_return(:gitlab_ci) + + expect do + migration.queue_batched_background_migration('MyJobClass', :events, :id, job_interval: 5.minutes) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + created_migration = Gitlab::Database::BackgroundMigration::BatchedMigration.last + + expect(created_migration.gitlab_schema).to eq('gitlab_ci') + end + end end describe '#finalize_batched_background_migration' do let!(:batched_migration) { create(:batched_background_migration, job_class_name: 'MyClass', table_name: :projects, column_name: :id, job_arguments: []) } + before do + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!) + end + it 'finalizes the migration' do allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| expect(runner).to receive(:finalize).with('MyClass', :projects, :id, []) @@ -204,4 +227,78 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d end end end + + describe '#delete_batched_background_migration' do + before do + expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!) + end + + context 'when migration exists' do + it 'deletes it' do + create( + :batched_background_migration, + job_class_name: 'MyJobClass', + table_name: :projects, + column_name: :id, + interval: 10.minutes, + min_value: 5, + max_value: 1005, + batch_class_name: 'MyBatchClass', + batch_size: 200, + sub_batch_size: 20, + job_arguments: [[:id], [:id_convert_to_bigint]] + ) + + expect do + migration.delete_batched_background_migration( + 'MyJobClass', + :projects, + :id, + [[:id], [:id_convert_to_bigint]]) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.from(1).to(0) + end + end + + context 'when migration does not exist' do + it 'does nothing' do + create( + :batched_background_migration, + job_class_name: 'SomeOtherJobClass', + table_name: :projects, + column_name: :id, + interval: 10.minutes, + min_value: 5, + max_value: 1005, + batch_class_name: 'MyBatchClass', + batch_size: 200, + sub_batch_size: 20, + job_arguments: [[:id], [:id_convert_to_bigint]] + ) + + expect do + migration.delete_batched_background_migration( + 'MyJobClass', + :projects, + :id, + [[:id], [:id_convert_to_bigint]]) + end.not_to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count } + end + end + end + + describe '#gitlab_schema_from_context' do + context 'when allowed_gitlab_schemas is not available' do + it 'defaults to :gitlab_main' do + expect(migration.gitlab_schema_from_context).to eq(:gitlab_main) + end + end + + context 'when allowed_gitlab_schemas is available' do + it 'uses schema from allowed_gitlab_schema' do + expect(migration).to receive(:allowed_gitlab_schemas).and_return([:gitlab_ci]) + + expect(migration.gitlab_schema_from_context).to eq(:gitlab_ci) + end + end + end end diff --git a/spec/lib/gitlab/patch/database_config_spec.rb b/spec/lib/gitlab/patch/database_config_spec.rb index 73dc84bb2ef..b06d28dbcd5 100644 --- a/spec/lib/gitlab/patch/database_config_spec.rb +++ b/spec/lib/gitlab/patch/database_config_spec.rb @@ -11,9 +11,6 @@ RSpec.describe Gitlab::Patch::DatabaseConfig do let(:configuration) { Rails::Application::Configuration.new(Rails.root) } before do - allow(File).to receive(:exist?).and_call_original - allow(File).to receive(:exist?).with(Rails.root.join("config/database_geo.yml")).and_return(false) - # The `AS::ConfigurationFile` calls `read` in `def initialize` # thus we cannot use `expect_next_instance_of` # rubocop:disable RSpec/AnyInstanceOf diff --git a/spec/lib/gitlab/subscription_portal_spec.rb b/spec/lib/gitlab/subscription_portal_spec.rb index 8d5a39baf77..098a58bff83 100644 --- a/spec/lib/gitlab/subscription_portal_spec.rb +++ b/spec/lib/gitlab/subscription_portal_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe ::Gitlab::SubscriptionPortal do using RSpec::Parameterized::TableSyntax + include SubscriptionPortalHelper let(:env_value) { nil } @@ -13,9 +14,9 @@ RSpec.describe ::Gitlab::SubscriptionPortal do describe '.default_subscriptions_url' do where(:test, :development, :result) do - false | false | 'https://customers.gitlab.com' - false | true | 'https://customers.staging.gitlab.com' - true | false | 'https://customers.staging.gitlab.com' + false | false | prod_customers_url + false | true | staging_customers_url + true | false | staging_customers_url end before do @@ -34,7 +35,7 @@ RSpec.describe ::Gitlab::SubscriptionPortal do subject { described_class.subscriptions_url } context 'when CUSTOMER_PORTAL_URL ENV is unset' do - it { is_expected.to eq('https://customers.staging.gitlab.com') } + it { is_expected.to eq(staging_customers_url) } end context 'when CUSTOMER_PORTAL_URL ENV is set' do @@ -54,17 +55,17 @@ RSpec.describe ::Gitlab::SubscriptionPortal do context 'url methods' do where(:method_name, :result) do - :default_subscriptions_url | 'https://customers.staging.gitlab.com' - :payment_form_url | 'https://customers.staging.gitlab.com/payment_forms/cc_validation' - :payment_validation_form_id | 'payment_method_validation' - :registration_validation_form_url | 'https://customers.staging.gitlab.com/payment_forms/cc_registration_validation' - :subscriptions_graphql_url | 'https://customers.staging.gitlab.com/graphql' - :subscriptions_more_minutes_url | 'https://customers.staging.gitlab.com/buy_pipeline_minutes' - :subscriptions_more_storage_url | 'https://customers.staging.gitlab.com/buy_storage' - :subscriptions_manage_url | 'https://customers.staging.gitlab.com/subscriptions' - :subscriptions_instance_review_url | 'https://customers.staging.gitlab.com/instance_review' - :subscriptions_gitlab_plans_url | 'https://customers.staging.gitlab.com/gitlab_plans' - :edit_account_url | 'https://customers.staging.gitlab.com/customers/edit' + :default_subscriptions_url | staging_customers_url + :payment_form_url | "#{staging_customers_url}/payment_forms/cc_validation" + :payment_validation_form_id | 'payment_method_validation' + :registration_validation_form_url | "#{staging_customers_url}/payment_forms/cc_registration_validation" + :subscriptions_graphql_url | "#{staging_customers_url}/graphql" + :subscriptions_more_minutes_url | "#{staging_customers_url}/buy_pipeline_minutes" + :subscriptions_more_storage_url | "#{staging_customers_url}/buy_storage" + :subscriptions_manage_url | "#{staging_customers_url}/subscriptions" + :subscriptions_instance_review_url | "#{staging_customers_url}/instance_review" + :subscriptions_gitlab_plans_url | "#{staging_customers_url}/gitlab_plans" + :edit_account_url | "#{staging_customers_url}/customers/edit" end with_them do @@ -79,7 +80,10 @@ RSpec.describe ::Gitlab::SubscriptionPortal do let(:group_id) { 153 } - it { is_expected.to eq("https://customers.staging.gitlab.com/gitlab/namespaces/#{group_id}/extra_seats") } + it do + url = "#{staging_customers_url}/gitlab/namespaces/#{group_id}/extra_seats" + is_expected.to eq(url) + end end describe '.upgrade_subscription_url' do @@ -88,7 +92,10 @@ RSpec.describe ::Gitlab::SubscriptionPortal do let(:group_id) { 153 } let(:plan_id) { 5 } - it { is_expected.to eq("https://customers.staging.gitlab.com/gitlab/namespaces/#{group_id}/upgrade/#{plan_id}") } + it do + url = "#{staging_customers_url}/gitlab/namespaces/#{group_id}/upgrade/#{plan_id}" + is_expected.to eq(url) + end end describe '.renew_subscription_url' do @@ -96,6 +103,9 @@ RSpec.describe ::Gitlab::SubscriptionPortal do let(:group_id) { 153 } - it { is_expected.to eq("https://customers.staging.gitlab.com/gitlab/namespaces/#{group_id}/renew") } + it do + url = "#{staging_customers_url}/gitlab/namespaces/#{group_id}/renew" + is_expected.to eq(url) + end end end |