Merge branch '4163-move-uploads-to-object-storage' into 'master'

Move uploads to object storage

Closes #4163

See merge request gitlab-org/gitlab-ee!3867

app/controllers/concerns/uploads_actions.rb

 module UploadsActions
   include Gitlab::Utils::StrongMemoize
+  UPLOAD_MOUNTS = %w(avatar attachment file logo header_logo).freeze
+
   def create
     link_to_file = UploadService.new(model, params[:file], uploader_class).execute
@@ -17,34 +19,71 @@ module UploadsActions
     end
   end
+  # This should either
+  #   - send the file directly
+  #   - or redirect to its URL
+  #
   def show
     return render_404 unless uploader.exists?
-    disposition = uploader.image_or_video? ? 'inline' : 'attachment'
-
-    expires_in 0.seconds, must_revalidate: true, private: true
-    send_file uploader.file.path, disposition: disposition
+    if uploader.file_storage?
+      disposition = uploader.image_or_video? ? 'inline' : 'attachment'
+      expires_in 0.seconds, must_revalidate: true, private: true
+      send_file uploader.file.path, disposition: disposition
+    else
+      redirect_to uploader.url
+    end
   end
   private
+  def uploader_class
+    raise NotImplementedError
+  end
+
+  def upload_mount
+    mounted_as = params[:mounted_as]
+    mounted_as if UPLOAD_MOUNTS.include?(mounted_as)
+  end
+
+  def uploader_mounted?
+    upload_model_class < CarrierWave::Mount::Extension && !upload_mount.nil?
+  end
+
   def uploader
     strong_memoize(:uploader) do
-      return if show_model.nil?
-      file_uploader = FileUploader.new(show_model, params[:secret])
-      file_uploader.retrieve_from_store!(params[:filename])
-      file_uploader
-    end
+      if uploader_mounted?
+        model.public_send(upload_mount) # rubocop:disable GitlabSecurity/PublicSend
+      else
+        build_uploader_from_upload || build_uploader_from_params
+      end
+    end
+  end
+  def build_uploader_from_upload
+    return nil unless params[:secret] && params[:filename]
+    upload_path = uploader_class.upload_path(params[:secret], params[:filename])
+    upload = Upload.find_by(uploader: uploader_class.to_s, path: upload_path)
+    upload&.build_uploader
+  end
+
+  def build_uploader_from_params
+    uploader = uploader_class.new(model, params[:secret])
+    uploader.retrieve_from_store!(params[:filename])
+    uploader
   end
   def image_or_video?
     uploader && uploader.exists? && uploader.image_or_video?
   end
-  def uploader_class
-    FileUploader
+  def find_model
+    nil
+  end
+
+  def model
+    strong_memoize(:model) { find_model }
   end
 end

app/controllers/groups/uploads_controller.rb

@@ -7,29 +7,23 @@ class Groups::UploadsController < Groups::ApplicationController
   private
-  def show_model
-    strong_memoize(:show_model) do
-      group_id = params[:group_id]
-
-      Group.find_by_full_path(group_id)
-    end
+  def upload_model_class
+    Group
   end
-  def authorize_upload_file!
-    render_404 unless can?(current_user, :upload_file, group)
+  def uploader_class
+    NamespaceFileUploader
   end
-  def uploader
-    strong_memoize(:uploader) do
-      file_uploader = uploader_class.new(show_model, params[:secret])
-      file_uploader.retrieve_from_store!(params[:filename])
-      file_uploader
-    end
-  end
-  def uploader_class
-    NamespaceFileUploader
+  def find_model
+    return @group if @group
+    group_id = params[:group_id]
+
+    Group.find_by_full_path(group_id)
   end
-  alias_method :model, :group
+  def authorize_upload_file!
+    render_404 unless can?(current_user, :upload_file, group)
+  end
 end

app/controllers/projects/lfs_storage_controller.rb

@@ -61,7 +61,7 @@ class Projects::LfsStorageController < Projects::GitHttpClientController
   def store_file(oid, size, tmp_file)
     # Define tmp_file_path early because we use it in "ensure"
