Add specs for deleting conflicting redirects

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