Revert "Merge branch '3867-port-to-ce' into 'master'"

This reverts commit 54a575f1bbba44573ab92dc58a4242f1ee734c5d, reversing
changes made to c63af942e5baf7849a94fa99da8494bcba28e3f8.

34 files changed, 453 insertions, 339 deletions

diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb
index 6a1869d1a48..67a11e56e94 100644
--- a/spec/controllers/groups/uploads_controller_spec.rb
+++ b/spec/controllers/groups/uploads_controller_spec.rb
@@ -6,7 +6,5 @@ describe Groups::UploadsController do
     { group_id: model }
   end
 
-  it_behaves_like 'handle uploads' do
-    let(:uploader_class) { NamespaceFileUploader }
-  end
+  it_behaves_like 'handle uploads'
 end
diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb
index 25a2e13fe1a..12cb7b2647f 100644
--- a/spec/controllers/projects/artifacts_controller_spec.rb
+++ b/spec/controllers/projects/artifacts_controller_spec.rb
@@ -145,7 +145,8 @@ describe Projects::ArtifactsController do
       context 'when using local file storage' do
         it_behaves_like 'a valid file' do
           let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
-          let(:archive_path) { JobArtifactUploader.root }
+          let(:store) { ObjectStoreUploader::LOCAL_STORE }
+          let(:archive_path) { JobArtifactUploader.local_store_path }
         end
       end
     end
diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb
index b7df42168e0..3a0c3faa7b4 100644
--- a/spec/controllers/projects/raw_controller_spec.rb
+++ b/spec/controllers/projects/raw_controller_spec.rb
@@ -53,7 +53,7 @@ describe Projects::RawController do
           end
 
           it 'serves the file' do