-    tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file)
+    tmp_file_path = File.join(LfsObjectUploader.workhorse_upload_path, tmp_file)
     object = LfsObject.find_or_create_by(oid: oid, size: size)
     file_exists = object.file.exists? || move_tmp_file_to_storage(object, tmp_file_path)

app/controllers/projects/uploads_controller.rb

 class Projects::UploadsController < Projects::ApplicationController
   include UploadsActions
+  # These will kick you out if you don't have access.
   skip_before_action :project, :repository,
     if: -> { action_name == 'show' && image_or_video? }
@@ -8,14 +9,20 @@ class Projects::UploadsController < Projects::ApplicationController
   private
-  def show_model
-    strong_memoize(:show_model) do
-      namespace = params[:namespace_id]
-      id = params[:project_id]
-      Project.find_by_full_path("#{namespace}/#{id}")
-    end
+  def upload_model_class
+    Project
+  end
+  def uploader_class
+    FileUploader
   end
-  alias_method :model, :project
+  def find_model
+    return @project if @project
+
+    namespace = params[:namespace_id]
+    id = params[:project_id]
+
+    Project.find_by_full_path("#{namespace}/#{id}")
+  end
 end

app/controllers/uploads_controller.rb

 class UploadsController < ApplicationController
   include UploadsActions
+  UnknownUploadModelError = Class.new(StandardError)
+
+  MODEL_CLASSES = {
+    "user"             => User,
+    "project"          => Project,
+    "note"             => Note,
+    "group"            => Group,
+    "appearance"       => Appearance,
+    "personal_snippet" => PersonalSnippet,
+    nil                => PersonalSnippet
+  }.freeze
+
+  rescue_from UnknownUploadModelError, with: :render_404
+
   skip_before_action :authenticate_user!
+  before_action :upload_mount_satisfied?
   before_action :find_model
   before_action :authorize_access!, only: [:show]
   before_action :authorize_create_access!, only: [:create]
-  private
+  def uploader_class
+    PersonalFileUploader
+  end
   def find_model
     return nil unless params[:id]
-    return render_404 unless upload_model && upload_mount
-
-    @model = upload_model.find(params[:id])
+    upload_model_class.find(params[:id])
   end
   def authorize_access!
@@ -53,55 +68,17 @@ class UploadsController < ApplicationController
     end
   end
-  def upload_model
-    upload_models = {
-      "user"    => User,
-      "project" => Project,
-      "note"    => Note,
-      "group"   => Group,
-      "appearance" => Appearance,
-      "personal_snippet" => PersonalSnippet
-    }
-
-    upload_models[params[:model]]
-  end
-
-  def upload_mount
-    return true unless params[:mounted_as]
-
-    upload_mounts = %w(avatar attachment file logo header_logo)
-
-    if upload_mounts.include?(params[:mounted_as])
-      params[:mounted_as]
-    end
+  def upload_model_class
+    MODEL_CLASSES[params[:model]] || raise(UnknownUploadModelError)
   end
-  def uploader
-    return @uploader if defined?(@uploader)
-
-    case model
-    when nil
-      @uploader = PersonalFileUploader.new(nil, params[:secret])
-
-      @uploader.retrieve_from_store!(params[:filename])
-    when PersonalSnippet
-      @uploader = PersonalFileUploader.new(model, params[:secret])
-
-      @uploader.retrieve_from_store!(params[:filename])
-    else
-      @uploader = @model.public_send(upload_mount) # rubocop:disable GitlabSecurity/PublicSend
-
-      redirect_to @uploader.url unless @uploader.file_storage?
-    end
-
-    @uploader
+  def upload_model_class_has_mounts?
+    upload_model_class < CarrierWave::Mount::Extension
   end
-  def uploader_class
-    PersonalFileUploader
-  end
-  def model
-    @model ||= find_model
+  def upload_mount_satisfied?
+    return true unless upload_model_class_has_mounts?
+    upload_model_class.uploader_options.has_key?(upload_mount)
   end
 end

