Revert "Merge branch '3867-port-to-ce' into 'master'"

This reverts commit 54a575f1, reversing
changes made to c63af942.

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
 
@@ -19,71 +17,34 @@ def create
     end
   end
 
-  # This should either
-  #   - send the file directly
-  #   - or redirect to its URL
-  #
   def show
     return render_404 unless uploader.exists?
 
-    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
+    disposition = uploader.image_or_video? ? 'inline' : 'attachment'
 
-  def uploader_class
-    raise NotImplementedError
-  end
+    expires_in 0.seconds, must_revalidate: true, private: true
 
-  def upload_mount
-    mounted_as = params[:mounted_as]
-    mounted_as if UPLOAD_MOUNTS.include?(mounted_as)
+    send_file uploader.file.path, disposition: disposition
   end
 
-  def uploader_mounted?
-    upload_model_class < CarrierWave::Mount::Extension && !upload_mount.nil?
-  end
+  private
 
   def uploader
     strong_memoize(:uploader) do
-      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]
+      return if show_model.nil?
 
-    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
+      file_uploader = FileUploader.new(show_model, params[:secret])
+      file_uploader.retrieve_from_store!(params[:filename])
 
-  def build_uploader_from_params
-    uploader = uploader_class.new(model, params[:secret])
-    uploader.retrieve_from_store!(params[:filename])
-    uploader
+      file_uploader
+    end
   end
 
   def image_or_video?
     uploader && uploader.exists? && uploader.image_or_video?
   end
 
-  def find_model
-    nil
-  end
-
-  def model
-    strong_memoize(:model) { find_model }
+  def uploader_class
+    FileUploader
   end
 end

app/controllers/groups/uploads_controller.rb

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

app/controllers/projects/lfs_storage_controller.rb

@@ -60,7 +60,7 @@ def tmp_filename
 
   def store_file(oid, size, tmp_file)
     # Define tmp_file_path early because we use it in "ensure"
-    tmp_file_path = File.join(LfsObjectUploader.workhorse_upload_path, tmp_file)
+    tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", 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? }
 
@@ -9,20 +8,14 @@ class Projects::UploadsController < Projects::ApplicationController
 
   private
 
-  def upload_model_class
-    Project
-  end
+  def show_model
+    strong_memoize(:show_model) do
+      namespace = params[:namespace_id]
+      id = params[:project_id]
 
-  def uploader_class
-    FileUploader
+      Project.find_by_full_path("#{namespace}/#{id}")
+    end
   end
 
-  def find_model
-    return @project if @project
-
-    namespace = params[:namespace_id]
-    id = params[:project_id]
-
-    Project.find_by_full_path("#{namespace}/#{id}")
-  end
+  alias_method :model, :project
 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]
 
-  def uploader_class
-    PersonalFileUploader
-  end
+  private
 
   def find_model
     return nil unless params[:id]
 
-    upload_model_class.find(params[:id])
+    return render_404 unless upload_model && upload_mount
+
+    @model = upload_model.find(params[:id])
   end
 
   def authorize_access!
@@ -68,17 +53,55 @@ def render_unauthorized
     end
   end
 
-  def upload_model_class
-    MODEL_CLASSES[params[:model]] || raise(UnknownUploadModelError)
+  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
   end
 
-  def upload_model_class_has_mounts?
-    upload_model_class < CarrierWave::Mount::Extension
+  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
   end
 
-  def upload_mount_satisfied?
-    return true unless upload_model_class_has_mounts?
+  def uploader_class
+    PersonalFileUploader
+  end
 
-    upload_model_class.uploader_options.has_key?(upload_mount)
+  def model
+    @model ||= find_model
   end
 end

app/models/appearance.rb

@@ -11,7 +11,6 @@ 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/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,14 +29,18 @@ class Group < Namespace
   has_many :variables, class_name: 'Ci::GroupVariable'
   has_many :custom_attributes, class_name: 'GroupCustomAttribute'
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
+  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 }
 
