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

14 files changed, 282 insertions, 387 deletions

diff --git a/app/uploaders/attachment_uploader.rb b/app/uploaders/attachment_uploader.rb
index 109eb2fea0b..cd819dc9bff 100644
--- a/app/uploaders/attachment_uploader.rb
+++ b/app/uploaders/attachment_uploader.rb
@@ -1,10 +1,12 @@
 class AttachmentUploader < GitlabUploader
-  include RecordsUploads
+  include RecordsUploads::Concern
+  include ObjectStorage::Concern
+  prepend ObjectStorage::Extension::RecordsUploads
   include UploaderHelper
 
-  storage :file
+  private
 
-  def store_dir
-    "#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
+  def dynamic_segment
+    File.join(model.class.to_s.underscore, mounted_as.to_s, model.id.to_s)
   end
 end
diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb
index cbb79376d5f..5848e6c6994 100644
--- a/app/uploaders/avatar_uploader.rb
+++ b/app/uploaders/avatar_uploader.rb
@@ -1,20 +1,13 @@
 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
diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb
index 00c2888d224..f37567d6141 100644
--- a/app/uploaders/file_mover.rb
+++ b/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
 
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 0b591e3bbbb..81952dacce4 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -1,23 +1,40 @@
+# 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>.*?)\)}
+  DYNAMIC_PATH_PATTERN = %r{(?<secret>\h{32})/(?<identifier>.*)}
 
-  storage :file
+  attr_accessor :model
+
+  def self.root
+    File.join(options.storage_path, 'uploads')
+  end
 
-  def self.absolute_path(upload_record)
+  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("]", "\\]")
+  def model_path_segment
+    self.class.model_path_segment(@model)
+  end
 
-    markdown = "[#{escaped_filename}](#{secure_url})"
+  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
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index 7f72b3ce471..ba2ceb0c8cf 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -1,64 +1,56 @@
 class GitlabUploader < CarrierWave::Uploader::Base
-  def self.absolute_path(upload_record)
-    File.join(CarrierWave.root, upload_record.path)
-  end
+  class_attribute :options
 
-  def self.root_dir
-    'uploads'
-  end
+  class << self
+    # DSL setter
+    def storage_options(options)
+      self.options = options
+    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?
+    def root
+      options.storage_path
+    end
 
-    File.join(root_dir, '-', 'system')
-  end
+    # represent the directory namespacing at the class level
+    def base_dir
+      options.fetch('base_dir', '')
+    end
 
-  def self.file_storage?
-    self.storage == CarrierWave::Storage::File
+    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
diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb
index a0757dbe6b2..3ad3e6ea32b 100644
--- a/app/uploaders/job_artifact_uploader.rb
+++ b/app/uploaders/job_artifact_uploader.rb
@@ -1,13 +1,8 @@
-class JobArtifactUploader < ObjectStoreUploader
-  storage_options Gitlab.config.artifacts
-
-  def self.local_store_path
-    Gitlab.config.artifacts.path
-  end
+class JobArtifactUploader < GitlabUploader
+  extend Workhorse::UploadPath
+  include ObjectStorage::Concern
 
-  def self.artifacts_upload_path
-    File.join(self.local_store_path, 'tmp/uploads/')
-  end
+  storage_options Gitlab.config.artifacts
 
   def size
     return super if model.size.nil?
@@ -15,9 +10,13 @@ class JobArtifactUploader < ObjectStoreUploader
     model.size
   end
 
+  def store_dir
+    dynamic_segment
+  end
+
   private
 