app/models/appearance.rb

@@ -11,6 +11,7 @@ class Appearance < ActiveRecord::Base
   mount_uploader :logo,         AttachmentUploader
   mount_uploader :header_logo,  AttachmentUploader
+
   has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   CACHE_KEY = 'current_appearance'.freeze

app/models/ci/build.rb

@@ -45,7 +45,7 @@ module Ci
     end
     scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }
-    scope :with_artifacts_stored_locally, ->() { with_artifacts.where(artifacts_file_store: [nil, LegacyArtifactUploader::LOCAL_STORE]) }
+    scope :with_artifacts_stored_locally, ->() { with_artifacts.where(artifacts_file_store: [nil, LegacyArtifactUploader::Store::LOCAL]) }
     scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
     scope :manual_actions, ->() { where(when: :manual, status: COMPLETED_STATUSES + [:manual]) }
     scope :ref_protected, -> { where(protected: true) }

app/models/concerns/avatarable.rb

 module Avatarable
   extend ActiveSupport::Concern
+  included do
+    prepend ShadowMethods
+
+    validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
+    validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
+
+    mount_uploader :avatar, AvatarUploader
+  end
+
+  module ShadowMethods
+    def avatar_url(**args)
+      # We use avatar_path instead of overriding avatar_url because of carrierwave.
+      # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864
+
+      avatar_path(only_path: args.fetch(:only_path, true)) || super
+    end
+  end
+
+  def avatar_type
+    unless self.avatar.image?
+      self.errors.add :avatar, "only images allowed"
+    end
+  end
+
   def avatar_path(only_path: true)
     return unless self[:avatar].present?

app/models/group.rb

@@ -29,18 +29,14 @@ class Group < Namespace
   has_many :variables, class_name: 'Ci::GroupVariable'
   has_many :custom_attributes, class_name: 'GroupCustomAttribute'
-  validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
+  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+
   validate :visibility_level_allowed_by_projects
   validate :visibility_level_allowed_by_sub_groups
   validate :visibility_level_allowed_by_parent
-  validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
-
   validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
-  mount_uploader :avatar, AvatarUploader
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   after_create :post_create_hook
   after_destroy :post_destroy_hook
   after_save :update_two_factor_requirement
@@ -116,12 +112,6 @@ class Group < Namespace
       visibility_level_allowed_by_sub_groups?(level)
   end
-  def avatar_url(**args)
-    # We use avatar_path instead of overriding avatar_url because of carrierwave.
-    # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864
-    avatar_path(args)
-  end
-
   def lfs_enabled?
     return false unless Gitlab.config.lfs.enabled
     return Gitlab.config.lfs.enabled if self[:lfs_enabled].nil?
@@ -193,12 +183,6 @@ class Group < Namespace
     owners.include?(user) && owners.size == 1
   end
-  def avatar_type
-    unless self.avatar.image?
-      self.errors.add :avatar, "only images allowed"
-    end
-  end
-
   def post_create_hook
     Gitlab::AppLogger.info("Group \"#{name}\" was created")

app/models/lfs_object.rb

@@ -7,7 +7,7 @@ class LfsObject < ActiveRecord::Base
   validates :oid, presence: true, uniqueness: true
-  scope :with_files_stored_locally, ->() { where(file_store: [nil, LfsObjectUploader::LOCAL_STORE]) }
+  scope :with_files_stored_locally, -> { where(file_store: [nil, LfsObjectUploader::Store::LOCAL]) }
   mount_uploader :file, LfsObjectUploader

app/models/note.rb

@@ -88,6 +88,7 @@ class Note < ActiveRecord::Base
     end
   end
+  # @deprecated attachments are handler by the MarkdownUploader
   mount_uploader :attachment, AttachmentUploader
   # Scopes

app/models/project.rb

@@ -255,9 +255,6 @@ class Project < ActiveRecord::Base
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
   validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