+  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
@@ -112,6 +116,12 @@ def visibility_level_allowed?(level = self.visibility_level)
       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?

app/models/note.rb

@@ -88,7 +88,6 @@ def values
     end
   end
 
-  # @deprecated attachments are handler by the MarkdownUploader
   mount_uploader :attachment, AttachmentUploader
 
   # Scopes

app/models/project.rb

@@ -256,6 +256,9 @@ 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
@@ -263,6 +266,7 @@ 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
@@ -285,6 +289,7 @@ class Project < ActiveRecord::Base
   scope :non_archived, -> { where(archived: false) }
   scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct }
   scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) }
+
   scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }
   scope :with_statistics, -> { includes(:statistics) }
   scope :with_shared_runners, -> { where(shared_runners_enabled: true) }
@@ -918,12 +923,20 @@ def jira_tracker?
     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)
-    Gitlab::Routing.url_helpers.project_avatar_url(self) if avatar_in_git
+    # 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)
   end
 
   # For compatibility with old code

app/models/upload.rb

@@ -9,11 +9,22 @@ class Upload < ActiveRecord::Base
   validates :model, presence: true
   validates :uploader, presence: true
 
-  before_save  :calculate_checksum!, if: :foreground_checksummable?
-  after_commit :schedule_checksum,   if: :checksummable?
+  before_save  :calculate_checksum, if:     :foreground_checksum?
+  after_commit :schedule_checksum,  unless: :foreground_checksum?
 
-  def self.hexdigest(path)
-    Digest::SHA256.file(path).hexdigest
+  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
+    )
   end
 
   def absolute_path
@@ -22,18 +33,10 @@ def absolute_path
     uploader_class.absolute_path(self)
   end
 
-  def calculate_checksum!
-    self.checksum = nil
-    return unless checksummable?
+  def calculate_checksum
+    return unless exist?
 
-    self.checksum = self.class.hexdigest(absolute_path)
-  end
-
-  def build_uploader
-    uploader_class.new(model).tap do |uploader|
-      uploader.upload = self
-      uploader.retrieve_from_store!(identifier)
-    end
+    self.checksum = Digest::SHA256.file(absolute_path).hexdigest
   end
 
   def exist?
@@ -42,16 +45,8 @@ def exist?
 
   private
 
-  def checksummable?
-    checksum.nil? && local? && exist?
-  end
-
-  def local?
-    true
-  end
-
-  def foreground_checksummable?
-    checksummable? && size <= CHECKSUM_THRESHOLD
+  def foreground_checksum?
+    size <= CHECKSUM_THRESHOLD
   end
 
   def schedule_checksum
@@ -62,10 +57,6 @@ def relative_path?
     !path.start_with?('/')
   end
 
-  def identifier
-    File.basename(path)
-  end
-
   def uploader_class
     Object.const_get(uploader)
   end

app/models/user.rb

@@ -137,7 +137,6 @@ def update_tracked_fields!(request)
   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
@@ -160,10 +159,12 @@ def update_tracked_fields!(request)
   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?
@@ -224,6 +225,9 @@ def inactive_message
     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) }
@@ -523,6 +527,12 @@ def namespace_move_dir_allowed
     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')
@@ -850,7 +860,9 @@ def temp_oauth_email?
   end
 
   def avatar_url(size: nil, scale: 2, **args)
-    GravatarService.new.execute(email, size, scale, username: username)
+    # 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)
   end
 
   def primary_email_verified?

app/services/projects/hashed_storage/migrate_attachments_service.rb

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

app/uploaders/attachment_uploader.rb

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

app/uploaders/avatar_uploader.rb

 class AvatarUploader < GitlabUploader
+  include RecordsUploads
   include UploaderHelper
-  include RecordsUploads::Concern
 
   storage :file
 
-  def exists?
-    model.avatar.file && model.avatar.file.present?
+  def store_dir
+    "#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
   end
 
-  def move_to_cache
-    false
+  def exists?
+    model.avatar.file && model.avatar.file.present?