From 9c510875813ec40186a7210b8e2dd1ed2873f8b0 Mon Sep 17 00:00:00 2001 From: Jon Yurek Date: Wed, 10 Dec 2008 15:31:37 -0500 Subject: [PATCH] #58 Applied Rob Anderton's patch, fixing many windows-related issues. --- lib/paperclip/attachment.rb | 3 ++ lib/paperclip/storage.rb | 2 +- test/attachment_test.rb | 19 +++++++------ test/geometry_test.rb | 2 +- test/integration_test.rb | 56 +++++++++++++++++++------------------ test/iostream_test.rb | 42 ++++++++++++++++++---------- test/paperclip_test.rb | 6 ++-- test/storage_test.rb | 4 +-- test/thumbnail_test.rb | 10 +++---- 9 files changed, 83 insertions(+), 61 deletions(-) diff --git a/lib/paperclip/attachment.rb b/lib/paperclip/attachment.rb index 7d649df3f..740ad2f72 100644 --- a/lib/paperclip/attachment.rb +++ b/lib/paperclip/attachment.rb @@ -62,6 +62,7 @@ def assign uploaded_file if uploaded_file.is_a?(Paperclip::Attachment) uploaded_file = uploaded_file.to_file(:original) + close_uploaded_file = true if uploaded_file.respond_to?(:close) end return nil unless valid_assignment?(uploaded_file) @@ -88,6 +89,7 @@ def assign uploaded_file # Reset the file size if the original file was reprocessed. instance_write(:file_size, uploaded_file.size.to_i) ensure + uploaded_file.close if close_uploaded_file validate end @@ -191,6 +193,7 @@ def self.interpolations # again. def reprocess! new_original = Tempfile.new("paperclip-reprocess") + new_original.binmode if old_original = to_file(:original) new_original.write( old_original.read ) new_original.rewind diff --git a/lib/paperclip/storage.rb b/lib/paperclip/storage.rb index 76c174180..c7f24270f 100644 --- a/lib/paperclip/storage.rb +++ b/lib/paperclip/storage.rb @@ -31,7 +31,7 @@ def exists?(style = default_style) # Returns representation of the data of the file assigned to the given # style, in the format most representative of the current storage. def to_file style = default_style - @queued_for_write[style] || (File.new(path(style)) if exists?(style)) + @queued_for_write[style] || (File.new(path(style), 'rb') if exists?(style)) end alias_method :to_io, :to_file diff --git a/test/attachment_test.rb b/test/attachment_test.rb index 22daa0b70..8cc778a79 100644 --- a/test/attachment_test.rb +++ b/test/attachment_test.rb @@ -77,7 +77,7 @@ class AttachmentTest < Test::Unit::TestCase @dummy.stubs(:id).returns(1024) @file = File.new(File.join(File.dirname(__FILE__), "fixtures", - "5k.png")) + "5k.png"), 'rb') @dummy.avatar = @file end @@ -95,7 +95,7 @@ class AttachmentTest < Test::Unit::TestCase @dummy.stubs(:id).returns(@id) @file = File.new(File.join(File.dirname(__FILE__), "fixtures", - "5k.png")) + "5k.png"), 'rb') @dummy.avatar = @file end @@ -131,7 +131,7 @@ class AttachmentTest < Test::Unit::TestCase setup do @file = File.new(File.join(File.dirname(__FILE__), "fixtures", - "5k.png")) + "5k.png"), 'rb') Paperclip::Thumbnail.stubs(:make) [:thumb, :large].each do |style| @dummy.avatar.stubs(:extra_options_for).with(style) @@ -213,7 +213,7 @@ class AttachmentTest < Test::Unit::TestCase @attachment = Paperclip::Attachment.new(:avatar, @instance) @file = File.new(File.join(File.dirname(__FILE__), "fixtures", - "5k.png")) + "5k.png"), 'rb') end should "raise if there are not the correct columns when you try to assign" do @@ -300,9 +300,11 @@ class AttachmentTest < Test::Unit::TestCase end should "return the real url" do - assert @attachment.to_file + file = @attachment.to_file + assert file assert_match %r{^/avatars/#{@instance.id}/original/5k\.png}, @attachment.url assert_match %r{^/avatars/#{@instance.id}/small/5k\.jpg}, @attachment.url(:small) + file.close end should "commit the files to disk" do @@ -310,6 +312,7 @@ class AttachmentTest < Test::Unit::TestCase io = @attachment.to_io(style) assert File.exists?(io) assert ! io.is_a?(::Tempfile) + io.close end end @@ -317,8 +320,7 @@ class AttachmentTest < Test::Unit::TestCase [[:large, 400, 61, "PNG"], [:medium, 100, 15, "GIF"], [:small, 32, 32, "JPEG"]].each do |style| - cmd = "identify -format '%w %h %b %m' " + - "#{@attachment.to_io(style.first).path}" + cmd = %Q[identify -format "%w %h %b %m" "#{@attachment.path(style.first)}"] out = `#{cmd}` width, height, size, format = out.split(" ") assert_equal style[1].to_s, width.to_s @@ -328,7 +330,8 @@ class AttachmentTest < Test::Unit::TestCase end should "still have its #file attribute not be nil" do - assert ! @attachment.to_file.nil? + assert ! (file = @attachment.to_file).nil? + file.close end context "and deleted" do diff --git a/test/geometry_test.rb b/test/geometry_test.rb index e1bc62f57..134372d69 100644 --- a/test/geometry_test.rb +++ b/test/geometry_test.rb @@ -93,7 +93,7 @@ class GeometryTest < Test::Unit::TestCase should "be generated from a file" do file = File.join(File.dirname(__FILE__), "fixtures", "5k.png") - file = File.new(file) + file = File.new(file, 'rb') assert_nothing_raised{ @geo = Paperclip::Geometry.from_file(file) } assert @geo.height > 0 assert @geo.width > 0 diff --git a/test/integration_test.rb b/test/integration_test.rb index 2e70ee4d2..278c9058f 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -4,7 +4,7 @@ class IntegrationTest < Test::Unit::TestCase context "Many models at once" do setup do rebuild_model - @file = File.new(File.join(FIXTURES_DIR, "5k.png")) + @file = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb') 300.times do |i| Dummy.create! :avatar => @file end @@ -24,13 +24,13 @@ class IntegrationTest < Test::Unit::TestCase @dummy = Dummy.new @file = File.new(File.join(File.dirname(__FILE__), "fixtures", - "5k.png")) + "5k.png"), 'rb') @dummy.avatar = @file assert @dummy.save end should "create its thumbnails properly" do - assert_match /\b50x50\b/, `identify '#{@dummy.avatar.path(:thumb)}'` + assert_match /\b50x50\b/, `identify "#{@dummy.avatar.path(:thumb)}"` end context "redefining its attachment styles" do @@ -44,7 +44,7 @@ class IntegrationTest < Test::Unit::TestCase end should "create its thumbnails properly" do - assert_match /\b150x25\b/, `identify '#{@dummy.avatar.path(:thumb)}'` + assert_match /\b150x25\b/, `identify "#{@dummy.avatar.path(:thumb)}"` end end end @@ -133,8 +133,8 @@ class IntegrationTest < Test::Unit::TestCase :url => "/:attachment/:class/:style/:id/:basename.:extension", :path => ":rails_root/tmp/:attachment/:class/:style/:id/:basename.:extension" @dummy = Dummy.new - @file = File.new(File.join(FIXTURES_DIR, "5k.png")) - @bad_file = File.new(File.join(FIXTURES_DIR, "bad.png")) + @file = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb') + @bad_file = File.new(File.join(FIXTURES_DIR, "bad.png"), 'rb') assert @dummy.avatar = @file assert @dummy.valid? @@ -146,18 +146,18 @@ class IntegrationTest < Test::Unit::TestCase ["300x46", :large], ["100x15", :medium], ["32x32", :thumb]].each do |geo, style| - cmd = %Q[identify -format "%wx%h" #{@dummy.avatar.to_file(style).path}] + cmd = %Q[identify -format "%wx%h" "#{@dummy.avatar.path(style)}"] assert_equal geo, `#{cmd}`.chomp, cmd end - saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.to_file(s).path } + saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.path(s) } @d2 = Dummy.find(@dummy.id) - assert_equal "100x15", `identify -format "%wx%h" #{@d2.avatar.to_file.path}`.chomp - assert_equal "434x66", `identify -format "%wx%h" #{@d2.avatar.to_file(:original).path}`.chomp - assert_equal "300x46", `identify -format "%wx%h" #{@d2.avatar.to_file(:large).path}`.chomp - assert_equal "100x15", `identify -format "%wx%h" #{@d2.avatar.to_file(:medium).path}`.chomp - assert_equal "32x32", `identify -format "%wx%h" #{@d2.avatar.to_file(:thumb).path}`.chomp + assert_equal "100x15", `identify -format "%wx%h" "#{@d2.avatar.path}"`.chomp + assert_equal "434x66", `identify -format "%wx%h" "#{@d2.avatar.path(:original)}"`.chomp + assert_equal "300x46", `identify -format "%wx%h" "#{@d2.avatar.path(:large)}"`.chomp + assert_equal "100x15", `identify -format "%wx%h" "#{@d2.avatar.path(:medium)}"`.chomp + assert_equal "32x32", `identify -format "%wx%h" "#{@d2.avatar.path(:thumb)}"`.chomp @dummy.avatar = "not a valid file but not nil" assert_equal File.basename(@file.path), @dummy.avatar_file_name @@ -186,10 +186,10 @@ class IntegrationTest < Test::Unit::TestCase assert_equal @dummy.avatar_file_name, @d2.avatar_file_name [:thumb, :medium, :large, :original].each do |style| - assert_equal @dummy.avatar.to_file(style).path, @d2.avatar.to_file(style).path + assert_equal @dummy.avatar.path(style), @d2.avatar.path(style) end - saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.to_file(s).path } + saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.path(s) } @d2.avatar = nil assert @d2.save @@ -203,7 +203,8 @@ class IntegrationTest < Test::Unit::TestCase expected = @dummy.avatar.to_file @dummy.avatar = "not a file" assert @dummy.valid? - assert_equal expected.path, @dummy.avatar.to_file.path + assert_equal expected.path, @dummy.avatar.path + expected.close @dummy.avatar = @bad_file assert ! @dummy.valid? @@ -234,19 +235,19 @@ class IntegrationTest < Test::Unit::TestCase context "that is assigned its file from another Paperclip attachment" do setup do @dummy2 = Dummy.new - @file2 = File.new(File.join(FIXTURES_DIR, "12k.png")) + @file2 = File.new(File.join(FIXTURES_DIR, "12k.png"), 'rb') assert @dummy2.avatar = @file2 @dummy2.save end should "work when assigned a file" do - assert_not_equal `identify -format "%wx%h" #{@dummy.avatar.to_file(:original).path}`, - `identify -format "%wx%h" #{@dummy2.avatar.to_file(:original).path}` + assert_not_equal `identify -format "%wx%h" "#{@dummy.avatar.path(:original)}"`, + `identify -format "%wx%h" "#{@dummy2.avatar.path(:original)}"` assert @dummy.avatar = @dummy2.avatar @dummy.save - assert_equal `identify -format "%wx%h" #{@dummy.avatar.to_file(:original).path}`, - `identify -format "%wx%h" #{@dummy2.avatar.to_file(:original).path}` + assert_equal `identify -format "%wx%h" "#{@dummy.avatar.path(:original)}"`, + `identify -format "%wx%h" "#{@dummy2.avatar.path(:original)}"` end should "work when assigned a nil file" do @@ -265,8 +266,9 @@ class IntegrationTest < Test::Unit::TestCase if ENV['S3_TEST_BUCKET'] def s3_files_for attachment [:thumb, :medium, :large, :original].inject({}) do |files, style| - data = `curl '#{attachment.url(style)}' 2>/dev/null`.chomp + data = `curl "#{attachment.url(style)}" 2>/dev/null`.chomp t = Tempfile.new("paperclip-test") + t.binmode t.write(data) t.rewind files[style] = t @@ -275,7 +277,7 @@ def s3_files_for attachment end def s3_headers_for attachment, style - `curl --head '#{attachment.url(style)}' 2>/dev/null`.split("\n").inject({}) do |h,head| + `curl --head "#{attachment.url(style)}" 2>/dev/null`.split("\n").inject({}) do |h,head| split_head = head.chomp.split(/\s*:\s*/, 2) h[split_head.first.downcase] = split_head.last unless split_head.empty? h @@ -295,8 +297,8 @@ def s3_headers_for attachment, style :bucket => ENV['S3_TEST_BUCKET'], :path => ":class/:attachment/:id/:style/:basename.:extension" @dummy = Dummy.new - @file = File.new(File.join(FIXTURES_DIR, "5k.png")) - @bad_file = File.new(File.join(FIXTURES_DIR, "bad.png")) + @file = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb') + @bad_file = File.new(File.join(FIXTURES_DIR, "bad.png"), 'rb') assert @dummy.avatar = @file assert @dummy.valid? @@ -310,7 +312,7 @@ def s3_headers_for attachment, style ["300x46", :large], ["100x15", :medium], ["32x32", :thumb]].each do |geo, style| - cmd = %Q[identify -format "%wx%h" #{@files_on_s3[style].path}] + cmd = %Q[identify -format "%wx%h" "#{@files_on_s3[style].path}"] assert_equal geo, `#{cmd}`.chomp, cmd end @@ -320,7 +322,7 @@ def s3_headers_for attachment, style ["300x46", :large], ["100x15", :medium], ["32x32", :thumb]].each do |geo, style| - cmd = %Q[identify -format "%wx%h" #{@d2_files[style].path}] + cmd = %Q[identify -format "%wx%h" "#{@d2_files[style].path}"] assert_equal geo, `#{cmd}`.chomp, cmd end diff --git a/test/iostream_test.rb b/test/iostream_test.rb index 7cdb9239b..3e4722498 100644 --- a/test/iostream_test.rb +++ b/test/iostream_test.rb @@ -11,29 +11,43 @@ class IOStreamTest < Test::Unit::TestCase context "A file" do setup do - @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png")) + @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png"), 'rb') end context "that is sent #stream_to" do - [["/tmp/iostream.string.test", File], - [Tempfile.new('iostream.test'), Tempfile]].each do |args| + context "and given a String" do + setup do + assert @result = @file.stream_to("/tmp/iostream.string.test") + end - context "and given a #{args[0].class.to_s}" do - setup do - assert @result = @file.stream_to(args[0]) - end + should "return a File" do + assert @result.is_a?(File) + end - should "return a #{args[1].to_s}" do - assert @result.is_a?(args[1]) - end + should "contain the same data as the original file" do + @file.rewind; @result.rewind + assert_equal @file.read, @result.read + end + end - should "contain the same data as the original file" do - @file.rewind; @result.rewind - assert_equal @file.read, @result.read - end + context "and given a Tempfile" do + setup do + tempfile = Tempfile.new('iostream.test') + tempfile.binmode + assert @result = @file.stream_to(tempfile) + end + + should "return a Tempfile" do + assert @result.is_a?(Tempfile) + end + + should "contain the same data as the original file" do + @file.rewind; @result.rewind + assert_equal @file.read, @result.read end end + end context "that is sent #to_tempfile" do diff --git a/test/paperclip_test.rb b/test/paperclip_test.rb index e87e0b202..9a24e6280 100644 --- a/test/paperclip_test.rb +++ b/test/paperclip_test.rb @@ -35,7 +35,7 @@ class PaperclipTest < Test::Unit::TestCase context "An ActiveRecord model with an 'avatar' attachment" do setup do rebuild_model :path => "tmp/:class/omg/:style.:extension" - @file = File.new(File.join(FIXTURES_DIR, "5k.png")) + @file = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb') end should "not error when trying to also create a 'blah' attachment" do @@ -176,8 +176,8 @@ def self.should_validate validation, options, valid_file, invalid_file [:content_type, {:content_type => "text/plain"}, "text.txt", "5k.png"], [:content_type, {:content_type => %r{image/.*}}, "5k.png", "text.txt"]].each do |args| validation, options, valid_file, invalid_file = args - valid_file &&= File.new(File.join(FIXTURES_DIR, valid_file)) - invalid_file &&= File.new(File.join(FIXTURES_DIR, invalid_file)) + valid_file &&= File.open(File.join(FIXTURES_DIR, valid_file), "rb") + invalid_file &&= File.open(File.join(FIXTURES_DIR, invalid_file), "rb") should_validate validation, options, valid_file, invalid_file end diff --git a/test/storage_test.rb b/test/storage_test.rb index 6276ae48c..255e832ee 100644 --- a/test/storage_test.rb +++ b/test/storage_test.rb @@ -77,7 +77,7 @@ class StorageTest < Test::Unit::TestCase context "when assigned" do setup do - @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png')) + @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'), 'rb') @dummy = Dummy.new @dummy.avatar = @file end @@ -144,7 +144,7 @@ class StorageTest < Test::Unit::TestCase context "when assigned" do setup do - @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png')) + @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'), 'rb') @dummy.avatar = @file end diff --git a/test/thumbnail_test.rb b/test/thumbnail_test.rb index cca378290..c2002d1cd 100644 --- a/test/thumbnail_test.rb +++ b/test/thumbnail_test.rb @@ -32,7 +32,7 @@ class ThumbnailTest < Test::Unit::TestCase context "An image" do setup do - @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png")) + @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png"), 'rb') end [["600x600>", "434x66"], @@ -45,7 +45,7 @@ class ThumbnailTest < Test::Unit::TestCase end should "start with dimensions of 434x66" do - cmd = %Q[identify -format "%wx%h" #{@file.path}] + cmd = %Q[identify -format "%wx%h" "#{@file.path}"] assert_equal "434x66", `#{cmd}`.chomp end @@ -59,7 +59,7 @@ class ThumbnailTest < Test::Unit::TestCase end should "be the size we expect it to be" do - cmd = %Q[identify -format "%wx%h" #{@thumb_result.path}] + cmd = %Q[identify -format "%wx%h" "#{@thumb_result.path}"] assert_equal args[1], `#{cmd}`.chomp end end @@ -97,7 +97,7 @@ class ThumbnailTest < Test::Unit::TestCase should "create the thumbnail when sent #make" do dst = @thumb.make - assert_match /100x50/, `identify #{dst.path}` + assert_match /100x50/, `identify "#{dst.path}"` end end @@ -119,7 +119,7 @@ class ThumbnailTest < Test::Unit::TestCase should "create the thumbnail when sent #make" do dst = @thumb.make - assert_match /100x50/, `identify #{dst.path}` + assert_match /100x50/, `identify "#{dst.path}"` end context "redefined to have bad convert_options setting" do