apply fixes from feedback42547-upload-store-mount-point

3 files changed, 37 insertions, 9 deletions

diff --git a/app/models/upload.rb b/app/models/upload.rb
index 28baee95091..2024228537a 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -30,8 +30,7 @@ class Upload < ActiveRecord::Base
   end
 
   def build_uploader
-    uploader_class.new(model, mount_point, **uploader_context)
-      .tap do |uploader|
+    uploader_class.new(model, mount_point, **uploader_context).tap do |uploader|
       uploader.upload = self
       uploader.retrieve_from_store!(identifier)
     end
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index cc4e7a38165..2310e67cb2e 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -110,14 +110,16 @@ class FileUploader < GitlabUploader
   end
 
   def upload=(value)
-    unless apply_context!(value.uploader_context)
-      if matches = DYNAMIC_PATH_PATTERN.match(value.path)
-        @secret = matches[:secret]
-        @identifier = matches[:identifier]
-      end
-    end
-
     super
+
+    return unless value
+    return if apply_context!(value.uploader_context)
+
+    # fallback to the regex based extraction
+    if matches = DYNAMIC_PATH_PATTERN.match(value.path)
+      @secret = matches[:secret]
+      @identifier = matches[:identifier]
+    end
   end
 
   def secret
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index 92f7467d247..a559681a079 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -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