Drop temporary tracking table when finished

2 files changed, 57 insertions, 42 deletions

diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb
index c794a2f152b..52f57408bfa 100644
--- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb
+++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb
@@ -1,11 +1,19 @@
 require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_uploads')
 
-describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sidekiq, schema: 20171103140253 do
+describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sidekiq, :temp_table_may_drop, schema: 20171103140253 do
   include TrackUntrackedUploadsHelpers
 
+  subject { described_class.new }
+
   let!(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) }
   let!(:uploads) { table(:uploads) }
 
+  before do
+    # Prevent the TrackUntrackedUploads migration from running PrepareUntrackedUploads job
+    allow(BackgroundMigrationWorker).to receive(:perform_async).and_return(true)
+  end
+
   context 'with untracked files and tracked files in untracked_files_for_uploads' do
     let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) }
     let!(:user1) { create(:user, :with_avatar) }
@@ -35,7 +43,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
 
     it 'adds untracked files to the uploads table' do
       expect do
-        described_class.new.perform(1, 1000)
+        subject.perform(1, 1000)
       end.to change { uploads.count }.from(4).to(8)
 
       expect(user2.uploads.count).to eq(1)
@@ -44,13 +52,15 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
     end
 
     it 'sets all added or confirmed tracked files to tracked' do
+      expect(subject).to receive(:drop_temp_table_if_finished) # Don't drop the table so we can look at it
+
       expect do
-        described_class.new.perform(1, 1000)
+        subject.perform(1, 1000)
       end.to change { untracked_files_for_uploads.where(tracked: true).count }.from(0).to(8)
     end
 
     it 'does not create duplicate uploads of already tracked files' do
-      described_class.new.perform(1, 1000)
+      subject.perform(1, 1000)
 
       expect(user1.uploads.count).to eq(1)
       expect(project1.uploads.count).to eq(2)
@@ -62,7 +72,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
       end_id = untracked_files_for_uploads.all.to_a[3].id
 
       expect do
-        described_class.new.perform(start_id, end_id)
+        subject.perform(start_id, end_id)
       end.to change { uploads.count }.from(4).to(6)
 
       expect(user1.uploads.count).to eq(1)
@@ -80,7 +90,7 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
       end_id = untracked_files_for_uploads.all.to_a[7].id
 
       expect do
-        described_class.new.perform(start_id, end_id)
+        subject.perform(start_id, end_id)
       end.to change { uploads.count }.from(4).to(6)
 
       expect(user1.uploads.count).to eq(1)
@@ -92,12 +102,24 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :migration, :sid
       # Only 4 have been either confirmed or added to uploads
       expect(untracked_files_for_uploads.where(tracked: true).count).to eq(4)
     end
+
+    it 'does not drop the temporary tracking table after processing the batch, if there are still untracked rows' do
+      subject.perform(1, untracked_files_for_uploads.last.id - 1)
+
+      expect(table_exists?(:untracked_files_for_uploads)).to be_truthy
+    end
+
+    it 'drops the temporary tracking table after processing the batch, if there are no untracked rows left' do
+      subject.perform(1, untracked_files_for_uploads.last.id)
+
+      expect(table_exists?(:untracked_files_for_uploads)).to be_falsey
+    end
   end
 
   context 'with no untracked files' do
     it 'does not add to the uploads table (and does not raise error)' do
       expect do
-        described_class.new.perform(1, 1000)
+        subject.perform(1, 1000)
       end.not_to change { uploads.count }.from(0)
     end
   end
diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
index d61135912dd..8fd20fd0bb3 100644
--- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
+++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
@@ -17,6 +17,13 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
     end
   end
 
+  around do |example|
+    # Especially important so the follow-up migration does not get run
+    Sidekiq::Testing.fake! do
+      example.run
+    end
+  end
+
   context 'when files were uploaded before and after hashed storage was enabled' do
     let!(:appearance) { create(:appearance, logo: uploaded_file, header_logo: uploaded_file) }
     let!(:user) { create(:user, :with_avatar) }
@@ -34,38 +41,30 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
     end
 
     it 'adds unhashed files to the untracked_files_for_uploads table' do
-      Sidekiq::Testing.fake! do
-        expect do
-          described_class.new.perform
-        end.to change { untracked_files_for_uploads.count }.from(0).to(5)
-      end
+      expect do
+        described_class.new.perform
+      end.to change { untracked_files_for_uploads.count }.from(0).to(5)
     end
 
     it 'adds files with paths relative to CarrierWave.root' do
-      Sidekiq::Testing.fake! do
-        described_class.new.perform
-        untracked_files_for_uploads.all.each do |file|
-          expect(file.path.start_with?('uploads/')).to be_truthy
-        end
+      described_class.new.perform
+      untracked_files_for_uploads.all.each do |file|
+        expect(file.path.start_with?('uploads/')).to be_truthy
       end
     end
 
     it 'does not add hashed files to the untracked_files_for_uploads table' do
-      Sidekiq::Testing.fake! do
-        described_class.new.perform
+      described_class.new.perform
 
-        hashed_file_path = project2.uploads.where(uploader: 'FileUploader').first.path
-        expect(untracked_files_for_uploads.where("path like '%#{hashed_file_path}%'").exists?).to be_falsey
-      end
+      hashed_file_path = project2.uploads.where(uploader: 'FileUploader').first.path
+      expect(untracked_files_for_uploads.where("path like '%#{hashed_file_path}%'").exists?).to be_falsey
     end
 
     it 'correctly schedules the follow-up background migration jobs' do
-      Sidekiq::Testing.fake! do
-        described_class.new.perform
+      described_class.new.perform
 
-        expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(1, 5)
-        expect(BackgroundMigrationWorker.jobs.size).to eq(1)
-      end
+      expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(1, 5)
+      expect(BackgroundMigrationWorker.jobs.size).to eq(1)
     end
 
     # E.g. from a previous failed run of this background migration
@@ -75,11 +74,9 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
       end
 
       it 'does not error or produce duplicates of existing data' do
-        Sidekiq::Testing.fake! do
-          expect do
-            described_class.new.perform
-          end.not_to change { untracked_files_for_uploads.count }.from(5)
-        end
+        expect do
+          described_class.new.perform
+        end.not_to change { untracked_files_for_uploads.count }.from(5)
       end
     end
 
@@ -97,11 +94,9 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
       end
 
       it 'does not add files from /uploads/tmp' do
-        Sidekiq::Testing.fake! do
-          expect do
-            described_class.new.perform
-          end.to change { untracked_files_for_uploads.count }.from(0).to(5)
-        end
+        expect do
+          described_class.new.perform
+        end.to change { untracked_files_for_uploads.count }.from(0).to(5)
       end
     end
   end
@@ -110,11 +105,9 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :migration, :side
   # may not have an upload directory because they have no uploads.
   context 'when no files were ever uploaded' do
     it 'does not add to the untracked_files_for_uploads table (and does not raise error)' do
-      Sidekiq::Testing.fake! do
-        expect do
-          described_class.new.perform
-        end.not_to change { untracked_files_for_uploads.count }.from(0)
-      end
+      expect do
+        described_class.new.perform
+      end.not_to change { untracked_files_for_uploads.count }.from(0)
     end
   end
 end