Commit 576a8c69 authored by Sean McGivern's avatar Sean McGivern

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
parents 055a02ed 8fe5bcde
...@@ -138,21 +138,18 @@ module ObjectStorage ...@@ -138,21 +138,18 @@ module ObjectStorage
include Report include Report
def self.enqueue!(uploads, mounted_as, to_store) def self.enqueue!(uploads, model_class, mounted_as, to_store)
sanity_check!(uploads, mounted_as) 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 end
# We need to be sure all the uploads are for the same uploader and model type # We need to be sure all the uploads are for the same uploader and model type
# and that the mount point exists if provided. # 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 upload = uploads.first
uploader_class = upload.uploader.constantize uploader_class = upload.uploader.constantize
model_class = uploads.first.model_type.constantize
uploader_types = uploads.map(&:uploader).uniq uploader_types = uploads.map(&:uploader).uniq
model_types = uploads.map(&:model_type).uniq model_types = uploads.map(&:model_type).uniq
model_has_mount = mounted_as.nil? || model_class.uploaders[mounted_as] == uploader_class model_has_mount = mounted_as.nil? || model_class.uploaders[mounted_as] == uploader_class
...@@ -162,7 +159,12 @@ module ObjectStorage ...@@ -162,7 +159,12 @@ module ObjectStorage
raise(SanityCheckError, "Mount point #{mounted_as} not found in #{model_class}.") unless model_has_mount raise(SanityCheckError, "Mount point #{mounted_as} not found in #{model_class}.") unless model_has_mount
end 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 @mounted_as = mounted_as&.to_sym
@to_store = to_store @to_store = to_store
...@@ -178,7 +180,17 @@ module ObjectStorage ...@@ -178,7 +180,17 @@ module ObjectStorage
end end
def sanity_check!(uploads) 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 end
def build_uploaders(uploads) def build_uploaders(uploads)
......
---
title: Fixed gitlab:uploads:migrate task failing for Groups' avatar.
merge_request: 18088
author:
type: fixed
...@@ -13,6 +13,7 @@ namespace :gitlab do ...@@ -13,6 +13,7 @@ namespace :gitlab do
def enqueue_batch(batch, index) def enqueue_batch(batch, index)
job = ObjectStorage::MigrateUploadsWorker.enqueue!(batch, job = ObjectStorage::MigrateUploadsWorker.enqueue!(batch,
@model_class,
@mounted_as, @mounted_as,
@to_store) @to_store)
puts "Enqueued job ##{index}: #{job}" puts "Enqueued job ##{index}: #{job}"
......
require 'rake_helper' require 'rake_helper'
describe 'gitlab:uploads:migrate rake tasks' do describe 'gitlab:uploads:migrate rake tasks' do
let!(:projects) { create_list(:project, 10, :with_avatar) } let(:model_class) { nil }
let(:model_class) { Project } let(:uploader_class) { nil }
let(:uploader_class) { AvatarUploader } let(:mounted_as) { nil }
let(:mounted_as) { :avatar }
let(:batch_size) { 3 } let(:batch_size) { 3 }
before do before do
...@@ -30,6 +29,14 @@ describe 'gitlab:uploads:migrate rake tasks' do ...@@ -30,6 +29,14 @@ describe 'gitlab:uploads:migrate rake tasks' do
end end
end end
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 it_behaves_like 'enqueue jobs in batch', batch: 4
context 'Upload has store = nil' do context 'Upload has store = nil' do
...@@ -37,6 +44,100 @@ describe 'gitlab:uploads:migrate rake tasks' do ...@@ -37,6 +44,100 @@ describe 'gitlab:uploads:migrate rake tasks' do
Upload.where(model: projects).update_all(store: nil) Upload.where(model: projects).update_all(store: nil)
end 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 }
before do
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 it_behaves_like 'enqueue jobs in batch', batch: 4
end end
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