-  validate :avatar_type,
-    if: ->(project) { project.avatar.present? && project.avatar_changed? }
-  validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
   validate :visibility_level_allowed_by_group
   validate :visibility_level_allowed_as_fork
   validate :check_wiki_path_conflict
@@ -265,7 +262,6 @@ class Project < ActiveRecord::Base
     presence: true,
     inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } }
-  mount_uploader :avatar, AvatarUploader
   has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   # Scopes
@@ -917,20 +913,12 @@ class Project < ActiveRecord::Base
     issues_tracker.to_param == 'jira'
   end
-  def avatar_type
-    unless self.avatar.image?
-      self.errors.add :avatar, 'only images allowed'
-    end
-  end
-
   def avatar_in_git
     repository.avatar
   end
   def avatar_url(**args)
-    # We use avatar_path instead of overriding avatar_url because of carrierwave.
-    # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864
-    avatar_path(args) || (Gitlab::Routing.url_helpers.project_avatar_url(self) if avatar_in_git)
+    Gitlab::Routing.url_helpers.project_avatar_url(self) if avatar_in_git
   end
   # For compatibility with old code

app/models/upload.rb

@@ -9,44 +9,52 @@ class Upload < ActiveRecord::Base
   validates :model, presence: true
   validates :uploader, presence: true
-  before_save  :calculate_checksum, if:     :foreground_checksum?
-  after_commit :schedule_checksum,  unless: :foreground_checksum?
-  def self.remove_path(path)
-    where(path: path).destroy_all
-  end
-
-  def self.record(uploader)
-    remove_path(uploader.relative_path)
-
-    create(
-      size: uploader.file.size,
-      path: uploader.relative_path,
-      model: uploader.model,
-      uploader: uploader.class.to_s
-    )
+  before_save  :calculate_checksum!, if: :foreground_checksummable?
+  after_commit :schedule_checksum,   if: :checksummable?
+  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
-    return unless exist?
+  def calculate_checksum!
+    self.checksum = nil
+    return unless checksummable?
     self.checksum = Digest::SHA256.file(absolute_path).hexdigest
   end
+  def build_uploader
+    uploader_class.new(model).tap do |uploader|
+      uploader.upload = self
+      uploader.retrieve_from_store!(identifier)
+    end
+  end
+
   def exist?
     File.exist?(absolute_path)
   end
   private
-  def foreground_checksum?
-    size <= CHECKSUM_THRESHOLD
+  def checksummable?
+    checksum.nil? && local? && exist?
+  end
+
+  def local?
+    return true if store.nil?
+
+    store == ObjectStorage::Store::LOCAL
+  end
+
+  def foreground_checksummable?
+    checksummable? && size <= CHECKSUM_THRESHOLD
   end
   def schedule_checksum
@@ -57,6 +65,10 @@ class Upload < ActiveRecord::Base
     !path.start_with?('/')
   end
+  def identifier
+    File.basename(path)
+  end
+
   def uploader_class
     Object.const_get(uploader)
   end

app/models/user.rb

@@ -134,6 +134,7 @@ class User < ActiveRecord::Base
   has_many :assigned_merge_requests,  dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" # rubocop:disable Cop/ActiveRecordDependent
   has_many :custom_attributes, class_name: 'UserCustomAttribute'
+  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   #
   # Validations
@@ -156,12 +157,10 @@ class User < ActiveRecord::Base
   validate :namespace_uniq, if: :username_changed?
   validate :namespace_move_dir_allowed, if: :username_changed?
-  validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
   validate :unique_email, if: :email_changed?
   validate :owns_notification_email, if: :notification_email_changed?
   validate :owns_public_email, if: :public_email_changed?
   validate :signup_domain_valid?, on: :create, if: ->(user) { !user.created_by_id }
-  validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
   before_validation :sanitize_attrs
   before_validation :set_notification_email, if: :email_changed?
@@ -223,9 +222,6 @@ class User < ActiveRecord::Base
     end
   end
