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

app/workers/object_storage/migrate_uploads_worker.rb

@@ -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

spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb

@@ -7,16 +7,11 @@ 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
-
+  shared_examples "uploads migration worker" do
     describe '.enqueue!' do
       def enqueue!
         described_class.enqueue!(uploads, Project, mounted_as, to_store)
@@ -50,14 +45,18 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
         end
       end
 
+      before do
+        stub_const("WrongModel", Class.new)
+      end
+
       context 'uploader types mismatch' do
-      let!(:outlier) { create(:upload, uploader: 'FileUploader') }
+        let!(:outlier) { create(:upload, uploader: 'GitlabUploader') }
 
         include_examples 'raises a SanityCheckError'
       end
 
       context 'model types mismatch' do
-      let!(:outlier) { create(:upload, model_type: 'Potato') }
+        let!(:outlier) { create(:upload, model_type: 'WrongModel') }
 
         include_examples 'raises a SanityCheckError'
       end
@@ -101,19 +100,45 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
       it 'migrates files' do
         perform
 
-      aggregate_failures do
-        projects.each do |project|
-          expect(project.reload.avatar.upload.local?).to be_falsey
-        end
-      end
+        expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0)
       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.")
+          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 "for AvatarUploader" do
+    let!(:projects) { create_list(:project, 10, :with_avatar) }
+    let(:mounted_as) { :avatar }
+
+    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