-            expect(controller).to receive(:send_file).with("#{LfsObjectUploader.root}/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: 'lfs_object.iso', disposition: 'attachment')
+            expect(controller).to receive(:send_file).with("#{Gitlab.config.shared.path}/lfs-objects/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: 'lfs_object.iso', disposition: 'attachment')
             get(:show,
                 namespace_id: public_project.namespace.to_param,
                 project_id: public_project,
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 376b229ffc9..b1f601a19e5 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -180,7 +180,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'image.png'
-
               response
             end
           end
@@ -197,7 +196,6 @@ describe UploadsController do
         it_behaves_like 'content not cached without revalidation' do
           subject do
             get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'image.png'
-
             response
           end
         end
@@ -222,7 +220,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png'
-
               response
             end
           end
@@ -242,7 +239,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png'
-
               response
             end
           end
@@ -295,7 +291,6 @@ describe UploadsController do
               it_behaves_like 'content not cached without revalidation' do
                 subject do
                   get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png'
-
                   response
                 end
               end
@@ -327,7 +322,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png'
-
               response
             end
           end
@@ -347,7 +341,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png'
-
               response
             end
           end
@@ -391,7 +384,6 @@ describe UploadsController do
               it_behaves_like 'content not cached without revalidation' do
                 subject do
                   get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png'
-
                   response
                 end
               end
@@ -428,7 +420,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
-
               response
             end
           end
@@ -448,7 +439,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
-
               response
             end
           end
@@ -501,7 +491,6 @@ describe UploadsController do
               it_behaves_like 'content not cached without revalidation' do
                 subject do
                   get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
-
                   response
                 end
               end
@@ -533,7 +522,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'appearance', mounted_as: 'header_logo', id: appearance.id, filename: 'dk.png'
-
               response
             end
           end
@@ -553,7 +541,6 @@ describe UploadsController do
           it_behaves_like 'content not cached without revalidation' do
             subject do
               get :show, model: 'appearance', mounted_as: 'logo', id: appearance.id, filename: 'dk.png'
-
               response
             end
           end
diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb
index 8c531cf5909..1512f5a0e58 100644
--- a/spec/factories/groups.rb
+++ b/spec/factories/groups.rb
@@ -18,7 +18,7 @@ FactoryBot.define do
     end
 
     trait :with_avatar do
-      avatar { fixture_file_upload('spec/fixtures/dk.png') }
+      avatar { File.open(Rails.root.join('spec/fixtures/dk.png')) }
     end
 
     trait :access_requestable do
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index 3f4e408b3a6..2defb4935ad 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -122,11 +122,11 @@ FactoryBot.define do
     end
 
     trait :with_attachment do
-      attachment { fixture_file_upload(Rails.root.join( "spec/fixtures/dk.png"), "image/png") }
+      attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png") }
     end
 
     trait :with_svg_attachment do
-      attachment { fixture_file_upload(Rails.root.join("spec/fixtures/unsanitized.svg"), "image/svg+xml") }
+      attachment { fixture_file_upload(Rails.root + "spec/fixtures/unsanitized.svg", "image/svg+xml") }
     end
 
     transient do
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 16d328a5bc2..d0f3911f730 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -90,7 +90,7 @@ FactoryBot.define do
     end
 
     trait :with_avatar do
-      avatar { fixture_file_upload('spec/fixtures/dk.png') }
+      avatar { File.open(Rails.root.join('spec/fixtures/dk.png')) }
     end
 
     trait :broken_storage do
diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
index c8cfe251d27..c39500faea1 100644
--- a/spec/factories/uploads.rb
+++ b/spec/factories/uploads.rb
@@ -1,42 +1,24 @@
 FactoryBot.define do
   factory :upload do
     model { build(:project) }
+    path { "uploads/-/system/project/avatar/avatar.jpg" }
     size 100.kilobytes
     uploader "AvatarUploader"
 
-    # we should build a mount agnostic upload by default
-    transient do
-      mounted_as :avatar
-      secret SecureRandom.hex
-    end
-
-    # this needs to comply with RecordsUpload::Concern#upload_path
-    path { File.join("uploads/-/system", model.class.to_s.underscore, mounted_as.to_s, 'avatar.jpg') }
-
-    trait :personal_snippet_upload do
+    trait :personal_snippet do
       model { build(:personal_snippet) }
-      path { File.join(secret, 'myfile.jpg') }
       uploader "PersonalFileUploader"
     end
 
     trait :issuable_upload do
-      path { File.join(secret, 'myfile.jpg') }
+      path { "#{SecureRandom.hex}/myfile.jpg" }
       uploader "FileUploader"
     end
 
     trait :namespace_upload do
+      path { "#{SecureRandom.hex}/myfile.jpg" }
       model { build(:group) }
-      path { File.join(secret, 'myfile.jpg') }
       uploader "NamespaceFileUploader"
     end
-
-    trait :attachment_upload do
-      transient do
-        mounted_as :attachment
-      end
-
-      model { build(:note) }
-      uploader "AttachmentUploader"
-    end
   end
 end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 769fd656e7a..e62e0b263ca 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -38,7 +38,7 @@ FactoryBot.define do
     end
 
     trait :with_avatar do
-      avatar { fixture_file_upload('spec/fixtures/dk.png') }
+      avatar { File.open(Rails.root.join('spec/fixtures/dk.png')) }
     end
 
     trait :two_factor_via_otp do
diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
index 370c2490b97..8bb9ebe0419 100644
--- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
+++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb
@@ -23,27 +23,6 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
     end
   end
 
-  # E.g. The installation is in use at the time of migration, and someone has
-  # just uploaded a file
-  shared_examples 'does not add files in /uploads/tmp' do
-    let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') }
-
-    before do
-      FileUtils.mkdir(File.dirname(tmp_file))
-      FileUtils.touch(tmp_file)
-    end
-
-    after do
-      FileUtils.rm(tmp_file)
-    end
-
-    it 'does not add files from /uploads/tmp' do
-      described_class.new.perform
-
-      expect(untracked_files_for_uploads.count).to eq(5)
-    end
-  end
-
   it 'ensures the untracked_files_for_uploads table exists' do
     expect do
       described_class.new.perform
@@ -130,8 +109,24 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
         end
       end
 
+      # E.g. The installation is in use at the time of migration, and someone has
+      # just uploaded a file
       context 'when there are files in /uploads/tmp' do
-        it_behaves_like 'does not add files in /uploads/tmp'
+        let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') }
+
+        before do
+          FileUtils.touch(tmp_file)
+        end
+
+        after do
+          FileUtils.rm(tmp_file)
+        end
+
+        it 'does not add files from /uploads/tmp' do
+          described_class.new.perform
+
+          expect(untracked_files_for_uploads.count).to eq(5)
+        end
       end
     end
   end
@@ -202,8 +197,24 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
         end
       end
 
+      # E.g. The installation is in use at the time of migration, and someone has
+      # just uploaded a file
       context 'when there are files in /uploads/tmp' do
-        it_behaves_like 'does not add files in /uploads/tmp'
+        let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') }
+
+        before do
+          FileUtils.touch(tmp_file)
+        end
+
+        after do
+          FileUtils.rm(tmp_file)
+        end
+
+        it 'does not add files from /uploads/tmp' do
+          described_class.new.perform
+
+          expect(untracked_files_for_uploads.count).to eq(5)
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
index 326ed2f2ecf..39e3b875c49 100644
--- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
+++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb
@@ -17,7 +17,7 @@ describe Gitlab::Gfm::UploadsRewriter do
     end
 
     let(:text) do
-      "Text and #{image_uploader.markdown_link} and #{zip_uploader.markdown_link}"
+      "Text and #{image_uploader.to_markdown} and #{zip_uploader.to_markdown}"
     end
 
     describe '#rewrite' do
diff --git a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb
index a685521cbf0..63992ea8ab8 100644
--- a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb
@@ -4,6 +4,7 @@ describe Gitlab::ImportExport::UploadsRestorer do
   describe 'bundle a project Git repo' do
     let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" }
     let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) }
+    let(:uploads_path) { FileUploader.dynamic_path_segment(project) }
 
     before do
       allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
@@ -25,9 +26,9 @@ describe Gitlab::ImportExport::UploadsRestorer do
       end
 
       it 'copies the uploads to the project path' do
-        subject.restore
+        restorer.restore
 
-        uploads = Dir.glob(File.join(subject.uploads_path, '**/*')).map { |file| File.basename(file) }
+        uploads = Dir.glob(File.join(uploads_path, '**/*')).map { |file| File.basename(file) }
 
         expect(uploads).to include('dummy.txt')
       end
@@ -43,9 +44,9 @@ describe Gitlab::ImportExport::UploadsRestorer do
       end
 
       it 'copies the uploads to the project path' do
-        subject.restore
+        restorer.restore
 
-        uploads = Dir.glob(File.join(subject.uploads_path, '**/*')).map { |file| File.basename(file) }
+        uploads = Dir.glob(File.join(uploads_path, '**/*')).map { |file| File.basename(file) }
 
         expect(uploads).to include('dummy.txt')
       end
diff --git a/spec/lib/gitlab/import_export/uploads_saver_spec.rb b/spec/lib/gitlab/import_export/uploads_saver_spec.rb
index 959779523f4..e8948de1f3a 100644
--- a/spec/lib/gitlab/import_export/uploads_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/uploads_saver_spec.rb
@@ -30,7 +30,7 @@ describe Gitlab::ImportExport::UploadsSaver do
       it 'copies the uploads to the export path' do
         saver.save
 
-        uploads = Dir.glob(File.join(saver.uploads_export_path, '**/*')).map { |file| File.basename(file) }
+        uploads = Dir.glob(File.join(shared.export_path, 'uploads', '**/*')).map { |file| File.basename(file) }
 
         expect(uploads).to include('banana_sample.gif')
       end
@@ -52,7 +52,7 @@ describe Gitlab::ImportExport::UploadsSaver do
       it 'copies the uploads to the export path' do
         saver.save
 
-        uploads = Dir.glob(File.join(saver.uploads_export_path, '**/*')).map { |file| File.basename(file) }
+        uploads = Dir.glob(File.join(shared.export_path, 'uploads', '**/*')).map { |file| File.basename(file) }
 
         expect(uploads).to include('banana_sample.gif')
       end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 6b7dbad128c..c3673a0e2a3 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -204,7 +204,7 @@ describe Namespace do
       let(:parent) { create(:group, name: 'parent', path: 'parent') }
       let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
       let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child, skip_disk_validation: true) }
-      let(:uploads_dir) { FileUploader.root }
+      let(:uploads_dir) { File.join(CarrierWave.root, FileUploader.base_dir) }
       let(:pages_dir) { File.join(TestEnv.pages_path) }
 
       before do
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index 42f3d609770..345382ea8c7 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -45,6 +45,51 @@ describe Upload do
     end
   end
 
