From d0354efcc8e5e28ac7a6bb4c7eb798c61d4a6d1a Mon Sep 17 00:00:00 2001 From: Vasily Fedoseyev Date: Fri, 5 Apr 2024 00:25:25 +0300 Subject: [PATCH] Delete intermediary tempfiles and tempfiles for styles --- lib/paperclip/attachment.rb | 8 +++++-- lib/paperclip/optimizer.rb | 20 ++++++++++-------- lib/paperclip/recursive_thumbnail.rb | 8 +++++-- lib/paperclip/storage/no_cache_s3.rb | 5 ++--- lib/paperclip/thumbnail.rb | 2 +- test/storage/no_cache_s3_test.rb | 31 +++++++++++++++++++++++++--- 6 files changed, 54 insertions(+), 20 deletions(-) diff --git a/lib/paperclip/attachment.rb b/lib/paperclip/attachment.rb index 5c2233a..3366b7d 100644 --- a/lib/paperclip/attachment.rb +++ b/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}") diff --git a/lib/paperclip/optimizer.rb b/lib/paperclip/optimizer.rb index ccd3316..0708f1c 100644 --- a/lib/paperclip/optimizer.rb +++ b/lib/paperclip/optimizer.rb @@ -3,12 +3,10 @@ require 'open3' module Paperclip class Optimizer < Processor def make - optimized_file_path = optimize(@file) - if optimized_file_path && File.exist?(optimized_file_path) - return File.open(optimized_file_path) - else - return @file - end + optimized_file = optimize(@file) + return @file unless optimized_file && optimized_file.size > 0 # rubocop:disable Style/ZeroLengthPredicate + + optimized_file 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 diff --git a/lib/paperclip/recursive_thumbnail.rb b/lib/paperclip/recursive_thumbnail.rb index 517553f..40bc3f5 100644 --- a/lib/paperclip/recursive_thumbnail.rb +++ b/lib/paperclip/recursive_thumbnail.rb @@ -5,8 +5,12 @@ module Paperclip # если по каким-то причинам не сформировался файл # для прыдущего размера не кидаем ексепшен и # генерим файл из оригинального - f = attachment.to_file(options[:thumbnail] || :original) rescue file - super f, options, attachment + source_style = options[:thumbnail] || :original + # 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 diff --git a/lib/paperclip/storage/no_cache_s3.rb b/lib/paperclip/storage/no_cache_s3.rb index d92444e..bffaa9a 100644 --- a/lib/paperclip/storage/no_cache_s3.rb +++ b/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[:original].close - queued_for_write[:original].unlink + queued_for_write.each_value do |file| + file.is_a?(Tempfile) && file.close! end queued_for_write.clear end diff --git a/lib/paperclip/thumbnail.rb b/lib/paperclip/thumbnail.rb index 5cee1b1..e1d428f 100644 --- a/lib/paperclip/thumbnail.rb +++ b/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 diff --git a/test/storage/no_cache_s3_test.rb b/test/storage/no_cache_s3_test.rb index 2d85431..b942ea9 100644 --- a/test/storage/no_cache_s3_test.rb +++ b/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 - @store2_stub = mock + @store1_stub = mock("store1") + @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=параметр')