diff --git a/.gitignore b/.gitignore index b6db9ec..604e863 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ gemfiles/*.lock gemfiles/.bundle/ tmp/ .rubocop-* +/coverage/ diff --git a/.pryrc b/.pryrc new file mode 100644 index 0000000..68b1922 --- /dev/null +++ b/.pryrc @@ -0,0 +1,7 @@ +# Alieases for debugger. +if defined?(PryByebug) || defined?(PryDebugger) + Pry.commands.alias_command 'c', 'continue' + Pry.commands.alias_command 's', 'step' + Pry.commands.alias_command 'n', 'next' + Pry.commands.alias_command 'f', 'finish' +end diff --git a/Gemfile b/Gemfile index 7700f4f..73e4987 100644 --- a/Gemfile +++ b/Gemfile @@ -15,6 +15,7 @@ gem 'rails' gem 'sidekiq' gem 'test-unit' +gem 'simplecov', require: false gem 'mocha' gem 'thoughtbot-shoulda', '>= 2.9.0' diff --git a/Rakefile b/Rakefile index 3582859..12a4fe0 100644 --- a/Rakefile +++ b/Rakefile @@ -3,7 +3,6 @@ require 'rake/testtask' require 'rdoc/task' $LOAD_PATH << File.join(File.dirname(__FILE__), 'lib') -require 'paperclip' import 'lib/tasks/paperclip_tasks.rake' @@ -19,7 +18,7 @@ Rake::TestTask.new(:test) do |t| end desc 'Start an IRB session with all necessary files required.' -task :shell do +task :shell => :load_paperclip do chdir File.dirname(__FILE__) exec 'irb -I lib/ -I lib/paperclip -r rubygems -r active_record -r tempfile -r init' end @@ -62,23 +61,28 @@ exclude_file_globs = ["test/s3.yml", "test/pkg/*", "test/tmp", "test/tmp/*"] -spec = Gem::Specification.new do |s| - s.name = "paperclip" - s.version = Paperclip::VERSION - s.author = "Jon Yurek" - s.email = "jyurek@thoughtbot.com" - s.homepage = "http://www.thoughtbot.com/projects/paperclip" - s.platform = Gem::Platform::RUBY - s.summary = "File attachments as attributes for ActiveRecord" - s.files = FileList[include_file_globs].to_a - FileList[exclude_file_globs].to_a - s.require_path = "lib" - s.test_files = FileList["test/**/test_*.rb"].to_a - s.rubyforge_project = "paperclip" - s.extra_rdoc_files = FileList["README*"].to_a - s.rdoc_options << '--line-numbers' << '--inline-source' - s.requirements << "ImageMagick" - s.add_development_dependency 'thoughtbot-shoulda' - s.add_development_dependency 'mocha' + +def spec + # TODO: require 'paperclip/version' + require 'paperclip' + Gem::Specification.new do |s| + s.name = "paperclip" + s.version = Paperclip::VERSION + s.author = "Jon Yurek" + s.email = "jyurek@thoughtbot.com" + s.homepage = "http://www.thoughtbot.com/projects/paperclip" + s.platform = Gem::Platform::RUBY + s.summary = "File attachments as attributes for ActiveRecord" + s.files = FileList[include_file_globs].to_a - FileList[exclude_file_globs].to_a + s.require_path = "lib" + s.test_files = FileList["test/**/test_*.rb"].to_a + s.rubyforge_project = "paperclip" + s.extra_rdoc_files = FileList["README*"].to_a + s.rdoc_options << '--line-numbers' << '--inline-source' + s.requirements << "ImageMagick" + s.add_development_dependency 'thoughtbot-shoulda' + s.add_development_dependency 'mocha' + end end desc "Print a list of the files to be put into the gem" 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=параметр') diff --git a/test/test_helper.rb b/test/test_helper.rb index 3e4684e..5e8c15b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require 'rubygems' +require 'bundler/setup' + require 'test/unit' require 'shoulda' require 'mocha/test_unit' @@ -13,6 +15,24 @@ require 'active_record' require 'active_support' require 'rails' +if ENV['COVERAGE'] + require 'simplecov' + + SimpleCov.external_at_exit = true + Test::Unit.at_exit do + SimpleCov.at_exit_behavior + end + SimpleCov.command_name 'test:units' + SimpleCov.start do + load_profile "test_frameworks" + + add_group "Storage", "lib/paperclip/storage/" + add_group "Libraries", "lib/" + + track_files "{lib}/**/*.rb" + end +end + ROOT = File.expand_path('../', __dir__) ENV['RAILS_ENV'] = 'test' @@ -20,9 +40,10 @@ class TestRailsApp < Rails::Application; end Rails.application.config.root = "#{ROOT}/tmp/rails" $LOAD_PATH << File.join(ROOT, 'lib') -$LOAD_PATH << File.join(ROOT, 'lib', 'paperclip') +$LOAD_PATH << File.join(ROOT, 'lib/paperclip') # ?? -require File.join(ROOT, 'lib', 'paperclip.rb') +require 'paperclip' +# require File.join(ROOT, 'lib/paperclip.rb') require 'shoulda_macros/paperclip'