-  mount_uploader :avatar, AvatarUploader
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   # Scopes
   scope :admins, -> { where(admin: true) }
   scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
@@ -521,12 +517,6 @@ class User < ActiveRecord::Base
     end
   end
-  def avatar_type
-    unless avatar.image?
-      errors.add :avatar, "only images allowed"
-    end
-  end
-
   def unique_email
     if !emails.exists?(email: email) && Email.exists?(email: email)
       errors.add(:email, 'has already been taken')
@@ -854,9 +844,7 @@ class User < ActiveRecord::Base
   end
   def avatar_url(size: nil, scale: 2, **args)
-    # We use avatar_path instead of overriding avatar_url because of carrierwave.
-    # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864
-    avatar_path(args) || GravatarService.new.execute(email, size, scale, username: username)
+    GravatarService.new.execute(email, size, scale, username: username)
   end
   def primary_email_verified?

app/services/projects/hashed_storage/migrate_attachments_service.rb

@@ -14,9 +14,9 @@ module Projects
         @old_path = project.full_path
         @new_path = project.disk_path
-        origin = FileUploader.dynamic_path_segment(project)
+        origin = FileUploader.absolute_base_dir(project)
         project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments]
-        target = FileUploader.dynamic_path_segment(project)
+        target = FileUploader.absolute_base_dir(project)
         result = move_folder!(origin, target)
         project.save!

app/uploaders/attachment_uploader.rb

 class AttachmentUploader < GitlabUploader
-  include RecordsUploads
+  include RecordsUploads::Concern
+  include ObjectStorage::Concern
+  prepend ObjectStorage::Extension::RecordsUploads
   include UploaderHelper
-  storage :file
-  def store_dir
-    "#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
+  private
+  def dynamic_segment
+    File.join(model.class.to_s.underscore, mounted_as.to_s, model.id.to_s)
   end
 end

app/uploaders/avatar_uploader.rb

 class AvatarUploader < GitlabUploader
-  include RecordsUploads
   include UploaderHelper
-
-  storage :file
-
-  def store_dir
-    "#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
-  end
+  include RecordsUploads::Concern
+  include ObjectStorage::Concern
+  prepend ObjectStorage::Extension::RecordsUploads
   def exists?
     model.avatar.file && model.avatar.file.present?
   end
-  # We set move_to_store and move_to_cache to 'false' to prevent stealing
-  # the avatar file from a project when forking it.
-  # https://gitlab.com/gitlab-org/gitlab-ce/issues/26158
   def move_to_store
     false
   end
@@ -22,4 +15,10 @@ class AvatarUploader < GitlabUploader
   def move_to_cache
     false
   end
+
+  private
+
+  def dynamic_segment
+    File.join(model.class.to_s.underscore, mounted_as.to_s, model.id.to_s)
+  end
 end

app/uploaders/file_mover.rb

@@ -21,13 +21,11 @@ class FileMover
   end
   def update_markdown
-    updated_text = model.read_attribute(update_field).gsub(temp_file_uploader.to_markdown, uploader.to_markdown)
+    updated_text = model.read_attribute(update_field)
+                        .gsub(temp_file_uploader.markdown_link, uploader.markdown_link)
     model.update_attribute(update_field, updated_text)
-
-    true
   rescue
     revert
-
     false
   end

app/uploaders/file_uploader.rb

+# This class breaks the actual CarrierWave concept.
+# Every uploader should use a base_dir that is model agnostic so we can build
+# back URLs from base_dir-relative paths saved in the `Upload` model.
+#
+# As the `.base_dir` is model dependent and **not** saved in the upload model (see #upload_path)
+# there is no way to build back the correct file path without the model, which defies
+# CarrierWave way of storing files.
+#
 class FileUploader < GitlabUploader
-  include RecordsUploads
   include UploaderHelper
+  include RecordsUploads::Concern
+  include ObjectStorage::Concern
+  prepend ObjectStorage::Extension::RecordsUploads
   MARKDOWN_PATTERN = %r{\!?\[.*?\]\(/uploads/(?<secret>[0-9a-f]{32})/(?<file>.*?)\)}