+  describe '.remove_path' do
+    it 'removes all records at the given path' do
+      described_class.create!(
+        size: File.size(__FILE__),
+        path: __FILE__,
+        model: build_stubbed(:user),
+        uploader: 'AvatarUploader'
+      )
+
+      expect { described_class.remove_path(__FILE__) }
+        .to change { described_class.count }.from(1).to(0)
+    end
+  end
+
+  describe '.record' do
+    let(:fake_uploader) do
+      double(
+        file: double(size: 12_345),
+        relative_path: 'foo/bar.jpg',
+        model: build_stubbed(:user),
+        class: 'AvatarUploader'
+      )
+    end
+
+    it 'removes existing paths before creation' do
+      expect(described_class).to receive(:remove_path)
+        .with(fake_uploader.relative_path)
+
+      described_class.record(fake_uploader)
+    end
+
+    it 'creates a new record and assigns size, path, model, and uploader' do
+      upload = described_class.record(fake_uploader)
+
+      aggregate_failures do
+        expect(upload).to be_persisted
+        expect(upload.size).to eq fake_uploader.file.size
+        expect(upload.path).to eq fake_uploader.relative_path
+        expect(upload.model_id).to eq fake_uploader.model.id
+        expect(upload.model_type).to eq fake_uploader.model.class.to_s
+        expect(upload.uploader).to eq fake_uploader.class
+      end
+    end
+  end
+
   describe '#absolute_path' do
     it 'returns the path directly when already absolute' do
       path = '/path/to/namespace/project/secret/file.jpg'
@@ -66,27 +111,27 @@ describe Upload do
     end
   end
 
-  describe '#calculate_checksum!' do
-    let(:upload) do
-      described_class.new(path: __FILE__,
-                          size: described_class::CHECKSUM_THRESHOLD - 1.megabyte)
-    end
-
-    it 'sets `checksum` to SHA256 sum of the file' do
+  describe '#calculate_checksum' do
+    it 'calculates the SHA256 sum' do
+      upload = described_class.new(
+        path: __FILE__,
+        size: described_class::CHECKSUM_THRESHOLD - 1.megabyte
+      )
       expected = Digest::SHA256.file(__FILE__).hexdigest
 
-      expect { upload.calculate_checksum! }
+      expect { upload.calculate_checksum }
         .to change { upload.checksum }.from(nil).to(expected)
     end
 
-    it 'sets `checksum` to nil for a non-existant file' do
-      expect(upload).to receive(:exist?).and_return(false)
+    it 'returns nil for a non-existant file' do
+      upload = described_class.new(
+        path: __FILE__,
+        size: described_class::CHECKSUM_THRESHOLD - 1.megabyte
+      )
 
-      checksum = Digest::SHA256.file(__FILE__).hexdigest
-      upload.checksum = checksum
+      expect(upload).to receive(:exist?).and_return(false)
 
-      expect { upload.calculate_checksum! }
-        .to change { upload.checksum }.from(checksum).to(nil)
+      expect(upload.calculate_checksum).to be_nil
     end
   end
 
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index c5c0b0c2867..cb66d23b77c 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -945,7 +945,7 @@ describe API::Runner do
         context 'when artifacts are being stored inside of tmp path' do
           before do
             # by configuring this path we allow to pass temp file from any path
