5 files changed, 41 insertions, 7 deletions

diff --git a/app/models/appearance.rb b/app/models/appearance.rb
index fb66dd0b766..f3cfc8ccd1e 100644
--- a/app/models/appearance.rb
+++ b/app/models/appearance.rb
@@ -1,4 +1,5 @@
 class Appearance < ActiveRecord::Base
+  include WithUploads
   include CacheMarkdownField
   include AfterCommitQueue
   include ObjectStorage::BackgroundMove
@@ -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..101d09f161d
--- /dev/null
+++ b/app/models/concerns/with_uploads.rb
@@ -0,0 +1,37 @@
+# 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 denepdent => :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).destroy_all
+  end
+end
diff --git a/app/models/group.rb b/app/models/group.rb
index cefca316399..107711e3cc5 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -2,6 +2,7 @@ require 'carrierwave/orm/activerecord'
 
 class Group < Namespace
   include Gitlab::ConfigHelper
+  include WithUploads
   include AfterCommitQueue
   include AccessRequestable
   include Avatarable
@@ -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..0b0d653c4af 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -4,6 +4,7 @@ class Project < ActiveRecord::Base
   include Gitlab::ConfigHelper
   include Gitlab::ShellAdapter
   include Gitlab::VisibilityLevel
+  include WithUploads
   include AccessRequestable
   include Avatarable
   include CacheMarkdownField
@@ -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 173ab38e20c..082ec76e86a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -3,6 +3,7 @@ require 'carrierwave/orm/activerecord'
 class User < ActiveRecord::Base
   extend Gitlab::ConfigHelper
 
+  include WithUploads
   include Gitlab::ConfigHelper
   include Gitlab::SQL::Pattern
   include AfterCommitQueue
@@ -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'