-  storage :file
-  def self.absolute_path(upload_record)
+  DYNAMIC_PATH_PATTERN = %r{(?<secret>\h{32})/(?<identifier>.*)}
+  attr_accessor :model
+
+  def self.root
+    File.join(options.storage_path, 'uploads')
+  end
+  def self.absolute_path(upload)
     File.join(
-      self.dynamic_path_segment(upload_record.model),
-      upload_record.path
+      absolute_base_dir(upload.model),
+      upload.path # already contain the dynamic_segment, see #upload_path
     )
   end
-  # Not using `GitlabUploader.base_dir` because all project namespaces are in
-  # the `public/uploads` dir.
-  #
-  def self.base_dir
-    root_dir
+  def self.base_dir(model)
+    model_path_segment(model)
+  end
+
+  # used in migrations and import/exports
+  def self.absolute_base_dir(model)
+    File.join(root, base_dir(model))
   end
   # Returns the part of `store_dir` that can change based on the model's current
@@ -29,63 +46,94 @@ class FileUploader < GitlabUploader
   # model - Object that responds to `full_path` and `disk_path`
   #
   # Returns a String without a trailing slash
-  def self.dynamic_path_segment(model)
+  def self.model_path_segment(model)
     if model.hashed_storage?(:attachments)
-      dynamic_path_builder(model.disk_path)
+      model.disk_path
     else
-      dynamic_path_builder(model.full_path)
+      model.full_path
     end
   end
-  # Auxiliary method to build dynamic path segment when not using a project model
-  #
-  # Prefer to use the `.dynamic_path_segment` as it includes Hashed Storage specific logic
-  def self.dynamic_path_builder(path)
-    File.join(CarrierWave.root, base_dir, path)
+  def self.upload_path(secret, identifier)
+    File.join(secret, identifier)
   end
-  attr_accessor :model
-  attr_reader :secret
+  def self.generate_secret
+    SecureRandom.hex
+  end
   def initialize(model, secret = nil)
     @model = model
-    @secret = secret || generate_secret
+    @secret = secret
   end
-  def store_dir
-    File.join(dynamic_path_segment, @secret)
+  def base_dir
+    self.class.base_dir(@model)
   end
-  def relative_path
-    self.file.path.sub("#{dynamic_path_segment}/", '')
+  # we don't need to know the actual path, an uploader instance should be
+  # able to yield the file content on demand, so we should build the digest
+  def absolute_path
+    self.class.absolute_path(@upload)
   end
-  def to_markdown
-    to_h[:markdown]
+  def upload_path
+    self.class.upload_path(dynamic_segment, identifier)
   end
-  def to_h
-    filename = image_or_video? ? self.file.basename : self.file.filename
-    escaped_filename = filename.gsub("]", "\\]")
-    markdown = "[#{escaped_filename}](#{secure_url})"
+  def model_path_segment
+    self.class.model_path_segment(@model)
+  end
+  def store_dir
+    File.join(base_dir, dynamic_segment)
+  end
+
+  def markdown_link
+    markdown = "[#{markdown_name}](#{secure_url})"
     markdown.prepend("!") if image_or_video? || dangerous?
+    markdown
+  end
+  def to_h
     {
-      alt:      filename,
+      alt:      markdown_name,
       url:      secure_url,
-      markdown: markdown
+      markdown: markdown_link
     }
   end
+  def filename
+    self.file.filename
+  end
+
+  # the upload does not hold the secret, but holds the path
+  # which contains the secret: extract it
+  def upload=(value)
+    if matches = DYNAMIC_PATH_PATTERN.match(value.path)
+      @secret = matches[:secret]
+      @identifier = matches[:identifier]
+    end
+
+    super
+  end
+
+  def secret
+    @secret ||= self.class.generate_secret
+  end
+
   private
