Merge pull request #95 from insales/more_tests

more tests

Rakefile

@@ -47,6 +47,11 @@ task :clean do
   Dir.glob("paperclip-*.gem").each{|f| FileUtils.rm f }
 end
 
+
+def spec
+  # TODO: require 'paperclip/version'
+  require 'paperclip'
+
   include_file_globs = ["README*",
                         "LICENSE",
                         "Rakefile",
@@ -62,9 +67,6 @@ exclude_file_globs = ["test/s3.yml",
                         "test/tmp",
                         "test/tmp/*"]
 
-def spec
-  # TODO: require 'paperclip/version'
-  require 'paperclip'
   Gem::Specification.new do |s|
     s.name              = "paperclip"
     s.version           = Paperclip::VERSION

lib/paperclip.rb

@@ -32,7 +32,6 @@ require 'paperclip/upfile'
 require 'paperclip/iostream'
 require 'paperclip/geometry'
 require 'paperclip/processor'
-require 'paperclip/tempfile'
 require 'paperclip/thumbnail'
 require 'paperclip/recursive_thumbnail'
 require 'paperclip/storage'
@@ -345,6 +344,6 @@ end
 
 # Set it all up.
 if Object.const_defined?("ActiveRecord")
-  ActiveRecord::Base.send(:include, Paperclip)
-  File.send(:include, Paperclip::Upfile)
+  ActiveSupport.on_load(:active_record) { include Paperclip }
+  File.include(Paperclip::Upfile)
 end

lib/paperclip/callback_compatability.rb

@@ -5,51 +5,21 @@ module Paperclip
     module_function
 
     def install_to(base)
-      mod = base.respond_to?(:set_callback) ? Rails3 : Rails21
-      base.extend(mod::Defining)
-      base.send(:include, mod::Running)
+      raise "#{base} does not respond to set_callback" unless base.respond_to?(:set_callback)
+
+      base.extend(Defining)
+      base.send(:include, Running)
     end
 
-    module Rails21
     module Defining
-        def define_paperclip_callbacks(*args)
-          args.each do |callback|
-            define_callbacks("before_#{callback}")
-            define_callbacks("after_#{callback}")
-          end
-        end
-      end
-
-      module Running
-        def run_paperclip_callbacks(callback, _opts = nil)
-          # The overall structure of this isn't ideal since after callbacks run even if
-          # befores return false. But this is how rails 3's callbacks work, unfortunately.
-          yield if run_callbacks(:"before_#{callback}") { |result, _object| result == false } != false
-          run_callbacks(:"after_#{callback}") { |result, _object| result == false }
-        end
-      end
-    end
-
-    module Rails3
-      module Defining
-        rails_version = Gem::Version.new(ActiveSupport::VERSION::STRING)
-        CALLBACK_OPTIONS =
-          if rails_version >= Gem::Version.new('5.0')
-            {}
-          elsif rails_version >= Gem::Version.new('4.1')
-            { terminator: ->(_target, result) { result == false } }
-          else
-            { terminator: 'result == false' }
-          end
-
       def define_paperclip_callbacks(*callbacks)
-          define_callbacks(*callbacks.flatten, CALLBACK_OPTIONS)
+        define_callbacks(*callbacks.flatten, {})
         callbacks.map(&:to_sym).each do |callback|
-            define_singleton_method "before_#{callback}" do |*args, &blk|
+          define_singleton_method :"before_#{callback}" do |*args, &blk|
             set_callback(callback, :before, *args, &blk)
           end
 
-            define_singleton_method "after_#{callback}" do |*args, &blk|
+          define_singleton_method :"after_#{callback}" do |*args, &blk|
             set_callback(callback, :after, *args, &blk)
           end
         end
@@ -63,5 +33,3 @@ module Paperclip
     end
   end
 end
-end
-

lib/paperclip/recursive_thumbnail.rb

@@ -7,14 +7,24 @@ module Paperclip
       # если по каким-то причинам не сформировался файл прыдущего размера - генерим из оригинального
       source_file = begin
         attachment.to_file(source_style)
-                    rescue
+      rescue StandardError
+        nil
+      end
+
+      unless source_file
         Paperclip.log "Using original for #{options}"
-                      file
+        source_file = file
       end
+
+      @original_file = file
       super(source_file, options, attachment)
+    end
+
+    def make
+      super
     ensure
-      if source_file != file && source_file.respond_to?(:close!) && !attachment&.queued_for_write&.value?(source_file)
-        source_file.close!
+      if @file != @original_file && @file.respond_to?(:close!) && !attachment&.queued_for_write&.value?(@file)
+        @file.close!
       end
     end
   end

lib/paperclip/storage/delayed_upload.rb

@@ -34,7 +34,7 @@ module Paperclip
         raise if model.exists?(id)
       rescue Errno::ENOENT => e
         raise if model.exists?(id)
-        Rollbar.warn(e, file_name: e.message.split(' - ')[-1])
+        Rollbar.warn(e, file_name: e.message.split(' - ')[-1]) if defined?(Rollbar)
       end
     end
   end

lib/paperclip/storage/filesystem.rb

@@ -56,7 +56,7 @@ module Paperclip
             # ignore file-not-found, let everything else pass
           end
           begin
-            while(true)
+            loop do
               path = File.dirname(path)
               if Dir.entries(path).empty?
                 FileUtils.rmdir(path)

lib/paperclip/tempfile.rb

@@ -1,20 +0,0 @@
-# frozen_string_literal: true
-
-require 'English'
-
-module Paperclip
-  # Due to how ImageMagick handles its image format conversion and how Tempfile
-  # handles its naming scheme, it is necessary to override how Tempfile makes
-  # its names so as to allow for file extensions. Idea taken from the comments
-  # on this blog post:
-  # http://marsorange.com/archives/of-mogrify-ruby-tempfile-dynamic-class-definitions
-  class Tempfile < ::Tempfile
-    # это похоже актуально только для java (не проверял, в mri 3.2 точно не вызывается)
-
-    # Replaces Tempfile's +make_tmpname+ with one that honors file extensions.
-    def make_tmpname(basename, n)
-      extension = File.extname(basename)
-      sprintf("%s,%d,%d%s", File.basename(basename, extension), $PROCESS_ID, n.to_i, extension)
-    end
-  end
-end

lib/paperclip/upfile.rb

@@ -39,11 +39,6 @@ module Paperclip
     def original_filename
       @original_filename ||= File.basename(path)
     end
-
-    # Returns the size of the file.
-    def size
-      File.size(self)
-    end
   end
 end
 

paperclip.gemspec

@@ -19,7 +19,7 @@ Gem::Specification.new do |s|
   s.rubyforge_project = %q{paperclip}
   s.summary = %q{File attachments as attributes for ActiveRecord}
 
-  s.add_dependency 'activesupport', '< 7.2'
+  s.add_dependency 'activesupport', [">= 5.0", '< 7.2']
   s.add_dependency 'addressable'
   s.add_dependency 'fastimage'
 end

test/attachment_test.rb

@@ -1,6 +1,6 @@
 require 'test_helper'
 
-class Dummy
+class Dummy # rubocop:disable Lint/EmptyClass
   # This is a dummy class
 end
 
@@ -134,6 +134,23 @@ class AttachmentTest < Test::Unit::TestCase
       @attachment.assign(@file)
       assert_equal "file.png", @attachment.path
     end
+
+    context "fast upload via nginx" do
+      should "return the right extension for the path" do
+        Tempfile.create do |tempfile|
+          content = "file contents"
+          tempfile.write(content)
+          upload = {
+            'original_name' => 'foo.jpg',
+            'content_type' => 'application/jpg',
+            'filepath' => tempfile.tap(&:rewind).path
+          }
+          @attachment.assign(upload)
+          assert_equal "foo.png", @attachment.path
+          assert_equal content, @attachment.queued_for_write[:original].tap(&:rewind).read
+        end
+      end
+    end
   end
 
   context "An attachment with both 'normal' and hash-style styles" do

test/matchers/have_attached_file_matcher_test.rb

@@ -19,5 +19,11 @@ class HaveAttachedFileMatcherTest < Test::Unit::TestCase
       @dummy_class.has_attached_file :avatar
       assert_accepts @matcher, @dummy_class
     end
+
+    should "have messages" do
+      assert_equal "have an attachment named avatar", @matcher.description
+      assert_equal "Should have an attachment named avatar", @matcher.failure_message
+      assert_equal "Should not have an attachment named avatar", @matcher.negative_failure_message
+    end
   end
 end

test/matchers/validate_attachment_content_type_matcher_test.rb

@@ -26,5 +26,19 @@ class ValidateAttachmentContentTypeMatcherTest < Test::Unit::TestCase
       @dummy_class.validates_attachment_content_type :avatar, :content_type => %r{image/.*}
       assert_accepts @matcher, @dummy_class
     end
+
+    should "have messages" do
+      assert_equal "validate the content types allowed on attachment avatar", @matcher.description
+      assert_equal(
+        "Content types image/png, image/jpeg should be accepted and audio/mp3, " \
+        "application/octet-stream rejected by avatar",
+        @matcher.failure_message
+      )
+      assert_equal(
+        "Content types image/png, image/jpeg should be rejected and audio/mp3, " \
+        "application/octet-stream accepted by avatar",
+        @matcher.negative_failure_message
+      )
+    end
   end
 end

test/matchers/validate_attachment_presence_matcher_test.rb

@@ -17,5 +17,11 @@ class ValidateAttachmentPresenceMatcherTest < Test::Unit::TestCase
       @dummy_class.validates_attachment_presence :avatar
       assert_accepts @matcher, @dummy_class
     end
+
+    should "have messages" do
+      assert_equal "require presence of attachment avatar", @matcher.description
+      assert_equal "Attachment avatar should be required", @matcher.failure_message
+      assert_equal "Attachment avatar should not be required", @matcher.negative_failure_message
+    end
   end
 end

test/matchers/validate_attachment_size_matcher_test.rb

@@ -31,6 +31,12 @@ class ValidateAttachmentSizeMatcherTest < Test::Unit::TestCase
         @dummy_class.validates_attachment_size :avatar, :in => 256..1024
         assert_accepts @matcher, @dummy_class
       end
+
+      should "have messages" do
+        assert_equal "validate the size of attachment avatar", @matcher.description
+        assert_equal "Attachment avatar must be between 256 and 1024 bytes", @matcher.failure_message
+        assert_equal "Attachment avatar cannot be between 256 and 1024 bytes", @matcher.negative_failure_message
+      end
     end
 
     context "validates_attachment_size with infinite range" do

test/optimizer_test.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class OptimizerTest < Test::Unit::TestCase
+  setup do
+    @pixel_jpg = Base64.decode64(
+      "/9j/4AAQSkZJRgABAQEASABIAAD/2wBDAP#{'/' * 86}wgALCAABAAEBAREA/8QAFBAB#{'A' * 21}P/aAAgBAQABPxA"
+    )
+    @pixel_png = Base64.decode64(
+      "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg"
+    )
+    @pixel_gif = Base64.decode64("R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7")
+  end
+
+  def with_tempfile(name, content)
+    Tempfile.create([File.basename(name), File.extname(name)]) do |tempfile|
+      tempfile.binmode
+      tempfile.write(content)
+      tempfile.flush
+      tempfile.rewind
+      yield tempfile
+    end
+  end
+
+  context "Paperclip::Optimizer" do
+    context "#real_content_type" do
+      should "detect jpeg" do
+        with_tempfile('pixel.jpg', @pixel_jpg) do |tempfile|
+          assert_equal("image/jpeg", Paperclip::Optimizer.new(tempfile, {}).real_content_type)
+        end
+      end
+
+      should("detect png") do
+        with_tempfile('image.svg.png', @pixel_png) do |tempfile|
+          assert_equal("image/png", Paperclip::Optimizer.new(tempfile, {}).real_content_type)
+        end
+      end
+
+      should("detect gif") do
+        with_tempfile('Spiked.gif', @pixel_gif) do |tempfile|
+          assert_equal("image/gif", Paperclip::Optimizer.new(tempfile, {}).real_content_type)
+        end
+      end
+    end
+
+    context "#make" do
+      should "process jpeg" do
+        with_tempfile('pixel.jpg', @pixel_jpg) do |tempfile|
+          res = Paperclip::Optimizer.new(tempfile, {}).make
+          assert_equal("image/jpeg", Paperclip::Upfile.content_type_from_file(res.path))
+        end
+      end
+
+      should("process png") do
+        with_tempfile('image.svg.png', @pixel_png) do |tempfile|
+          res = Paperclip::Optimizer.new(tempfile, {}).make
+          assert_equal("image/png", Paperclip::Upfile.content_type_from_file(res.path))
+        end
+      end
+
+      should("process gif") do
+        with_tempfile('Spiked.gif', @pixel_gif) do |tempfile|
+          res = Paperclip::Optimizer.new(tempfile, {}).make
+          assert_equal("image/gif", Paperclip::Upfile.content_type_from_file(res.path))
+        end
+      end
+
+      should("pass others") do
+        invalid_content = "invalid gif"
+        with_tempfile('Spiked.gif', invalid_content) do |tempfile|
+          res = Paperclip::Optimizer.new(tempfile, {}).make
+          assert_equal(tempfile, res)
+          assert_equal(invalid_content, res.read)
+        end
+      end
+
+      should("handle errors") do
+        invalid_content = "invalid gif"
+        with_tempfile('Spiked.gif', invalid_content) do |tempfile|
+          processor = Paperclip::Optimizer.new(tempfile, {})
+          processor.stubs(real_content_type: 'image/gif')
+          Open3.stubs(:capture3).raises("lala")
+          Paperclip.expects(:log).with { _1.include?("lala") }
+          res = processor.make
+          assert_equal(tempfile, res)
+          assert_equal(invalid_content, res.read)
+        end
+      end
+    end
+  end
+end

test/paperclip_test.rb

@@ -16,7 +16,7 @@ class PaperclipTest < Test::Unit::TestCase
       should "execute the right command" do
         Paperclip.expects(:path_for_command).with("convert").returns("/usr/bin/convert")
         Paperclip.expects(:bit_bucket).returns("/dev/null")
-        Paperclip.expects(:"`").with("timeout 30 /usr/bin/convert #{@file_path} #{@file_path2} 2>/dev/null")
+        Paperclip.expects(:`).with("timeout 30 /usr/bin/convert #{@file_path} #{@file_path2} 2>/dev/null")
         Paperclip.run("convert", "#{@file_path} #{@file_path2}")
       end
     end
@@ -33,7 +33,7 @@ class PaperclipTest < Test::Unit::TestCase
     should "execute the right command" do
       Paperclip.expects(:path_for_command).with("convert").returns("convert")
       Paperclip.expects(:bit_bucket).returns("/dev/null")
-      Paperclip.expects(:"`").with("timeout 30 convert #{@file_path} #{@file_path2} 2>/dev/null")
+      Paperclip.expects(:`).with("timeout 30 convert #{@file_path} #{@file_path2} 2>/dev/null")
       Paperclip.run("convert", "#{@file_path} #{@file_path2}")
     end
 
@@ -41,7 +41,7 @@ class PaperclipTest < Test::Unit::TestCase
       Paperclip.options[:log_command] = true
       Paperclip.expects(:bit_bucket).returns("/dev/null")
       Paperclip.expects(:log).with("convert #{@file_path} #{@file_path2} 2>/dev/null")
-      Paperclip.expects(:"`").with("timeout 30 convert #{@file_path} #{@file_path2} 2>/dev/null")
+      Paperclip.expects(:`).with("timeout 30 convert #{@file_path} #{@file_path2} 2>/dev/null")
       Paperclip.run("convert", "#{@file_path} #{@file_path2}")
     end
   end
@@ -110,7 +110,7 @@ class PaperclipTest < Test::Unit::TestCase
 
     context "a validation with an if guard clause" do
       setup do
-        Dummy.send(:"validates_attachment_presence", :avatar, :if => lambda{|i| i.foo })
+        Dummy.send(:validates_attachment_presence, :avatar, if: ->(i) { i.foo })
         @dummy = Dummy.new
       end
 
@@ -129,7 +129,7 @@ class PaperclipTest < Test::Unit::TestCase
 
     context "a validation with an unless guard clause" do
       setup do
-        Dummy.send(:"validates_attachment_presence", :avatar, :unless => lambda{|i| i.foo })
+        Dummy.send(:validates_attachment_presence, :avatar, unless: ->(i) { i.foo })
         @dummy = Dummy.new
       end
 

test/plural_cache_test.rb

@@ -1,7 +1,7 @@
 require 'test_helper'
 
 class PluralCacheTest < Test::Unit::TestCase
-  class BigBox; end
+  class BigBox; end # rubocop:disable Lint/EmptyClass
 
   should 'cache pluralizations' do
     cache = Paperclip::Interpolations::PluralCache.new

test/railtie_test.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class RailtieTest < Test::Unit::TestCase
+  should "load processors" do
+    FileUtils.mkdir_p('tmp/rails/lib/paperclip_processors')
+    Rails.root.join('lib/paperclip_processors/some_custom_processor.rb').write <<~RUBY
+      class Paperclip::SomeCustomProcessor < Paperclip::Processor
+      end
+    RUBY
+    Paperclip::Railtie.initializers.each(&:run)
+    assert defined?(Paperclip::SomeCustomProcessor)
+  end
+
+  should "load rake tasks" do
+    require 'rake'
+    require 'rake/testtask'
+
+    Rails.application.load_tasks
+    assert_equal Rake::Task, Rake.application["paperclip:refresh:thumbnails"].class
+  end
+end

test/recursive_thumbnail_test.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class RecursiveThumbnailTest < Test::Unit::TestCase
+  setup do
+    Paperclip::Geometry.stubs from_file: Paperclip::Geometry.parse('1x1')
+    @original_file = stub("originalfile", path: 'originalfile.txt')
+    @attachment = attachment({})
+  end
+
+  should "use original when style not present" do
+    processor = Paperclip::RecursiveThumbnail.new(@original_file, { thumbnail: :missing, geometry: '1x1' }, @attachment)
+    assert_equal @original_file, processor.file
+  end
+
+  should "use original when style failed to download" do
+    @attachment.expects(:to_file).with(:missing).raises("cannot haz filez")
+    processor = Paperclip::RecursiveThumbnail.new(@original_file, { thumbnail: :missing, geometry: '1x1' }, @attachment)
+    assert_equal @original_file, processor.file
+  end
+
+  should "use style when present" do
+    style_file = stub("stylefile", path: 'style.txt')
+    style_file.expects(:close!).once
+    @original_file.expects(:close!).never
+    @attachment.expects(:to_file).with(:existent).returns(style_file)
+    processor = Paperclip::RecursiveThumbnail.new(
+      @original_file, { thumbnail: :existent, geometry: '1x1' }, @attachment
+    )
+    Paperclip.stubs run: ""
+    assert_equal style_file, processor.file
+    res = processor.make
+    assert_equal Tempfile, res.class
+  end
+end

test/test_helper.rb

@@ -30,6 +30,7 @@ if ENV['COVERAGE']
     add_group "Libraries", "lib/"
 
     track_files "{lib}/**/*.rb"
+    add_filter "lib/tasks/paperclip_tasks.rake" # TODO: вообще по-хорошема надо и его покрыть
   end
 end
 
@@ -99,6 +100,8 @@ def rebuild_class(options = {})
 end
 
 class FakeModel
+  def self.set_callback(...); end
+
   include Paperclip
 
   attr_accessor :avatar_file_name,

test/thumbnail_test.rb

@@ -65,7 +65,7 @@ class ThumbnailTest < Test::Unit::TestCase
 
       should "send the right command to convert when sent #make" do
         Paperclip::Thumbnail.any_instance.stubs(:gamma_correction_if_needed).returns("PNG\nPaletteAlpha")
-        Paperclip.expects(:"`").with do |arg|
+        Paperclip.expects(:`).with do |arg|
           arg.match? %r{convert\s+"#{File.expand_path(@thumb.file.path)}\[0\]"\s+-auto-orient\s+-resize\s+\"x50\"\s+-crop\s+\"100x50\+114\+0\"\s+\+repage\s+}
         end
         @thumb.make
@@ -90,7 +90,7 @@ class ThumbnailTest < Test::Unit::TestCase
 
       should "send the right command to convert when sent #make" do
         Paperclip::Thumbnail.any_instance.stubs(:gamma_correction_if_needed).returns("PNG\nPaletteAlpha")
-        Paperclip.expects(:"`").with do |arg|
+        Paperclip.expects(:`).with do |arg|
           arg.match? %r{convert\s+"#{File.expand_path(@thumb.file.path)}\[0\]"\s+-auto-orient\s+-resize\s+"x50"\s+-crop\s+"100x50\+114\+0"\s+\+repage\s+-strip\s+-depth\s+8\s+}
         end
         @thumb.make

test/upfile_test.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class UpfileTest < Test::Unit::TestCase
+  context "content_type_from_ext" do
+    {
+      'lala' => 'application/x-octet-stream',
+      'lala.foo' => 'application/x-foo',
+      '__.jpg' => "image/jpeg",
+      '_.jpeg' => "image/jpeg",
+      '__.tif' => "image/tiff",
+      '_.tiff' => "image/tiff",
+
+      '_.png' => "image/png",
+      '_.gif' => "image/gif",
+      '_.bmp' => "image/bmp",
+      "_.webp" => "image/webp",
+
+      # '_.pngfoo' => 'application/x-pngfoo', # ??
+      # '_.htmfoo' => 'application/x-htmfoo', # ?
+
+      '_.csv' => "text/csv",
+      '_.xml' => "text/xml",
+      '_.css' => "text/css",
+      '_.js' => "text/js", # ???
+
+      '_.html' => 'text/html',
+      '__.htm' => 'text/html',
+
+      "_.txt" => "text/plain",
+      "_.liquid" => "text/x-liquid",
+      '_.svg' => 'image/svg+xml',
+      '_.xls' => 'application/vnd.ms-excel'
+    }.each_pair do |example, result|
+      should "return #{result} for #{example}" do
+        assert_equal result, Paperclip::Upfile.content_type_from_ext(example)
+      end
+    end
+  end
+end