adapt the models that uses mounted uploaders

`ObjectStorage::Concern` adds delegation to the model that
mount an uploader that uses `RecordsUploads::Concern` to track
its uploads. The basic implementation is to use the uploader
to build paths that can be used to fetch back the related `Upload`
instance so it can be used to build the uploader back (which is
needed to build the correct file URL).

app/models/appearance.rb

@@ -30,4 +30,21 @@ class Appearance < ActiveRecord::Base
       errors.add(:single_appearance_row, 'Only 1 appearances row can exist')
     end
   end
+
+  def logo_upload(uploader)
+    find_upload(uploader, logo_identifier)
+  end
+
+  def header_logo_upload(uploader)
+    find_upload(uploader, header_logo_identifier)
+  end
+
+  private
+
+  def find_upload(uploader, identifier)
+    return unless identifier
+
+    paths = uploader.store_dirs.map { |store, path| File.join(path, identifier) }
+    uploads.where(uploader: uploader.class.to_s, paths: paths)&.last
+  end
 end

app/models/concerns/avatarable.rb

 module Avatarable
   extend ActiveSupport::Concern
 
+  included do
+    class_eval do
+      validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
+      validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
+
+      mount_uploader :avatar, AvatarUploader
+
+      # the AvatarUploader < RecordsUploads::Concern
+      # TODO: rename to avatar_uploads and use a scope
+      has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+    end
+  end
+
+  def avatar_type
+    unless self.avatar.image?
+      self.errors.add :avatar, "only images allowed"
+    end
+  end
+
+  # As the associated `Upload` may have multiple difference paths, we need to find
+  # the last one that fits. There should only be one upload per file anyways.
+  #
+  def avatar_upload(uploader)
+    return unless avatar_identifier
+
+    paths = uploader.store_dirs.map {|store, path| File.join(path, avatar_identifier) }
+    uploads.where(uploader: uploader.class.to_s, path: paths)&.last
+  end
+
   def avatar_path(only_path: true)
     return unless self[:avatar].present?
 

app/models/group.rb

@@ -40,21 +40,15 @@ class Group < Namespace
   # here since Group inherits from Namespace, the entity_type would be set to `Namespace`.
   has_many :audit_events, -> { where(entity_type: Group) }, foreign_key: 'entity_id'
 
-  validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
   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 }
 
   validates :repository_size_limit,
             numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true }
 
-  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
@@ -135,9 +129,7 @@ class Group < Namespace
   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)
+    avatar_path(**args)
   end
 
   def lfs_enabled?
@@ -212,12 +204,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 human_ldap_access
     Gitlab::Access.options_with_owner.key ldap_access
   end

app/models/note.rb

@@ -92,7 +92,6 @@ class Note < ActiveRecord::Base
 
   # @deprecated attachments are handler by the MarkdownUploader
   mount_uploader :attachment, AttachmentUploader
-  deprecate :attachment => 'Use the Markdown uploader instead'
 
   # Scopes
   scope :searchable, -> { where(system: false) }
@@ -206,6 +205,13 @@ class Note < ActiveRecord::Base
     current_application_settings.max_attachment_size.megabytes.to_i
   end
 
+  def attachment_upload(uploader)
+    return unless attachment_identifier
+
+    paths = uploader.store_dirs.map { |store, path| File.join(path, attachment_identifier) }
+    Upload.where(model: self, uploader: uploader.class.to_s, path: paths)&.last
+  end
+
   def hook_attrs
     attributes
   end

app/models/project.rb

@@ -259,9 +259,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
@@ -925,20 +922,6 @@ 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_uploader(uploader)
-    return uploader unless avatar_identifier
-
-    paths = uploader.store_dirs.map {|store, path| File.join(path, avatar_identifier) }
-    uploader.upload = uploads.where(uploader: 'AvatarUploader', path: paths)&.last
-    uploader.object_store = uploader.upload&.store # TODO: move this to RecordsUploads
-  end
-
   def avatar_in_git
     repository.avatar
   end
@@ -946,7 +929,7 @@ class Project < ActiveRecord::Base
   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)
+    avatar_path(**args) || (Gitlab::Routing.url_helpers.project_avatar_url(self) if avatar_in_git)
   end
 
   # For compatibility with old code

app/models/upload.rb

@@ -9,7 +9,7 @@ class Upload < ActiveRecord::Base
   validates :model, presence: true
   validates :uploader, presence: true
 
-  before_save  :calculate_checksum, if:     :foreground_checksum?
+  before_save  :calculate_checksum!, if:     :foreground_checksum?
   after_commit :schedule_checksum,   unless: :foreground_checksum?
 
   def self.remove_path(path)
@@ -19,47 +19,49 @@ class Upload < ActiveRecord::Base
   def self.record(uploader)
     upload = uploader.upload || new
 
-    binding.pry
     upload.update_attributes(
       size: uploader.file.size,
-      path: uploader.dynamic_path,
+      path: uploader.upload_path,
       model: uploader.model,
       uploader: uploader.class.to_s,
       store: uploader.try(:object_store) || ObjectStorage::Store::LOCAL
     )
-  end
-
-  def self.hexdigest(absolute_path)
-    return unless File.exist?(absolute_path)
-
-    Digest::SHA256.file(absolute_path).hexdigest
+    upload
   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
+  def calculate_checksum!
+    self.checksum = nil
+
+    return unless local?
     return unless exist?
 
-    self.checksum = self.class.hexdigest(absolute_path)
+    self.checksum = Digest::SHA256.file(absolute_path).hexdigest
+  end
+
+  def build_uploader(from = nil)
+    (from || uploader_class.new(model)).tap do |uploader|
+      uploader.upload = self
+    end
   end
 
   def exist?
     File.exist?(absolute_path)
   end
 
-  def build_uploader(from = nil)
-    uploader = from || uploader_class.new(model)
+  private
 
-    uploader.upload = self
-    uploader.object_store = store
-    uploader
-  end
+  def local?
+    return true if store.nil?
 
-  #private
+    store == ObjectStorage::Store::LOCAL
+  end
 
   def foreground_checksum?
     size <= CHECKSUM_THRESHOLD

app/models/user.rb

@@ -158,12 +158,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?
@@ -227,9 +225,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) }
@@ -532,12 +527,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')
@@ -870,7 +859,7 @@ class User < ActiveRecord::Base
   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)
+    avatar_path(**args) || GravatarService.new.execute(email, size, scale, username: username)
   end
 
   def primary_email_verified?