-  def dynamic_path_segment
-    self.class.dynamic_path_segment(model)
+  def markdown_name
+    (image_or_video? ? File.basename(filename, File.extname(filename)) : filename).gsub("]", "\\]")
   end
-  def generate_secret
-    SecureRandom.hex
+  def identifier
+    @identifier ||= filename
+  end
+
+  def dynamic_segment
+    secret
   end
   def secure_url

app/uploaders/gitlab_uploader.rb

 class GitlabUploader < CarrierWave::Uploader::Base
-  def self.absolute_path(upload_record)
-    File.join(CarrierWave.root, upload_record.path)
-  end
-  def self.root_dir
-    'uploads'
-  end
-  # When object storage is used, keep the `root_dir` as `base_dir`.
-  # The files aren't really in folders there, they just have a name.
-  # The files that contain user input in their name, also contain a hash, so
-  # the names are still unique
-  #
-  # This method is overridden in the `FileUploader`
-  def self.base_dir
-    return root_dir unless file_storage?
-    File.join(root_dir, '-', 'system')
-  end
-  def self.file_storage?
-    self.storage == CarrierWave::Storage::File
+  class_attribute :options
+  class << self
+    # DSL setter
+    def storage_options(options)
+      self.options = options
+    end
+    def root
+      options.storage_path
+    end
+    # represent the directory namespacing at the class level
+    def base_dir
+      options.fetch('base_dir', '')
+    end
+    def file_storage?
+      storage == CarrierWave::Storage::File
+    end
+
+    def absolute_path(upload_record)
+      File.join(root, upload_record.path)
+    end
   end
+  storage_options Gitlab.config.uploads
+
   delegate :base_dir, :file_storage?, to: :class
   def file_cache_storage?
     cache_storage.is_a?(CarrierWave::Storage::File)
   end
-  # Reduce disk IO
   def move_to_cache
-    true
+    file_storage?
   end
-  # Reduce disk IO
   def move_to_store
-    true
-  end
-
-  # Designed to be overridden by child uploaders that have a dynamic path
-  # segment -- that is, a path that changes based on mutable attributes of its
-  # associated model
-  #
-  # For example, `FileUploader` builds the storage path based on the associated
-  # project model's `path_with_namespace` value, which can change when the
-  # project or its containing namespace is moved or renamed.
-  def relative_path
-    self.file.path.sub("#{root}/", '')
+    file_storage?
   end
   def exists?
     file.present?
   end
-  # Override this if you don't want to save files by default to the Rails.root directory
+  def cache_dir
+    File.join(root, base_dir, 'tmp/cache')
+  end
+
   def work_dir
-    # Default path set by CarrierWave:
-    # https://github.com/carrierwaveuploader/carrierwave/blob/v1.0.0/lib/carrierwave/uploader/cache.rb#L182
-    CarrierWave.tmp_path
+    File.join(root, base_dir, 'tmp/work')
   end
   def filename
@@ -67,6 +59,17 @@ class GitlabUploader < CarrierWave::Uploader::Base
   private
+  # Designed to be overridden by child uploaders that have a dynamic path
+  # segment -- that is, a path that changes based on mutable attributes of its
+  # associated model
+  #
+  # For example, `FileUploader` builds the storage path based on the associated
+  # project model's `path_with_namespace` value, which can change when the
+  # project or its containing namespace is moved or renamed.
+  def dynamic_segment
+    raise(NotImplementedError)
+  end
+
   # To prevent files from moving across filesystems, override the default
   # implementation:
   # http://github.com/carrierwaveuploader/carrierwave/blob/v1.0.0/lib/carrierwave/uploader/cache.rb#L181-L183
@@ -74,6 +77,6 @@ class GitlabUploader < CarrierWave::Uploader::Base
     # To be safe, keep this directory outside of the the cache directory
     # because calling CarrierWave.clean_cache_files! will remove any files in
     # the cache directory.
-    File.join(work_dir, @cache_id, version_name.to_s, for_file)
+    File.join(work_dir, cache_id, version_name.to_s, for_file)
   end
 end