Skip to content

Commit

Permalink
Applied PR thoughtbot#1903 from thoughtbot/paperclip
Browse files Browse the repository at this point in the history
curl -L https://github.com/thoughtbot/paperclip/pull/1903.patch > 1903.patch
git apply --3way 1903.patch

Fixed conflict in lib/papeclip/storage/s3.rb by adding .freeze to string
literals in http/https prefixing.
  • Loading branch information
bheeshmar committed Apr 18, 2017
1 parent 15af98d commit 78ea6dc
Show file tree
Hide file tree
Showing 15 changed files with 532 additions and 197 deletions.
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ gemfile:
- gemfiles/3.2.gemfile
- gemfiles/4.1.gemfile
- gemfiles/4.2.gemfile
- gemfiles/3.2.awsv1.gemfile
- gemfiles/4.1.awsv1.gemfile
- gemfiles/4.2.awsv1.gemfile

matrix:
fast_finish: true
Expand Down
18 changes: 18 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,11 +1,29 @@
appraise "3.2" do
gem "rails", "~> 3.2.0"
gem "aws-sdk", "~> 2.0"
end

appraise "4.1" do
gem "rails", "~> 4.1.0"
gem "aws-sdk", "~> 2.0"
end

appraise "4.2" do
gem "rails", "~> 4.2.0"
gem "aws-sdk", "~> 2.0"
end

appraise "3.2.awsv1" do
gem "rails", "~> 3.2.0"
gem "aws-sdk", "~> 1.5"
end

appraise "4.1.awsv1" do
gem "rails", "~> 4.1.0"
gem "aws-sdk", "~> 1.5"
end

appraise "4.2.awsv1" do
gem "rails", "~> 4.2.0"
gem "aws-sdk", "~> 1.5"
end
1 change: 1 addition & 0 deletions features/basic_integration.feature
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Feature: Rails integration
bucket: paperclip
access_key_id: access_key
secret_access_key: secret_key
s3_region: us-west-2
"""
And I start the rails application
When I go to the new user page
Expand Down
8 changes: 6 additions & 2 deletions features/step_definitions/s3_steps.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
When /^I attach the file "([^"]*)" to "([^"]*)" on S3$/ do |file_path, field|
definition = Paperclip::AttachmentRegistry.definitions_for(User)[field.downcase.to_sym]
path = "https://paperclip.s3.amazonaws.com#{definition[:path]}"
path = if defined?(::AWS)
"https://paperclip.s3.amazonaws.com#{definition[:path]}"
else
"https://paperclip.s3-us-west-2.amazonaws.com#{definition[:path]}"
end
path.gsub!(':filename', File.basename(file_path))
path.gsub!(/:([^\/\.]+)/) do |match|
"([^\/\.]+)"
end
FakeWeb.register_uri(:put, Regexp.new(path), :body => "OK")
FakeWeb.register_uri(:put, Regexp.new(path), :body => defined?(::AWS) ? "OK" : "<xml></xml>")
step "I attach the file \"#{file_path}\" to \"#{field}\""
end

Expand Down
20 changes: 20 additions & 0 deletions gemfiles/3.2.awsv1.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "sqlite3", "~> 1.3.8", :platforms => :ruby
gem "jruby-openssl", :platforms => :jruby
gem "activerecord-jdbcsqlite3-adapter", :platforms => :jruby
gem "rubysl", :platforms => :rbx
gem "racc", :platforms => :rbx
gem "pry"
gem "rails", "~> 3.2.0"
gem "aws-sdk", "~> 1.5"

group :development, :test do
gem "mime-types", "~> 1.16"
gem "builder"
gem "rubocop", :require => false
end

gemspec :path => "../"
1 change: 1 addition & 0 deletions gemfiles/3.2.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ gem "rubysl", :platforms => :rbx
gem "racc", :platforms => :rbx
gem "pry"
gem "rails", "~> 3.2.0"
gem "aws-sdk", "~> 2.0"

group :development, :test do
gem "mime-types", "~> 1.16"
Expand Down
20 changes: 20 additions & 0 deletions gemfiles/4.1.awsv1.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "sqlite3", "~> 1.3.8", :platforms => :ruby
gem "jruby-openssl", :platforms => :jruby
gem "activerecord-jdbcsqlite3-adapter", :platforms => :jruby
gem "rubysl", :platforms => :rbx
gem "racc", :platforms => :rbx
gem "pry"
gem "rails", "~> 4.1.0"
gem "aws-sdk", "~> 1.5"

group :development, :test do
gem "mime-types", "~> 1.16"
gem "builder"
gem "rubocop", :require => false
end

gemspec :path => "../"
1 change: 1 addition & 0 deletions gemfiles/4.1.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ gem "rubysl", :platforms => :rbx
gem "racc", :platforms => :rbx
gem "pry"
gem "rails", "~> 4.1.0"
gem "aws-sdk", "~> 2.0"

group :development, :test do
gem "mime-types", "~> 1.16"
Expand Down
20 changes: 20 additions & 0 deletions gemfiles/4.2.awsv1.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "sqlite3", "~> 1.3.8", :platforms => :ruby
gem "jruby-openssl", :platforms => :jruby
gem "activerecord-jdbcsqlite3-adapter", :platforms => :jruby
gem "rubysl", :platforms => :rbx
gem "racc", :platforms => :rbx
gem "pry"
gem "rails", "~> 4.2.0"
gem "aws-sdk", "~> 1.5"

group :development, :test do
gem "mime-types", "~> 1.16"
gem "builder"
gem "rubocop", :require => false
end

gemspec :path => "../"
1 change: 1 addition & 0 deletions gemfiles/4.2.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ gem "rubysl", :platforms => :rbx
gem "racc", :platforms => :rbx
gem "pry"
gem "rails", "~> 4.2.0"
gem "aws-sdk", "~> 2.0"

group :development, :test do
gem "mime-types", "~> 1.16"
Expand Down
105 changes: 80 additions & 25 deletions lib/paperclip/storage/s3.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ module Storage
# * +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.aws.amazon.com/AmazonS3/latest/dev/ACLOverview.html)
# The default for Paperclip is :public_read.
# The default for Paperclip is :public_read (aws-sdk v1) / public-read (aws-sdk v2).
#
# You can set permission on a per style bases by doing the following:
# :s3_permissions => {
Expand Down Expand Up @@ -93,6 +93,7 @@ module Storage
# S3 (strictly speaking) does not support directories, you can still use a / to
# separate parts of your file name.
# * +s3_host_name+: If you are using your bucket in Tokyo region etc, write host_name.
# * +s3_region+: For aws-sdk v2, s3_region is required.
# * +s3_metadata+: These key/value pairs will be stored with the
# object. This option works by prefixing each key with
# "x-amz-meta-" before sending it as a header on the object
Expand All @@ -114,20 +115,26 @@ module S3
def self.extended base
begin
require 'aws-sdk'
const_set('AWS_CLASS', defined?(::Aws) ? ::Aws : ::AWS)
const_set('AWS_BASE_ERROR',
defined?(::Aws) ? Aws::Errors::ServiceError : AWS::Errors::Base)
const_set('DEFAULT_PERMISSION',
defined?(::AWS) ? :public_read : :'public-read')

rescue LoadError => e
e.message << " (You may need to install the aws-sdk gem)"
raise e
end unless defined?(AWS::Core)
end unless defined?(AWS_CLASS)

# Overriding log formatter to make sure it return a UTF-8 string
if defined?(AWS::Core::LogFormatter)
AWS::Core::LogFormatter.class_eval do
if defined?(AWS_CLASS::Core::LogFormatter)
AWS_CLASS::Core::LogFormatter.class_eval do
def summarize_hash(hash)
hash.map { |key, value| ":#{key}=>#{summarize_value(value)}".force_encoding('UTF-8') }.sort.join(',')
end
end
elsif defined?(AWS::Core::ClientLogging)
AWS::Core::ClientLogging.class_eval do
elsif defined?(AWS_CLASS::Core::ClientLogging)
AWS_CLASS::Core::ClientLogging.class_eval do
def sanitize_hash(hash)
hash.map { |key, value| "#{sanitize_value(key)}=>#{sanitize_value(value)}".force_encoding('UTF-8') }.sort.join(',')
end
Expand All @@ -141,15 +148,15 @@ def sanitize_hash(hash)
Proc.new do |style, attachment|
permission = (@s3_permissions[style.to_s.to_sym] || @s3_permissions[:default])
permission = permission.call(attachment, style) if permission.respond_to?(:call)
(permission == :public_read) ? 'http'.freeze : 'https'.freeze
(permission == DEFAULT_PERMISSION) ? 'http'.freeze : 'https'.freeze
end
@s3_metadata = @options[:s3_metadata] || {}
@s3_headers = {}
merge_s3_headers(@options[:s3_headers], @s3_headers, @s3_metadata)

@s3_storage_class = set_storage_class(@options[:s3_storage_class])

@s3_server_side_encryption = :aes256
@s3_server_side_encryption = "AES256"
if @options[:s3_server_side_encryption].blank?
@s3_server_side_encryption = false
end
Expand Down Expand Up @@ -182,8 +189,13 @@ def sanitize_hash(hash)

def expiring_url(time = 3600, style_name = default_style)
if path(style_name)
base_options = { :expires => time, :secure => use_secure_protocol?(style_name) }
s3_object(style_name).url_for(:read, base_options.merge(s3_url_options)).to_s
if aws_v1?
base_options = { :expires => time, :secure => use_secure_protocol?(style_name) }
s3_object(style_name).url_for(:read, base_options.merge(s3_url_options)).to_s
else
base_options = { :expires_in => time }
s3_object(style_name).presigned_url(:get, base_options.merge(s3_url_options)).to_s
end
else
url(style_name)
end
Expand All @@ -200,6 +212,13 @@ def s3_host_name
host_name || s3_credentials[:s3_host_name] || "s3.amazonaws.com".freeze
end

def s3_region
region = @options[:s3_region]
region = region.call(self) if region.is_a?(Proc)

region || s3_credentials[:s3_region]
end

def s3_host_alias
@s3_host_alias = @options[:s3_host_alias]
@s3_host_alias = @s3_host_alias.call(self) if @s3_host_alias.respond_to?(:call)
Expand All @@ -220,7 +239,11 @@ def bucket_name

def s3_interface
@s3_interface ||= begin
config = { :s3_endpoint => s3_host_name }
config = if aws_v1?
{ :s3_endpoint => s3_host_name }
else
{ :region => s3_region }
end

if using_http_proxy?

Expand All @@ -234,7 +257,7 @@ def s3_interface
config[:proxy_uri] = URI::HTTP.build(proxy_opts)
end

[:access_key_id, :secret_access_key, :credential_provider].each do |opt|
[:access_key_id, :secret_access_key, :credential_provider, :credentials].each do |opt|
config[opt] = s3_credentials[opt] if s3_credentials[opt]
end

Expand All @@ -244,15 +267,31 @@ def s3_interface

def obtain_s3_instance_for(options)
instances = (Thread.current[:paperclip_s3_instances] ||= {})
instances[options] ||= AWS::S3.new(options)
instances[options] ||= if aws_v1?
AWS_CLASS::S3.new(options)
else
AWS_CLASS::S3::Resource.new(options)
end
end

def s3_bucket
@s3_bucket ||= s3_interface.buckets[bucket_name]
@s3_bucket ||= if aws_v1?
s3_interface.buckets[bucket_name]
else
s3_interface.bucket(bucket_name)
end
end

def style_name_as_path(style_name)
path(style_name).sub(%r{\A/},'')
end

def s3_object style_name = default_style
s3_bucket.objects[path(style_name).sub(%r{\A/},'')]
if aws_v1?
s3_bucket.objects[style_name_as_path(style_name)]
else
s3_bucket.object(style_name_as_path(style_name))
end
end

def using_http_proxy?
Expand All @@ -277,7 +316,7 @@ def http_proxy_password

def set_permissions permissions
permissions = { :default => permissions } unless permissions.respond_to?(:merge)
permissions.merge :default => (permissions[:default] || :public_read)
permissions.merge :default => (permissions[:default] || DEFAULT_PERMISSION)
end

def set_storage_class(storage_class)
Expand All @@ -297,7 +336,7 @@ def exists?(style = default_style)
else
false
end
rescue AWS::Errors::Base => e
rescue AWS_BASE_ERROR => e
false
end

Expand All @@ -323,7 +362,11 @@ def s3_protocol(style = default_style, with_colon = false)
end

def create_bucket
s3_interface.buckets.create(bucket_name)
if aws_v1?
s3_interface.buckets.create(bucket_name)
else
s3_interface.bucket(bucket_name).create
end
end

def flush_writes #:nodoc:
Expand Down Expand Up @@ -356,11 +399,15 @@ def flush_writes #:nodoc:
write_options[:metadata] = @s3_metadata unless @s3_metadata.empty?
write_options.merge!(@s3_headers)

s3_object(style).write(file, write_options)
rescue AWS::S3::Errors::NoSuchBucket
if aws_v1?
s3_object(style).write(file, write_options)
else
s3_object(style).upload_file(file.path, write_options)
end
rescue AWS_CLASS::S3::Errors::NoSuchBucket
create_bucket
retry
rescue AWS::S3::Errors::SlowDown
rescue AWS_CLASS::S3::Errors::SlowDown
retries += 1
if retries <= 5
sleep((2 ** retries) * 0.5)
Expand All @@ -382,8 +429,12 @@ def flush_deletes #:nodoc:
@queued_for_delete.each do |path|
begin
log("deleting #{path}")
s3_bucket.objects[path.sub(%r{\A/},'')].delete
rescue AWS::Errors::Base => e
if aws_v1?
s3_bucket.objects[path.sub(%r{\A/},'')]
else
s3_bucket.object(path.sub(%r{\A/},''))
end.delete
rescue AWS_BASE_ERROR => e
# Ignore this.
end
end
Expand All @@ -393,17 +444,21 @@ def flush_deletes #:nodoc:
def copy_to_local_file(style, local_dest_path)
log("copying #{path(style)} to local file #{local_dest_path}")
::File.open(local_dest_path, 'wb') do |local_file|
s3_object(style).read do |chunk|
s3_object(style).send(aws_v1? ? :read : :get) do |chunk|
local_file.write(chunk)
end
end
rescue AWS::Errors::Base => e
rescue AWS_BASE_ERROR => e
warn("#{e} - cannot copy #{path(style)} to local file #{local_dest_path}")
false
end

private

def aws_v1?
Gem::Version.new(AWS_CLASS::VERSION) < Gem::Version.new(2)
end

def find_credentials creds
case creds
when File
Expand Down
Loading

0 comments on commit 78ea6dc

Please sign in to comment.