diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-04-02 13:00:31 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-04-02 13:00:31 +0000 |
commit | 576a8c6960e39165a05839b5dd490c7cad397ce2 (patch) | |
tree | d52a1a98263d491c8d36b9b85c4ef96c85e83b8c | |
parent | 055a02edca0a49938542b57bc3652a0da5464f22 (diff) | |
parent | 8fe5bcde569e9e5f45e397005a915142d20d319d (diff) | |
download | gitlab-ce-576a8c6960e39165a05839b5dd490c7cad397ce2.tar.gz |
Merge branch '44776-fix-upload-migrate-fails-for-group' into 'master'
Resolve "Migrate upload task fails for Groups"
Closes #44776
See merge request gitlab-org/gitlab-ce!18088
4 files changed, 135 insertions, 16 deletions
diff --git a/app/workers/object_storage/migrate_uploads_worker.rb b/app/workers/object_storage/migrate_uploads_worker.rb index 01ed123e6c8..a6b2c251254 100644 --- a/app/workers/object_storage/migrate_uploads_worker.rb +++ b/app/workers/object_storage/migrate_uploads_worker.rb @@ -138,21 +138,18 @@ module ObjectStorage include Report - def self.enqueue!(uploads, mounted_as, to_store) - sanity_check!(uploads, mounted_as) + def self.enqueue!(uploads, model_class, mounted_as, to_store) + sanity_check!(uploads, model_class, mounted_as) - perform_async(uploads.ids, mounted_as, to_store) + perform_async(uploads.ids, model_class.to_s, mounted_as, to_store) end # We need to be sure all the uploads are for the same uploader and model type # and that the mount point exists if provided. # - def self.sanity_check!(uploads, mounted_as) + def self.sanity_check!(uploads, model_class, mounted_as) upload = uploads.first - uploader_class = upload.uploader.constantize - model_class = uploads.first.model_type.constantize - uploader_types = uploads.map(&:uploader).uniq model_types = uploads.map(&:model_type).uniq model_has_mount = mounted_as.nil? || model_class.uploaders[mounted_as] == uploader_class @@ -162,7 +159,12 @@ module ObjectStorage raise(SanityCheckError, "Mount point #{mounted_as} not found in #{model_class}.") unless model_has_mount end - def perform(ids, mounted_as, to_store) + def perform(*args) + args_check!(args) + + (ids, model_type, mounted_as, to_store) = args + + @model_class = model_type.constantize @mounted_as = mounted_as&.to_sym @to_store = to_store @@ -178,7 +180,17 @@ module ObjectStorage end def sanity_check!(uploads) - self.class.sanity_check!(uploads, @mounted_as) + self.class.sanity_check!(uploads, @model_class, @mounted_as) + end + + def args_check!(args) + return if args.count == 4 + + case args.count + when 3 then raise SanityCheckError, "Job is missing the `model_type` argument." + else + raise SanityCheckError, "Job has wrong arguments format." + end end def build_uploaders(uploads) diff --git a/changelogs/unreleased/44776-fix-upload-migrate-fails-for-group.yml b/changelogs/unreleased/44776-fix-upload-migrate-fails-for-group.yml new file mode 100644 index 00000000000..6094fcd0b3e --- /dev/null +++ b/changelogs/unreleased/44776-fix-upload-migrate-fails-for-group.yml @@ -0,0 +1,5 @@ +--- +title: Fixed gitlab:uploads:migrate task failing for Groups' avatar. +merge_request: 18088 +author: +type: fixed diff --git a/lib/tasks/gitlab/uploads/migrate.rake b/lib/tasks/gitlab/uploads/migrate.rake index e7e656a911b..78e18992a8e 100644 --- a/lib/tasks/gitlab/uploads/migrate.rake +++ b/lib/tasks/gitlab/uploads/migrate.rake @@ -13,6 +13,7 @@ namespace :gitlab do def enqueue_batch(batch, index) job = ObjectStorage::MigrateUploadsWorker.enqueue!(batch, + @model_class, @mounted_as, @to_store) puts "Enqueued job ##{index}: #{job}" diff --git a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb index 53b1e45ba54..6fcfae358ec 100644 --- a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb +++ b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb @@ -1,10 +1,9 @@ require 'rake_helper' describe 'gitlab:uploads:migrate rake tasks' do - let!(:projects) { create_list(:project, 10, :with_avatar) } - let(:model_class) { Project } - let(:uploader_class) { AvatarUploader } - let(:mounted_as) { :avatar } + let(:model_class) { nil } + let(:uploader_class) { nil } + let(:mounted_as) { nil } let(:batch_size) { 3 } before do @@ -30,11 +29,113 @@ describe 'gitlab:uploads:migrate rake tasks' do end end - it_behaves_like 'enqueue jobs in batch', batch: 4 + context "for AvatarUploader" do + let(:uploader_class) { AvatarUploader } + let(:mounted_as) { :avatar } + + context "for Project" do + let(:model_class) { Project } + let!(:projects) { create_list(:project, 10, :with_avatar) } + + it_behaves_like 'enqueue jobs in batch', batch: 4 + + context 'Upload has store = nil' do + before do + Upload.where(model: projects).update_all(store: nil) + end + + it_behaves_like 'enqueue jobs in batch', batch: 4 + end + end + + context "for Group" do + let(:model_class) { Group } + + before do + create_list(:group, 10, :with_avatar) + end + + it_behaves_like 'enqueue jobs in batch', batch: 4 + end + + context "for User" do + let(:model_class) { User } + + before do + create_list(:user, 10, :with_avatar) + end + + it_behaves_like 'enqueue jobs in batch', batch: 4 + end + end + + context "for AttachmentUploader" do + let(:uploader_class) { AttachmentUploader } + + context "for Note" do + let(:model_class) { Note } + let(:mounted_as) { :attachment } + + before do + create_list(:note, 10, :with_attachment) + end + + it_behaves_like 'enqueue jobs in batch', batch: 4 + end + + context "for Appearance" do + let(:model_class) { Appearance } + let(:mounted_as) { :logo } + + before do + create(:appearance, :with_logos) + end + + %i(logo header_logo).each do |mount| + it_behaves_like 'enqueue jobs in batch', batch: 1 do + let(:mounted_as) { mount } + end + end + end + end + + context "for FileUploader" do + let(:uploader_class) { FileUploader } + let(:model_class) { Project } + + before do + create_list(:project, 10) do |model| + uploader_class.new(model) + .store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end + end + + it_behaves_like 'enqueue jobs in batch', batch: 4 + end + + context "for PersonalFileUploader" do + let(:uploader_class) { PersonalFileUploader } + let(:model_class) { PersonalSnippet } + + before do + create_list(:personal_snippet, 10) do |model| + uploader_class.new(model) + .store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end + end + + it_behaves_like 'enqueue jobs in batch', batch: 4 + end + + context "for NamespaceFileUploader" do + let(:uploader_class) { NamespaceFileUploader } + let(:model_class) { Snippet } - context 'Upload has store = nil' do before do - Upload.where(model: projects).update_all(store: nil) + create_list(:snippet, 10) do |model| + uploader_class.new(model) + .store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end end it_behaves_like 'enqueue jobs in batch', batch: 4 |