diff options
Diffstat (limited to 'spec/lib/gitlab/background_migration')
19 files changed, 646 insertions, 202 deletions
diff --git a/spec/lib/gitlab/background_migration/backfill_artifact_expiry_date_spec.rb b/spec/lib/gitlab/background_migration/backfill_artifact_expiry_date_spec.rb deleted file mode 100644 index f5d2224747a..00000000000 --- a/spec/lib/gitlab/background_migration/backfill_artifact_expiry_date_spec.rb +++ /dev/null @@ -1,82 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::BackfillArtifactExpiryDate, :migration, schema: 20210301200959 do - subject(:perform) { migration.perform(1, 99) } - - let(:migration) { described_class.new } - let(:artifact_outside_id_range) { create_artifact!(id: 100, created_at: 1.year.ago, expire_at: nil) } - let(:artifact_outside_date_range) { create_artifact!(id: 40, created_at: Time.current, expire_at: nil) } - let(:old_artifact) { create_artifact!(id: 10, created_at: 16.months.ago, expire_at: nil) } - let(:recent_artifact) { create_artifact!(id: 20, created_at: 1.year.ago, expire_at: nil) } - let(:artifact_with_expiry) { create_artifact!(id: 30, created_at: 1.year.ago, expire_at: Time.current + 1.day) } - - before do - table(:namespaces).create!(id: 1, name: 'the-namespace', path: 'the-path') - table(:projects).create!(id: 1, name: 'the-project', namespace_id: 1) - table(:ci_builds).create!(id: 1, allow_failure: false) - end - - context 'when current date is before the 22nd' do - before do - travel_to(Time.zone.local(2020, 1, 1, 0, 0, 0)) - end - - it 'backfills the expiry date for old artifacts' do - expect(old_artifact.reload.expire_at).to eq(nil) - - perform - - expect(old_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2020, 4, 22, 0, 0, 0)) - end - - it 'backfills the expiry date for recent artifacts' do - expect(recent_artifact.reload.expire_at).to eq(nil) - - perform - - expect(recent_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2021, 1, 22, 0, 0, 0)) - end - end - - context 'when current date is after the 22nd' do - before do - travel_to(Time.zone.local(2020, 1, 23, 0, 0, 0)) - end - - it 'backfills the expiry date for old artifacts' do - expect(old_artifact.reload.expire_at).to eq(nil) - - perform - - expect(old_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2020, 5, 22, 0, 0, 0)) - end - - it 'backfills the expiry date for recent artifacts' do - expect(recent_artifact.reload.expire_at).to eq(nil) - - perform - - expect(recent_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2021, 2, 22, 0, 0, 0)) - end - end - - it 'does not touch artifacts with expiry date' do - expect { perform }.not_to change { artifact_with_expiry.reload.expire_at } - end - - it 'does not touch artifacts outside id range' do - expect { perform }.not_to change { artifact_outside_id_range.reload.expire_at } - end - - it 'does not touch artifacts outside date range' do - expect { perform }.not_to change { artifact_outside_date_range.reload.expire_at } - end - - private - - def create_artifact!(**args) - table(:ci_job_artifacts).create!(**args, project_id: 1, job_id: 1, file_type: 1) - end -end diff --git a/spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_spec.rb b/spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_spec.rb index 1158eedfe7c..84611c88806 100644 --- a/spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillDraftStatusOnMergeRequests, end end - it "updates all open draft merge request's draft field to true" do + it "updates all eligible draft merge request's draft field to true" do mr_count = merge_requests.all.count expect { subject.perform(mr_ids.first, mr_ids.last) } diff --git a/spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_with_corrected_regex_spec.rb b/spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_with_corrected_regex_spec.rb new file mode 100644 index 00000000000..e6e10977143 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_with_corrected_regex_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillDraftStatusOnMergeRequestsWithCorrectedRegex, + :migration, schema: 20220326161803 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:merge_requests) { table(:merge_requests) } + + let(:group) { namespaces.create!(name: 'gitlab', path: 'gitlab') } + let(:project) { projects.create!(namespace_id: group.id) } + + let(:draft_prefixes) { ["[Draft]", "(Draft)", "Draft:", "Draft", "[WIP]", "WIP:", "WIP"] } + + def create_merge_request(params) + common_params = { + target_project_id: project.id, + target_branch: 'feature1', + source_branch: 'master' + } + + merge_requests.create!(common_params.merge(params)) + end + + context "for MRs with #draft? == true titles but draft attribute false" do + let(:mr_ids) { merge_requests.all.collect(&:id) } + + before do + draft_prefixes.each do |prefix| + (1..4).each do |n| + create_merge_request( + title: "#{prefix} This is a title", + draft: false, + state_id: n + ) + + create_merge_request( + title: "This is a title with the #{prefix} in a weird spot", + draft: false, + state_id: n + ) + end + end + end + + it "updates all eligible draft merge request's draft field to true" do + mr_count = merge_requests.all.count + + expect { subject.perform(mr_ids.first, mr_ids.last) } + .to change { MergeRequest.where(draft: false).count } + .from(mr_count).to(mr_count - draft_prefixes.length) + end + + it "marks successful slices as completed" do + expect(subject).to receive(:mark_job_as_succeeded).with(mr_ids.first, mr_ids.last) + + subject.perform(mr_ids.first, mr_ids.last) + end + + it_behaves_like 'marks background migration job records' do + let!(:non_eligible_mrs) do + Array.new(2) do + create_merge_request( + title: "Not a d-r-a-f-t 1", + draft: false, + state_id: 1 + ) + end + end + + let(:arguments) { [non_eligible_mrs.first.id, non_eligible_mrs.last.id] } + end + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_group_features_spec.rb b/spec/lib/gitlab/background_migration/backfill_group_features_spec.rb index 4705f0d0ab9..d84bc479554 100644 --- a/spec/lib/gitlab/background_migration/backfill_group_features_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_group_features_spec.rb @@ -6,7 +6,15 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillGroupFeatures, :migration, s let(:group_features) { table(:group_features) } let(:namespaces) { table(:namespaces) } - subject { described_class.new(connection: ActiveRecord::Base.connection) } + subject do + described_class.new(start_id: 1, + end_id: 4, + batch_table: :namespaces, + batch_column: :id, + sub_batch_size: 10, + pause_ms: 0, + connection: ActiveRecord::Base.connection) + end describe '#perform' do it 'creates settings for all group namespaces in range' do @@ -19,7 +27,7 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillGroupFeatures, :migration, s group_features.create!(id: 1, group_id: 4) expect(group_features.count).to eq 1 - expect { subject.perform(1, 4, :namespaces, :id, 10, 0, 4) }.to change { group_features.count }.by(2) + expect { subject.perform(4) }.to change { group_features.count }.by(2) expect(group_features.count).to eq 3 expect(group_features.all.pluck(:group_id)).to contain_exactly(1, 3, 4) diff --git a/spec/lib/gitlab/background_migration/backfill_integrations_enable_ssl_verification_spec.rb b/spec/lib/gitlab/background_migration/backfill_integrations_enable_ssl_verification_spec.rb new file mode 100644 index 00000000000..b3825a7c4ea --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_integrations_enable_ssl_verification_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillIntegrationsEnableSslVerification, schema: 20220425121410 do + let(:migration) { described_class.new } + let(:integrations) { described_class::Integration } + + before do + integrations.create!(id: 1, type_new: 'Integrations::Bamboo') # unaffected integration + integrations.create!(id: 2, type_new: 'Integrations::DroneCi') # no properties + integrations.create!(id: 3, type_new: 'Integrations::DroneCi', + properties: {}) # no URL + integrations.create!(id: 4, type_new: 'Integrations::DroneCi', + properties: { 'drone_url' => '' }) # blank URL + integrations.create!(id: 5, type_new: 'Integrations::DroneCi', + properties: { 'drone_url' => 'https://example.com:foo' }) # invalid URL + integrations.create!(id: 6, type_new: 'Integrations::DroneCi', + properties: { 'drone_url' => 'https://example.com' }) # unknown URL + integrations.create!(id: 7, type_new: 'Integrations::DroneCi', + properties: { 'drone_url' => 'http://cloud.drone.io' }) # no HTTPS + integrations.create!(id: 8, type_new: 'Integrations::DroneCi', + properties: { 'drone_url' => 'https://cloud.drone.io' }) # known URL + integrations.create!(id: 9, type_new: 'Integrations::Teamcity', + properties: { 'teamcity_url' => 'https://example.com' }) # unknown URL + integrations.create!(id: 10, type_new: 'Integrations::Teamcity', + properties: { 'teamcity_url' => 'https://foo.bar.teamcity.com' }) # unknown URL + integrations.create!(id: 11, type_new: 'Integrations::Teamcity', + properties: { 'teamcity_url' => 'https://teamcity.com' }) # unknown URL + integrations.create!(id: 12, type_new: 'Integrations::Teamcity', + properties: { 'teamcity_url' => 'https://customer.teamcity.com' }) # known URL + end + + def properties(id) + integrations.find(id).properties + end + + it 'enables SSL verification for known-good hostnames', :aggregate_failures do + migration.perform(1, 12) + + # Bamboo + expect(properties(1)).to be_nil + + # DroneCi + expect(properties(2)).to be_nil + expect(properties(3)).not_to include('enable_ssl_verification') + expect(properties(4)).not_to include('enable_ssl_verification') + expect(properties(5)).not_to include('enable_ssl_verification') + expect(properties(6)).not_to include('enable_ssl_verification') + expect(properties(7)).not_to include('enable_ssl_verification') + expect(properties(8)).to include('enable_ssl_verification' => true) + + # Teamcity + expect(properties(9)).not_to include('enable_ssl_verification') + expect(properties(10)).not_to include('enable_ssl_verification') + expect(properties(11)).not_to include('enable_ssl_verification') + expect(properties(12)).to include('enable_ssl_verification' => true) + end + + it 'only updates records within the given ID range', :aggregate_failures do + migration.perform(1, 8) + + expect(properties(8)).to include('enable_ssl_verification' => true) + expect(properties(12)).not_to include('enable_ssl_verification') + end + + it 'marks the job as succeeded' do + expect(Gitlab::Database::BackgroundMigrationJob).to receive(:mark_all_as_succeeded) + .with('BackfillIntegrationsEnableSslVerification', [1, 10]) + + migration.perform(1, 10) + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb b/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb index 8f765a7a536..d8a7ec775dd 100644 --- a/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_integrations_type_new_spec.rb @@ -2,10 +2,19 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::BackfillIntegrationsTypeNew do +RSpec.describe Gitlab::BackgroundMigration::BackfillIntegrationsTypeNew, :migration, schema: 20220212120735 do let(:migration) { described_class.new } let(:integrations) { table(:integrations) } - let(:namespaced_integrations) { Gitlab::Integrations::StiType.namespaced_integrations } + + let(:namespaced_integrations) do + Set.new(%w[ + Asana Assembla Bamboo Bugzilla Buildkite Campfire Confluence CustomIssueTracker Datadog + Discord DroneCi EmailsOnPush Ewm ExternalWiki Flowdock HangoutsChat Harbor Irker Jenkins Jira Mattermost + MattermostSlashCommands MicrosoftTeams MockCi MockMonitoring Packagist PipelinesEmail Pivotaltracker + Prometheus Pushover Redmine Shimo Slack SlackSlashCommands Teamcity UnifyCircuit WebexTeams Youtrack Zentao + Github GitlabSlackApplication + ]).freeze + end before do integrations.connection.execute 'ALTER TABLE integrations DISABLE TRIGGER "trigger_type_new_on_insert"' diff --git a/spec/lib/gitlab/background_migration/backfill_note_discussion_id_spec.rb b/spec/lib/gitlab/background_migration/backfill_note_discussion_id_spec.rb new file mode 100644 index 00000000000..dcb4ede36af --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_note_discussion_id_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillNoteDiscussionId do + let(:migration) { described_class.new } + let(:notes_table) { table(:notes) } + let(:existing_discussion_id) { Digest::SHA1.hexdigest('test') } + + before do + notes_table.create!(id: 1, noteable_type: 'Issue', noteable_id: 2, discussion_id: existing_discussion_id) + notes_table.create!(id: 2, noteable_type: 'Issue', noteable_id: 1, discussion_id: nil) + notes_table.create!(id: 3, noteable_type: 'MergeRequest', noteable_id: 1, discussion_id: nil) + notes_table.create!(id: 4, noteable_type: 'Commit', commit_id: RepoHelpers.sample_commit.id, discussion_id: nil) + notes_table.create!(id: 5, noteable_type: 'Issue', noteable_id: 2, discussion_id: nil) + notes_table.create!(id: 6, noteable_type: 'MergeRequest', noteable_id: 2, discussion_id: nil) + end + + it 'updates records in the specified batch', :aggregate_failures do + migration.perform(1, 5) + + expect(notes_table.where(discussion_id: nil).count).to eq(1) + + expect(notes_table.find(1).discussion_id).to eq(existing_discussion_id) + notes_table.where(id: 2..5).each do |n| + expect(n.discussion_id).to match(/\A[0-9a-f]{40}\z/) + end + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_project_settings_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_settings_spec.rb new file mode 100644 index 00000000000..525c236b644 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_project_settings_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillProjectSettings, :migration, schema: 20220324165436 do + let(:migration) { described_class.new } + let(:namespaces_table) { table(:namespaces) } + let(:projects_table) { table(:projects) } + let(:project_settings_table) { table(:project_settings) } + + let(:table_name) { 'projects' } + let(:batch_column) { :id } + let(:sub_batch_size) { 2 } + let(:pause_ms) { 0 } + + subject(:perform_migration) { migration.perform(1, 30, table_name, batch_column, sub_batch_size, pause_ms) } + + before do + namespaces_table.create!(id: 1, name: 'namespace', path: 'namespace-path', type: 'Group') + projects_table.create!(id: 11, name: 'group-project-1', path: 'group-project-path-1', namespace_id: 1) + projects_table.create!(id: 12, name: 'group-project-2', path: 'group-project-path-2', namespace_id: 1) + project_settings_table.create!(project_id: 11) + + namespaces_table.create!(id: 2, name: 'namespace', path: 'namespace-path', type: 'User') + projects_table.create!(id: 21, name: 'user-project-1', path: 'user--project-path-1', namespace_id: 2) + projects_table.create!(id: 22, name: 'user-project-2', path: 'user-project-path-2', namespace_id: 2) + project_settings_table.create!(project_id: 21) + end + + it 'backfills project settings when it does not exist', :aggregate_failures do + expect(project_settings_table.count).to eq 2 + + queries = ActiveRecord::QueryRecorder.new do + perform_migration + end + + expect(queries.count).to eq(5) + + expect(project_settings_table.count).to eq 4 + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_topics_title_spec.rb b/spec/lib/gitlab/background_migration/backfill_topics_title_spec.rb new file mode 100644 index 00000000000..3c46456eed0 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_topics_title_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillTopicsTitle, schema: 20220331133802 do + it 'correctly backfills the title of the topics' do + topics = table(:topics) + + topic_1 = topics.create!(name: 'topic1') + topic_2 = topics.create!(name: 'topic2', title: 'Topic 2') + topic_3 = topics.create!(name: 'topic3') + topic_4 = topics.create!(name: 'topic4') + + subject.perform(topic_1.id, topic_3.id) + + expect(topic_1.reload.title).to eq('topic1') + expect(topic_2.reload.title).to eq('Topic 2') + expect(topic_3.reload.title).to eq('topic3') + expect(topic_4.reload.title).to be_nil + end +end diff --git a/spec/lib/gitlab/background_migration/batched_migration_job_spec.rb b/spec/lib/gitlab/background_migration/batched_migration_job_spec.rb new file mode 100644 index 00000000000..f8b3a8681f0 --- /dev/null +++ b/spec/lib/gitlab/background_migration/batched_migration_job_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BatchedMigrationJob do + describe '#perform' do + let(:connection) { Gitlab::Database.database_base_models[:main].connection } + + let(:job_class) { Class.new(described_class) } + + let(:job_instance) do + job_class.new(start_id: 1, end_id: 10, + batch_table: '_test_table', + batch_column: 'id', + sub_batch_size: 2, + pause_ms: 1000, + connection: connection) + end + + subject(:perform_job) { job_instance.perform } + + it 'raises an error if not overridden' do + expect { perform_job }.to raise_error(NotImplementedError, /must implement perform/) + end + + context 'when the subclass uses sub-batching' do + let(:job_class) do + Class.new(described_class) do + def perform(*job_arguments) + each_sub_batch( + operation_name: :update, + batching_arguments: { order_hint: :updated_at }, + batching_scope: -> (relation) { relation.where.not(bar: nil) } + ) do |sub_batch| + sub_batch.update_all('to_column = from_column') + end + end + end + end + + let(:test_table) { table(:_test_table) } + + before do + allow(job_instance).to receive(:sleep) + + connection.create_table :_test_table do |t| + t.timestamps_with_timezone null: false + t.integer :from_column, null: false + t.text :bar + t.integer :to_column + end + + test_table.create!(id: 1, from_column: 5, bar: 'value') + test_table.create!(id: 2, from_column: 10, bar: 'value') + test_table.create!(id: 3, from_column: 15) + test_table.create!(id: 4, from_column: 20, bar: 'value') + end + + after do + connection.drop_table(:_test_table) + end + + it 'calls the operation for each sub-batch' do + expect { perform_job }.to change { test_table.where(to_column: nil).count }.from(4).to(1) + + expect(test_table.order(:id).pluck(:to_column)).to contain_exactly(5, 10, nil, 20) + end + + it 'instruments the batch operation' do + expect(job_instance.batch_metrics.affected_rows).to be_empty + + expect(job_instance.batch_metrics).to receive(:instrument_operation).with(:update).twice.and_call_original + + perform_job + + expect(job_instance.batch_metrics.affected_rows[:update]).to contain_exactly(2, 1) + end + + it 'pauses after each sub-batch' do + expect(job_instance).to receive(:sleep).with(1.0).twice + + perform_job + end + + context 'when batching_arguments are given' do + it 'forwards them for batching' do + expect(job_instance).to receive(:parent_batch_relation).and_return(test_table) + + expect(test_table).to receive(:each_batch).with(column: 'id', of: 2, order_hint: :updated_at) + + perform_job + end + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/copy_column_using_background_migration_job_spec.rb b/spec/lib/gitlab/background_migration/copy_column_using_background_migration_job_spec.rb index 90d9bbb42c3..78bd1afd8d2 100644 --- a/spec/lib/gitlab/background_migration/copy_column_using_background_migration_job_spec.rb +++ b/spec/lib/gitlab/background_migration/copy_column_using_background_migration_job_spec.rb @@ -3,123 +3,134 @@ require 'spec_helper' RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJob do - let(:table_name) { :_test_copy_primary_key_test } - let(:test_table) { table(table_name) } - let(:sub_batch_size) { 1000 } - let(:pause_ms) { 0 } - let(:connection) { ApplicationRecord.connection } - - let(:helpers) do - ActiveRecord::Migration.new.extend(Gitlab::Database::MigrationHelpers) - end - - before do - connection.execute(<<~SQL) - CREATE TABLE #{table_name} - ( - id integer NOT NULL, - name character varying, - fk integer NOT NULL, - #{helpers.convert_to_bigint_column(:id)} bigint DEFAULT 0 NOT NULL, - #{helpers.convert_to_bigint_column(:fk)} bigint DEFAULT 0 NOT NULL, - name_convert_to_text text DEFAULT 'no name' - ); - SQL - - # Insert some data, it doesn't make a difference - test_table.create!(id: 11, name: 'test1', fk: 1) - test_table.create!(id: 12, name: 'test2', fk: 2) - test_table.create!(id: 15, name: nil, fk: 3) - test_table.create!(id: 19, name: 'test4', fk: 4) - end + it { expect(described_class).to be < Gitlab::BackgroundMigration::BatchedMigrationJob } - after do - # Make sure that the temp table we created is dropped (it is not removed by the database_cleaner) - connection.execute(<<~SQL) - DROP TABLE IF EXISTS #{table_name}; - SQL - end + describe '#perform' do + let(:table_name) { :_test_copy_primary_key_test } + let(:test_table) { table(table_name) } + let(:sub_batch_size) { 1000 } + let(:pause_ms) { 0 } + let(:connection) { ApplicationRecord.connection } + + let(:helpers) do + ActiveRecord::Migration.new.extend(Gitlab::Database::MigrationHelpers) + end - subject(:copy_columns) { described_class.new(connection: connection) } + let(:copy_job) do + described_class.new(start_id: 12, + end_id: 20, + batch_table: table_name, + batch_column: 'id', + sub_batch_size: sub_batch_size, + pause_ms: pause_ms, + connection: connection) + end - it { expect(described_class).to be < Gitlab::BackgroundMigration::BaseJob } + before do + connection.execute(<<~SQL) + CREATE TABLE #{table_name} + ( + id integer NOT NULL, + name character varying, + fk integer NOT NULL, + #{helpers.convert_to_bigint_column(:id)} bigint DEFAULT 0 NOT NULL, + #{helpers.convert_to_bigint_column(:fk)} bigint DEFAULT 0 NOT NULL, + name_convert_to_text text DEFAULT 'no name' + ); + SQL + + # Insert some data, it doesn't make a difference + test_table.create!(id: 11, name: 'test1', fk: 1) + test_table.create!(id: 12, name: 'test2', fk: 2) + test_table.create!(id: 15, name: nil, fk: 3) + test_table.create!(id: 19, name: 'test4', fk: 4) + end - describe '#perform' do - let(:migration_class) { described_class.name } + after do + # Make sure that the temp table we created is dropped (it is not removed by the database_cleaner) + connection.execute(<<~SQL) + DROP TABLE IF EXISTS #{table_name}; + SQL + end it 'copies all primary keys in range' do temporary_column = helpers.convert_to_bigint_column(:id) - copy_columns.perform(12, 15, table_name, 'id', sub_batch_size, pause_ms, 'id', temporary_column) - expect(test_table.where("id = #{temporary_column}").pluck(:id)).to contain_exactly(12, 15) - expect(test_table.where(temporary_column => 0).pluck(:id)).to contain_exactly(11, 19) - expect(test_table.all.count).to eq(4) + copy_job.perform('id', temporary_column) + + expect(test_table.count).to eq(4) + expect(test_table.where("id = #{temporary_column}").pluck(:id)).to contain_exactly(12, 15, 19) + expect(test_table.where(temporary_column => 0).pluck(:id)).to contain_exactly(11) end it 'copies all foreign keys in range' do temporary_column = helpers.convert_to_bigint_column(:fk) - copy_columns.perform(10, 14, table_name, 'id', sub_batch_size, pause_ms, 'fk', temporary_column) - expect(test_table.where("fk = #{temporary_column}").pluck(:id)).to contain_exactly(11, 12) - expect(test_table.where(temporary_column => 0).pluck(:id)).to contain_exactly(15, 19) - expect(test_table.all.count).to eq(4) + copy_job.perform('fk', temporary_column) + + expect(test_table.count).to eq(4) + expect(test_table.where("fk = #{temporary_column}").pluck(:id)).to contain_exactly(12, 15, 19) + expect(test_table.where(temporary_column => 0).pluck(:id)).to contain_exactly(11) end it 'copies columns with NULLs' do - expect(test_table.where("name_convert_to_text = 'no name'").count).to eq(4) + expect { copy_job.perform('name', 'name_convert_to_text') } + .to change { test_table.where("name_convert_to_text = 'no name'").count }.from(4).to(1) - copy_columns.perform(10, 20, table_name, 'id', sub_batch_size, pause_ms, 'name', 'name_convert_to_text') - - expect(test_table.where('name = name_convert_to_text').pluck(:id)).to contain_exactly(11, 12, 19) + expect(test_table.where('name = name_convert_to_text').pluck(:id)).to contain_exactly(12, 19) expect(test_table.where('name is NULL and name_convert_to_text is NULL').pluck(:id)).to contain_exactly(15) - expect(test_table.where("name_convert_to_text = 'no name'").count).to eq(0) end - it 'copies multiple columns when given' do - columns_to_copy_from = %w[id fk] - id_tmp_column = helpers.convert_to_bigint_column('id') - fk_tmp_column = helpers.convert_to_bigint_column('fk') - columns_to_copy_to = [id_tmp_column, fk_tmp_column] + context 'when multiple columns are given' do + let(:id_tmp_column) { helpers.convert_to_bigint_column('id') } + let(:fk_tmp_column) { helpers.convert_to_bigint_column('fk') } + let(:columns_to_copy_from) { %w[id fk] } + let(:columns_to_copy_to) { [id_tmp_column, fk_tmp_column] } - subject.perform(10, 15, table_name, 'id', sub_batch_size, pause_ms, columns_to_copy_from, columns_to_copy_to) + it 'copies all values in the range' do + copy_job.perform(columns_to_copy_from, columns_to_copy_to) - expect(test_table.where("id = #{id_tmp_column} AND fk = #{fk_tmp_column}").pluck(:id)).to contain_exactly(11, 12, 15) - expect(test_table.where(id_tmp_column => 0).where(fk_tmp_column => 0).pluck(:id)).to contain_exactly(19) - expect(test_table.all.count).to eq(4) - end + expect(test_table.count).to eq(4) + expect(test_table.where("id = #{id_tmp_column} AND fk = #{fk_tmp_column}").pluck(:id)).to contain_exactly(12, 15, 19) + expect(test_table.where(id_tmp_column => 0).where(fk_tmp_column => 0).pluck(:id)).to contain_exactly(11) + end - it 'raises error when number of source and target columns does not match' do - columns_to_copy_from = %w[id fk] - columns_to_copy_to = [helpers.convert_to_bigint_column(:id)] + context 'when the number of source and target columns does not match' do + let(:columns_to_copy_to) { [id_tmp_column] } - expect do - subject.perform(10, 15, table_name, 'id', sub_batch_size, pause_ms, columns_to_copy_from, columns_to_copy_to) - end.to raise_error(ArgumentError, 'number of source and destination columns must match') + it 'raises an error' do + expect do + copy_job.perform(columns_to_copy_from, columns_to_copy_to) + end.to raise_error(ArgumentError, 'number of source and destination columns must match') + end + end end it 'tracks timings of queries' do - expect(copy_columns.batch_metrics.timings).to be_empty + expect(copy_job.batch_metrics.timings).to be_empty - copy_columns.perform(10, 20, table_name, 'id', sub_batch_size, pause_ms, 'name', 'name_convert_to_text') + copy_job.perform('name', 'name_convert_to_text') - expect(copy_columns.batch_metrics.timings[:update_all]).not_to be_empty + expect(copy_job.batch_metrics.timings[:update_all]).not_to be_empty end context 'pause interval between sub-batches' do - it 'sleeps for the specified time between sub-batches' do - sub_batch_size = 2 + let(:pause_ms) { 5 } - expect(copy_columns).to receive(:sleep).with(0.005) + it 'sleeps for the specified time between sub-batches' do + expect(copy_job).to receive(:sleep).with(0.005) - copy_columns.perform(10, 12, table_name, 'id', sub_batch_size, 5, 'name', 'name_convert_to_text') + copy_job.perform('name', 'name_convert_to_text') end - it 'treats negative values as 0' do - sub_batch_size = 2 + context 'when pause_ms value is negative' do + let(:pause_ms) { -5 } - expect(copy_columns).to receive(:sleep).with(0) + it 'treats it as a 0' do + expect(copy_job).to receive(:sleep).with(0) - copy_columns.perform(10, 12, table_name, 'id', sub_batch_size, -5, 'name', 'name_convert_to_text') + copy_job.perform('name', 'name_convert_to_text') + end end end end diff --git a/spec/lib/gitlab/background_migration/expire_o_auth_tokens_spec.rb b/spec/lib/gitlab/background_migration/expire_o_auth_tokens_spec.rb new file mode 100644 index 00000000000..cffcda0a2ca --- /dev/null +++ b/spec/lib/gitlab/background_migration/expire_o_auth_tokens_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::ExpireOAuthTokens, :migration, schema: 20220428133724 do + let(:migration) { described_class.new } + let(:oauth_access_tokens_table) { table(:oauth_access_tokens) } + + let(:table_name) { 'oauth_access_tokens' } + + subject(:perform_migration) do + described_class.new(start_id: 1, + end_id: 30, + batch_table: :oauth_access_tokens, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: ActiveRecord::Base.connection) + .perform + end + + before do + oauth_access_tokens_table.create!(id: 1, token: 's3cr3t-1', expires_in: nil) + oauth_access_tokens_table.create!(id: 2, token: 's3cr3t-2', expires_in: 42) + oauth_access_tokens_table.create!(id: 3, token: 's3cr3t-3', expires_in: nil) + end + + it 'adds expiry to oauth tokens', :aggregate_failures do + expect(ActiveRecord::QueryRecorder.new { perform_migration }.count).to eq(3) + + expect(oauth_access_tokens_table.find(1).expires_in).to eq(7_200) + expect(oauth_access_tokens_table.find(2).expires_in).to eq(42) + expect(oauth_access_tokens_table.find(3).expires_in).to eq(7_200) + end +end diff --git a/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb b/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb index a111007a984..65d55f85a98 100644 --- a/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb +++ b/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Gitlab::BackgroundMigration::ExtractProjectTopicsIntoSeparateTabl # Tagging records expect { tagging_1.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { tagging_2.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect { other_tagging.reload }.not_to raise_error(ActiveRecord::RecordNotFound) + expect { other_tagging.reload }.not_to raise_error expect { tagging_3.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { tagging_4.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { tagging_5.reload }.to raise_error(ActiveRecord::RecordNotFound) diff --git a/spec/lib/gitlab/background_migration/job_coordinator_spec.rb b/spec/lib/gitlab/background_migration/job_coordinator_spec.rb index c1351481505..95847c67d94 100644 --- a/spec/lib/gitlab/background_migration/job_coordinator_spec.rb +++ b/spec/lib/gitlab/background_migration/job_coordinator_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Gitlab::BackgroundMigration::JobCoordinator do it 'raises an error' do expect do described_class.for_tracking_database('notvalid') - end.to raise_error(ArgumentError, /tracking_database must be one of/) + end.to raise_error(ArgumentError, /must be one of/) end end end diff --git a/spec/lib/gitlab/background_migration/merge_topics_with_same_name_spec.rb b/spec/lib/gitlab/background_migration/merge_topics_with_same_name_spec.rb index 254b4fea698..2c2c048992f 100644 --- a/spec/lib/gitlab/background_migration/merge_topics_with_same_name_spec.rb +++ b/spec/lib/gitlab/background_migration/merge_topics_with_same_name_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::MergeTopicsWithSameName, schema: 20220223124428 do +RSpec.describe Gitlab::BackgroundMigration::MergeTopicsWithSameName, schema: 20220331133802 do def set_avatar(topic_id, avatar) topic = ::Projects::Topic.find(topic_id) topic.avatar = avatar @@ -16,49 +16,62 @@ RSpec.describe Gitlab::BackgroundMigration::MergeTopicsWithSameName, schema: 202 topics = table(:topics) project_topics = table(:project_topics) - group = namespaces.create!(name: 'group', path: 'group') - project_1 = projects.create!(namespace_id: group.id, visibility_level: 20) - project_2 = projects.create!(namespace_id: group.id, visibility_level: 10) - project_3 = projects.create!(namespace_id: group.id, visibility_level: 0) + group_1 = namespaces.create!(name: 'space1', type: 'Group', path: 'space1') + group_2 = namespaces.create!(name: 'space2', type: 'Group', path: 'space2') + group_3 = namespaces.create!(name: 'space3', type: 'Group', path: 'space3') + proj_space_1 = namespaces.create!(name: 'proj1', path: 'proj1', type: 'Project', parent_id: group_1.id) + proj_space_2 = namespaces.create!(name: 'proj2', path: 'proj2', type: 'Project', parent_id: group_2.id) + proj_space_3 = namespaces.create!(name: 'proj3', path: 'proj3', type: 'Project', parent_id: group_3.id) + project_1 = projects.create!(namespace_id: group_1.id, project_namespace_id: proj_space_1.id, visibility_level: 20) + project_2 = projects.create!(namespace_id: group_2.id, project_namespace_id: proj_space_2.id, visibility_level: 10) + project_3 = projects.create!(namespace_id: group_3.id, project_namespace_id: proj_space_3.id, visibility_level: 0) topic_1_keep = topics.create!( name: 'topic1', + title: 'Topic 1', description: 'description 1 to keep', total_projects_count: 2, non_private_projects_count: 2 ) topic_1_remove = topics.create!( name: 'TOPIC1', + title: 'Topic 1', description: 'description 1 to remove', total_projects_count: 2, non_private_projects_count: 1 ) topic_2_remove = topics.create!( name: 'topic2', + title: 'Topic 2', total_projects_count: 0 ) topic_2_keep = topics.create!( name: 'TOPIC2', + title: 'Topic 2', description: 'description 2 to keep', total_projects_count: 1 ) topic_3_remove_1 = topics.create!( name: 'topic3', + title: 'Topic 3', total_projects_count: 2, non_private_projects_count: 1 ) topic_3_keep = topics.create!( name: 'Topic3', + title: 'Topic 3', total_projects_count: 2, non_private_projects_count: 2 ) topic_3_remove_2 = topics.create!( name: 'TOPIC3', + title: 'Topic 3', description: 'description 3 to keep', total_projects_count: 2, non_private_projects_count: 1 ) topic_4_keep = topics.create!( - name: 'topic4' + name: 'topic4', + title: 'Topic 4' ) project_topics_1 = [] diff --git a/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb b/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb index 90dd3e14606..e38edfc3643 100644 --- a/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb +++ b/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' RSpec.describe Gitlab::BackgroundMigration::NullifyOrphanRunnerIdOnCiBuilds, :migration, schema: 20220223112304 do let(:namespaces) { table(:namespaces) } let(:projects) { table(:projects) } - let(:ci_runners) { table(:ci_runners) } - let(:ci_pipelines) { table(:ci_pipelines) } - let(:ci_builds) { table(:ci_builds) } + let(:ci_runners) { table(:ci_runners, database: :ci) } + let(:ci_pipelines) { table(:ci_pipelines, database: :ci) } + let(:ci_builds) { table(:ci_builds, database: :ci) } subject { described_class.new } @@ -26,9 +26,9 @@ RSpec.describe Gitlab::BackgroundMigration::NullifyOrphanRunnerIdOnCiBuilds, :mi describe '#perform' do let(:namespace) { namespaces.create!(name: 'test', path: 'test', type: 'Group') } let(:project) { projects.create!(namespace_id: namespace.id, name: 'test') } - let(:pipeline) { ci_pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: 'success') } it 'nullifies runner_id for orphan ci_builds in range' do + pipeline = ci_pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: 'success') ci_runners.create!(id: 2, runner_type: 'project_type') ci_builds.create!(id: 5, type: 'Ci::Build', commit_id: pipeline.id, runner_id: 2) diff --git a/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_on_projects_spec.rb b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_on_projects_spec.rb index d02f7245c15..71020746fa7 100644 --- a/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_on_projects_spec.rb +++ b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_on_projects_spec.rb @@ -6,32 +6,47 @@ RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenEncrypte let(:namespaces) { table(:namespaces) } let(:projects) { table(:projects) } - let(:perform) { described_class.new.perform(1, 4) } + subject(:background_migration) { described_class.new } before do namespaces.create!(id: 123, name: 'sample', path: 'sample') projects.create!(id: 1, namespace_id: 123, runners_token_encrypted: 'duplicate') projects.create!(id: 2, namespace_id: 123, runners_token_encrypted: 'a-runners-token') - projects.create!(id: 3, namespace_id: 123, runners_token_encrypted: 'duplicate') + projects.create!(id: 3, namespace_id: 123, runners_token_encrypted: 'duplicate-2') projects.create!(id: 4, namespace_id: 123, runners_token_encrypted: nil) projects.create!(id: 5, namespace_id: 123, runners_token_encrypted: 'duplicate-2') - projects.create!(id: 6, namespace_id: 123, runners_token_encrypted: 'duplicate-2') + projects.create!(id: 6, namespace_id: 123, runners_token_encrypted: 'duplicate') + projects.create!(id: 7, namespace_id: 123, runners_token_encrypted: 'another-runners-token') + projects.create!(id: 8, namespace_id: 123, runners_token_encrypted: 'another-runners-token') end describe '#up' do - before do - stub_const("#{described_class}::SUB_BATCH_SIZE", 2) - end - it 'nullifies duplicate tokens', :aggregate_failures do - perform + background_migration.perform(1, 2) + background_migration.perform(3, 4) - expect(projects.count).to eq(6) + expect(projects.count).to eq(8) expect(projects.all.pluck(:id, :runners_token_encrypted).to_h).to eq( - { 1 => nil, 2 => 'a-runners-token', 3 => nil, 4 => nil, 5 => 'duplicate-2', 6 => 'duplicate-2' } - ) - expect(projects.pluck(:runners_token_encrypted).uniq).to match_array [nil, 'a-runners-token', 'duplicate-2'] + { + 1 => nil, + 2 => 'a-runners-token', + 3 => nil, + 4 => nil, + 5 => 'duplicate-2', + 6 => 'duplicate', + 7 => 'another-runners-token', + 8 => 'another-runners-token' + }) + expect(projects.pluck(:runners_token_encrypted).uniq).to match_array [ + nil, 'a-runners-token', 'duplicate', 'duplicate-2', 'another-runners-token' + ] + end + + it 'does not touch projects outside id range' do + expect do + background_migration.perform(1, 2) + end.not_to change { projects.where(id: [3..8]).each(&:reload).map(&:updated_at) } end end end diff --git a/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_on_projects_spec.rb b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_on_projects_spec.rb index fd61047d851..7d3df69bee2 100644 --- a/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_on_projects_spec.rb +++ b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_on_projects_spec.rb @@ -6,32 +6,47 @@ RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenValuesOn let(:namespaces) { table(:namespaces) } let(:projects) { table(:projects) } - let(:perform) { described_class.new.perform(1, 4) } + subject(:background_migration) { described_class.new } before do namespaces.create!(id: 123, name: 'sample', path: 'sample') projects.create!(id: 1, namespace_id: 123, runners_token: 'duplicate') projects.create!(id: 2, namespace_id: 123, runners_token: 'a-runners-token') - projects.create!(id: 3, namespace_id: 123, runners_token: 'duplicate') + projects.create!(id: 3, namespace_id: 123, runners_token: 'duplicate-2') projects.create!(id: 4, namespace_id: 123, runners_token: nil) projects.create!(id: 5, namespace_id: 123, runners_token: 'duplicate-2') - projects.create!(id: 6, namespace_id: 123, runners_token: 'duplicate-2') + projects.create!(id: 6, namespace_id: 123, runners_token: 'duplicate') + projects.create!(id: 7, namespace_id: 123, runners_token: 'another-runners-token') + projects.create!(id: 8, namespace_id: 123, runners_token: 'another-runners-token') end describe '#up' do - before do - stub_const("#{described_class}::SUB_BATCH_SIZE", 2) - end - it 'nullifies duplicate tokens', :aggregate_failures do - perform + background_migration.perform(1, 2) + background_migration.perform(3, 4) - expect(projects.count).to eq(6) + expect(projects.count).to eq(8) expect(projects.all.pluck(:id, :runners_token).to_h).to eq( - { 1 => nil, 2 => 'a-runners-token', 3 => nil, 4 => nil, 5 => 'duplicate-2', 6 => 'duplicate-2' } - ) - expect(projects.pluck(:runners_token).uniq).to match_array [nil, 'a-runners-token', 'duplicate-2'] + { + 1 => nil, + 2 => 'a-runners-token', + 3 => nil, + 4 => nil, + 5 => 'duplicate-2', + 6 => 'duplicate', + 7 => 'another-runners-token', + 8 => 'another-runners-token' + }) + expect(projects.pluck(:runners_token).uniq).to match_array [ + nil, 'a-runners-token', 'duplicate', 'duplicate-2', 'another-runners-token' + ] + end + + it 'does not touch projects outside id range' do + expect do + background_migration.perform(1, 2) + end.not_to change { projects.where(id: [3..8]).each(&:reload).map(&:updated_at) } end end end diff --git a/spec/lib/gitlab/background_migration/reset_too_many_tags_skipped_registry_imports_spec.rb b/spec/lib/gitlab/background_migration/reset_too_many_tags_skipped_registry_imports_spec.rb new file mode 100644 index 00000000000..3f59b0a24a3 --- /dev/null +++ b/spec/lib/gitlab/background_migration/reset_too_many_tags_skipped_registry_imports_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::ResetTooManyTagsSkippedRegistryImports, :migration, + :aggregate_failures, + schema: 20220502173045 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:container_repositories) { table(:container_repositories) } + + subject(:background_migration) { described_class.new } + + let!(:namespace) { namespaces.create!(id: 1, path: 'foo', name: 'foo') } + let!(:project) { projects.create!(id: 1, project_namespace_id: 1, namespace_id: 1, path: 'bar', name: 'bar') } + + let!(:container_repository1) do + container_repositories.create!(id: 1, + project_id: 1, + name: 'a', + migration_state: 'import_skipped', + migration_skipped_at: Time.zone.now, + migration_skipped_reason: 2, + migration_pre_import_started_at: Time.zone.now, + migration_pre_import_done_at: Time.zone.now, + migration_import_started_at: Time.zone.now, + migration_import_done_at: Time.zone.now, + migration_aborted_at: Time.zone.now, + migration_retries_count: 2, + migration_aborted_in_state: 'importing') + end + + let!(:container_repository2) do + container_repositories.create!(id: 2, + project_id: 1, + name: 'b', + migration_state: 'import_skipped', + migration_skipped_at: Time.zone.now, + migration_skipped_reason: 2) + end + + let!(:container_repository3) do + container_repositories.create!(id: 3, + project_id: 1, + name: 'c', + migration_state: 'import_skipped', + migration_skipped_at: Time.zone.now, + migration_skipped_reason: 1) + end + + # This is an unlikely state, but included here to test the edge case + let!(:container_repository4) do + container_repositories.create!(id: 4, + project_id: 1, + name: 'd', + migration_state: 'default', + migration_skipped_reason: 2) + end + + describe '#up' do + it 'resets only qualified container repositories', :aggregate_failures do + background_migration.perform(1, 4) + + expect(container_repository1.reload.migration_state).to eq('default') + expect(container_repository1.migration_skipped_reason).to eq(nil) + expect(container_repository1.migration_pre_import_started_at).to eq(nil) + expect(container_repository1.migration_pre_import_done_at).to eq(nil) + expect(container_repository1.migration_import_started_at).to eq(nil) + expect(container_repository1.migration_import_done_at).to eq(nil) + expect(container_repository1.migration_aborted_at).to eq(nil) + expect(container_repository1.migration_skipped_at).to eq(nil) + expect(container_repository1.migration_retries_count).to eq(0) + expect(container_repository1.migration_aborted_in_state).to eq(nil) + + expect(container_repository2.reload.migration_state).to eq('default') + expect(container_repository2.migration_skipped_reason).to eq(nil) + + expect(container_repository3.reload.migration_state).to eq('import_skipped') + expect(container_repository3.migration_skipped_reason).to eq(1) + + expect(container_repository4.reload.migration_state).to eq('default') + expect(container_repository4.migration_skipped_reason).to eq(2) + end + end +end |