From a14ebd4117938bff705325d474a7ea9ffacedc32 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 10 May 2018 09:21:03 +0000 Subject: Merge branch '46147-remove-model-redefinition-worker' into 'master' Resolve "NoMethodError: undefined method `uploader_context' for # { 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 diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index 7a7dcb71680..aed62f97448 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -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 -- cgit v1.2.3