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

Store uploader context in uploads

Closes #42547

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

6 files changed, 50 insertions, 18 deletions

diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index 61554029d09..7ad79a1e56c 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -70,7 +70,7 @@ module UploadsActions
   end
 
   def build_uploader_from_params
-    uploader = uploader_class.new(model, params[:secret])
+    uploader = uploader_class.new(model, secret: params[:secret])
     uploader.retrieve_from_store!(params[:filename])
     uploader
   end
diff --git a/app/models/upload.rb b/app/models/upload.rb
index fb55fd8007b..2024228537a 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -30,7 +30,7 @@ class Upload < ActiveRecord::Base
   end
 
   def build_uploader
-    uploader_class.new(model).tap do |uploader|
+    uploader_class.new(model, mount_point, **uploader_context).tap do |uploader|
       uploader.upload = self
       uploader.retrieve_from_store!(identifier)
     end
@@ -40,6 +40,13 @@ class Upload < ActiveRecord::Base
     File.exist?(absolute_path)
   end
 
+  def uploader_context
+    {
+      identifier: identifier,
+      secret: secret
+    }.compact
+  end
+
   private
 
   def checksummable?
@@ -62,11 +69,15 @@ class Upload < ActiveRecord::Base
     !path.start_with?('/')
   end
 
+  def uploader_class
+    Object.const_get(uploader)
+  end
+
   def identifier
     File.basename(path)
   end
 
-  def uploader_class
-    Object.const_get(uploader)
+  def mount_point
+    super&.to_sym
   end
 end
diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb
index e7af1483d23..8f56f09c9f7 100644
--- a/app/uploaders/file_mover.rb
+++ b/app/uploaders/file_mover.rb
@@ -49,11 +49,11 @@ class FileMover
   end
 
   def uploader
-    @uploader ||= PersonalFileUploader.new(model, secret)
+    @uploader ||= PersonalFileUploader.new(model, secret: secret)
   end
 
   def temp_file_uploader
-    @temp_file_uploader ||= PersonalFileUploader.new(nil, secret)
+    @temp_file_uploader ||= PersonalFileUploader.new(nil, secret: secret)
   end
 
   def revert
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 85ae9863b13..2310e67cb2e 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -62,9 +62,11 @@ class FileUploader < GitlabUploader
 
   attr_accessor :model
 
-  def initialize(model, secret = nil)
+  def initialize(model, mounted_as = nil, **uploader_context)
+    super(model, nil, **uploader_context)
+
     @model = model
-    @secret = secret
+    apply_context!(uploader_context)
   end
 
   def base_dir
@@ -107,15 +109,17 @@ class FileUploader < GitlabUploader
     self.file.filename
   end
 
-  # the upload does not hold the secret, but holds the path
-  # which contains the secret: extract it
   def upload=(value)
+    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
-
-    super
   end
 
   def secret
@@ -124,6 +128,18 @@ class FileUploader < GitlabUploader
 
   private
 
+  def apply_context!(uploader_context)
+    @secret, @identifier = uploader_context.values_at(:secret, :identifier)
+
+    !!(@secret && @identifier)
+  end
+
+  def build_upload
+    super.tap do |upload|
+      upload.secret = secret
+    end
+  end
+
   def markdown_name
     (image_or_video? ? File.basename(filename, File.extname(filename)) : filename).gsub("]", "\\]")
   end
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index b12829efe73..a9e5c028b03 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -29,6 +29,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
 
   delegate :base_dir, :file_storage?, to: :class
 
+  def initialize(model, mounted_as = nil, **uploader_context)
+    super(model, mounted_as)
+  end
+
   def file_cache_storage?
     cache_storage.is_a?(CarrierWave::Storage::File)
   end
diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb
index dfb8dccec57..458928bc067 100644
--- a/app/uploaders/records_uploads.rb
+++ b/app/uploaders/records_uploads.rb
@@ -24,7 +24,7 @@ module RecordsUploads
         uploads.where(path: upload_path).delete_all
         upload.destroy! if upload
 
-        self.upload = build_upload_from_uploader(self)
+        self.upload = build_upload
         upload.save!
       end
     end
@@ -39,12 +39,13 @@ module RecordsUploads
       Upload.order(id: :desc).where(uploader: self.class.to_s)
     end
 
-    def build_upload_from_uploader(uploader)
+    def build_upload
       Upload.new(
-        size: uploader.file.size,
-        path: uploader.upload_path,
-        model: uploader.model,
-        uploader: uploader.class.to_s
+        uploader: self.class.to_s,
+        size: file.size,
+        path: upload_path,
+        model: model,
+        mount_point: mounted_as
       )
     end