Commit a14ebd41 authored by Douwe Maan's avatar Douwe Maan Committed by Mayra Cabrera

Merge branch '46147-remove-model-redefinition-worker' into 'master'

Resolve "NoMethodError: undefined method `uploader_context' for #<ObjectStorage::MigrateUploadsWorker::Upload:0x00007f59e..."

Closes #46147

See merge request gitlab-org/gitlab-ce!18820
parent 383b32d6
......@@ -9,85 +9,6 @@ module ObjectStorage
SanityCheckError = Class.new(StandardError)
class Upload < ActiveRecord::Base
# Upper limit for foreground checksum processing
CHECKSUM_THRESHOLD = 100.megabytes
belongs_to :model, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
validates :size, presence: true
validates :path, presence: true
validates :model, presence: true
validates :uploader, presence: true
before_save :calculate_checksum!, if: :foreground_checksummable?
after_commit :schedule_checksum, if: :checksummable?
scope :stored_locally, -> { where(store: [nil, ObjectStorage::Store::LOCAL]) }
scope :stored_remotely, -> { where(store: ObjectStorage::Store::REMOTE) }
def self.hexdigest(path)
Digest::SHA256.file(path).hexdigest
end
def absolute_path
raise ObjectStorage::RemoteStoreError, "Remote object has no absolute path." unless local?
return path unless relative_path?
uploader_class.absolute_path(self)
end
def calculate_checksum!
self.checksum = nil
return unless checksummable?
self.checksum = self.class.hexdigest(absolute_path)
end
def build_uploader(mounted_as = nil)
uploader_class.new(model, mounted_as).tap do |uploader|
uploader.upload = self
uploader.retrieve_from_store!(identifier)
end
end
def exist?
File.exist?(absolute_path)
end
def local?
return true if store.nil?
store == ObjectStorage::Store::LOCAL
end
private
def checksummable?
checksum.nil? && local? && exist?
end
def foreground_checksummable?
checksummable? && size <= CHECKSUM_THRESHOLD
end
def schedule_checksum
UploadChecksumWorker.perform_async(id)
end
def relative_path?
!path.start_with?('/')
end
def identifier
File.basename(path)
end
def uploader_class
Object.const_get(uploader)
end
end
class MigrationResult
attr_reader :upload
attr_accessor :error
......
......@@ -7,113 +7,138 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
end
end
let!(:projects) { create_list(:project, 10, :with_avatar) }
let(:uploads) { Upload.all }
let(:model_class) { Project }
let(:mounted_as) { :avatar }
let(:uploads) { Upload.all }
let(:to_store) { ObjectStorage::Store::REMOTE }
before do
stub_uploads_object_storage(AvatarUploader)
end
describe '.enqueue!' do
def enqueue!
described_class.enqueue!(uploads, Project, mounted_as, to_store)
end
shared_examples "uploads migration worker" do
describe '.enqueue!' do
def enqueue!
described_class.enqueue!(uploads, Project, mounted_as, to_store)
end
it 'is guarded by .sanity_check!' do
expect(described_class).to receive(:perform_async)
expect(described_class).to receive(:sanity_check!)
it 'is guarded by .sanity_check!' do
expect(described_class).to receive(:perform_async)
expect(described_class).to receive(:sanity_check!)
enqueue!
end
enqueue!
end
context 'sanity_check! fails' do
include_context 'sanity_check! fails'
context 'sanity_check! fails' do
include_context 'sanity_check! fails'
it 'does not enqueue a job' do
expect(described_class).not_to receive(:perform_async)
it 'does not enqueue a job' do
expect(described_class).not_to receive(:perform_async)
expect { enqueue! }.to raise_error(described_class::SanityCheckError)
expect { enqueue! }.to raise_error(described_class::SanityCheckError)
end
end
end
end
describe '.sanity_check!' do
shared_examples 'raises a SanityCheckError' do
let(:mount_point) { nil }
describe '.sanity_check!' do
shared_examples 'raises a SanityCheckError' do
let(:mount_point) { nil }
it do
expect { described_class.sanity_check!(uploads, model_class, mount_point) }
.to raise_error(described_class::SanityCheckError)
it do
expect { described_class.sanity_check!(uploads, model_class, mount_point) }
.to raise_error(described_class::SanityCheckError)
end
end
end
context 'uploader types mismatch' do
let!(:outlier) { create(:upload, uploader: 'FileUploader') }
before do
stub_const("WrongModel", Class.new)
end
include_examples 'raises a SanityCheckError'
end
context 'uploader types mismatch' do
let!(:outlier) { create(:upload, uploader: 'GitlabUploader') }
context 'model types mismatch' do
let!(:outlier) { create(:upload, model_type: 'Potato') }
include_examples 'raises a SanityCheckError'
end
include_examples 'raises a SanityCheckError'
end
context 'model types mismatch' do
let!(:outlier) { create(:upload, model_type: 'WrongModel') }
context 'mount point not found' do
include_examples 'raises a SanityCheckError' do
let(:mount_point) { :potato }
include_examples 'raises a SanityCheckError'
end
end
end
describe '#perform' do
def perform
described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, to_store)
rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures
# swallow
context 'mount point not found' do
include_examples 'raises a SanityCheckError' do
let(:mount_point) { :potato }
end
end
end
shared_examples 'outputs correctly' do |success: 0, failures: 0|
total = success + failures
describe '#perform' do
def perform
described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, to_store)
rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures
# swallow
end
shared_examples 'outputs correctly' do |success: 0, failures: 0|
total = success + failures
if success > 0
it 'outputs the reports' do
expect(Rails.logger).to receive(:info).with(%r{Migrated #{success}/#{total} files})
if success > 0
it 'outputs the reports' do
expect(Rails.logger).to receive(:info).with(%r{Migrated #{success}/#{total} files})
perform
perform
end
end
end
if failures > 0
it 'outputs upload failures' do
expect(Rails.logger).to receive(:warn).with(/Error .* I am a teapot/)
if failures > 0
it 'outputs upload failures' do
expect(Rails.logger).to receive(:warn).with(/Error .* I am a teapot/)
perform
perform
end
end
end
end
it_behaves_like 'outputs correctly', success: 10
it_behaves_like 'outputs correctly', success: 10
it 'migrates files' do
perform
it 'migrates files' do
perform
expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0)
end
aggregate_failures do
projects.each do |project|
expect(project.reload.avatar.upload.local?).to be_falsey
context 'migration is unsuccessful' do
before do
allow_any_instance_of(ObjectStorage::Concern)
.to receive(:migrate!).and_raise(CarrierWave::UploadError, "I am a teapot.")
end
it_behaves_like 'outputs correctly', failures: 10
end
end
end
context 'migration is unsuccessful' do
before do
allow_any_instance_of(ObjectStorage::Concern).to receive(:migrate!).and_raise(CarrierWave::UploadError, "I am a teapot.")
end
context "for AvatarUploader" do
let!(:projects) { create_list(:project, 10, :with_avatar) }
let(:mounted_as) { :avatar }
it_behaves_like 'outputs correctly', failures: 10
before do
stub_uploads_object_storage(AvatarUploader)
end
it_behaves_like "uploads migration worker"
end
context "for FileUploader" do
let!(:projects) { create_list(:project, 10) }
let(:secret) { SecureRandom.hex }
let(:mounted_as) { nil }
before do
stub_uploads_object_storage(FileUploader)
projects.map do |project|
uploader = FileUploader.new(project)
uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt'))
end
end
it_behaves_like "uploads migration worker"
end
end
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