Merge branch '42547-upload-store-mount-point' into 'master'

Store uploader context in uploads

Closes #42547

See merge request gitlab-org/gitlab-ce!16779

5 files changed, 48 insertions, 11 deletions

diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
index c8cfe251d27..ff3a2a76acc 100644
--- a/spec/factories/uploads.rb
+++ b/spec/factories/uploads.rb
@@ -3,36 +3,40 @@ FactoryBot.define do
     model { build(:project) }
     size 100.kilobytes
     uploader "AvatarUploader"
+    mount_point :avatar
+    secret nil
 
     # we should build a mount agnostic upload by default
     transient do
-      mounted_as :avatar
-      secret SecureRandom.hex
+      filename 'myfile.jpg'
     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') }
+    path { File.join("uploads/-/system", model.class.to_s.underscore, mount_point.to_s, 'avatar.jpg') }
 
     trait :personal_snippet_upload do
-      model { build(:personal_snippet) }
-      path { File.join(secret, 'myfile.jpg') }
       uploader "PersonalFileUploader"
+      path { File.join(secret, filename) }
+      model { build(:personal_snippet) }
+      secret SecureRandom.hex
     end
 
     trait :issuable_upload do
-      path { File.join(secret, 'myfile.jpg') }
       uploader "FileUploader"
+      path { File.join(secret, filename) }
+      secret SecureRandom.hex
     end
 
     trait :namespace_upload do
       model { build(:group) }
-      path { File.join(secret, 'myfile.jpg') }
+      path { File.join(secret, filename) }
       uploader "NamespaceFileUploader"
+      secret SecureRandom.hex
     end
 
     trait :attachment_upload do
       transient do
-        mounted_as :attachment
+        mount_point :attachment
       end
 
       model { build(:note) }
diff --git a/spec/lib/file_size_validator_spec.rb b/spec/lib/file_size_validator_spec.rb
index c44bc1840df..ebd907ecb7f 100644
--- a/spec/lib/file_size_validator_spec.rb
+++ b/spec/lib/file_size_validator_spec.rb
@@ -2,8 +2,8 @@ require 'spec_helper'
 
 describe FileSizeValidator do
   let(:validator) { described_class.new(options) }
-  let(:attachment) { AttachmentUploader.new }
   let(:note) { create(:note) }
+  let(:attachment) { AttachmentUploader.new(note) }
 
   describe 'options uses an integer' do
     let(:options) { { maximum: 10, attributes: { attachment: attachment } } }
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index 42f3d609770..0dcaa026332 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -103,4 +103,10 @@ describe Upload do
       expect(upload).not_to exist
     end
   end
+
+  describe "#uploader_context" do
+    subject { create(:upload, :issuable_upload, secret: 'secret', filename: 'file.txt') }
+
+    it { expect(subject.uploader_context).to match(a_hash_including(secret: 'secret', identifier: 'file.txt')) }
+  end
 end
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index a72f853df75..a559681a079 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -40,7 +40,7 @@ describe FileUploader do
   end
 
   describe 'initialize' do
-    let(:uploader) { described_class.new(double, 'secret') }
+    let(:uploader) { described_class.new(double, secret: 'secret') }
 
     it 'accepts a secret parameter' do
       expect(described_class).not_to receive(:generate_secret)
@@ -54,4 +54,31 @@ describe FileUploader do
       expect(uploader.secret).to eq('secret')
     end
   end
+
+  describe '#upload=' do
+    let(:secret) { SecureRandom.hex }
+    let(:upload) { create(:upload, :issuable_upload, secret: secret, filename: 'file.txt') }
+
+    it 'handles nil' do
+      expect(uploader).not_to receive(:apply_context!)
+
+      uploader.upload = nil
+    end
+
+    it 'extract the uploader context from it' do
+      expect(uploader).to receive(:apply_context!).with(a_hash_including(secret: secret, identifier: 'file.txt'))
+
+      uploader.upload = upload
+    end
+
+    context 'uploader_context is empty' do
+      it 'fallbacks to regex based extraction' do
+        expect(upload).to receive(:uploader_context).and_return({})
+
+        uploader.upload = upload
+        expect(uploader.secret).to eq(secret)
+        expect(uploader.instance_variable_get(:@identifier)).to eq('file.txt')
+      end
+    end
+  end
 end
diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb
index a144b39f74f..60e35dcf235 100644
--- a/spec/uploaders/gitlab_uploader_spec.rb
+++ b/spec/uploaders/gitlab_uploader_spec.rb
@@ -4,7 +4,7 @@ require 'carrierwave/storage/fog'
 describe GitlabUploader do
   let(:uploader_class) { Class.new(described_class) }
 
-  subject { uploader_class.new }
+  subject { uploader_class.new(double) }
 
   describe '#file_storage?' do
     context 'when file storage is used' do