remove file after `Upload#destroy`

it will also automatically prune empty directories for
`FileUploader`-based uploaders.

3 files changed, 37 insertions, 0 deletions

diff --git a/app/models/upload.rb b/app/models/upload.rb
index 2024228537a..1bbcc4e381d 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -12,6 +12,10 @@ class Upload < ActiveRecord::Base
   before_save  :calculate_checksum!, if: :foreground_checksummable?
   after_commit :schedule_checksum,   if: :checksummable?
 
+  # as the FileUploader is not mounted, the default CarrierWave ActiveRecord
+  # hooks are not executed and the file will not be deleted
+  after_commit :delete_file!, on: :destroy, if: -> { uploader_class <= FileUploader }
+
   def self.hexdigest(path)
     Digest::SHA256.file(path).hexdigest
   end
@@ -49,6 +53,10 @@ class Upload < ActiveRecord::Base
 
   private
 
+  def delete_file!
+    build_uploader.remove!
+  end
+
   def checksummable?
     checksum.nil? && local? && exist?
   end
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 2310e67cb2e..bde1161dfa8 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -15,6 +15,8 @@ class FileUploader < GitlabUploader
 
   storage :file
 
+  after :remove, :prune_store_dir
+
   def self.root
     File.join(options.storage_path, 'uploads')
   end
@@ -140,6 +142,10 @@ class FileUploader < GitlabUploader
     end
   end
 
+  def prune_store_dir
+    storage.delete_dir!(store_dir) # only remove when empty
+  end
+
   def markdown_name
     (image_or_video? ? File.basename(filename, File.extname(filename)) : filename).gsub("]", "\\]")
   end
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index a559681a079..80af2a206af 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -48,6 +48,29 @@ describe FileUploader do
     end
   end
 
+  describe 'callbacks' do
+    describe '#prune_store_dir after :remove' do
+      before do
+        uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt'))
+      end
+
+      def store_dir
+        File.expand_path(uploader.store_dir, uploader.root)
+      end
+
+      it 'is called' do
+        expect(uploader).to receive(:prune_store_dir).once
+
+        uploader.remove!
+      end
+
+      it 'prune the store directory' do
+        expect { uploader.remove! }
+          .to change { File.exists?(store_dir) }.from(true).to(false)
+      end
+    end
+  end
+
   describe '#secret' do
     it 'generates a secret if none is provided' do
       expect(described_class).to receive(:generate_secret).and_return('secret')