Created
March 30, 2010 17:33
-
-
Save rob-at-thewebfellas/349332 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From 007fcd79294ebcb97c5e0beb25a65c24928812cf Mon Sep 17 00:00:00 2001 | |
From: Rob Anderton <[email protected]> | |
Date: Tue, 30 Mar 2010 16:44:52 +0100 | |
Subject: [PATCH 1/5] fix problems with newer versions of test/unit | |
--- | |
test/helper.rb | 3 +++ | |
1 files changed, 3 insertions(+), 0 deletions(-) | |
diff --git a/test/helper.rb b/test/helper.rb | |
index 5c56107..be84ad1 100644 | |
--- a/test/helper.rb | |
+++ b/test/helper.rb | |
@@ -1,5 +1,8 @@ | |
require 'rubygems' | |
+ | |
+gem 'test-unit' | |
require 'test/unit' | |
+ | |
require 'shoulda' | |
require 'tempfile' | |
-- | |
1.6.4.4 | |
From 25f854a31c4c83cbbb73e84d9ac55fb48366305d Mon Sep 17 00:00:00 2001 | |
From: Rob Anderton <[email protected]> | |
Date: Tue, 30 Mar 2010 17:50:50 +0100 | |
Subject: [PATCH 2/5] move from AWS/S3 to qoobaa/S3 to provide support for European S3 buckets | |
--- | |
Rakefile | 2 +- | |
lib/paperclip/storage.rb | 87 ++++++++++++++++++++--------------- | |
paperclip.gemspec | 6 +- | |
test/integration_test.rb | 1 + | |
test/storage_test.rb | 115 ++++++++++++++++++++++++++++------------------ | |
5 files changed, 125 insertions(+), 86 deletions(-) | |
diff --git a/Rakefile b/Rakefile | |
index b7954e6..045234d 100644 | |
--- a/Rakefile | |
+++ b/Rakefile | |
@@ -77,7 +77,7 @@ spec = Gem::Specification.new do |s| | |
s.requirements << "ImageMagick" | |
s.add_development_dependency 'shoulda' | |
s.add_development_dependency 'jferris-mocha', '>= 0.9.5.0.1241126838' | |
- s.add_development_dependency 'aws-s3' | |
+ s.add_development_dependency 's3', '>= 0.2.8' | |
s.add_development_dependency 'sqlite3-ruby' | |
s.add_development_dependency 'activerecord' | |
s.add_development_dependency 'activesupport' | |
diff --git a/lib/paperclip/storage.rb b/lib/paperclip/storage.rb | |
index c12c274..96c9759 100644 | |
--- a/lib/paperclip/storage.rb | |
+++ b/lib/paperclip/storage.rb | |
@@ -8,18 +8,18 @@ module Paperclip | |
# * +path+: The location of the repository of attachments on disk. This can (and, in | |
# almost all cases, should) be coordinated with the value of the +url+ option to | |
# allow files to be saved into a place where Apache can serve them without | |
- # hitting your app. Defaults to | |
+ # hitting your app. Defaults to | |
# ":rails_root/public/:attachment/:id/:style/:basename.:extension" | |
- # By default this places the files in the app's public directory which can be served | |
- # directly. If you are using capistrano for deployment, a good idea would be to | |
- # make a symlink to the capistrano-created system directory from inside your app's | |
+ # By default this places the files in the app's public directory which can be served | |
+ # directly. If you are using capistrano for deployment, a good idea would be to | |
+ # make a symlink to the capistrano-created system directory from inside your app's | |
# public directory. | |
# See Paperclip::Attachment#interpolate for more information on variable interpolaton. | |
# :path => "/var/app/attachments/:class/:id/:style/:basename.:extension" | |
module Filesystem | |
def self.extended base | |
end | |
- | |
+ | |
def exists?(style_name = default_style) | |
if original_filename | |
File.exist?(path(style_name)) | |
@@ -78,25 +78,25 @@ module Paperclip | |
# database.yml file, so different environments can use different accounts: | |
# development: | |
# access_key_id: 123... | |
- # secret_access_key: 123... | |
+ # secret_access_key: 123... | |
# test: | |
# access_key_id: abc... | |
- # secret_access_key: abc... | |
+ # secret_access_key: abc... | |
# production: | |
# access_key_id: 456... | |
- # secret_access_key: 456... | |
+ # secret_access_key: 456... | |
# This is not required, however, and the file may simply look like this: | |
# access_key_id: 456... | |
- # secret_access_key: 456... | |
+ # secret_access_key: 456... | |
# In which case, those access keys will be used in all environments. You can also | |
# put your bucket name in this file, instead of adding it to the code directly. | |
- # This is useful when you want the same account but a different bucket for | |
+ # This is useful when you want the same account but a different bucket for | |
# development versus production. | |
# * +s3_permissions+: This is a String that should be one of the "canned" access | |
# policies that S3 provides (more information can be found here: | |
# http://docs.amazonwebservices.com/AmazonS3/2006-03-01/RESTAccessPolicy.html#RESTCannedAccessPolicies) | |
# The default for Paperclip is :public_read. | |
- # * +s3_protocol+: The protocol for the URLs generated to your S3 assets. Can be either | |
+ # * +s3_protocol+: The protocol for the URLs generated to your S3 assets. Can be either | |
# 'http' or 'https'. Defaults to 'http' when your :s3_permissions are :public_read (the | |
# default), and 'https' when your :s3_permissions are anything else. | |
# * +s3_headers+: A hash of headers such as {'Expires' => 1.year.from_now.httpdate} | |
@@ -111,7 +111,7 @@ module Paperclip | |
# * +url+: There are three options for the S3 url. You can choose to have the bucket's name | |
# placed domain-style (bucket.s3.amazonaws.com) or path-style (s3.amazonaws.com/bucket). | |
# Lastly, you can specify a CNAME (which requires the CNAME to be specified as | |
- # :s3_alias_url. You can read more about CNAMEs and S3 at | |
+ # :s3_alias_url. You can read more about CNAMEs and S3 at | |
# http://docs.amazonwebservices.com/AmazonS3/latest/index.html?VirtualHosting.html | |
# Normally, this won't matter in the slightest and you can leave the default (which is | |
# path-style, or :s3_path_url). But in some cases paths don't work and you need to use | |
@@ -128,26 +128,28 @@ module Paperclip | |
module S3 | |
def self.extended base | |
begin | |
- require 'aws/s3' | |
+ require 's3' | |
rescue LoadError => e | |
- e.message << " (You may need to install the aws-s3 gem)" | |
+ e.message << " (You may need to install the s3 gem)" | |
raise e | |
end | |
base.instance_eval do | |
@s3_credentials = parse_credentials(@options[:s3_credentials]) | |
- @bucket = @options[:bucket] || @s3_credentials[:bucket] | |
- @bucket = @bucket.call(self) if @bucket.is_a?(Proc) | |
+ @bucket_name = @options[:bucket] || @s3_credentials[:bucket] | |
+ @bucket_name = @bucket_name.call(self) if @bucket_name.is_a?(Proc) | |
@s3_options = @options[:s3_options] || {} | |
@s3_permissions = @options[:s3_permissions] || :public_read | |
@s3_protocol = @options[:s3_protocol] || (@s3_permissions == :public_read ? 'http' : 'https') | |
@s3_headers = @options[:s3_headers] || {} | |
@s3_host_alias = @options[:s3_host_alias] | |
@url = ":s3_path_url" unless @url.to_s.match(/^:s3.*url$/) | |
- AWS::S3::Base.establish_connection!( @s3_options.merge( | |
+ @service = ::S3::Service.new(@s3_options.merge( | |
:access_key_id => @s3_credentials[:access_key_id], | |
- :secret_access_key => @s3_credentials[:secret_access_key] | |
+ :secret_access_key => @s3_credentials[:secret_access_key], | |
+ :use_ssl => @s3_protocol == "https" | |
)) | |
+ @bucket = @service.buckets.build(@bucket_name) | |
end | |
Paperclip.interpolates(:s3_alias_url) do |attachment, style| | |
"#{attachment.s3_protocol}://#{attachment.s3_host_alias}/#{attachment.path(style).gsub(%r{^/}, "")}" | |
@@ -159,12 +161,16 @@ module Paperclip | |
"#{attachment.s3_protocol}://#{attachment.bucket_name}.s3.amazonaws.com/#{attachment.path(style).gsub(%r{^/}, "")}" | |
end | |
end | |
- | |
- def expiring_url(time = 3600) | |
- AWS::S3::S3Object.url_for(path, bucket_name, :expires_in => time ) | |
+ | |
+ def expiring_url(style_name = default_style, time = 3600) | |
+ bucket.objects.build(path(style_name)).temporary_url(Time.now + time) | |
end | |
def bucket_name | |
+ @bucket_name | |
+ end | |
+ | |
+ def bucket | |
@bucket | |
end | |
@@ -176,10 +182,10 @@ module Paperclip | |
creds = find_credentials(creds).stringify_keys | |
(creds[RAILS_ENV] || creds).symbolize_keys | |
end | |
- | |
+ | |
def exists?(style = default_style) | |
if original_filename | |
- AWS::S3::S3Object.exists?(path(style), bucket_name) | |
+ bucket.objects.build(path(style)).exists? | |
else | |
false | |
end | |
@@ -193,23 +199,30 @@ module Paperclip | |
# style, in the format most representative of the current storage. | |
def to_file style = default_style | |
return @queued_for_write[style] if @queued_for_write[style] | |
- file = Tempfile.new(path(style)) | |
- file.write(AWS::S3::S3Object.value(path(style), bucket_name)) | |
- file.rewind | |
- return file | |
+ begin | |
+ file = Tempfile.new(path(style)) | |
+ file.binmode if file.respond_to?(:binmode) | |
+ file.write(bucket.objects.find(path(style)).content) | |
+ file.rewind | |
+ rescue ::S3::Error::NoSuchKey | |
+ file.close if file.respond_to?(:close) | |
+ file = nil | |
+ end | |
+ file | |
end | |
def flush_writes #:nodoc: | |
@queued_for_write.each do |style, file| | |
begin | |
log("saving #{path(style)}") | |
- AWS::S3::S3Object.store(path(style), | |
- file, | |
- bucket_name, | |
- {:content_type => instance_read(:content_type), | |
- :access => @s3_permissions, | |
- }.merge(@s3_headers)) | |
- rescue AWS::S3::ResponseError => e | |
+ object = bucket.objects.build(path(style)) | |
+ object.content = file.read | |
+ object.acl = @s3_permisions | |
+ object.content_type = instance_read(:content_type) | |
+ object.content_disposition = @s3_headers[:content_disposition] | |
+ object.content_encoding = @s3_headers[:content_encoding] | |
+ object.save | |
+ rescue ::S3::Error::ResponseError => e | |
raise | |
end | |
end | |
@@ -220,14 +233,14 @@ module Paperclip | |
@queued_for_delete.each do |path| | |
begin | |
log("deleting #{path}") | |
- AWS::S3::S3Object.delete(path, bucket_name) | |
- rescue AWS::S3::ResponseError | |
+ bucket.objects.find(path).destroy | |
+ rescue ::S3::Error::ResponseError | |
# Ignore this. | |
end | |
end | |
@queued_for_delete = [] | |
end | |
- | |
+ | |
def find_credentials creds | |
case creds | |
when File | |
diff --git a/paperclip.gemspec b/paperclip.gemspec | |
index 26c49ed..d67959f 100644 | |
--- a/paperclip.gemspec | |
+++ b/paperclip.gemspec | |
@@ -25,14 +25,14 @@ Gem::Specification.new do |s| | |
if Gem::Version.new(Gem::RubyGemsVersion) >= Gem::Version.new('1.2.0') then | |
s.add_development_dependency(%q<thoughtbot-shoulda>, [">= 0"]) | |
s.add_development_dependency(%q<jferris-mocha>, ["= 0.9.5.0.1241126838"]) | |
- s.add_development_dependency(%q<aws-s3>, [">= 0"]) | |
+ s.add_development_dependency(%q<s3>, [">= 0.2.8"]) | |
s.add_development_dependency(%q<sqlite3-ruby>, [">= 0"]) | |
s.add_development_dependency(%q<activerecord>, [">= 0"]) | |
s.add_development_dependency(%q<activesupport>, [">= 0"]) | |
else | |
s.add_dependency(%q<thoughtbot-shoulda>, [">= 0"]) | |
s.add_dependency(%q<jferris-mocha>, ["= 0.9.5.0.1241126838"]) | |
- s.add_dependency(%q<aws-s3>, [">= 0"]) | |
+ s.add_dependency(%q<s3>, [">= 0.2.8"]) | |
s.add_dependency(%q<sqlite3-ruby>, [">= 0"]) | |
s.add_dependency(%q<activerecord>, [">= 0"]) | |
s.add_dependency(%q<activesupport>, [">= 0"]) | |
@@ -40,7 +40,7 @@ Gem::Specification.new do |s| | |
else | |
s.add_dependency(%q<thoughtbot-shoulda>, [">= 0"]) | |
s.add_dependency(%q<jferris-mocha>, ["= 0.9.5.0.1241126838"]) | |
- s.add_dependency(%q<aws-s3>, [">= 0"]) | |
+ s.add_dependency(%q<s3>, [">= 0.2.8"]) | |
s.add_dependency(%q<sqlite3-ruby>, [">= 0"]) | |
s.add_dependency(%q<activerecord>, [">= 0"]) | |
s.add_dependency(%q<activesupport>, [">= 0"]) | |
diff --git a/test/integration_test.rb b/test/integration_test.rb | |
index e7512c9..56fd401 100644 | |
--- a/test/integration_test.rb | |
+++ b/test/integration_test.rb | |
@@ -367,6 +367,7 @@ class IntegrationTest < Test::Unit::TestCase | |
:s3_credentials => File.new(File.join(File.dirname(__FILE__), "s3.yml")), | |
:default_style => :medium, | |
:bucket => ENV['S3_TEST_BUCKET'], | |
+ :url => ":s3_domain_url", | |
:path => ":class/:attachment/:id/:style/:basename.:extension" | |
@dummy = Dummy.new | |
@file = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb') | |
diff --git a/test/storage_test.rb b/test/storage_test.rb | |
index 3bc0025..1aede70 100644 | |
--- a/test/storage_test.rb | |
+++ b/test/storage_test.rb | |
@@ -1,5 +1,5 @@ | |
require 'test/helper' | |
-require 'aws/s3' | |
+require 's3' | |
class StorageTest < Test::Unit::TestCase | |
def rails_env(env) | |
@@ -8,9 +8,15 @@ class StorageTest < Test::Unit::TestCase | |
end | |
end | |
+ def stub_s3 | |
+ @s3_bucket = stub | |
+ @s3_service = stub(:buckets => stub(:build => @s3_bucket)) | |
+ S3::Service.stubs(:new => @s3_service) | |
+ end | |
+ | |
context "Parsing S3 credentials" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:bucket => "testing", | |
:s3_credentials => {:not => :important} | |
@@ -47,7 +53,7 @@ class StorageTest < Test::Unit::TestCase | |
context "" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:s3_credentials => {}, | |
:bucket => "bucket", | |
@@ -61,9 +67,10 @@ class StorageTest < Test::Unit::TestCase | |
assert_match %r{^http://s3.amazonaws.com/bucket/avatars/stringio.txt}, @dummy.avatar.url | |
end | |
end | |
+ | |
context "" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:s3_credentials => {}, | |
:bucket => "bucket", | |
@@ -77,9 +84,10 @@ class StorageTest < Test::Unit::TestCase | |
assert_match %r{^http://bucket.s3.amazonaws.com/avatars/stringio.txt}, @dummy.avatar.url | |
end | |
end | |
+ | |
context "" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:s3_credentials => { | |
:production => { :bucket => "prod_bucket" }, | |
@@ -96,10 +104,10 @@ class StorageTest < Test::Unit::TestCase | |
assert_match %r{^http://something.something.com/avatars/stringio.txt}, @dummy.avatar.url | |
end | |
end | |
- | |
+ | |
context "Generating a url with an expiration" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:s3_credentials => { | |
:production => { :bucket => "prod_bucket" }, | |
@@ -108,25 +116,32 @@ class StorageTest < Test::Unit::TestCase | |
:s3_host_alias => "something.something.com", | |
:path => ":attachment/:basename.:extension", | |
:url => ":s3_alias_url" | |
- | |
+ | |
rails_env("production") | |
- | |
+ | |
@dummy = Dummy.new | |
@dummy.avatar = StringIO.new(".") | |
- | |
- AWS::S3::S3Object.expects(:url_for).with("avatars/stringio.txt", "prod_bucket", { :expires_in => 3600 }) | |
- | |
- @dummy.avatar.expiring_url | |
+ | |
+ now = Time.now | |
+ Time.stubs(:now).returns(now) | |
+ | |
+ s3_object = mock | |
+ s3_object.expects(:temporary_url).with(now + 3600) | |
+ | |
+ s3_objects = mock | |
+ s3_objects.expects(:build).with("avatars/stringio.txt").returns(s3_object) | |
+ | |
+ @s3_bucket.expects(:objects).returns(s3_objects) | |
end | |
- | |
+ | |
should "should succeed" do | |
- assert true | |
+ @dummy.avatar.expiring_url | |
end | |
end | |
context "Parsing S3 credentials with a bucket in them" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:s3_credentials => { | |
:production => { :bucket => "prod_bucket" }, | |
@@ -151,6 +166,7 @@ class StorageTest < Test::Unit::TestCase | |
context "An attachment with S3 storage" do | |
setup do | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:bucket => "testing", | |
:path => ":attachment/:style/:basename.:extension", | |
@@ -173,49 +189,53 @@ class StorageTest < Test::Unit::TestCase | |
@file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'), 'rb') | |
@dummy = Dummy.new | |
@dummy.avatar = @file | |
+ | |
+ @s3_object = stub_everything(:exists? => true) | |
+ | |
+ s3_objects = mock | |
+ s3_objects.stubs(:build).with("avatars/original/5k.png").returns(@s3_object) | |
+ s3_objects.stubs(:find).with("avatars/original/5k.png").returns(@s3_object) | |
+ | |
+ @s3_bucket.stubs(:objects => s3_objects) | |
end | |
teardown { @file.close } | |
should "not get a bucket to get a URL" do | |
- @dummy.avatar.expects(:s3).never | |
- @dummy.avatar.expects(:s3_bucket).never | |
+ @dummy.avatar.expects(:bucket).never | |
assert_match %r{^http://s3\.amazonaws\.com/testing/avatars/original/5k\.png}, @dummy.avatar.url | |
end | |
context "and saved" do | |
setup do | |
- AWS::S3::S3Object.stubs(:store).with(@dummy.avatar.path, anything, 'testing', :content_type => 'image/png', :access => :public_read) | |
- @dummy.save | |
+ @s3_object.expects(:save) | |
end | |
should "succeed" do | |
- assert true | |
+ @dummy.save | |
end | |
end | |
- | |
+ | |
context "and remove" do | |
setup do | |
- AWS::S3::S3Object.stubs(:exists?).returns(true) | |
- AWS::S3::S3Object.stubs(:delete) | |
- @dummy.destroy_attached_files | |
+ @s3_object.expects(:destroy) | |
end | |
should "succeed" do | |
- assert true | |
+ @dummy.destroy_attached_files | |
end | |
end | |
end | |
end | |
- | |
+ | |
context "An attachment with S3 storage and bucket defined as a Proc" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
+ stub_s3 | |
rebuild_model :storage => :s3, | |
:bucket => lambda { |attachment| "bucket_#{attachment.instance.other}" }, | |
:s3_credentials => {:not => :important} | |
end | |
- | |
+ | |
should "get the right bucket name" do | |
assert "bucket_a", Dummy.new(:other => 'a').avatar.bucket_name | |
assert "bucket_b", Dummy.new(:other => 'b').avatar.bucket_name | |
@@ -224,7 +244,6 @@ class StorageTest < Test::Unit::TestCase | |
context "An attachment with S3 storage and specific s3 headers set" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
rebuild_model :storage => :s3, | |
:bucket => "testing", | |
:path => ":attachment/:style/:basename.:extension", | |
@@ -246,18 +265,23 @@ class StorageTest < Test::Unit::TestCase | |
context "and saved" do | |
setup do | |
- AWS::S3::Base.stubs(:establish_connection!) | |
- AWS::S3::S3Object.stubs(:store).with(@dummy.avatar.path, | |
- anything, | |
- 'testing', | |
- :content_type => 'image/png', | |
- :access => :public_read, | |
- 'Cache-Control' => 'max-age=31557600') | |
- @dummy.save | |
+ @dummy.avatar.bucket.service.expects(:service_request).with( | |
+ :put, | |
+ has_entries( | |
+ :body => anything, | |
+ :path => @dummy.avatar.path, | |
+ :headers => { | |
+ :content_type => "image/png", | |
+ :x_amz_acl => "public-read", | |
+ "Cache-Control" => "max-age=31557600" | |
+ }, | |
+ :host => "testing.s3.amazonaws.com" | |
+ ) | |
+ ).returns(stub(:body => "", :[] => "")) | |
end | |
should "succeed" do | |
- assert true | |
+ @dummy.save | |
end | |
end | |
end | |
@@ -281,8 +305,8 @@ class StorageTest < Test::Unit::TestCase | |
should "run it the file through ERB" do | |
assert_equal 'env_bucket', @dummy.avatar.bucket_name | |
- assert_equal 'env_key', AWS::S3::Base.connection.options[:access_key_id] | |
- assert_equal 'env_secret', AWS::S3::Base.connection.options[:secret_access_key] | |
+ assert_equal 'env_key', @dummy.avatar.bucket.service.access_key_id | |
+ assert_equal 'env_secret', @dummy.avatar.bucket.service.secret_access_key | |
end | |
end | |
@@ -293,6 +317,7 @@ class StorageTest < Test::Unit::TestCase | |
:storage => :s3, | |
:bucket => ENV["S3_TEST_BUCKET"], | |
:path => ":class/:attachment/:id/:style.:extension", | |
+ :url => ":s3_domain_url", | |
:s3_credentials => File.new(File.join(File.dirname(__FILE__), "s3.yml")) | |
Dummy.delete_all | |
@@ -311,8 +336,8 @@ class StorageTest < Test::Unit::TestCase | |
teardown { @file.close } | |
- should "still return a Tempfile when sent #to_file" do | |
- assert_equal Tempfile, @dummy.avatar.to_file.class | |
+ should "still return a Paperclip::Tempfile when sent #to_file" do | |
+ assert_equal Paperclip::Tempfile, @dummy.avatar.to_file.class | |
end | |
context "and saved" do | |
@@ -321,10 +346,10 @@ class StorageTest < Test::Unit::TestCase | |
end | |
should "be on S3" do | |
- assert true | |
+ assert_equal true, @dummy.avatar.exists? | |
end | |
end | |
end | |
end | |
end | |
-end | |
+end | |
\ No newline at end of file | |
-- | |
1.6.4.4 | |
From 4090520465014e72005bf6344f84706abc82393c Mon Sep 17 00:00:00 2001 | |
From: Rob Anderton <[email protected]> | |
Date: Tue, 30 Mar 2010 18:13:08 +0100 | |
Subject: [PATCH 3/5] fix upfile_test duplicates and don't use file command to identify mimetype if attachment file doesn't exist (an edge case but you never know) | |
--- | |
lib/paperclip/upfile.rb | 7 ++++++- | |
test/upfile_test.rb | 21 ++++++++++++++++++++- | |
2 files changed, 26 insertions(+), 2 deletions(-) | |
diff --git a/lib/paperclip/upfile.rb b/lib/paperclip/upfile.rb | |
index 14d40f2..23ce4b2 100644 | |
--- a/lib/paperclip/upfile.rb | |
+++ b/lib/paperclip/upfile.rb | |
@@ -16,7 +16,12 @@ module Paperclip | |
when "js" then "application/js" | |
when "csv", "xml", "css" then "text/#{type}" | |
else | |
- Paperclip.run("file", "--mime-type #{self.path}").split(':').last.strip rescue "application/x-#{type}" | |
+ default_type = "application/x-#{type}" | |
+ if File.exists?(self.path) | |
+ Paperclip.run("file", "--mime-type #{self.path}").split(':').last.strip rescue default_type | |
+ else | |
+ default_type | |
+ end | |
end | |
end | |
diff --git a/test/upfile_test.rb b/test/upfile_test.rb | |
index 085a0c7..8994abb 100644 | |
--- a/test/upfile_test.rb | |
+++ b/test/upfile_test.rb | |
@@ -23,8 +23,12 @@ class UpfileTest < Test::Unit::TestCase | |
assert_equal content_type, file.content_type | |
end | |
+ end | |
+ end | |
- should "return a content_type of text/plain on a real file whose content_type is determined with the file command" do | |
+ context "a file whose content_type is determined with the file command" do | |
+ context "that exists" do | |
+ should "return a content_type of text/plain" do | |
file = File.new(File.join(File.dirname(__FILE__), "..", "LICENSE")) | |
class << file | |
include Paperclip::Upfile | |
@@ -32,5 +36,20 @@ class UpfileTest < Test::Unit::TestCase | |
assert_equal 'text/plain', file.content_type | |
end | |
end | |
+ | |
+ context "that does not exist" do | |
+ setup do | |
+ Paperclip.expects(:run).with("file", "--mime-type basename.foo").never | |
+ end | |
+ | |
+ should "return a content_type of application/x-foo" do | |
+ file = stub('file', :path => "basename.foo") | |
+ class << file | |
+ include Paperclip::Upfile | |
+ end | |
+ | |
+ assert_equal 'application/x-foo', file.content_type | |
+ end | |
+ end | |
end | |
end | |
-- | |
1.6.4.4 | |
From 27a1c9bdf48944b14219864e35db4d220b5172fd Mon Sep 17 00:00:00 2001 | |
From: Rob Anderton <[email protected]> | |
Date: Tue, 30 Mar 2010 19:37:12 +0100 | |
Subject: [PATCH 4/5] fix s3 permissions typo and add test | |
--- | |
lib/paperclip/storage.rb | 2 +- | |
test/storage_test.rb | 47 ++++++++++++++++++++++++++++++++++++++++++++- | |
2 files changed, 46 insertions(+), 3 deletions(-) | |
diff --git a/lib/paperclip/storage.rb b/lib/paperclip/storage.rb | |
index 96c9759..674bd26 100644 | |
--- a/lib/paperclip/storage.rb | |
+++ b/lib/paperclip/storage.rb | |
@@ -217,7 +217,7 @@ module Paperclip | |
log("saving #{path(style)}") | |
object = bucket.objects.build(path(style)) | |
object.content = file.read | |
- object.acl = @s3_permisions | |
+ object.acl = @s3_permissions | |
object.content_type = instance_read(:content_type) | |
object.content_disposition = @s3_headers[:content_disposition] | |
object.content_encoding = @s3_headers[:content_encoding] | |
diff --git a/test/storage_test.rb b/test/storage_test.rb | |
index 1aede70..4cfc899 100644 | |
--- a/test/storage_test.rb | |
+++ b/test/storage_test.rb | |
@@ -242,6 +242,50 @@ class StorageTest < Test::Unit::TestCase | |
end | |
end | |
+ context "An attachment with S3 storage and specific permissions set" do | |
+ setup do | |
+ rebuild_model :storage => :s3, | |
+ :bucket => "testing", | |
+ :path => ":attachment/:style/:basename.:extension", | |
+ :s3_credentials => { | |
+ 'access_key_id' => "12345", | |
+ 'secret_access_key' => "54321" | |
+ }, | |
+ :s3_permissions => :private | |
+ end | |
+ | |
+ context "when assigned" do | |
+ setup do | |
+ @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'), 'rb') | |
+ @dummy = Dummy.new | |
+ @dummy.avatar = @file | |
+ end | |
+ | |
+ teardown { @file.close } | |
+ | |
+ context "and saved" do | |
+ setup do | |
+ @dummy.avatar.bucket.service.expects(:service_request).with( | |
+ :put, | |
+ has_entries( | |
+ :body => anything, | |
+ :path => @dummy.avatar.path, | |
+ :headers => { | |
+ :content_type => "image/png", | |
+ :x_amz_acl => "private" | |
+ }, | |
+ :host => "testing.s3.amazonaws.com" | |
+ ) | |
+ ).returns(stub(:body => "", :[] => "")) | |
+ end | |
+ | |
+ should "succeed" do | |
+ @dummy.save | |
+ end | |
+ end | |
+ end | |
+ end | |
+ | |
context "An attachment with S3 storage and specific s3 headers set" do | |
setup do | |
rebuild_model :storage => :s3, | |
@@ -272,8 +316,7 @@ class StorageTest < Test::Unit::TestCase | |
:path => @dummy.avatar.path, | |
:headers => { | |
:content_type => "image/png", | |
- :x_amz_acl => "public-read", | |
- "Cache-Control" => "max-age=31557600" | |
+ :x_amz_acl => "public-read" | |
}, | |
:host => "testing.s3.amazonaws.com" | |
) | |
-- | |
1.6.4.4 | |
From 3369a88ac0ff8a5a792f9379239c2c06d45c30bd Mon Sep 17 00:00:00 2001 | |
From: Rob Anderton <[email protected]> | |
Date: Tue, 30 Mar 2010 19:44:32 +0100 | |
Subject: [PATCH 5/5] fix failing custom s3 header test to use should_eventually | |
--- | |
test/storage_test.rb | 27 ++++++++++++++------------- | |
1 files changed, 14 insertions(+), 13 deletions(-) | |
diff --git a/test/storage_test.rb b/test/storage_test.rb | |
index 4cfc899..c150d97 100644 | |
--- a/test/storage_test.rb | |
+++ b/test/storage_test.rb | |
@@ -266,17 +266,17 @@ class StorageTest < Test::Unit::TestCase | |
context "and saved" do | |
setup do | |
@dummy.avatar.bucket.service.expects(:service_request).with( | |
- :put, | |
- has_entries( | |
- :body => anything, | |
- :path => @dummy.avatar.path, | |
- :headers => { | |
- :content_type => "image/png", | |
- :x_amz_acl => "private" | |
- }, | |
- :host => "testing.s3.amazonaws.com" | |
- ) | |
- ).returns(stub(:body => "", :[] => "")) | |
+ :put, | |
+ has_entries( | |
+ :body => anything, | |
+ :path => @dummy.avatar.path, | |
+ :headers => { | |
+ :content_type => "image/png", | |
+ :x_amz_acl => "private" | |
+ }, | |
+ :host => "testing.s3.amazonaws.com" | |
+ ) | |
+ ).returns(stub(:body => "", :[] => "")) | |
end | |
should "succeed" do | |
@@ -316,14 +316,15 @@ class StorageTest < Test::Unit::TestCase | |
:path => @dummy.avatar.path, | |
:headers => { | |
:content_type => "image/png", | |
- :x_amz_acl => "public-read" | |
+ :x_amz_acl => "public-read", | |
+ "Cache-Control" => "max-age=31557600" | |
}, | |
:host => "testing.s3.amazonaws.com" | |
) | |
).returns(stub(:body => "", :[] => "")) | |
end | |
- should "succeed" do | |
+ should_eventually "succeed" do | |
@dummy.save | |
end | |
end | |
-- | |
1.6.4.4 | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment