From 21919348c3ee3980dc7bd1bc1ff9e943be134735 Mon Sep 17 00:00:00 2001 From: Tim Assmann Date: Wed, 19 Oct 2011 14:54:33 +0200 Subject: [PATCH 1/4] added support for a proc to set s3_permission with specs, still needs a bit of work --- lib/paperclip/storage/s3.rb | 12 ++++++---- test/storage/s3_test.rb | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index 0a9102597..8736f26b6 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -80,8 +80,10 @@ def self.extended base @s3_options = @options.s3_options || {} @s3_permissions = set_permissions(@options.s3_permissions) @s3_protocol = @options.s3_protocol || - Proc.new do |style| - (@s3_permissions[style.to_sym] || @s3_permissions[:default]) == :public_read ? 'http' : 'https' + Proc.new do |style, attachment| + permission = (@s3_permissions[style.to_sym] || @s3_permissions[:default]) + permission = permission.call(attachment, style) if permission.is_a?(Proc) + permission == :public_read ? 'http' : 'https' end @s3_headers = @options.s3_headers || {} @@ -184,7 +186,7 @@ def exists?(style = default_style) def s3_protocol(style = default_style) if @s3_protocol.is_a?(Proc) - @s3_protocol.call(style) + @s3_protocol.call(style, self) else @s3_protocol end @@ -212,11 +214,13 @@ def flush_writes #:nodoc: @queued_for_write.each do |style, file| begin log("saving #{path(style)}") + s3_permission = @s3_permissions[style] || @s3_permissions[:default] + s3_permission = s3_permission.call(self, style) if s3_permission.is_a?(Proc) AWS::S3::S3Object.store(path(style), file, bucket_name, {:content_type => file.content_type.to_s.strip, - :access => (@s3_permissions[style] || @s3_permissions[:default]), + :access => (s3_permission), }.merge(@s3_headers)) rescue AWS::S3::NoSuchBucket => e create_bucket diff --git a/test/storage/s3_test.rb b/test/storage/s3_test.rb index a50ac1079..6641a808f 100644 --- a/test/storage/s3_test.rb +++ b/test/storage/s3_test.rb @@ -632,5 +632,53 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError end end end + + context "proc permission set" do + setup do + rebuild_model :storage => :s3, + :bucket => "testing", + :path => ":attachment/:style/:basename.:extension", + :styles => { + :thumb => "80x80>" + }, + :s3_credentials => { + 'access_key_id' => "12345", + 'secret_access_key' => "54321" + }, + :s3_permissions => lambda{|attachment, style| attachment.instance.private_attachment? && style.to_sym != :thumb ? 'private' : 'public-read' } + end + + context "when assigned" do + setup do + @file = File.new(File.join(File.dirname(__FILE__), '..', 'fixtures', '5k.png'), 'rb') + @dummy = Dummy.new + @dummy.stubs(:private_attachment? => true) + @dummy.avatar = @file + end + + teardown { @file.close } + + context "and saved" do + setup do + AWS::S3::Base.stubs(:establish_connection!) + [:thumb, :original].each do |style| + AWS::S3::S3Object.expects(:store).with("avatars/#{style}/5k.png", + anything, + 'testing', + :content_type => 'image/png', + :access => style == :thumb ? 'public-read' : 'private') + end + @dummy.save + end + + should "succeed" do + @dummy.avatar.url() =~ /^https:/ + @dummy.avatar.url(:thumb) =~ /^http:/ + assert true + end + end + end + + end end end From 15712902d8136fc0b46c3c8b26da51113a5cc1d5 Mon Sep 17 00:00:00 2001 From: Alexander Presber Date: Wed, 19 Oct 2011 17:18:04 +0200 Subject: [PATCH 2/4] require pathname formulate assert conditions --- test/helper.rb | 1 + test/storage/s3_test.rb | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/helper.rb b/test/helper.rb index 91f39488e..30ccdf01e 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -10,6 +10,7 @@ require 'active_support' require 'mime/types' require 'pry' +require 'pathname' puts "Testing against version #{ActiveRecord::VERSION::STRING}" diff --git a/test/storage/s3_test.rb b/test/storage/s3_test.rb index 6641a808f..701452fc7 100644 --- a/test/storage/s3_test.rb +++ b/test/storage/s3_test.rb @@ -672,9 +672,8 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError end should "succeed" do - @dummy.avatar.url() =~ /^https:/ - @dummy.avatar.url(:thumb) =~ /^http:/ - assert true + assert_equal 0, @dummy.avatar.url() =~ /^https:/ + assert_equal 0, @dummy.avatar.url(:thumb) =~ /^http:/ end end end From 3a6ca7d9c7ba6a6ccd751bb1a10721e3798b5b66 Mon Sep 17 00:00:00 2001 From: Alexander Presber Date: Thu, 20 Oct 2011 11:08:03 +0200 Subject: [PATCH 3/4] fixed wrong permissions ('private' and 'public-read') into the right ones (:private and :public_read) in the s3_test completed test for lambda permission value --- lib/paperclip/storage/s3.rb | 6 ++--- test/storage/s3_test.rb | 54 ++++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index 8736f26b6..ae1d097a2 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -81,9 +81,9 @@ def self.extended base @s3_permissions = set_permissions(@options.s3_permissions) @s3_protocol = @options.s3_protocol || Proc.new do |style, attachment| - permission = (@s3_permissions[style.to_sym] || @s3_permissions[:default]) - permission = permission.call(attachment, style) if permission.is_a?(Proc) - permission == :public_read ? 'http' : 'https' + permission = (@s3_permissions[style.to_sym] || @s3_permissions[:default]) + permission = permission.call(attachment, style) if permission.is_a?(Proc) + (permission == :public_read) ? 'http' : 'https' end @s3_headers = @options.s3_headers || {} diff --git a/test/storage/s3_test.rb b/test/storage/s3_test.rb index 701452fc7..07df50636 100644 --- a/test/storage/s3_test.rb +++ b/test/storage/s3_test.rb @@ -509,7 +509,7 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError end context "S3 Permissions" do - context "defaults to public-read" do + context "defaults to :public_read" do setup do rebuild_model :storage => :s3, :bucket => "testing", @@ -556,7 +556,7 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError 'access_key_id' => "12345", 'secret_access_key' => "54321" }, - :s3_permissions => 'private' + :s3_permissions => :private end context "when assigned" do @@ -575,7 +575,7 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError anything, 'testing', :content_type => 'image/png', - :access => 'private') + :access => :private) @dummy.save end @@ -599,8 +599,8 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError 'secret_access_key' => "54321" }, :s3_permissions => { - :original => 'private', - :thumb => 'public-read' + :original => :private, + :thumb => :public_read } end @@ -621,7 +621,7 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError anything, 'testing', :content_type => 'image/png', - :access => style == :thumb ? 'public-read' : 'private') + :access => style == :thumb ? :public_read : :private) end @dummy.save end @@ -635,17 +635,21 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError context "proc permission set" do setup do - rebuild_model :storage => :s3, - :bucket => "testing", - :path => ":attachment/:style/:basename.:extension", - :styles => { - :thumb => "80x80>" - }, - :s3_credentials => { - 'access_key_id' => "12345", - 'secret_access_key' => "54321" - }, - :s3_permissions => lambda{|attachment, style| attachment.instance.private_attachment? && style.to_sym != :thumb ? 'private' : 'public-read' } + rebuild_model( + :storage => :s3, + :bucket => "testing", + :path => ":attachment/:style/:basename.:extension", + :styles => { + :thumb => "80x80>" + }, + :s3_credentials => { + 'access_key_id' => "12345", + 'secret_access_key' => "54321" + }, + :s3_permissions => lambda {|attachment, style| + attachment.instance.private_attachment? && style.to_sym != :thumb ? :private : :public_read + } + ) end context "when assigned" do @@ -662,18 +666,20 @@ class AWS::S3::NoSuchBucket < AWS::S3::ResponseError setup do AWS::S3::Base.stubs(:establish_connection!) [:thumb, :original].each do |style| - AWS::S3::S3Object.expects(:store).with("avatars/#{style}/5k.png", - anything, - 'testing', - :content_type => 'image/png', - :access => style == :thumb ? 'public-read' : 'private') + AWS::S3::S3Object.expects(:store).with( + "avatars/#{style}/5k.png", + anything, + 'testing', + :content_type => 'image/png', + :access => style == :thumb ? :public_read : :private + ) end @dummy.save end should "succeed" do - assert_equal 0, @dummy.avatar.url() =~ /^https:/ - assert_equal 0, @dummy.avatar.url(:thumb) =~ /^http:/ + assert @dummy.avatar.url().include? "https://" + assert @dummy.avatar.url(:thumb).include? "http://" end end end From f40969342eec12c278f0cc890643c024d87af423 Mon Sep 17 00:00:00 2001 From: Alexander Presber Date: Thu, 20 Oct 2011 17:12:39 +0200 Subject: [PATCH 4/4] expose s3_permissions analoguous to s3_protocol --- lib/paperclip/storage/s3.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index ae1d097a2..253cdb2d5 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -184,6 +184,12 @@ def exists?(style = default_style) end end + def s3_permissions(style = default_style) + s3_permissions = @s3_permissions[style] || @s3_permissions[:default] + s3_permissions = s3_permissions.call(self, style) if s3_permissions.is_a?(Proc) + s3_permissions + end + def s3_protocol(style = default_style) if @s3_protocol.is_a?(Proc) @s3_protocol.call(style, self) @@ -214,13 +220,11 @@ def flush_writes #:nodoc: @queued_for_write.each do |style, file| begin log("saving #{path(style)}") - s3_permission = @s3_permissions[style] || @s3_permissions[:default] - s3_permission = s3_permission.call(self, style) if s3_permission.is_a?(Proc) AWS::S3::S3Object.store(path(style), file, bucket_name, {:content_type => file.content_type.to_s.strip, - :access => (s3_permission), + :access => s3_permissions(style), }.merge(@s3_headers)) rescue AWS::S3::NoSuchBucket => e create_bucket