-            allow(JobArtifactUploader).to receive(:workhorse_upload_path).and_return('/')
+            allow(JobArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
           end
 
           context 'when job has been erased' do
@@ -1122,7 +1122,7 @@ describe API::Runner do
             # by configuring this path we allow to pass file from @tmpdir only
             # but all temporary files are stored in system tmp directory
             @tmpdir = Dir.mktmpdir
-            allow(JobArtifactUploader).to receive(:workhorse_upload_path).and_return(@tmpdir)
+            allow(JobArtifactUploader).to receive(:artifacts_upload_path).and_return(@tmpdir)
           end
 
           after do
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 930ef49b7f3..bee918a20aa 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -958,7 +958,7 @@ describe 'Git LFS API and storage' do
             end
 
             it 'responds with status 200, location of lfs store and object details' do
-              expect(json_response['StoreLFSPath']).to eq(LfsObjectUploader.workhorse_upload_path)
+              expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload")
               expect(json_response['LfsOid']).to eq(sample_oid)
               expect(json_response['LfsSize']).to eq(sample_size)
             end
@@ -1075,7 +1075,7 @@ describe 'Git LFS API and storage' do
             end
 
             it 'with location of lfs store and object details' do
-              expect(json_response['StoreLFSPath']).to eq(LfsObjectUploader.workhorse_upload_path)
+              expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload")
               expect(json_response['LfsOid']).to eq(sample_oid)
               expect(json_response['LfsSize']).to eq(sample_size)
             end
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 322c91065e7..388c9d63c7b 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -6,7 +6,7 @@ describe Issues::MoveService do
   let(:title) { 'Some issue' }
   let(:description) { 'Some issue description' }
   let(:old_project) { create(:project) }
-  let(:new_project) { create(:project, group: create(:group)) }
+  let(:new_project) { create(:project) }
   let(:milestone1) { create(:milestone, project_id: old_project.id, title: 'v9.0') }
 
   let(:old_issue) do
@@ -250,7 +250,7 @@ describe Issues::MoveService do
 
         context 'issue description with uploads' do
           let(:uploader) { build(:file_uploader, project: old_project) }
-          let(:description) { "Text and #{uploader.markdown_link}" }
+          let(:description) { "Text and #{uploader.to_markdown}" }
 
           include_context 'issue move executed'
 
diff --git a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
index 15699574b3a..50e59954f73 100644
--- a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
+++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
@@ -6,7 +6,7 @@ describe Projects::HashedStorage::MigrateAttachmentsService do
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
   let(:hashed_storage) { Storage::HashedProject.new(project) }
 
-  let!(:upload) { Upload.find_by(path: file_uploader.upload_path) }
+  let!(:upload) { Upload.find_by(path: file_uploader.relative_path) }
   let(:file_uploader) { build(:file_uploader, project: project) }
   let(:old_path) { File.join(base_path(legacy_storage), upload.path) }
   let(:new_path) { File.join(base_path(hashed_storage), upload.path) }
@@ -58,6 +58,6 @@ describe Projects::HashedStorage::MigrateAttachmentsService do
   end
 
   def base_path(storage)
-    File.join(FileUploader.root, storage.disk_path)
+    FileUploader.dynamic_path_builder(storage.disk_path)
   end
 end
diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
index 7ce80c82439..935c08221e0 100644
--- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
@@ -2,8 +2,6 @@ shared_examples 'handle uploads' do
   let(:user)  { create(:user) }
   let(:jpg)   { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
   let(:txt)   { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
-  let(:secret) { FileUploader.generate_secret }
-  let(:uploader_class) { FileUploader }
 
   describe "POST #create" do
     context 'when a user is not authorized to upload a file' do
@@ -67,12 +65,7 @@ shared_examples 'handle uploads' do
 
   describe "GET #show" do
     let(:show_upload) do
-      get :show, params.merge(secret: secret, filename: "rails_sample.jpg")
-    end
-
-    before do
-      expect(FileUploader).to receive(:generate_secret).and_return(secret)
-      UploadService.new(model, jpg, uploader_class).execute
+      get :show, params.merge(secret: "123456", filename: "image.jpg")
     end
 
     context "when the model is public" do
@@ -82,6 +75,11 @@ shared_examples 'handle uploads' do
 
       context "when not signed in" do
         context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
           it "responds with status 200" do
             show_upload
 
@@ -90,10 +88,6 @@ shared_examples 'handle uploads' do
         end
 
         context "when the file doesn't exist" do
-          before do
-            allow_any_instance_of(FileUploader).to receive(:exists?).and_return(false)
-          end
-
           it "responds with status 404" do
             show_upload
 
@@ -108,6 +102,11 @@ shared_examples 'handle uploads' do
         end
 
         context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
           it "responds with status 200" do
             show_upload
 
@@ -116,10 +115,6 @@ shared_examples 'handle uploads' do
         end
 
         context "when the file doesn't exist" do
-          before do
-            allow_any_instance_of(FileUploader).to receive(:exists?).and_return(false)
-          end
-
           it "responds with status 404" do
             show_upload
 
@@ -136,6 +131,11 @@ shared_examples 'handle uploads' do
 
       context "when not signed in" do
         context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
           context "when the file is an image" do
             before do
               allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
@@ -149,10 +149,6 @@ shared_examples 'handle uploads' do
           end
 
           context "when the file is not an image" do
-            before do
-              allow_any_instance_of(FileUploader).to receive(:image?).and_return(false)
-            end
-
             it "redirects to the sign in page" do
               show_upload
 
@@ -162,10 +158,6 @@ shared_examples 'handle uploads' do
         end
 
         context "when the file doesn't exist" do
-          before do
-            allow_any_instance_of(FileUploader).to receive(:exists?).and_return(false)
-          end
-
           it "redirects to the sign in page" do
             show_upload
 
@@ -185,6 +177,11 @@ shared_examples 'handle uploads' do
           end
 
           context "when the file exists" do
+            before do
+              allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+              allow(jpg).to receive(:exists?).and_return(true)
+            end
+
             it "responds with status 200" do
               show_upload
 
@@ -193,10 +190,6 @@ shared_examples 'handle uploads' do
           end
 
           context "when the file doesn't exist" do
-            before do
-              allow_any_instance_of(FileUploader).to receive(:exists?).and_return(false)
-            end
-
             it "responds with status 404" do
               show_upload
 
@@ -207,6 +200,11 @@ shared_examples 'handle uploads' do
 
         context "when the user doesn't have access to the model" do
           context "when the file exists" do
+            before do
+              allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+              allow(jpg).to receive(:exists?).and_return(true)
+            end
+
             context "when the file is an image" do
               before do
                 allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
@@ -220,10 +218,6 @@ shared_examples 'handle uploads' do
             end
 
             context "when the file is not an image" do
-              before do
-                allow_any_instance_of(FileUploader).to receive(:image?).and_return(false)
-              end
-
               it "responds with status 404" do
                 show_upload
 
@@ -233,10 +227,6 @@ shared_examples 'handle uploads' do
           end
 
           context "when the file doesn't exist" do
-            before do
-              allow_any_instance_of(FileUploader).to receive(:exists?).and_return(false)
-            end
-
             it "responds with status 404" do
               show_upload
 
diff --git a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
deleted file mode 100644
index 934d53e7bba..00000000000
--- a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
+++ /dev/null
@@ -1,48 +0,0 @@
-shared_examples "matches the method pattern" do |method|
-  let(:target) { subject }
-  let(:args) { nil }
-  let(:pattern) { patterns[method] }
-
-  it do
-    return skip "No pattern provided, skipping." unless pattern
-
-    expect(target.method(method).call(*args)).to match(pattern)
-  end
-end
-
-shared_examples "builds correct paths" do |**patterns|
-  let(:patterns) { patterns }
-
-  before do
-    allow(subject).to receive(:filename).and_return('<filename>')
-  end
-
-  describe "#store_dir" do
-    it_behaves_like "matches the method pattern", :store_dir
-  end
-
-  describe "#cache_dir" do
-    it_behaves_like "matches the method pattern", :cache_dir
-  end
-
-  describe "#work_dir" do
-    it_behaves_like "matches the method pattern", :work_dir
-  end
-
-  describe "#upload_path" do
-    it_behaves_like "matches the method pattern", :upload_path
-  end
-
-  describe ".absolute_path" do
-    it_behaves_like "matches the method pattern", :absolute_path do
-      let(:target) { subject.class }
-      let(:args) { [upload] }
-    end
-  end
-
-  describe ".base_dir" do
-    it_behaves_like "matches the method pattern", :base_dir do
-      let(:target) { subject.class }
-    end
-  end
-end
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index c275522159c..9e5f08fbc51 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -237,7 +237,7 @@ module TestEnv
   end
 
   def artifacts_path
-    Gitlab.config.artifacts.storage_path
+    Gitlab.config.artifacts.path
   end
 
   # When no cached assets exist, manually hit the root path to create them
diff --git a/spec/support/track_untracked_uploads_helpers.rb b/spec/support/track_untracked_uploads_helpers.rb
index 5752078d2a0..d05eda08201 100644
--- a/spec/support/track_untracked_uploads_helpers.rb
+++ b/spec/support/track_untracked_uploads_helpers.rb
@@ -1,6 +1,6 @@
 module TrackUntrackedUploadsHelpers
   def uploaded_file
-    fixture_path = Rails.root.join('spec/fixtures/rails_sample.jpg')
+    fixture_path = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')
     fixture_file_upload(fixture_path)
   end
 
diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb
index 091ba824fc6..04ee6e9bfad 100644
--- a/spec/uploaders/attachment_uploader_spec.rb
+++ b/spec/uploaders/attachment_uploader_spec.rb
@@ -1,14 +1,28 @@
 require 'spec_helper'
 
 describe AttachmentUploader do
-  let(:note) { create(:note, :with_attachment) }
-  let(:uploader) { note.attachment }
-  let(:upload) { create(:upload, :attachment_upload, model: uploader.model) }
+  let(:uploader) { described_class.new(build_stubbed(:user)) }
 
-  subject { uploader }
+  describe "#store_dir" do
+    it "stores in the system dir" do
+      expect(uploader.store_dir).to start_with("uploads/-/system/user")
+    end
 
-  it_behaves_like 'builds correct paths',
-                  store_dir: %r[uploads/-/system/note/attachment/],
-                  upload_path: %r[uploads/-/system/note/attachment/],
-                  absolute_path: %r[#{CarrierWave.root}/uploads/-/system/note/attachment/]
+    it "uses the old path when using object storage" do
+      expect(described_class).to receive(:file_storage?).and_return(false)
+      expect(uploader.store_dir).to start_with("uploads/user")
+    end
+  end
+
+  describe '#move_to_cache' do
+    it 'is true' do
+      expect(uploader.move_to_cache).to eq(true)
+    end
+  end
+
+  describe '#move_to_store' do
+    it 'is true' do
+      expect(uploader.move_to_store).to eq(true)
+    end
+  end
 end
diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb
index bf9028c9260..1dc574699d8 100644
--- a/spec/uploaders/avatar_uploader_spec.rb
+++ b/spec/uploaders/avatar_uploader_spec.rb
@@ -1,16 +1,18 @@
 require 'spec_helper'
 
 describe AvatarUploader do
-  let(:model) { create(:user, :with_avatar) }
-  let(:uploader) { described_class.new(model, :avatar) }
-  let(:upload) { create(:upload, model: model) }
+  let(:uploader) { described_class.new(build_stubbed(:user)) }
 
-  subject { uploader }
+  describe "#store_dir" do
+    it "stores in the system dir" do
+      expect(uploader.store_dir).to start_with("uploads/-/system/user")
+    end
 
-  it_behaves_like 'builds correct paths',
-                  store_dir: %r[uploads/-/system/user/avatar/],
-                  upload_path: %r[uploads/-/system/user/avatar/],
-                  absolute_path: %r[#{CarrierWave.root}/uploads/-/system/user/avatar/]
+    it "uses the old path when using object storage" do
+      expect(described_class).to receive(:file_storage?).and_return(false)
+      expect(uploader.store_dir).to start_with("uploads/user")
+    end
+  end
 
   describe '#move_to_cache' do
     it 'is false' do
diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb
index bc024cd307c..0cf462e9553 100644
--- a/spec/uploaders/file_mover_spec.rb
+++ b/spec/uploaders/file_mover_spec.rb
@@ -3,13 +3,13 @@ require 'spec_helper'
 describe FileMover do
   let(:filename) { 'banana_sample.gif' }
   let(:file) { fixture_file_upload(Rails.root.join('spec', 'fixtures', filename)) }
-  let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) }
-
   let(:temp_description) do
-    "test ![banana_sample](/#{temp_file_path}) "\
-    "same ![banana_sample](/#{temp_file_path}) "
+    'test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) same ![banana_sample]'\
+    '(/uploads/-/system/temp/secret55/banana_sample.gif)'
   end
-  let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) }
+  let(:temp_file_path) { File.join('secret55', filename).to_s }
+  let(:file_path) { File.join('uploads', '-', 'system', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s }
+
   let(:snippet) { create(:personal_snippet, description: temp_description) }
 
   subject { described_class.new(file_path, snippet).execute }
@@ -28,8 +28,8 @@ describe FileMover do
 
         expect(snippet.reload.description)
           .to eq(
-            "test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
-            "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "
+            "test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"\
+            " same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"
           )
       end
 
@@ -50,8 +50,8 @@ describe FileMover do
 
         expect(snippet.reload.description)
           .to eq(
-            "test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) "\
-            "same ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) "
+            "test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif)"\
+            " same ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif)"
           )
       end
 
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index a72f853df75..845516e5004 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -1,57 +1,118 @@
 require 'spec_helper'
 
 describe FileUploader do
-  let(:group) { create(:group, name: 'awesome') }
-  let(:project) { create(:project, namespace: group, name: 'project') }
-  let(:uploader) { described_class.new(project) }
-  let(:upload)  { double(model: project, path: 'secret/foo.jpg') }
+  let(:uploader) { described_class.new(build_stubbed(:project)) }
 
-  subject { uploader }
+  context 'legacy storage' do
+    let(:project) { build_stubbed(:project) }
+
+    describe '.absolute_path' do
+      it 'returns the correct absolute path by building it dynamically' do
+        upload = double(model: project, path: 'secret/foo.jpg')
+
+        dynamic_segment = project.full_path
 
-  shared_examples 'builds correct legacy storage paths' do
-    include_examples 'builds correct paths',
-                     store_dir: %r{awesome/project/\h+},
-                     absolute_path: %r{#{described_class.root}/awesome/project/secret/foo.jpg}
+        expect(described_class.absolute_path(upload))
+          .to end_with("#{dynamic_segment}/secret/foo.jpg")
+      end
+    end
+
+    describe "#store_dir" do
+      it "stores in the namespace path" do
+        uploader = described_class.new(project)
+
+        expect(uploader.store_dir).to include(project.full_path)
+        expect(uploader.store_dir).not_to include("system")
+      end
+    end
   end
 
-  shared_examples 'uses hashed storage' do
+  context 'hashed storage' do
     context 'when rolled out attachments' do
-      before do
-        allow(project).to receive(:disk_path).and_return('ca/fe/fe/ed')
+      let(:project) { build_stubbed(:project, :hashed) }
+
+      describe '.absolute_path' do
+        it 'returns the correct absolute path by building it dynamically' do
+          upload = double(model: project, path: 'secret/foo.jpg')
+
+          dynamic_segment = project.disk_path
+
+          expect(described_class.absolute_path(upload))
+            .to end_with("#{dynamic_segment}/secret/foo.jpg")
+        end
       end
 
-      let(:project) { build_stubbed(:project, :hashed, namespace: group, name: 'project') }
+      describe "#store_dir" do
+        it "stores in the namespace path" do
+          uploader = described_class.new(project)
 
-      it_behaves_like 'builds correct paths',
-                      store_dir: %r{ca/fe/fe/ed/\h+},
-                      absolute_path: %r{#{described_class.root}/ca/fe/fe/ed/secret/foo.jpg}
+          expect(uploader.store_dir).to include(project.disk_path)
+          expect(uploader.store_dir).not_to include("system")
+        end
+      end
     end
 
     context 'when only repositories are rolled out' do
-      let(:project) { build_stubbed(:project, namespace: group, name: 'project', storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) }
+      let(:project) { build_stubbed(:project, storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) }
 
-      it_behaves_like 'builds correct legacy storage paths'
-    end
-  end
+      describe '.absolute_path' do
+        it 'returns the correct absolute path by building it dynamically' do
+          upload = double(model: project, path: 'secret/foo.jpg')
 
-  context 'legacy storage' do
-    it_behaves_like 'builds correct legacy storage paths'
-    include_examples 'uses hashed storage'
+          dynamic_segment = project.full_path
+
+          expect(described_class.absolute_path(upload))
+            .to end_with("#{dynamic_segment}/secret/foo.jpg")
+        end
+      end
+
+      describe "#store_dir" do
+        it "stores in the namespace path" do
+          uploader = described_class.new(project)
+
+          expect(uploader.store_dir).to include(project.full_path)
+          expect(uploader.store_dir).not_to include("system")
+        end
+      end
+    end
   end
 
   describe 'initialize' do
-    let(:uploader) { described_class.new(double, 'secret') }
+    it 'generates a secret if none is provided' do
+      expect(SecureRandom).to receive(:hex).and_return('secret')
+
+      uploader = described_class.new(double)
+
+      expect(uploader.secret).to eq 'secret'
+    end
 
     it 'accepts a secret parameter' do
-      expect(described_class).not_to receive(:generate_secret)
-      expect(uploader.secret).to eq('secret')
+      expect(SecureRandom).not_to receive(:hex)
+
+      uploader = described_class.new(double, 'secret')
+
+      expect(uploader.secret).to eq 'secret'
     end
   end
 
-  describe '#secret' do
-    it 'generates a secret if none is provided' do
-      expect(described_class).to receive(:generate_secret).and_return('secret')
-      expect(uploader.secret).to eq('secret')
+  describe '#move_to_cache' do
+    it 'is true' do
+      expect(uploader.move_to_cache).to eq(true)
+    end
+  end
+
+  describe '#move_to_store' do
+    it 'is true' do
+      expect(uploader.move_to_store).to eq(true)
+    end
+  end
+
+  describe '#relative_path' do
+    it 'removes the leading dynamic path segment' do
+      fixture = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')
+      uploader.store!(fixture_file_upload(fixture))
+
+      expect(uploader.relative_path).to match(%r{\A\h{32}/rails_sample.jpg\z})
     end
   end
 end
diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb
index d606404e95d..a067c3e75f4 100644
--- a/spec/uploaders/job_artifact_uploader_spec.rb
+++ b/spec/uploaders/job_artifact_uploader_spec.rb
@@ -3,13 +3,33 @@ require 'spec_helper'
 describe JobArtifactUploader do
   let(:job_artifact) { create(:ci_job_artifact) }
   let(:uploader) { described_class.new(job_artifact, :file) }
+  let(:local_path) { Gitlab.config.artifacts.path }
 
-  subject { uploader }
+  describe '#store_dir' do
+    subject { uploader.store_dir }
 
-  it_behaves_like "builds correct paths",
-                  store_dir: %r[\h{2}/\h{2}/\h{64}/\d{4}_\d{1,2}_\d{1,2}/\d+/\d+\z],
-                  cache_dir: %r[artifacts/tmp/cache],
-                  work_dir: %r[artifacts/tmp/work]
+    let(:path) { "#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/#{job_artifact.job_id}/#{job_artifact.id}" }
+
+    context 'when using local storage' do
+      it { is_expected.to start_with(local_path) }
+      it { is_expected.to match(%r{\h{2}/\h{2}/\h{64}/\d{4}_\d{1,2}_\d{1,2}/\d+/\d+\z}) }
+      it { is_expected.to end_with(path) }
+    end
+  end
+
+  describe '#cache_dir' do
+    subject { uploader.cache_dir }
+
+    it { is_expected.to start_with(local_path) }
+    it { is_expected.to end_with('/tmp/cache') }
+  end
+
+  describe '#work_dir' do
+    subject { uploader.work_dir }
+
+    it { is_expected.to start_with(local_path) }
+    it { is_expected.to end_with('/tmp/work') }
+  end
 
   context 'file is stored in valid local_path' do
     let(:file) do
@@ -23,7 +43,7 @@ describe JobArtifactUploader do
 
     subject { uploader.file.path }
 
-    it { is_expected.to start_with("#{uploader.root}/#{uploader.class.base_dir}") }
+    it { is_expected.to start_with(local_path) }
     it { is_expected.to include("/#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/") }
     it { is_expected.to include("/#{job_artifact.job_id}/#{job_artifact.id}/") }
     it { is_expected.to end_with("ci_build_artifacts.zip") }
diff --git a/spec/uploaders/legacy_artifact_uploader_spec.rb b/spec/uploaders/legacy_artifact_uploader_spec.rb
index 54c6a8b869b..efeffb78772 100644
--- a/spec/uploaders/legacy_artifact_uploader_spec.rb
+++ b/spec/uploaders/legacy_artifact_uploader_spec.rb
@@ -3,22 +3,49 @@ require 'rails_helper'
 describe LegacyArtifactUploader do
   let(:job) { create(:ci_build) }
   let(:uploader) { described_class.new(job, :legacy_artifacts_file) }
-  let(:local_path) { described_class.root }
+  let(:local_path) { Gitlab.config.artifacts.path }
 
-  subject { uploader }
+  describe '.local_store_path' do
+    subject { described_class.local_store_path }
 
-  # TODO: move to Workhorse::UploadPath
-  describe '.workhorse_upload_path' do
-    subject { described_class.workhorse_upload_path }
+    it "delegate to artifacts path" do
+      expect(Gitlab.config.artifacts).to receive(:path)
+
+      subject
+    end
+  end
+
+  describe '.artifacts_upload_path' do
+    subject { described_class.artifacts_upload_path }
 
     it { is_expected.to start_with(local_path) }
-    it { is_expected.to end_with('tmp/uploads') }
+    it { is_expected.to end_with('tmp/uploads/') }
+  end
+
+  describe '#store_dir' do
+    subject { uploader.store_dir }
+
+    let(:path) { "#{job.created_at.utc.strftime('%Y_%m')}/#{job.project_id}/#{job.id}" }
+
+    context 'when using local storage' do
+      it { is_expected.to start_with(local_path) }
+      it { is_expected.to end_with(path) }
+    end
   end
 
-  it_behaves_like "builds correct paths",
-                  store_dir: %r[\d{4}_\d{1,2}/\d+/\d+\z],
-                  cache_dir: %r[artifacts/tmp/cache],
-                  work_dir: %r[artifacts/tmp/work]
+  describe '#cache_dir' do
+    subject { uploader.cache_dir }
+
+    it { is_expected.to start_with(local_path) }
+    it { is_expected.to end_with('/tmp/cache') }
+  end
+
+  describe '#work_dir' do
+    subject { uploader.work_dir }
+
+    it { is_expected.to start_with(local_path) }
+    it { is_expected.to end_with('/tmp/work') }
+  end
 
   describe '#filename' do
     # we need to use uploader, as this makes to use mounter
@@ -42,7 +69,7 @@ describe LegacyArtifactUploader do
 
     subject { uploader.file.path }
 
-    it { is_expected.to start_with("#{uploader.root}") }
+    it { is_expected.to start_with(local_path) }
     it { is_expected.to include("/#{job.created_at.utc.strftime('%Y_%m')}/") }
     it { is_expected.to include("/#{job.project_id}/") }
     it { is_expected.to end_with("ci_build_artifacts.zip") }
diff --git a/spec/uploaders/lfs_object_uploader_spec.rb b/spec/uploaders/lfs_object_uploader_spec.rb
index 6ebc885daa8..7088bc23334 100644
--- a/spec/uploaders/lfs_object_uploader_spec.rb
+++ b/spec/uploaders/lfs_object_uploader_spec.rb
@@ -2,13 +2,39 @@ require 'spec_helper'
 
 describe LfsObjectUploader do
   let(:lfs_object) { create(:lfs_object, :with_file) }
-  let(:uploader) { described_class.new(lfs_object, :file) }
+  let(:uploader) { described_class.new(lfs_object) }
   let(:path) { Gitlab.config.lfs.storage_path }
 
-  subject { uploader }
+  describe '#move_to_cache' do
+    it 'is true' do
+      expect(uploader.move_to_cache).to eq(true)
+    end
+  end
 
-  it_behaves_like "builds correct paths",
-                  store_dir: %r[\h{2}/\h{2}],
-                  cache_dir: %r[/lfs-objects/tmp/cache],
-                  work_dir: %r[/lfs-objects/tmp/work]
+  describe '#move_to_store' do
+    it 'is true' do
+      expect(uploader.move_to_store).to eq(true)
+    end
+  end
+
+  describe '#store_dir' do
+    subject { uploader.store_dir }
+
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with("#{lfs_object.oid[0, 2]}/#{lfs_object.oid[2, 2]}") }
+  end
+
+  describe '#cache_dir' do
+    subject { uploader.cache_dir }
+
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with('/tmp/cache') }
+  end
+
+  describe '#work_dir' do
+    subject { uploader.work_dir }
+
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with('/tmp/work') }
+  end
 end
diff --git a/spec/uploaders/namespace_file_uploader_spec.rb b/spec/uploaders/namespace_file_uploader_spec.rb
index 24a2fc0f72e..c6c4500c179 100644
--- a/spec/uploaders/namespace_file_uploader_spec.rb
+++ b/spec/uploaders/namespace_file_uploader_spec.rb
@@ -1,16 +1,21 @@
 require 'spec_helper'
 
-IDENTIFIER = %r{\h+/\S+}
-
 describe NamespaceFileUploader do
   let(:group) { build_stubbed(:group) }
   let(:uploader) { described_class.new(group) }
-  let(:upload) { create(:upload, :namespace_upload, model: group) }
 
-  subject { uploader }
+  describe "#store_dir" do
+    it "stores in the namespace id directory" do
+      expect(uploader.store_dir).to include(group.id.to_s)
+    end
+  end
+
+  describe ".absolute_path" do
+    it "stores in thecorrect directory" do
+      upload_record = create(:upload, :namespace_upload, model: group)
 
-  it_behaves_like 'builds correct paths',
-                  store_dir: %r[uploads/-/system/namespace/\d+],
-                  upload_path: IDENTIFIER,
-                  absolute_path: %r[#{CarrierWave.root}/uploads/-/system/namespace/\d+/#{IDENTIFIER}]
+      expect(described_class.absolute_path(upload_record))
+        .to include("-/system/namespace/#{group.id}")
+    end
+  end
 end
diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb
index ed1fba6edda..cbafa9f478d 100644
--- a/spec/uploaders/personal_file_uploader_spec.rb
+++ b/spec/uploaders/personal_file_uploader_spec.rb
@@ -1,27 +1,25 @@
 require 'spec_helper'
 
-IDENTIFIER = %r{\h+/\S+}
-
 describe PersonalFileUploader do
-  let(:model) { create(:personal_snippet) }
-  let(:uploader) { described_class.new(model) }
-  let(:upload) { create(:upload, :personal_snippet_upload) }
+  let(:uploader) { described_class.new(build_stubbed(:project)) }
+  let(:snippet) { create(:personal_snippet) }
 
-  subject { uploader }
+  describe '.absolute_path' do
+    it 'returns the correct absolute path by building it dynamically' do
+      upload = double(model: snippet, path: 'secret/foo.jpg')
 
-  it_behaves_like 'builds correct paths',
-                  store_dir: %r[uploads/-/system/personal_snippet/\d+],
-                  upload_path: IDENTIFIER,
-                  absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet/\d+/#{IDENTIFIER}]
+      dynamic_segment = "personal_snippet/#{snippet.id}"
 
-  describe '#to_h' do
-    before do
-      subject.instance_variable_set(:@secret, 'secret')
+      expect(described_class.absolute_path(upload)).to end_with("/-/system/#{dynamic_segment}/secret/foo.jpg")
     end
+  end
+
+  describe '#to_h' do
+    it 'returns the hass' do
+      uploader = described_class.new(snippet, 'secret')
 
-    it 'is correct' do
       allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name'))
-      expected_url = "/uploads/-/system/personal_snippet/#{model.id}/secret/file_name"
+      expected_url = "/uploads/-/system/personal_snippet/#{snippet.id}/secret/file_name"
 
       expect(uploader.to_h).to eq(
         alt: 'file_name',
diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb
index 9a3e5d83e01..7ef7fb7d758 100644
--- a/spec/uploaders/records_uploads_spec.rb
+++ b/spec/uploaders/records_uploads_spec.rb
@@ -3,16 +3,16 @@ require 'rails_helper'
 describe RecordsUploads do
   let!(:uploader) do
     class RecordsUploadsExampleUploader < GitlabUploader
-      include RecordsUploads::Concern
+      include RecordsUploads
 
       storage :file
 
-      def dynamic_segment
-        'co/fe/ee'
+      def model
+        FactoryBot.build_stubbed(:user)
       end
     end
 
-    RecordsUploadsExampleUploader.new(build_stubbed(:user))
+    RecordsUploadsExampleUploader.new
   end
 
   def upload_fixture(filename)
@@ -20,55 +20,48 @@ describe RecordsUploads do
   end
 
   describe 'callbacks' do
-    let(:upload) { create(:upload) }
-
-    before do
-      uploader.upload = upload
-    end
-
-    it '#record_upload after `store`' do
+    it 'calls `record_upload` after `store`' do
       expect(uploader).to receive(:record_upload).once
 
       uploader.store!(upload_fixture('doc_sample.txt'))
     end
 
-    it '#destroy_upload after `remove`' do
+    it 'calls `destroy_upload` after `remove`' do
+      expect(uploader).to receive(:destroy_upload).once
+
       uploader.store!(upload_fixture('doc_sample.txt'))
 
-      expect(uploader).to receive(:destroy_upload).once
       uploader.remove!
     end
   end
 
   describe '#record_upload callback' do
-    it 'creates an Upload record after store' do
-      expect { uploader.store!(upload_fixture('rails_sample.jpg')) }.to change { Upload.count }.by(1)
-    end
+    it 'returns early when not using file storage' do
+      allow(uploader).to receive(:file_storage?).and_return(false)
+      expect(Upload).not_to receive(:record)
 
-    it 'creates a new record and assigns size, path, model, and uploader' do
       uploader.store!(upload_fixture('rails_sample.jpg'))
-
-      upload = uploader.upload
-      aggregate_failures do
-        expect(upload).to be_persisted
-        expect(upload.size).to eq uploader.file.size
-        expect(upload.path).to eq uploader.upload_path
-        expect(upload.model_id).to eq uploader.model.id
-        expect(upload.model_type).to eq uploader.model.class.to_s
-        expect(upload.uploader).to eq uploader.class.to_s
-      end
     end
 
-    it "does not create an Upload record when the file doesn't exist" do
+    it "returns early when the file doesn't exist" do
       allow(uploader).to receive(:file).and_return(double(exists?: false))
+      expect(Upload).not_to receive(:record)
 
-      expect { uploader.store!(upload_fixture('rails_sample.jpg')) }.not_to change { Upload.count }
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+    end
+
+    it 'creates an Upload record after store' do
+      expect(Upload).to receive(:record)
+        .with(uploader)
+
+      uploader.store!(upload_fixture('rails_sample.jpg'))
     end
 
     it 'does not create an Upload record if model is missing' do
-      allow_any_instance_of(RecordsUploadsExampleUploader).to receive(:model).and_return(nil)
+      expect_any_instance_of(RecordsUploadsExampleUploader).to receive(:model).and_return(nil)
+      expect(Upload).not_to receive(:record).with(uploader)
 
-      expect { uploader.store!(upload_fixture('rails_sample.jpg')) }.not_to change { Upload.count }
+      uploader.store!(upload_fixture('rails_sample.jpg'))
     end
 
     it 'it destroys Upload records at the same path before recording' do
@@ -79,15 +72,29 @@ describe RecordsUploads do
         uploader: uploader.class.to_s
       )
 
-      uploader.upload = existing
       uploader.store!(upload_fixture('rails_sample.jpg'))
 
       expect { existing.reload }.to raise_error(ActiveRecord::RecordNotFound)
-      expect(Upload.count).to eq(1)
+      expect(Upload.count).to eq 1
     end
   end
 
   describe '#destroy_upload callback' do
+    it 'returns early when not using file storage' do
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+
+      allow(uploader).to receive(:file_storage?).and_return(false)
+      expect(Upload).not_to receive(:remove_path)
+
+      uploader.remove!
+    end
+
+    it 'returns early when file is nil' do
+      expect(Upload).not_to receive(:remove_path)
+
+      uploader.remove!
+    end
+
     it 'it destroys Upload records at the same path after removal' do
       uploader.store!(upload_fixture('rails_sample.jpg'))
 
diff --git a/spec/workers/upload_checksum_worker_spec.rb b/spec/workers/upload_checksum_worker_spec.rb
index 9e50ce15871..911360da66c 100644
--- a/spec/workers/upload_checksum_worker_spec.rb
+++ b/spec/workers/upload_checksum_worker_spec.rb
@@ -2,31 +2,18 @@ require 'rails_helper'
 
 describe UploadChecksumWorker do
   describe '#perform' do
-    subject { described_class.new }
-
-    context 'without a valid record' do
-      it 'rescues ActiveRecord::RecordNotFound' do
-        expect { subject.perform(999_999) }.not_to raise_error
-      end
+    it 'rescues ActiveRecord::RecordNotFound' do
+      expect { described_class.new.perform(999_999) }.not_to raise_error
     end
 
-    context 'with a valid record' do
-      let(:upload) { create(:user, :with_avatar).avatar.upload }
-
-      before do
-        expect(Upload).to receive(:find).and_return(upload)
-        allow(upload).to receive(:foreground_checksumable?).and_return(false)
-      end
+    it 'calls calculate_checksum_without_delay and save!' do
+      upload = spy
+      expect(Upload).to receive(:find).with(999_999).and_return(upload)
 
-      it 'calls calculate_checksum!' do
-        expect(upload).to receive(:calculate_checksum!)
-        subject.perform(upload.id)
-      end
+      described_class.new.perform(999_999)
 
-      it 'calls save!' do
-        expect(upload).to receive(:save!)
-        subject.perform(upload.id)
-      end
+      expect(upload).to have_received(:calculate_checksum)
+      expect(upload).to have_received(:save!)
     end
   end
 end