Skip to content

Commit

Permalink
added streaming support [#17]
Browse files Browse the repository at this point in the history
  • Loading branch information
Jakub Kuźma committed Sep 26, 2010
1 parent a0af780 commit f7fd538
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 28 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
s3 (0.3.3)
s3 (0.3.4)
proxies

GEM
Expand Down
1 change: 1 addition & 0 deletions Rakefile
@@ -1,5 +1,6 @@
require "bundler"
Bundler::GemHelper.install_tasks
Bundler.setup

require "rake/testtask"
require "rake/rdoctask"
Expand Down
1 change: 1 addition & 0 deletions lib/s3.rb
Expand Up @@ -15,6 +15,7 @@
require "s3/bucket"
require "s3/connection"
require "s3/exceptions"
require "s3/request"
require "s3/object"
require "s3/service"
require "s3/signature"
Expand Down
32 changes: 15 additions & 17 deletions lib/s3/connection.rb
Expand Up @@ -19,14 +19,18 @@ class Connection
# * <tt>:timeout</tt> - Timeout to use by the Net::HTTP object
# (60 by default)
# * <tt>:proxy</tt> - Hash for Net::HTTP Proxy settings
# { :host => "proxy.mydomain.com", :port => "80, :user => "user_a", :password => "secret" }
# { :host => "proxy.mydomain.com", :port => "80, :user => "user_a", :password => "secret" }
# * <tt>:proxy</tt> - Hash for Net::HTTP Proxy settings
# * <tt>:chunk_size</tt> - Size of a chunk when streaming
# (1048576 (1 MiB) by default)
def initialize(options = {})
@access_key_id = options.fetch(:access_key_id)
@secret_access_key = options.fetch(:secret_access_key)
@use_ssl = options.fetch(:use_ssl, false)
@debug = options.fetch(:debug, false)
@timeout = options.fetch(:timeout, 60)
@proxy = options.fetch(:proxy, nil)
@chunk_size = options.fetch(:chunk_size, 1048576)
end

# Makes request with given HTTP method, sets missing parameters,
Expand All @@ -53,7 +57,7 @@ def initialize(options = {})
def request(method, options)
host = options.fetch(:host, HOST)
path = options.fetch(:path)
body = options.fetch(:body, "")
body = options.fetch(:body, nil)
params = options.fetch(:params, {})
headers = options.fetch(:headers, {})

Expand All @@ -63,14 +67,21 @@ def request(method, options)
end

path = URI.escape(path)
request = request_class(method).new(path)
request = Request.new(@chunk_size, method.to_s.upcase, !!body, true, path)

headers = self.class.parse_headers(headers)
headers.each do |key, value|
request[key] = value
end

request.body = body
if body
if body.respond_to?(:read)
request.body_stream = body
else
request.body = body
end
request.content_length = body.respond_to?(:lstat) ? body.stat.size : body.size
end

send_request(host, request)
end
Expand Down Expand Up @@ -143,19 +154,6 @@ def self.parse_headers(headers)

private

def request_class(method)
case method
when :get
request_class = Net::HTTP::Get
when :head
request_class = Net::HTTP::Head
when :put
request_class = Net::HTTP::Put
when :delete
request_class = Net::HTTP::Delete
end
end

def port
use_ssl ? 443 : 80
end
Expand Down
7 changes: 3 additions & 4 deletions lib/s3/object.rb
Expand Up @@ -40,8 +40,8 @@ def key=(key)
def acl=(acl)
@acl = acl.to_s.gsub("_", "-") if acl
end
# Assigns a new storage class (RRS) to the object. Please note

# Assigns a new storage class (RRS) to the object. Please note
# that the storage class is not retrieved from the server and set
# to "STANDARD" by default.
#
Expand Down Expand Up @@ -185,8 +185,7 @@ def object_headers(options = {})
end

def put_object
body = content.is_a?(IO) ? content.read : content
response = object_request(:put, :body => body, :headers => dump_headers)
response = object_request(:put, :body => content, :headers => dump_headers)
parse_headers(response)
end

Expand Down
31 changes: 31 additions & 0 deletions lib/s3/request.rb
@@ -0,0 +1,31 @@
module S3
# Class responsible for sending chunked requests
# properly. Net::HTTPGenericRequest has hardcoded chunk_size, so we
# inherit the class and override chunk_size.
class Request < Net::HTTPGenericRequest
def initialize(chunk_size, m, reqbody, resbody, path, initheader = nil)
@chunk_size = chunk_size
super(m, reqbody, resbody, path, initheader)
end

private

def send_request_with_body_stream(sock, ver, path, f)
unless content_length() or chunked?
raise ArgumentError, "Content-Length not given and Transfer-Encoding is not `chunked'"
end
supply_default_content_type
write_header sock, ver, path
if chunked?
while s = f.read(@chunk_size)
sock.write(sprintf("%x\r\n", s.length) << s << "\r\n")
end
sock.write "0\r\n\r\n"
else
while s = f.read(@chunk_size)
sock.write s
end
end
end
end
end
6 changes: 5 additions & 1 deletion lib/s3/service.rb
Expand Up @@ -22,12 +22,15 @@ def ==(other)
# (false by default)
# * <tt>:timeout</tt> - Timeout to use by the Net::HTTP object
# (60 by default)
# * <tt>:chunk_size</tt> - Size of a chunk when streaming
# (1048576 (1 MiB) by default)
def initialize(options)
@access_key_id = options.fetch(:access_key_id)
@secret_access_key = options.fetch(:secret_access_key)
@use_ssl = options.fetch(:use_ssl, false)
@timeout = options.fetch(:timeout, 60)
@debug = options.fetch(:debug, false)
@chunk_size = options.fetch(:chunk_size, 1048576)

raise ArgumentError, "Missing proxy settings. Must specify at least :host." if options[:proxy] && !options[:proxy][:host]
@proxy = options.fetch(:proxy, nil)
Expand Down Expand Up @@ -74,7 +77,8 @@ def connection
:use_ssl => @use_ssl,
:timeout => @timeout,
:debug => @debug,
:proxy => @proxy)
:proxy => @proxy,
:chunk_size => @chunk_size)
end
@connection
end
Expand Down
2 changes: 1 addition & 1 deletion lib/s3/version.rb
@@ -1,3 +1,3 @@
module S3
VERSION = "0.3.4"
VERSION = "0.3.5"
end
5 changes: 1 addition & 4 deletions test/test_helper.rb
@@ -1,6 +1,3 @@
require "rubygems"
require "bundler/setup"

