From 95a99380e5f953bcedea8ffaa01ddf5658dda168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petteri=20K=C3=A4=C3=A4p=C3=A4?= Date: Sun, 24 May 2015 15:23:36 +0300 Subject: [PATCH 1/3] Initial implementation of passing s3 options to put, copy_from and presigned_post (issue refile/refile-s3#3) --- lib/refile/s3.rb | 24 ++++++++++++++++++++---- spec/refile/s3_spec.rb | 41 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/lib/refile/s3.rb b/lib/refile/s3.rb index 59dcafb..95a6874 100644 --- a/lib/refile/s3.rb +++ b/lib/refile/s3.rb @@ -36,6 +36,8 @@ class S3 def initialize(access_key_id:, secret_access_key:, region:, bucket:, max_size: nil, prefix: nil, hasher: Refile::RandomHasher.new, **s3_options) @access_key_id = access_key_id @secret_access_key = secret_access_key + @s3_presigned_post_options = s3_options.delete(:s3_presigned_post_options) { {} } + @s3_object_operation_options = s3_options.delete(:s3_object_operation_options) { {} } @s3_options = { access_key_id: access_key_id, secret_access_key: secret_access_key, region: region }.merge s3_options @s3 = Aws::S3::Resource.new @s3_options @bucket_name = bucket @@ -52,12 +54,14 @@ def initialize(access_key_id:, secret_access_key:, region:, bucket:, max_size: n verify_uploadable def upload(uploadable) id = @hasher.hash(uploadable) - if uploadable.is_a?(Refile::File) and uploadable.backend.is_a?(S3) and uploadable.backend.access_key_id == access_key_id - object(id).copy_from(copy_source: [@bucket_name, uploadable.backend.object(uploadable.id).key].join("/")) + operation, options = if upload_via_copy_operation?(uploadable) + [:copy_from, { copy_source: [@bucket_name, uploadable.backend.object(uploadable.id).key].join("/") }] else - object(id).put(body: uploadable, content_length: uploadable.size) + [:put, { body: uploadable, content_length: uploadable.size }] end + object(id).send(operation, s3_object_operation_options(options)) + Refile::File.new(self, id) end @@ -137,11 +141,23 @@ def clear!(confirm = nil) # @return [Refile::Signature] def presign id = RandomHasher.new.hash - signature = @bucket.presigned_post(key: [*@prefix, id].join("/")) + signature = @bucket.presigned_post(s3_presigned_post_options(key: [*@prefix, id].join("/"))) signature.content_length_range(0..@max_size) if @max_size Signature.new(as: "file", id: id, url: signature.url.to_s, fields: signature.fields) end + def s3_presigned_post_options(options) + @s3_presigned_post_options.merge(options) + end + + def s3_object_operation_options(options) + @s3_object_operation_options.merge(options) + end + + def upload_via_copy_operation?(uploadable) + uploadable.is_a?(Refile::File) and uploadable.backend.is_a?(S3) and uploadable.backend.access_key_id == access_key_id + end + verify_id def object(id) @bucket.object([*@prefix, id].join("/")) end diff --git a/spec/refile/s3_spec.rb b/spec/refile/s3_spec.rb index ab8394f..cf609e8 100644 --- a/spec/refile/s3_spec.rb +++ b/spec/refile/s3_spec.rb @@ -3,10 +3,49 @@ WebMock.allow_net_connect! -config = YAML.load_file("s3.yml").map { |k, v| [k.to_sym, v] }.to_h +s3_config = YAML.load_file("s3.yml").map { |k, v| [k.to_sym, v] }.to_h RSpec.describe Refile::S3 do + let(:config) { s3_config } let(:backend) { Refile::S3.new(max_size: 100, **config) } it_behaves_like :backend + + describe '@s3_options' do + let(:value) { backend.instance_variable_get(:@s3_options) } + + it 'is initialized' do + %i(access_key_id secret_access_key region).each do |attribute| + expect(value[attribute]).to eq(config[attribute]) + end + end + end + + %i(s3_object_operation_options s3_presigned_post_options).each do |option| + describe "@#{option}" do + let(:additional_config) do + { "#{option}".to_sym => { server_side_encryption: 'aes256' } } + end + + let(:config) { additional_config.merge s3_config } + let(:value) { backend.send(:instance_variable_get, "@#{option}") } + + it 'is initialized' do + expect(value[:server_side_encryption]).to eq('aes256') + expect(backend.instance_variable_get(:@s3_options)).not_to have_key(option) + end + end + + describe ".#{option}" do + let(:additional_config) do + { "#{option}".to_sym => { server_side_encryption: 'aes256' } } + end + + let(:config) { additional_config.merge s3_config } + + it 'is merges provided options' do + expect(backend.send(option, { server_side_encryption: 'aws:kms' })[:server_side_encryption]).to eq('aws:kms') + end + end + end end From 8fb78f4406ffcb0e7aab311ba6a51e34caa49747 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petteri=20K=C3=A4=C3=A4p=C3=A4?= Date: Mon, 25 May 2015 19:08:10 +0300 Subject: [PATCH 2/3] Alternative implementation of passing s3 options to put, copy_from and presigned_post (issue refile/refile-s3#3) --- lib/refile/s3.rb | 24 ++++++------ spec/refile/s3_spec.rb | 85 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 82 insertions(+), 27 deletions(-) diff --git a/lib/refile/s3.rb b/lib/refile/s3.rb index 95a6874..5c6db30 100644 --- a/lib/refile/s3.rb +++ b/lib/refile/s3.rb @@ -21,6 +21,13 @@ class S3 attr_reader :access_key_id, :max_size + S3_AVAILABLE_OPTIONS = { + client: %i(access_key_id region secret_access_key), + copy_from: %i(copy_source server_side_encryption), + presigned_post: %i(key server_side_encryption), + put: %i(body content_length server_side_encryption) + } + # Sets up an S3 backend with the given credentials. # # @param [String] access_key_id @@ -36,10 +43,8 @@ class S3 def initialize(access_key_id:, secret_access_key:, region:, bucket:, max_size: nil, prefix: nil, hasher: Refile::RandomHasher.new, **s3_options) @access_key_id = access_key_id @secret_access_key = secret_access_key - @s3_presigned_post_options = s3_options.delete(:s3_presigned_post_options) { {} } - @s3_object_operation_options = s3_options.delete(:s3_object_operation_options) { {} } @s3_options = { access_key_id: access_key_id, secret_access_key: secret_access_key, region: region }.merge s3_options - @s3 = Aws::S3::Resource.new @s3_options + @s3 = Aws::S3::Resource.new s3_options_for(:client) @bucket_name = bucket @bucket = @s3.bucket @bucket_name @hasher = hasher @@ -60,7 +65,7 @@ def initialize(access_key_id:, secret_access_key:, region:, bucket:, max_size: n [:put, { body: uploadable, content_length: uploadable.size }] end - object(id).send(operation, s3_object_operation_options(options)) + object(id).send(operation, s3_options_for(operation, options)) Refile::File.new(self, id) end @@ -141,17 +146,14 @@ def clear!(confirm = nil) # @return [Refile::Signature] def presign id = RandomHasher.new.hash - signature = @bucket.presigned_post(s3_presigned_post_options(key: [*@prefix, id].join("/"))) + signature = @bucket.presigned_post(s3_options_for(:presigned_post, key: [*@prefix, id].join("/"))) signature.content_length_range(0..@max_size) if @max_size Signature.new(as: "file", id: id, url: signature.url.to_s, fields: signature.fields) end - def s3_presigned_post_options(options) - @s3_presigned_post_options.merge(options) - end - - def s3_object_operation_options(options) - @s3_object_operation_options.merge(options) + def s3_options_for(operation, options = {}) + keys = S3_AVAILABLE_OPTIONS.fetch(operation) + @s3_options.merge(options).select { |key, _| keys.include?(key) } end def upload_via_copy_operation?(uploadable) diff --git a/spec/refile/s3_spec.rb b/spec/refile/s3_spec.rb index cf609e8..0139065 100644 --- a/spec/refile/s3_spec.rb +++ b/spec/refile/s3_spec.rb @@ -19,32 +19,85 @@ expect(value[attribute]).to eq(config[attribute]) end end - end - - %i(s3_object_operation_options s3_presigned_post_options).each do |option| - describe "@#{option}" do - let(:additional_config) do - { "#{option}".to_sym => { server_side_encryption: 'aes256' } } - end - let(:config) { additional_config.merge s3_config } - let(:value) { backend.send(:instance_variable_get, "@#{option}") } + context 'given additional configuration options' do + let(:config) { { server_side_encryption: 'aes256' }.merge s3_config } it 'is initialized' do expect(value[:server_side_encryption]).to eq('aes256') - expect(backend.instance_variable_get(:@s3_options)).not_to have_key(option) end end + end - describe ".#{option}" do - let(:additional_config) do - { "#{option}".to_sym => { server_side_encryption: 'aes256' } } + describe '.s3_options_for' do + let(:config) do + { + access_key_id: 'xyz', + secret_access_key: 'abcd1234', + region: 'sa-east-1', + bucket: 'my-bucket', + server_side_encryption: 'aes256' + } + end + let(:options) { {} } + let(:value) { backend.s3_options_for key, options } + + describe 'given operation' do + context 'client' do + let(:key) { :client } + + it 'returns valid options' do + expect(value).to eq( + access_key_id: 'xyz', + secret_access_key: 'abcd1234', + region: 'sa-east-1' + ) + end end - let(:config) { additional_config.merge s3_config } + context 'copy_from' do + let(:key) { :copy_from } + let(:options) { { copy_source: 'xyz' } } + + it 'returns valid options' do + expect(value).to eq( + copy_source: 'xyz', + server_side_encryption: 'aes256' + ) + end + end + + context 'presigned_post' do + let(:key) { :presigned_post } + let(:options) { { key: 'xyz' } } + + it 'returns valid options' do + expect(value).to eq( + key: 'xyz', + server_side_encryption: 'aes256' + ) + end + end + + context 'put' do + let(:key) { :put } + let(:options) { { body: 'xyz', content_length: 3 } } + + it 'returns valid options' do + expect(value).to eq( + body: 'xyz', + content_length: 3, + server_side_encryption: 'aes256' + ) + end + end + end + + context 'given an invalid operation' do + let(:key) { :invalid } - it 'is merges provided options' do - expect(backend.send(option, { server_side_encryption: 'aws:kms' })[:server_side_encryption]).to eq('aws:kms') + it 'raises an error' do + expect { value }.to raise_error(KeyError) end end end From 6425083a182a90f70a46488c2b3ba0c308e6cb52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petteri=20K=C3=A4=C3=A4p=C3=A4?= Date: Mon, 8 Jun 2015 18:18:42 +0300 Subject: [PATCH 3/3] Add storage_class S3 option --- lib/refile/s3.rb | 6 +++--- spec/refile/s3_spec.rb | 12 ++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/refile/s3.rb b/lib/refile/s3.rb index 918c36d..6f08eb9 100644 --- a/lib/refile/s3.rb +++ b/lib/refile/s3.rb @@ -32,9 +32,9 @@ class S3 S3_AVAILABLE_OPTIONS = { client: %i(access_key_id region secret_access_key), - copy_from: %i(copy_source server_side_encryption), - presigned_post: %i(key server_side_encryption), - put: %i(body content_length server_side_encryption) + copy_from: %i(copy_source server_side_encryption storage_class), + presigned_post: %i(key server_side_encryption storage_class), + put: %i(body content_length server_side_encryption storage_class) } # Sets up an S3 backend diff --git a/spec/refile/s3_spec.rb b/spec/refile/s3_spec.rb index 0139065..5830093 100644 --- a/spec/refile/s3_spec.rb +++ b/spec/refile/s3_spec.rb @@ -36,7 +36,8 @@ secret_access_key: 'abcd1234', region: 'sa-east-1', bucket: 'my-bucket', - server_side_encryption: 'aes256' + server_side_encryption: 'aes256', + storage_class: 'STANDARD' } end let(:options) { {} } @@ -62,7 +63,8 @@ it 'returns valid options' do expect(value).to eq( copy_source: 'xyz', - server_side_encryption: 'aes256' + server_side_encryption: 'aes256', + storage_class: 'STANDARD' ) end end @@ -74,7 +76,8 @@ it 'returns valid options' do expect(value).to eq( key: 'xyz', - server_side_encryption: 'aes256' + server_side_encryption: 'aes256', + storage_class: 'STANDARD' ) end end @@ -87,7 +90,8 @@ expect(value).to eq( body: 'xyz', content_length: 3, - server_side_encryption: 'aes256' + server_side_encryption: 'aes256', + storage_class: 'STANDARD' ) end end