diff options
author | Michael Kozono <mkozono@gmail.com> | 2017-09-07 18:50:03 -0700 |
---|---|---|
committer | Michael Kozono <mkozono@gmail.com> | 2017-09-14 14:17:23 -0700 |
commit | f1e963bd89e737868a3a49358d714a288738c2e8 (patch) | |
tree | f6dabb0d992f24eef6ce23f40781a797e8f0bc5b | |
parent | bedcb7f43ddb8d6e2cce9424e3d988fe1d5a7cc8 (diff) | |
download | gitlab-ce-f1e963bd89e737868a3a49358d714a288738c2e8.tar.gz |
Add specs for deleting conflicting redirects
-rw-r--r-- | db/post_migrate/20170907170235_delete_conflicting_redirect_routes.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/background_migration/delete_conflicting_redirect_routes_range.rb (renamed from lib/gitlab/background_migration/delete_conflicting_redirect_routes.rb) | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range_spec.rb | 40 | ||||
-rw-r--r-- | spec/migrations/delete_conflicting_redirect_routes_spec.rb | 51 |
4 files changed, 94 insertions, 3 deletions
diff --git a/db/post_migrate/20170907170235_delete_conflicting_redirect_routes.rb b/db/post_migrate/20170907170235_delete_conflicting_redirect_routes.rb index 4111c884c28..73bb4603eb6 100644 --- a/db/post_migrate/20170907170235_delete_conflicting_redirect_routes.rb +++ b/db/post_migrate/20170907170235_delete_conflicting_redirect_routes.rb @@ -7,7 +7,7 @@ class DeleteConflictingRedirectRoutes < ActiveRecord::Migration DOWNTIME = false BATCH_SIZE = 1000 # Number of rows to process per job JOB_BUFFER_SIZE = 1000 # Number of jobs to bulk queue at a time - MIGRATION = 'DeleteConflictingRedirectRoutes'.freeze + MIGRATION = 'DeleteConflictingRedirectRoutesRange'.freeze disable_ddl_transaction! diff --git a/lib/gitlab/background_migration/delete_conflicting_redirect_routes.rb b/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range.rb index c33b6d513da..a45fb6b6d0c 100644 --- a/lib/gitlab/background_migration/delete_conflicting_redirect_routes.rb +++ b/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range.rb @@ -1,6 +1,6 @@ module Gitlab module BackgroundMigration - class DeleteConflictingRedirectRoutes + class DeleteConflictingRedirectRoutesRange class Route < ActiveRecord::Base self.table_name = 'routes' end @@ -17,7 +17,7 @@ module Gitlab conflicts = RedirectRoute.where(routes_match_redirects_clause(start_id, end_id)) num_rows = conflicts.delete_all - Rails.logger.info("Gitlab::BackgroundMigration::DeleteConflictingRedirectRoutes [#{start_id}, #{end_id}] - Deleted #{num_rows} redirect routes that were conflicting with routes.") + Rails.logger.info("Gitlab::BackgroundMigration::DeleteConflictingRedirectRoutesRange [#{start_id}, #{end_id}] - Deleted #{num_rows} redirect routes that were conflicting with routes.") end def migrate? diff --git a/spec/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range_spec.rb b/spec/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range_spec.rb new file mode 100644 index 00000000000..5c471cbdeda --- /dev/null +++ b/spec/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::DeleteConflictingRedirectRoutesRange, :migration, schema: 20170907170235 do + let!(:redirect_routes) { table(:redirect_routes) } + let!(:routes) { table(:routes) } + + before do + routes.create!(id: 1, source_id: 1, source_type: 'Namespace', path: 'foo1') + routes.create!(id: 2, source_id: 2, source_type: 'Namespace', path: 'foo2') + routes.create!(id: 3, source_id: 3, source_type: 'Namespace', path: 'foo3') + routes.create!(id: 4, source_id: 4, source_type: 'Namespace', path: 'foo4') + routes.create!(id: 5, source_id: 5, source_type: 'Namespace', path: 'foo5') + + # Valid redirects + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'bar') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'bar2') + redirect_routes.create!(source_id: 2, source_type: 'Namespace', path: 'bar3') + + # Conflicting redirects + redirect_routes.create!(source_id: 2, source_type: 'Namespace', path: 'foo1') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo2') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo3') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo4') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo5') + end + + it 'deletes the conflicting redirect_routes in the range' do + expect(redirect_routes.count).to eq(8) + + expect do + described_class.new.perform(1, 3) + end.to change { redirect_routes.where("path like 'foo%'").count }.from(5).to(2) + + expect do + described_class.new.perform(4, 5) + end.to change { redirect_routes.where("path like 'foo%'").count }.from(2).to(0) + + expect(redirect_routes.count).to eq(3) + end +end diff --git a/spec/migrations/delete_conflicting_redirect_routes_spec.rb b/spec/migrations/delete_conflicting_redirect_routes_spec.rb new file mode 100644 index 00000000000..5d277028b96 --- /dev/null +++ b/spec/migrations/delete_conflicting_redirect_routes_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170907170235_delete_conflicting_redirect_routes') + +describe DeleteConflictingRedirectRoutes, :migration, :sidekiq do + let!(:redirect_routes) { table(:redirect_routes) } + let!(:routes) { table(:routes) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 2) + stub_const("#{described_class.name}::JOB_BUFFER_SIZE", 2) + + routes.create!(id: 1, source_id: 1, source_type: 'Namespace', path: 'foo1') + routes.create!(id: 2, source_id: 2, source_type: 'Namespace', path: 'foo2') + routes.create!(id: 3, source_id: 3, source_type: 'Namespace', path: 'foo3') + routes.create!(id: 4, source_id: 4, source_type: 'Namespace', path: 'foo4') + routes.create!(id: 5, source_id: 5, source_type: 'Namespace', path: 'foo5') + + # Valid redirects + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'bar') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'bar2') + redirect_routes.create!(source_id: 2, source_type: 'Namespace', path: 'bar3') + + # Conflicting redirects + redirect_routes.create!(source_id: 2, source_type: 'Namespace', path: 'foo1') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo2') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo3') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo4') + redirect_routes.create!(source_id: 1, source_type: 'Namespace', path: 'foo5') + end + + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq([described_class::MIGRATION, [1, 2]]) + expect(BackgroundMigrationWorker.jobs[1]['args']).to eq([described_class::MIGRATION, [3, 4]]) + expect(BackgroundMigrationWorker.jobs[2]['args']).to eq([described_class::MIGRATION, [5, 5]]) + expect(BackgroundMigrationWorker.jobs.size).to eq 3 + end + end + end + + it 'schedules background migrations' do + Sidekiq::Testing.inline! do + expect do + migrate! + end.to change { redirect_routes.count }.from(8).to(3) + end + end +end |