Commit 0bbbd7c8 authored by Michael Kozono's avatar Michael Kozono

Merge branch 'fix-uploads-migrator' into 'master'

Migrate only AttachmentUploader note uploads

See merge request gitlab-org/gitlab!18873
parents 547a9267 1c71de07
...@@ -18,6 +18,7 @@ module Gitlab ...@@ -18,6 +18,7 @@ module Gitlab
def execute def execute
return unless upload return unless upload
return unless upload.model_type == 'Note'
if !project if !project
# if we don't have models associated with the upload we can not move it # if we don't have models associated with the upload we can not move it
......
...@@ -14,7 +14,7 @@ module Gitlab ...@@ -14,7 +14,7 @@ module Gitlab
include Database::MigrationHelpers include Database::MigrationHelpers
def perform(start_id, end_id) def perform(start_id, end_id)
Upload.where(id: start_id..end_id, uploader: 'AttachmentUploader').find_each do |upload| Upload.where(id: start_id..end_id, uploader: 'AttachmentUploader', model_type: 'Note').find_each do |upload|
LegacyUploadMover.new(upload).execute LegacyUploadMover.new(upload).execute
end end
end end
......
...@@ -15,7 +15,7 @@ namespace :gitlab do ...@@ -15,7 +15,7 @@ namespace :gitlab do
batch_size = 5000 batch_size = 5000
delay_interval = 5.minutes.to_i delay_interval = 5.minutes.to_i
Upload.where(uploader: 'AttachmentUploader').each_batch(of: batch_size) do |relation, index| Upload.where(uploader: 'AttachmentUploader', model_type: 'Note').each_batch(of: batch_size) do |relation, index|
start_id, end_id = relation.pluck('MIN(id), MAX(id)').first start_id, end_id = relation.pluck('MIN(id), MAX(id)').first
delay = index * delay_interval delay = index * delay_interval
......
...@@ -91,15 +91,26 @@ describe Gitlab::BackgroundMigration::LegacyUploadMover do ...@@ -91,15 +91,26 @@ describe Gitlab::BackgroundMigration::LegacyUploadMover do
end end
end end
context 'when no model found for the upload' do context 'when no note found for the upload' do
before do before do
legacy_upload.model = nil legacy_upload.model_id = nil
legacy_upload.model_type = 'Note'
expect_error_log expect_error_log
end end
it_behaves_like 'legacy upload deletion' it_behaves_like 'legacy upload deletion'
end end
context 'when upload does not belong to a note' do
before do
legacy_upload.model = create(:appearance)
end
it 'does not remove the upload' do
expect { described_class.new(legacy_upload).execute }.not_to change { Upload.count }
end
end
context 'when the upload move fails' do context 'when the upload move fails' do
before do before do
expect(FileUploader).to receive(:copy_to).and_raise('failed') expect(FileUploader).to receive(:copy_to).and_raise('failed')
......
...@@ -35,6 +35,8 @@ describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do ...@@ -35,6 +35,8 @@ describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do
let!(:legacy_upload_no_file) { create_upload(note2, false) } let!(:legacy_upload_no_file) { create_upload(note2, false) }
let!(:legacy_upload_legacy_project) { create_upload(note_legacy) } let!(:legacy_upload_legacy_project) { create_upload(note_legacy) }
let!(:appearance) { create(:appearance, :with_logo) }
let(:start_id) { 1 } let(:start_id) { 1 }
let(:end_id) { 10000 } let(:end_id) { 10000 }
...@@ -52,12 +54,18 @@ describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do ...@@ -52,12 +54,18 @@ describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do
expect(File.exist?(legacy_upload_legacy_project.absolute_path)).to be_falsey expect(File.exist?(legacy_upload_legacy_project.absolute_path)).to be_falsey
end end
it 'removes all AttachmentUploader records' do it 'removes all Note AttachmentUploader records' do
expect { subject }.to change { Upload.where(uploader: 'AttachmentUploader').count }.from(3).to(0) expect { subject }.to change { Upload.where(uploader: 'AttachmentUploader').count }.from(4).to(1)
end end
it 'creates new uploads for successfully migrated records' do it 'creates new uploads for successfully migrated records' do
expect { subject }.to change { Upload.where(uploader: 'FileUploader').count }.from(0).to(2) expect { subject }.to change { Upload.where(uploader: 'FileUploader').count }.from(0).to(2)
end end
it 'does not remove appearance uploads' do
subject
expect(appearance.logo.file).to exist
end
end end
# rubocop: enable RSpec/FactoriesInMigrationSpecs # rubocop: enable RSpec/FactoriesInMigrationSpecs
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment