Merge branch 'mk-add-old-attachments-to-uploads-table' into 'master'

Add old files to uploads table

See merge request gitlab-org/gitlab-ce!15270

4 files changed, 777 insertions, 1 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
new file mode 100644
index 00000000000..b80df6956b0
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb
@@ -0,0 +1,510 @@
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do
+  include TrackUntrackedUploadsHelpers
+
+  subject { described_class.new }
+
+  let!(:untracked_files_for_uploads) { described_class::UntrackedFile }
+  let!(:uploads) { described_class::Upload }
+
+  before do
+    DatabaseCleaner.clean
+    drop_temp_table_if_exists
+    ensure_temporary_tracking_table_exists
+    uploads.delete_all
+  end
+
+  after(:all) do
+    drop_temp_table_if_exists
+  end
+
+  context 'with untracked files and tracked files in untracked_files_for_uploads' do
+    let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
+    let!(:user1) { create(:user, :with_avatar) }
+    let!(:user2) { create(:user, :with_avatar) }
+    let!(:project1) { create(:project, :with_avatar) }
+    let!(:project2) { create(:project, :with_avatar) }
+
+    before do
+      UploadService.new(project1, uploaded_file, FileUploader).execute # Markdown upload
+      UploadService.new(project2, uploaded_file, FileUploader).execute # Markdown upload
+
+      # File records created by PrepareUntrackedUploads
+      untracked_files_for_uploads.create!(path: appearance.uploads.first.path)
+      untracked_files_for_uploads.create!(path: appearance.uploads.last.path)
+      untracked_files_for_uploads.create!(path: user1.uploads.first.path)
+      untracked_files_for_uploads.create!(path: user2.uploads.first.path)
+      untracked_files_for_uploads.create!(path: project1.uploads.first.path)
+      untracked_files_for_uploads.create!(path: project2.uploads.first.path)
+      untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project1.full_path}/#{project1.uploads.last.path}")
+      untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project2.full_path}/#{project2.uploads.last.path}")
+
+      # Untrack 4 files
+      user2.uploads.delete_all
+      project2.uploads.delete_all # 2 files: avatar and a Markdown upload
+      appearance.uploads.where("path like '%header_logo%'").delete_all
+    end
+
+    it 'adds untracked files to the uploads table' do
+      expect do
+        subject.perform(1, untracked_files_for_uploads.last.id)
+      end.to change { uploads.count }.from(4).to(8)
+
+      expect(user2.uploads.count).to eq(1)
+      expect(project2.uploads.count).to eq(2)
+      expect(appearance.uploads.count).to eq(2)
+    end
+
+    it 'deletes rows after processing them' do
+      expect(subject).to receive(:drop_temp_table_if_finished) # Don't drop the table so we can look at it
+
+      expect do
+        subject.perform(1, untracked_files_for_uploads.last.id)
+      end.to change { untracked_files_for_uploads.count }.from(8).to(0)
+    end
+
+    it 'does not create duplicate uploads of already tracked files' do
+      subject.perform(1, untracked_files_for_uploads.last.id)
+
+      expect(user1.uploads.count).to eq(1)
+      expect(project1.uploads.count).to eq(2)
+      expect(appearance.uploads.count).to eq(2)
+    end
+
+    it 'uses the start and end batch ids [only 1st half]' do
+      ids = untracked_files_for_uploads.all.order(:id).pluck(:id)
+      start_id = ids[0]
+      end_id = ids[3]
+
+      expect do
+        subject.perform(start_id, end_id)
+      end.to change { uploads.count }.from(4).to(6)
+
+      expect(user1.uploads.count).to eq(1)
+      expect(user2.uploads.count).to eq(1)
+      expect(appearance.uploads.count).to eq(2)
+      expect(project1.uploads.count).to eq(2)
+      expect(project2.uploads.count).to eq(0)
+
+      # Only 4 have been either confirmed or added to uploads
+      expect(untracked_files_for_uploads.count).to eq(4)
+    end
+
+    it 'uses the start and end batch ids [only 2nd half]' do
+      ids = untracked_files_for_uploads.all.order(:id).pluck(:id)
+      start_id = ids[4]
+      end_id = ids[7]
+
+      expect do
+        subject.perform(start_id, end_id)
+      end.to change { uploads.count }.from(4).to(6)
+
+      expect(user1.uploads.count).to eq(1)
+      expect(user2.uploads.count).to eq(0)
+      expect(appearance.uploads.count).to eq(1)
+      expect(project1.uploads.count).to eq(2)
+      expect(project2.uploads.count).to eq(2)
+
+      # Only 4 have been either confirmed or added to uploads
+      expect(untracked_files_for_uploads.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(ActiveRecord::Base.connection.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(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey
+    end
+
+    it 'does not block a whole batch because of one bad path' do
+      untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project2.full_path}/._7d37bf4c747916390e596744117d5d1a")
+      expect(untracked_files_for_uploads.count).to eq(9)
+      expect(uploads.count).to eq(4)
+
+      subject.perform(1, untracked_files_for_uploads.last.id)
+
+      expect(untracked_files_for_uploads.count).to eq(1)
+      expect(uploads.count).to eq(8)
+    end
+
+    it 'an unparseable path is shown in error output' do
+      bad_path = "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{project2.full_path}/._7d37bf4c747916390e596744117d5d1a"
+      untracked_files_for_uploads.create!(path: bad_path)
+
+      expect(Rails.logger).to receive(:error).with(/Error parsing path "#{bad_path}":/)
+
+      subject.perform(1, untracked_files_for_uploads.last.id)
+    end
+  end
+
+  context 'with no untracked files' do
+    it 'does not add to the uploads table (and does not raise error)' do
+      expect do
+        subject.perform(1, 1000)
+      end.not_to change { uploads.count }.from(0)
+    end
+  end
+
+  describe 'upload outcomes for each path pattern' do
+    shared_examples_for 'non_markdown_file' do
+      let!(:expected_upload_attrs) { model.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') }
+      let!(:untracked_file) { untracked_files_for_uploads.create!(path: expected_upload_attrs['path']) }
+
+      before do
+        model.uploads.delete_all
+      end
+
+      it 'creates an Upload record' do
+        expect do
+          subject.perform(1, untracked_files_for_uploads.last.id)
+        end.to change { model.reload.uploads.count }.from(0).to(1)
+
+        expect(model.uploads.first.attributes).to include(expected_upload_attrs)
+      end
+    end
+
+    context 'for an appearance logo file path' do
+      let(:model) { create_or_update_appearance(logo: uploaded_file) }
+
+      it_behaves_like 'non_markdown_file'
+    end
+
+    context 'for an appearance header_logo file path' do
+      let(:model) { create_or_update_appearance(header_logo: uploaded_file) }
+
+      it_behaves_like 'non_markdown_file'
+    end
+
+    context 'for a pre-Markdown Note attachment file path' do
+      class Note < ActiveRecord::Base
+        has_many :uploads, as: :model, dependent: :destroy
+      end
+
+      let(:model) { create(:note, :with_attachment) }
+
+      it_behaves_like 'non_markdown_file'
+    end
+
+    context 'for a user avatar file path' do
+      let(:model) { create(:user, :with_avatar) }
+
+      it_behaves_like 'non_markdown_file'
+    end
+
+    context 'for a group avatar file path' do
+      let(:model) { create(:group, :with_avatar) }
+
+      it_behaves_like 'non_markdown_file'
+    end
+
+    context 'for a project avatar file path' do
+      let(:model) { create(:project, :with_avatar) }
+
+      it_behaves_like 'non_markdown_file'
+    end
+
+    context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
+      let(:model) { create(:project) }
+
+      before do
+        # Upload the file
+        UploadService.new(model, uploaded_file, FileUploader).execute
+
+        # Create the untracked_files_for_uploads record
+        untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{model.full_path}/#{model.uploads.first.path}")
+
+        # Save the expected upload attributes
+        @expected_upload_attrs = model.reload.uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum')
+
+        # Untrack the file
+        model.reload.uploads.delete_all
+      end
+
+      it 'creates an Upload record' do
+        expect do
+          subject.perform(1, untracked_files_for_uploads.last.id)
+        end.to change { model.reload.uploads.count }.from(0).to(1)
+
+        expect(model.uploads.first.attributes).to include(@expected_upload_attrs)
+      end
+    end
+  end
+end
+
+describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do
+  include TrackUntrackedUploadsHelpers
+
+  let(:upload_class) { Gitlab::BackgroundMigration::PopulateUntrackedUploads::Upload }
+
+  before(:all) do
+    ensure_temporary_tracking_table_exists
+  end
+
+  after(:all) do
+    drop_temp_table_if_exists
+  end
+
+  describe '#upload_path' do
+    def assert_upload_path(file_path, expected_upload_path)
+      untracked_file = create_untracked_file(file_path)
+
+      expect(untracked_file.upload_path).to eq(expected_upload_path)
+    end
+
+    context 'for an appearance logo file path' do
+      it 'returns the file path relative to the CarrierWave root' do
+        assert_upload_path('/-/system/appearance/logo/1/some_logo.jpg', 'uploads/-/system/appearance/logo/1/some_logo.jpg')
+      end
+    end
+
+    context 'for an appearance header_logo file path' do
+      it 'returns the file path relative to the CarrierWave root' do
+        assert_upload_path('/-/system/appearance/header_logo/1/some_logo.jpg', 'uploads/-/system/appearance/header_logo/1/some_logo.jpg')
+      end
+    end
+
+    context 'for a pre-Markdown Note attachment file path' do
+      it 'returns the file path relative to the CarrierWave root' do
+        assert_upload_path('/-/system/note/attachment/1234/some_attachment.pdf', 'uploads/-/system/note/attachment/1234/some_attachment.pdf')
+      end
+    end
+
+    context 'for a user avatar file path' do
+      it 'returns the file path relative to the CarrierWave root' do
+        assert_upload_path('/-/system/user/avatar/1234/avatar.jpg', 'uploads/-/system/user/avatar/1234/avatar.jpg')
+      end
+    end
+
+    context 'for a group avatar file path' do
+      it 'returns the file path relative to the CarrierWave root' do
+        assert_upload_path('/-/system/group/avatar/1234/avatar.jpg', 'uploads/-/system/group/avatar/1234/avatar.jpg')
+      end
+    end
+
+    context 'for a project avatar file path' do
+      it 'returns the file path relative to the CarrierWave root' do
+        assert_upload_path('/-/system/project/avatar/1234/avatar.jpg', 'uploads/-/system/project/avatar/1234/avatar.jpg')
+      end
+    end
+
+    context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
+      it 'returns the file path relative to the project directory in uploads' do
+        project = create(:project)
+        random_hex = SecureRandom.hex
+
+        assert_upload_path("/#{project.full_path}/#{random_hex}/Some file.jpg", "#{random_hex}/Some file.jpg")
+      end
+    end
+  end
+
+  describe '#uploader' do
+    def assert_uploader(file_path, expected_uploader)
+      untracked_file = create_untracked_file(file_path)
+
+      expect(untracked_file.uploader).to eq(expected_uploader)
+    end
+
+    context 'for an appearance logo file path' do
+      it 'returns AttachmentUploader as a string' do
+        assert_uploader('/-/system/appearance/logo/1/some_logo.jpg', 'AttachmentUploader')
+      end
+    end
+
+    context 'for an appearance header_logo file path' do
+      it 'returns AttachmentUploader as a string' do
+        assert_uploader('/-/system/appearance/header_logo/1/some_logo.jpg', 'AttachmentUploader')
+      end
+    end
+
+    context 'for a pre-Markdown Note attachment file path' do
+      it 'returns AttachmentUploader as a string' do
+        assert_uploader('/-/system/note/attachment/1234/some_attachment.pdf', 'AttachmentUploader')
+      end
+    end
+
+    context 'for a user avatar file path' do
+      it 'returns AvatarUploader as a string' do
+        assert_uploader('/-/system/user/avatar/1234/avatar.jpg', 'AvatarUploader')
+      end
+    end
+
+    context 'for a group avatar file path' do
+      it 'returns AvatarUploader as a string' do
+        assert_uploader('/-/system/group/avatar/1234/avatar.jpg', 'AvatarUploader')
+      end
+    end
+
+    context 'for a project avatar file path' do
+      it 'returns AvatarUploader as a string' do
+        assert_uploader('/-/system/project/avatar/1234/avatar.jpg', 'AvatarUploader')
+      end
+    end
+
+    context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
+      it 'returns FileUploader as a string' do
+        project = create(:project)
+
+        assert_uploader("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", 'FileUploader')
+      end
+    end
+  end
+
+  describe '#model_type' do
+    def assert_model_type(file_path, expected_model_type)
+      untracked_file = create_untracked_file(file_path)
+
+      expect(untracked_file.model_type).to eq(expected_model_type)
+    end
+
+    context 'for an appearance logo file path' do
+      it 'returns Appearance as a string' do
+        assert_model_type('/-/system/appearance/logo/1/some_logo.jpg', 'Appearance')
+      end
+    end
+
+    context 'for an appearance header_logo file path' do
+      it 'returns Appearance as a string' do
+        assert_model_type('/-/system/appearance/header_logo/1/some_logo.jpg', 'Appearance')
+      end
+    end
+
+    context 'for a pre-Markdown Note attachment file path' do
+      it 'returns Note as a string' do
+        assert_model_type('/-/system/note/attachment/1234/some_attachment.pdf', 'Note')
+      end
+    end
+
+    context 'for a user avatar file path' do
+      it 'returns User as a string' do
+        assert_model_type('/-/system/user/avatar/1234/avatar.jpg', 'User')
+      end
+    end
+
+    context 'for a group avatar file path' do
+      it 'returns Namespace as a string' do
+        assert_model_type('/-/system/group/avatar/1234/avatar.jpg', 'Namespace')
+      end
+    end
+
+    context 'for a project avatar file path' do
+      it 'returns Project as a string' do
+        assert_model_type('/-/system/project/avatar/1234/avatar.jpg', 'Project')
+      end
+    end
+
+    context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
+      it 'returns Project as a string' do
+        project = create(:project)
+
+        assert_model_type("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", 'Project')
+      end
+    end
+  end
+
+  describe '#model_id' do
+    def assert_model_id(file_path, expected_model_id)
+      untracked_file = create_untracked_file(file_path)
+
+      expect(untracked_file.model_id).to eq(expected_model_id)
+    end
+
+    context 'for an appearance logo file path' do
+      it 'returns the ID as a string' do
+        assert_model_id('/-/system/appearance/logo/1/some_logo.jpg', 1)
+      end
+    end
+
+    context 'for an appearance header_logo file path' do
+      it 'returns the ID as a string' do
+        assert_model_id('/-/system/appearance/header_logo/1/some_logo.jpg', 1)
+      end
+    end
+
+    context 'for a pre-Markdown Note attachment file path' do
+      it 'returns the ID as a string' do
+        assert_model_id('/-/system/note/attachment/1234/some_attachment.pdf', 1234)
+      end
+    end
+
+    context 'for a user avatar file path' do
+      it 'returns the ID as a string' do
+        assert_model_id('/-/system/user/avatar/1234/avatar.jpg', 1234)
+      end
+    end
+
+    context 'for a group avatar file path' do
+      it 'returns the ID as a string' do
+        assert_model_id('/-/system/group/avatar/1234/avatar.jpg', 1234)
+      end
+    end
+
+    context 'for a project avatar file path' do
+      it 'returns the ID as a string' do
+        assert_model_id('/-/system/project/avatar/1234/avatar.jpg', 1234)
+      end
+    end
+
+    context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
+      it 'returns the ID as a string' do
+        project = create(:project)
+
+        assert_model_id("/#{project.full_path}/#{SecureRandom.hex}/Some file.jpg", project.id)
+      end
+    end
+  end
+
+  describe '#file_size' do
+    context 'for an appearance logo file path' do
+      let(:appearance) { create_or_update_appearance(logo: uploaded_file) }
+      let(:untracked_file) { described_class.create!(path: appearance.uploads.first.path) }
+
+      it 'returns the file size' do
+        expect(untracked_file.file_size).to eq(35255)
+      end
+
+      it 'returns the same thing that CarrierWave would return' do
+        expect(untracked_file.file_size).to eq(appearance.logo.size)
+      end
+    end
+
+    context 'for a project avatar file path' do
+      let(:project) { create(:project, avatar: uploaded_file) }
+      let(:untracked_file) { described_class.create!(path: project.uploads.first.path) }
+
+      it 'returns the file size' do
+        expect(untracked_file.file_size).to eq(35255)
+      end
+
+      it 'returns the same thing that CarrierWave would return' do
+        expect(untracked_file.file_size).to eq(project.avatar.size)
+      end
+    end
+
+    context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do
+      let(:project) { create(:project) }
+      let(:untracked_file) { create_untracked_file("/#{project.full_path}/#{project.uploads.first.path}") }
+
+      before do
+        UploadService.new(project, uploaded_file, FileUploader).execute
+      end
+
+      it 'returns the file size' do
+        expect(untracked_file.file_size).to eq(35255)
+      end
+
+      it 'returns the same thing that CarrierWave would return' do
+        expect(untracked_file.file_size).to eq(project.uploads.first.size)
+      end
+    end
+  end
+
+  def create_untracked_file(path_relative_to_upload_dir)
+    described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}#{path_relative_to_upload_dir}")
+  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
new file mode 100644
index 00000000000..cd3f1a45270
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
@@ -0,0 +1,242 @@
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
+  include TrackUntrackedUploadsHelpers
+
+  let!(:untracked_files_for_uploads) { described_class::UntrackedFile }
+
+  matcher :be_scheduled_migration do |*expected|
+    match do |migration|
+      BackgroundMigrationWorker.jobs.any? do |job|
+        job['args'] == [migration, expected]
+      end
+    end
+
+    failure_message do |migration|
+      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
+    end
+  end
+
+  before do
+    DatabaseCleaner.clean
+
+    drop_temp_table_if_exists
+  end
+
+  after do
+    drop_temp_table_if_exists
+  end
+
+  around do |example|
+    # Especially important so the follow-up migration does not get run
+    Sidekiq::Testing.fake! do
+      example.run
+    end
+  end
+
+  it 'ensures the untracked_files_for_uploads table exists' do
+    expect do
+      described_class.new.perform
+    end.to change { ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) }.from(false).to(true)
+  end
+
+  it 'has a path field long enough for really long paths' do
+    described_class.new.perform
+
+    component = 'a' * 255
+
+    long_path = [
+      'uploads',
+      component, # project.full_path
+      component  # filename
+    ].flatten.join('/')
+
+    record = untracked_files_for_uploads.create!(path: long_path)
+    expect(record.reload.path.size).to eq(519)
+  end
+
+  context "test bulk insert with ON CONFLICT DO NOTHING or IGNORE" do
+    around do |example|
+      # If this is CI, we use Postgres 9.2 so this whole context should be
+      # skipped since we're unable to use ON CONFLICT DO NOTHING or IGNORE.
+      if described_class.new.send(:can_bulk_insert_and_ignore_duplicates?)
+        example.run
+      end
+    end
+
+    context 'when files were uploaded before and after hashed storage was enabled' do
+      let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
+      let!(:user) { create(:user, :with_avatar) }
+      let!(:project1) { create(:project, :with_avatar) }
+      let(:project2) { create(:project) } # instantiate after enabling hashed_storage
+
+      before do
+        # Markdown upload before enabling hashed_storage
+        UploadService.new(project1, uploaded_file, FileUploader).execute
+
+        stub_application_setting(hashed_storage_enabled: true)
+
+        # Markdown upload after enabling hashed_storage
+        UploadService.new(project2, uploaded_file, FileUploader).execute
+      end
+
+      it 'adds unhashed files to the untracked_files_for_uploads table' do
+        described_class.new.perform
+
+        expect(untracked_files_for_uploads.count).to eq(5)
+      end
+
+      it 'adds files with paths relative to CarrierWave.root' do
+        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
+        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
+
+      it 'correctly schedules the follow-up background migration jobs' do
+        described_class.new.perform
+
+        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
+      context 'when there is existing data in untracked_files_for_uploads' do
+        before do
+          described_class.new.perform
+        end
+
+        it 'does not error or produce duplicates of existing data' do
+          expect do
+            described_class.new.perform
+          end.not_to change { untracked_files_for_uploads.count }.from(5)
+        end
+      end
+
+      # E.g. The installation is in use at the time of migration, and someone has
+      # just uploaded a file
+      context 'when there are files in /uploads/tmp' do
+        let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') }
+
+        before do
+          FileUtils.touch(tmp_file)
+        end
+
+        after do
+          FileUtils.rm(tmp_file)
+        end
+
+        it 'does not add files from /uploads/tmp' do
+          described_class.new.perform
+
+          expect(untracked_files_for_uploads.count).to eq(5)
+        end
+      end
+    end
+  end
+
+  context 'test bulk insert without ON CONFLICT DO NOTHING or IGNORE' do
+    before do
+      # If this is CI, we use Postgres 9.2 so this stub has no effect.
+      #
+      # If this is being run on Postgres 9.5+ or MySQL, then this stub allows us
+      # to test the bulk insert functionality without ON CONFLICT DO NOTHING or
+      # IGNORE.
+      allow_any_instance_of(described_class).to receive(:postgresql_pre_9_5?).and_return(true)
+    end
+
+    context 'when files were uploaded before and after hashed storage was enabled' do
+      let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) }
+      let!(:user) { create(:user, :with_avatar) }
+      let!(:project1) { create(:project, :with_avatar) }
+      let(:project2) { create(:project) } # instantiate after enabling hashed_storage
+
+      before do
+        # Markdown upload before enabling hashed_storage
+        UploadService.new(project1, uploaded_file, FileUploader).execute
+
+        stub_application_setting(hashed_storage_enabled: true)
+
+        # Markdown upload after enabling hashed_storage
+        UploadService.new(project2, uploaded_file, FileUploader).execute
+      end
+
+      it 'adds unhashed files to the untracked_files_for_uploads table' do
+        described_class.new.perform
+
+        expect(untracked_files_for_uploads.count).to eq(5)
+      end
+
+      it 'adds files with paths relative to CarrierWave.root' do
+        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
+        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
+
+      it 'correctly schedules the follow-up background migration jobs' do
+        described_class.new.perform
+
+        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
+      context 'when there is existing data in untracked_files_for_uploads' do
+        before do
+          described_class.new.perform
+        end
+
+        it 'does not error or produce duplicates of existing data' do
+          expect do
+            described_class.new.perform
+          end.not_to change { untracked_files_for_uploads.count }.from(5)
+        end
+      end
+
+      # E.g. The installation is in use at the time of migration, and someone has
+      # just uploaded a file
+      context 'when there are files in /uploads/tmp' do
+        let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') }
+
+        before do
+          FileUtils.touch(tmp_file)
+        end
+
+        after do
+          FileUtils.rm(tmp_file)
+        end
+
+        it 'does not add files from /uploads/tmp' do
+          described_class.new.perform
+
+          expect(untracked_files_for_uploads.count).to eq(5)
+        end
+      end
+    end
+  end
+
+  # Very new or lightly-used installations that are running this migration
+  # 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
+      described_class.new.perform
+
+      expect(untracked_files_for_uploads.count).to eq(0)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb
index a5657b81952..b2f13fae73f 100644
--- a/spec/lib/gitlab/database_spec.rb
+++ b/spec/lib/gitlab/database_spec.rb
@@ -221,6 +221,22 @@ describe Gitlab::Database do
       described_class.bulk_insert('test', rows)
     end
 
+    it 'does not quote values of a column in the disable_quote option' do
+      [1, 2, 4, 5].each do |i|
+        expect(connection).to receive(:quote).with(i)
+      end
+
+      described_class.bulk_insert('test', rows, disable_quote: :c)
+    end
+
+    it 'does not quote values of columns in the disable_quote option' do
+      [2, 5].each do |i|
+        expect(connection).to receive(:quote).with(i)
+      end
+
+      described_class.bulk_insert('test', rows, disable_quote: [:a, :c])
+    end
+
     it 'handles non-UTF-8 data' do
       expect { described_class.bulk_insert('test', [{ a: "\255" }]) }.not_to raise_error
     end
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 3137a72fdc4..e872a5290c5 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Utils do
-  delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, to: :described_class
+  delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, to: :described_class
 
   describe '.slugify' do
     {
@@ -59,4 +59,12 @@ describe Gitlab::Utils do
       expect(random_string).to be_kind_of(String)
     end
   end
+
+  describe '.which' do
+    it 'finds the full path to an executable binary' do
+      expect(File).to receive(:executable?).with('/bin/sh').and_return(true)
+
+      expect(which('sh', 'PATH' => '/bin')).to eq('/bin/sh')
+    end
+  end
 end