Bundler.require
require "test/unit"
require "mocha"
require "s3"

9 comments on commit f7fd538

@jacquescrocker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this commit might have busted something. When I switched my bundle to git master, I started getting this error: https://gist.github.com/096e94da348d4a2fe708

Seems like an infinite loop or something. When I switch to the ref right before this commit, it works fine.

Here's the code that's causing it. Pretty standard. The file I'm uploading is about 200K

new_object = bucket.objects.build(file_name)
new_object.content = file_content
new_object.content_type = "image/png"
new_object.save

@qoobaa
Copy link
Owner

@qoobaa qoobaa commented on f7fd538 Oct 5, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for info, I'm gonna check it.

@qoobaa
Copy link
Owner

@qoobaa qoobaa commented on f7fd538 Oct 5, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not able to reproduce this error. I'm uploading 220 kiB file without any problems on 1.8.7 and 1.9.2.

@jacquescrocker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the exact file it is choking on, if that helps: http://cl.ly/76efafb7611acbc71ff6

If that file doesnt help repro it for you, I'll dig in an try to get you a better reproducable test case soon

@qoobaa
Copy link
Owner

@qoobaa qoobaa commented on f7fd538 Oct 5, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which version of Ruby is it?

@jacquescrocker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ruby 1.8.7 (2010-06-23 patchlevel 299) [i686-darwin10.4.0]

@qoobaa
Copy link
Owner

@qoobaa qoobaa commented on f7fd538 Oct 5, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://assets.jah.pl/test - uploaded without any problems on Ruby 1.8.7-p302.

@jacquescrocker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No prob, thanks for trying it out. I'll try to track down a reproducable test case

@qoobaa
Copy link
Owner

@qoobaa qoobaa commented on f7fd538 Oct 5, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just added your gmail account in my IM, you can talk to me using jabber.

Please sign in to comment.