5 files changed, 43 insertions, 7 deletions
diff --git a/app/models/appearance.rb b/app/models/appearance.rb
index fb66dd0b766..f8713138a93 100644
--- a/app/models/appearance.rb
+++ b/app/models/appearance.rb
@@ -2,6 +2,7 @@ class Appearance < ActiveRecord::Base
   include CacheMarkdownField
   include AfterCommitQueue
   include ObjectStorage::BackgroundMove
+  include WithUploads
 
   cache_markdown_field :description
   cache_markdown_field :new_project_guidelines
@@ -14,8 +15,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:#{Gitlab::VERSION}".freeze
 
   after_commit :flush_redis_cache
diff --git a/app/models/concerns/with_uploads.rb b/app/models/concerns/with_uploads.rb
new file mode 100644
index 00000000000..e7cfffb775b
--- /dev/null
+++ b/app/models/concerns/with_uploads.rb
@@ -0,0 +1,39 @@
+# Mounted uploaders are destroyed by carrierwave's after_commit
+# hook. This hook fetches upload location (local vs remote) from
+# Upload model. So it's neccessary to make sure that during that
+# after_commit hook model's associated uploads are not deleted yet.
+# IOW we can not use dependent: :destroy :
+# has_many :uploads, as: :model, dependent: :destroy
+#
+# And because not-mounted uploads require presence of upload's
+# object model when destroying them (FileUploader's `build_upload` method
+# references `model` on delete), we can not use after_commit hook for these
+# uploads.
+#
+# Instead FileUploads are destroyed in before_destroy hook and remaining uploads
+# are destroyed by the carrierwave's after_commit hook.
+
+module WithUploads
+  extend ActiveSupport::Concern
+
+  # Currently there is no simple way how to select only not-mounted
+  # uploads, it should be all FileUploaders so we select them by
+  # `uploader` class
+  FILE_UPLOADERS = %w(PersonalFileUploader NamespaceFileUploader FileUploader).freeze
+
+  included do
+    has_many :uploads, as: :model
+
+    before_destroy :destroy_file_uploads
+  end
+
+  # mounted uploads are deleted in carrierwave's after_commit hook,
+  # but FileUploaders which are not mounted must be deleted explicitly and
+  # it can not be done in after_commit because FileUploader requires loads
+  # associated model on destroy (which is already deleted in after_commit)
+  def destroy_file_uploads
+    self.uploads.where(uploader: FILE_UPLOADERS).find_each do |upload|
+      upload.destroy
+    end
+  end
+end
diff --git a/app/models/group.rb b/app/models/group.rb
index cefca316399..8fb77a7869d 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -10,6 +10,7 @@ class Group < Namespace
   include LoadedInGroupList
   include GroupDescendant
   include TokenAuthenticatable
+  include WithUploads
 
   has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
   alias_method :members, :group_members
@@ -30,8 +31,6 @@ 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
-
   has_many :boards
   has_many :badges, class_name: 'GroupBadge'
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 534a0e630af..0975e64e995 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -23,6 +23,7 @@ class Project < ActiveRecord::Base
   include ::Gitlab::Utils::StrongMemoize
   include ChronicDurationAttribute
   include FastDestroyAll::Helpers
+  include WithUploads
 
   extend Gitlab::ConfigHelper
 
@@ -301,8 +302,6 @@ class Project < ActiveRecord::Base
     inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } }
   validates :variables, variable_duplicates: { scope: :environment_scope }
 
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-
   # Scopes
   scope :pending_delete, -> { where(pending_delete: true) }
   scope :without_deleted, -> { where(pending_delete: false) }
diff --git a/app/models/user.rb b/app/models/user.rb
index 226a4489261..474fde36c02 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -17,6 +17,7 @@ class User < ActiveRecord::Base
   include IgnorableColumn
   include BulkMemberAccessLoad
   include BlocksJsonSerialization
+  include WithUploads
 
   DEFAULT_NOTIFICATION_LEVEL = :participating
 
@@ -137,7 +138,6 @@ class User < ActiveRecord::Base
 
   has_many :custom_attributes, class_name: 'UserCustomAttribute'
   has_many :callouts, class_name: 'UserCallout'
-  has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :term_agreements
   belongs_to :accepted_term, class_name: 'ApplicationSetting::Term'