-  def default_path
+  def dynamic_segment
     creation_date = model.created_at.utc.strftime('%Y_%m_%d')
 
     File.join(disk_hash[0..1], disk_hash[2..3], disk_hash,
diff --git a/app/uploaders/legacy_artifact_uploader.rb b/app/uploaders/legacy_artifact_uploader.rb
index 476a46c1754..b726b053493 100644
--- a/app/uploaders/legacy_artifact_uploader.rb
+++ b/app/uploaders/legacy_artifact_uploader.rb
@@ -1,17 +1,16 @@
-class LegacyArtifactUploader < ObjectStoreUploader
-  storage_options Gitlab.config.artifacts
+class LegacyArtifactUploader < GitlabUploader
+  extend Workhorse::UploadPath
+  include ObjectStorage::Concern
 
-  def self.local_store_path
-    Gitlab.config.artifacts.path
-  end
+  storage_options Gitlab.config.artifacts
 
-  def self.artifacts_upload_path
-    File.join(self.local_store_path, 'tmp/uploads/')
+  def store_dir
+    dynamic_segment
   end
 
   private
 
-  def default_path
+  def dynamic_segment
     File.join(model.created_at.utc.strftime('%Y_%m'), model.project_id.to_s, model.id.to_s)
   end
 end
diff --git a/app/uploaders/lfs_object_uploader.rb b/app/uploaders/lfs_object_uploader.rb
index fa42e4710b7..e7cce1bbb0a 100644
--- a/app/uploaders/lfs_object_uploader.rb
+++ b/app/uploaders/lfs_object_uploader.rb
@@ -1,17 +1,25 @@
-class LfsObjectUploader < ObjectStoreUploader
-  storage_options Gitlab.config.lfs
+class LfsObjectUploader < GitlabUploader
+  extend Workhorse::UploadPath
+  include ObjectStorage::Concern
 
-  def self.local_store_path
-    Gitlab.config.lfs.storage_path
+  # LfsObject are in `tmp/upload` instead of `tmp/uploads`
+  def self.workhorse_upload_path
+    File.join(root, 'tmp/upload')
   end
 
+  storage_options Gitlab.config.lfs
+
   def filename
     model.oid[4..-1]
   end
 
+  def store_dir
+    dynamic_segment
+  end
+
   private
 
-  def default_path
-    "#{model.oid[0, 2]}/#{model.oid[2, 2]}"
+  def dynamic_segment
+    File.join(model.oid[0, 2], model.oid[2, 2])
   end
 end
diff --git a/app/uploaders/namespace_file_uploader.rb b/app/uploaders/namespace_file_uploader.rb
index 672126e9ec2..269415b1926 100644
--- a/app/uploaders/namespace_file_uploader.rb
+++ b/app/uploaders/namespace_file_uploader.rb
@@ -1,15 +1,26 @@
 class NamespaceFileUploader < FileUploader
-  def self.base_dir
-    File.join(root_dir, '-', 'system', 'namespace')
+  # Re-Override
+  def self.root
+    options.storage_path
   end
 
-  def self.dynamic_path_segment(model)
-    dynamic_path_builder(model.id.to_s)
+  def self.base_dir(model)
+    File.join(options.base_dir, 'namespace', model_path_segment(model))
   end
 
-  private
+  def self.model_path_segment(model)
+    File.join(model.id.to_s)
+  end
+
+  # Re-Override
+  def store_dir
+    store_dirs[object_store]
+  end
 
-  def secure_url
-    File.join('/uploads', @secret, file.filename)
+  def store_dirs
+    {
+      Store::LOCAL => File.join(base_dir, dynamic_segment),
+      Store::REMOTE => File.join('namespace', model_path_segment, dynamic_segment)
+    }
   end
 end
diff --git a/app/uploaders/object_store_uploader.rb b/app/uploaders/object_store_uploader.rb
deleted file mode 100644
index bb25dc4219f..00000000000
--- a/app/uploaders/object_store_uploader.rb
+++ /dev/null
@@ -1,215 +0,0 @@
-require 'fog/aws'
-require 'carrierwave/storage/fog'
-
-class ObjectStoreUploader < GitlabUploader
-  before :store, :set_default_local_store
-  before :store, :verify_license!
-
-  LOCAL_STORE = 1
-  REMOTE_STORE = 2
-
-  class << self
-    def storage_options(options)
-      @storage_options = options
-    end
-
-    def object_store_options
-      @storage_options&.object_store
-    end
-
-    def object_store_enabled?
-      object_store_options&.enabled
-    end
-
-    def background_upload_enabled?
-      object_store_options&.background_upload
-    end
-
-    def object_store_credentials
-      @object_store_credentials ||= object_store_options&.connection&.to_hash&.deep_symbolize_keys
-    end
-
-    def object_store_directory
-      object_store_options&.remote_directory
-    end
-
-    def local_store_path
-      raise NotImplementedError
-    end
-  end
-
-  def file_storage?
-    storage.is_a?(CarrierWave::Storage::File)
-  end
-
-  def file_cache_storage?
-    cache_storage.is_a?(CarrierWave::Storage::File)
-  end
-
-  def real_object_store
-    model.public_send(store_serialization_column) # rubocop:disable GitlabSecurity/PublicSend
-  end
-
-  def object_store
-    subject.public_send(:"#{field}_store")
-  end
-
-  def object_store=(value)
-    @storage = nil
-    model.public_send(:"#{store_serialization_column}=", value) # rubocop:disable GitlabSecurity/PublicSend
-  end
-
-  def store_dir
-    if file_storage?
-      default_local_path
-    else
-      default_path
-    end
-  end
-
-  def use_file
-    if file_storage?
-      return yield path
-    end
-
-    begin
-      cache_stored_file!
-      yield cache_path
-    ensure
-      cache_storage.delete_dir!(cache_path(nil))
-    end
-  end
-
-  def filename
-    super || file&.filename
-  end
-
-  def migrate!(new_store)
-    raise 'Undefined new store' unless new_store
-
-    return unless object_store != new_store
-    return unless file
-
-    old_file = file
-    old_store = object_store
-
-    # for moving remote file we need to first store it locally
-    cache_stored_file! unless file_storage?
-
-    # change storage
-    self.object_store = new_store
-
-    with_callbacks(:store, file) do
-      storage.store!(file).tap do |new_file|
-        # since we change storage store the new storage
-        # in case of failure delete new file
-        begin
-          model.save!
-        rescue => e
-          new_file.delete
-          self.object_store = old_store
-          raise e
-        end
-
-        old_file.delete
-      end
-    end
-  end
-
-  def schedule_migration_to_object_storage(*args)
-    return unless self.class.object_store_enabled?
-    return unless self.class.background_upload_enabled?
-    return unless self.licensed?
-    return unless self.file_storage?
-
-    ObjectStorageUploadWorker.perform_async(self.class.name, model.class.name, mounted_as, model.id)
-  end
-
-  def fog_directory
-    self.class.object_store_options.remote_directory
-  end
-
-  def fog_credentials
-    self.class.object_store_options.connection
-  end
-
-  def fog_public
-    false
-  end
-
-  def move_to_store
-    file.try(:storage) == storage
-  end
-
-  def move_to_cache
-    file.try(:storage) == cache_storage
-  end
-
-  # We block storing artifacts on Object Storage, not receiving
-  def verify_license!(new_file)
-    return if file_storage?
-
-    raise 'Object Storage feature is missing' unless licensed?
-  end
-
-  def exists?
-    file.try(:exists?)
-  end
-
-  def cache_dir
-    File.join(self.class.local_store_path, 'tmp/cache')
-  end
-
-  # Override this if you don't want to save local files by default to the Rails.root directory
-  def work_dir
-    # Default path set by CarrierWave:
-    # https://github.com/carrierwaveuploader/carrierwave/blob/v1.1.0/lib/carrierwave/uploader/cache.rb#L182
-    # CarrierWave.tmp_path
-    File.join(self.class.local_store_path, 'tmp/work')
-  end
-
-  def licensed?
-    License.feature_available?(:object_storage)
-  end
-
-  private
-
-  def set_default_local_store(new_file)
-    self.object_store = LOCAL_STORE unless self.object_store
-  end
-
-  def default_local_path
-    File.join(self.class.local_store_path, default_path)
-  end
-
-  def default_path
-    raise NotImplementedError
-  end
-
-  def serialization_column
-    model.class.uploader_option(mounted_as, :mount_on) || mounted_as
-  end
-
-  def store_serialization_column
-    :"#{serialization_column}_store"
-  end
-
-  def storage
-    @storage ||=
-      if object_store == REMOTE_STORE
-        remote_storage
-      else
-        local_storage
-      end
-  end
-
-  def remote_storage
-    raise 'Object Storage is not enabled' unless self.class.object_store_enabled?
-
-    CarrierWave::Storage::Fog.new(self)
-  end
-
-  def local_storage
-    CarrierWave::Storage::File.new(self)
-  end
-end
diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb
index 3298ad104ec..440972affec 100644
--- a/app/uploaders/personal_file_uploader.rb
+++ b/app/uploaders/personal_file_uploader.rb
@@ -1,23 +1,40 @@
 class PersonalFileUploader < FileUploader
-  def self.dynamic_path_segment(model)
-    File.join(CarrierWave.root, model_path(model))
+  # Re-Override
+  def self.root
+    options.storage_path
   end
 
-  def self.base_dir
-    File.join(root_dir, '-', 'system')
+  def self.base_dir(model)
+    File.join(options.base_dir, model_path_segment(model))
   end
 
-  private
+  def self.model_path_segment(model)
+    return 'temp/' unless model
 
-  def secure_url
-    File.join(self.class.model_path(model), secret, file.filename)
+    File.join(model.class.to_s.underscore, model.id.to_s)
+  end
+
+  def object_store
+    return Store::LOCAL unless model
+
+    super
+  end
+
+  # Revert-Override
+  def store_dir
+    store_dirs[object_store]
+  end
+
+  def store_dirs
+    {
+      Store::LOCAL => File.join(base_dir, dynamic_segment),
+      Store::REMOTE => File.join(model_path_segment, dynamic_segment)
+    }
   end
 
-  def self.model_path(model)
-    if model
-      File.join("/#{base_dir}", model.class.to_s.underscore, model.id.to_s)
-    else
-      File.join("/#{base_dir}", 'temp')
-    end
+  private
+
+  def secure_url
+    File.join('/', base_dir, secret, file.filename)
   end
 end
diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb
index feb4f04d7b7..dfb8dccec57 100644
--- a/app/uploaders/records_uploads.rb
+++ b/app/uploaders/records_uploads.rb
@@ -1,35 +1,61 @@
 module RecordsUploads
-  extend ActiveSupport::Concern
+  module Concern
+    extend ActiveSupport::Concern
 
-  included do
-    after :store,   :record_upload
-    before :remove, :destroy_upload
-  end
+    attr_accessor :upload
 
-  # After storing an attachment, create a corresponding Upload record
-  #
-  # NOTE: We're ignoring the argument passed to this callback because we want
-  # the `SanitizedFile` object from `CarrierWave::Uploader::Base#file`, not the
-  # `Tempfile` object the callback gets.
-  #
-  # Called `after :store`
-  def record_upload(_tempfile = nil)
-    return unless model
-    return unless file_storage?
-    return unless file.exists?
-
-    Upload.record(self)
-  end
+    included do
+      after  :store,  :record_upload
+      before :remove, :destroy_upload
+    end
+
+    # After storing an attachment, create a corresponding Upload record
+    #
+    # NOTE: We're ignoring the argument passed to this callback because we want
+    # the `SanitizedFile` object from `CarrierWave::Uploader::Base#file`, not the
+    # `Tempfile` object the callback gets.
+    #
+    # Called `after :store`
+    def record_upload(_tempfile = nil)
+      return unless model
+      return unless file && file.exists?
+
+      Upload.transaction do
+        uploads.where(path: upload_path).delete_all
+        upload.destroy! if upload
+
+        self.upload = build_upload_from_uploader(self)
+        upload.save!
+      end
+    end
+
+    def upload_path
+      File.join(store_dir, filename.to_s)
+    end
+
+    private
+
+    def uploads
+      Upload.order(id: :desc).where(uploader: self.class.to_s)
+    end
 
-  private
+    def build_upload_from_uploader(uploader)
+      Upload.new(
+        size: uploader.file.size,
+        path: uploader.upload_path,
+        model: uploader.model,
+        uploader: uploader.class.to_s
+      )
+    end
 
-  # Before removing an attachment, destroy any Upload records at the same path
-  #
-  # Called `before :remove`
-  def destroy_upload(*args)
-    return unless file_storage?
-    return unless file
+    # Before removing an attachment, destroy any Upload records at the same path
+    #
+    # Called `before :remove`
+    def destroy_upload(*args)
+      return unless file && file.exists?
 
-    Upload.remove_path(relative_path)
+      self.upload = nil
+      uploads.where(path: upload_path).delete_all
+    end
   end
 end
diff --git a/app/uploaders/uploader_helper.rb b/app/uploaders/uploader_helper.rb
index 7635c20ab3a..fd446d31092 100644
--- a/app/uploaders/uploader_helper.rb
+++ b/app/uploaders/uploader_helper.rb
@@ -32,14 +32,7 @@ module UploaderHelper
   def extension_match?(extensions)
     return false unless file
 
-    extension =
-      if file.respond_to?(:extension)
-        file.extension
-      else
-        # Not all CarrierWave storages respond to :extension
-        File.extname(file.path).delete('.')
-      end
-
+    extension = file.try(:extension) || File.extname(file.path).delete('.')
     extensions.include?(extension.downcase)
   end
 end
diff --git a/app/uploaders/workhorse.rb b/app/uploaders/workhorse.rb
new file mode 100644
index 00000000000..782032cf516
--- /dev/null
+++ b/app/uploaders/workhorse.rb
@@ -0,0 +1,7 @@
+module Workhorse
+  module UploadPath
+    def workhorse_upload_path
+      File.join(root, base_dir, 'tmp/uploads')
+    end
+  end
+end