Delete intermediary tempfiles and tempfiles for styles

lib/paperclip/attachment.rb

@@ -314,7 +314,7 @@ module Paperclip
     # thumbnails forcefully, by reobtaining the original file and going through
     # the post-process again.
     def reprocess!
-      new_original = Tempfile.new("paperclip-reprocess-#{instance.id}-")
+      new_original = Tempfile.new("paperclip-reprocess-#{instance.class.table_name}-#{instance.id}-")
       new_original.binmode
       old_original = to_file(:original)
       new_original.write( old_original.read )
@@ -431,7 +431,11 @@ module Paperclip
         begin
           raise RuntimeError.new("Style #{name} has no processors defined.") if args[:processors].blank?
           queued_for_write[name] = args[:processors].inject(queued_for_write[:original]) do |file, processor|
-            Paperclip.processor(processor).make(file, args, self)
+            Paperclip.processor(processor).make(file, args, self).tap do |new_file|
+              # closing intermediary tempfiles
+              file.close! if new_file != file && file.is_a?(Tempfile) &&
+                             (name == :original || !queued_for_write.value?(file))
+            end
           end
         rescue PaperclipError => e
           log("An error was received while processing: #{e.inspect}")

lib/paperclip/optimizer.rb

@@ -3,12 +3,10 @@ require 'open3'
 module Paperclip
   class Optimizer < Processor
     def make
-      optimized_file_path  = optimize(@file)
+      optimized_file = optimize(@file)
-      if optimized_file_path && File.exist?(optimized_file_path)
+      return @file unless optimized_file && optimized_file.size > 0 # rubocop:disable Style/ZeroLengthPredicate
-        return File.open(optimized_file_path)
+
-      else
+      optimized_file
-        return @file
-      end
     end
 
     def real_content_type
@@ -18,9 +16,9 @@ module Paperclip
     def optimize(_file)
       # TODO: use the arg?
       src = @file.path
-      dst = "#{src}-#{SecureRandom.hex}"
+      dst_file = Tempfile.new(["#{File.basename(src)}-optim", File.extname(src)])
       src_shell = src.shellescape
-      dst_shell = dst.shellescape
+      dst_shell = dst_file.path.shellescape
       cmd = case real_content_type
             when 'image/jpeg', 'image/jpg', 'image/pjpeg'
               "cp #{src_shell} #{dst_shell} && jpegoptim --all-progressive -q --strip-com --strip-exif --strip-iptc -- #{dst_shell}"
@@ -32,7 +30,11 @@ module Paperclip
               return
             end
       run_and_verify!(cmd)
-      dst
+      dst_file
+    rescue StandardError => e
+      dst_file.close!
+      Paperclip.log("Error: cannot optimize: #{e}")
+      nil
     end
 
     private

lib/paperclip/recursive_thumbnail.rb

@@ -5,8 +5,12 @@ module Paperclip
       # если по каким-то причинам не сформировался файл
       # для прыдущего размера не кидаем ексепшен и
       # генерим файл из оригинального
-      f = attachment.to_file(options[:thumbnail] || :original) rescue file
+      source_style = options[:thumbnail] || :original
-      super f, options, attachment
+      # TODO: вообще queued_for_write[source_style] место в NoCacheS3#to_file
+      f = attachment&.queued_for_write&.dig(source_style)&.tap(&:rewind)
+      # TODO: и надо сносить файл если все же была загрузка
+      f ||= attachment.to_file(source_style) rescue file # rubocop:disable Style/RescueModifier
+      super(f, options, attachment)
     end
   end
 end

lib/paperclip/storage/no_cache_s3.rb

@@ -142,9 +142,8 @@ module Paperclip
           (self.class.store_ids - [self.class.main_store_id]).each { |store_id| enqueue_sync_job(store_id) }
         end
         # HACK: Iostream пишет в tempfile, и он нигде не закрывается. Будем закрывать хотя бы тут
-        if queued_for_write[:original].is_a?(Tempfile)
+        queued_for_write.each_value do |file|
-          queued_for_write[:original].close
+          file.is_a?(Tempfile) && file.close!
-          queued_for_write[:original].unlink
         end
         queued_for_write.clear
       end

lib/paperclip/thumbnail.rb

@@ -46,7 +46,7 @@ module Paperclip
     def make
       src = @file
       ext = @format.present? ? ".#{@format}" : nil
-      dst = Tempfile.new([@basename, ext])
+      dst = Tempfile.new(["#{@basename}-thumb-", ext])
       dst.binmode
 
       command = <<-end_command

test/storage/no_cache_s3_test.rb

@@ -4,6 +4,7 @@ require 'test_helper'
 require 'sidekiq'
 require 'sidekiq/testing'
 require 'aws-sdk-s3'
+require 'base64'
 
 require 'delayed_paperclip'
 DelayedPaperclip::Railtie.insert
@@ -29,6 +30,12 @@ class NoCacheS3Test < Test::Unit::TestCase
       stores: {
         store_1: { access_key_id: '123', secret_access_key: '123', region: 'r', bucket: 'buck' },
         store_2: { access_key_id: '456', secret_access_key: '456', region: 'r', bucket: 'buck' }
+      },
+      styles: {
+        original: { geometry: '4x4>', processors: %i[thumbnail optimizer] },
+        medium: '3x3',
+        small: { geometry: '2x2', processors: [:recursive_thumbnail], thumbnail: :medium },
+        micro: { geometry: '1x1', processors: [:recursive_thumbnail], thumbnail: :small }
       }
     )
     modify_table(:dummies) do |table|
@@ -36,12 +43,14 @@ class NoCacheS3Test < Test::Unit::TestCase
       table.boolean :avatar_synced_to_store_2, null: false, default: false
     end
     @instance = Dummy.create
-    @store1_stub = mock
+    @store1_stub = mock("store1")
-    @store2_stub = mock
+    @store2_stub = mock("store2")
     @store1_stub.stubs(:url).returns('http://store.local')
     @store2_stub.stubs(:url).returns('http://store.local')
     @instance.avatar.class.stubs(:stores).returns({ store_1: @store1_stub, store_2: @store2_stub })
-    Dummy::AvatarAttachment.any_instance.stubs(:to_file).returns(stub_file('text.txt', 'qwe'))
+    Dummy::AvatarAttachment.any_instance.stubs(:to_file).returns(
+      stub_file('pixel.gif', Base64.decode64('R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw'))
+    )
   end
 
   teardown { TEST_ROOT.rmtree if TEST_ROOT.exist? }
@@ -83,6 +92,22 @@ class NoCacheS3Test < Test::Unit::TestCase
     end
   end
 
+  context "reprocess" do
+    setup do
+      Sidekiq::Testing.fake!
+      @instance.update_columns avatar_file_name: 'foo.gif', avatar_content_type: 'image/gif'
+    end
+
+    should "delete tmp files" do
+      @store1_stub.expects(:put_object).times(1 + (@instance.avatar.options[:styles].keys - [:original]).size)
+      # Paperclip.expects(:log).with { puts "Log: #{_1}"; true }.at_least(3)
+      existing_files = Dir.children(Dir.tmpdir)
+      @instance.avatar.reprocess!
+      leftover_files = (Dir.children(Dir.tmpdir) - existing_files).sort
+      assert_empty(leftover_files)
+    end
+  end
+
   context 'generating presigned_url' do
     setup do
       Dummy::AvatarAttachment.any_instance.stubs(:storage_url).returns('http://домен.pф/ключ?param1=параметр')