From 7bdca06bd4d9a41765e83c30da9744521e2eb455 Mon Sep 17 00:00:00 2001 From: Rein Henrichs Date: Sat, 8 Nov 2008 13:59:57 -0500 Subject: [PATCH] Moving send_request and friends into HTTParty::Request --- lib/httparty.rb | 113 +++++++--------------------- lib/httparty/request.rb | 100 +++++++++++++++++++++++++ spec/httparty/request_spec.rb | 106 ++++++++++++++++++++++++++ spec/httparty_spec.rb | 137 +++++++--------------------------- 4 files changed, 259 insertions(+), 197 deletions(-) create mode 100644 lib/httparty/request.rb create mode 100644 spec/httparty/request_spec.rb diff --git a/lib/httparty.rb b/lib/httparty.rb index 9da9e5a..7e02cb9 100644 --- a/lib/httparty.rb +++ b/lib/httparty.rb @@ -8,18 +8,23 @@ require 'active_support' directory = File.dirname(__FILE__) $:.unshift(directory) unless $:.include?(directory) || $:.include?(File.expand_path(directory)) +require 'httparty/request' + module HTTParty class UnsupportedFormat < StandardError; end class RedirectionTooDeep < StandardError; end + + AllowedFormats = {:xml => 'text/xml', :json => 'application/json'} def self.included(base) base.extend ClassMethods end - AllowedFormats = {:xml => 'text/xml', :json => 'application/json'} - SupportedHTTPMethods = [Net::HTTP::Get, Net::HTTP::Post, Net::HTTP::Put, Net::HTTP::Delete] - module ClassMethods + def default_options + @@default_options ||= {} + end + # # Set an http proxy # @@ -28,43 +33,42 @@ module HTTParty # http_proxy http://myProxy, 1080 # .... def http_proxy(addr=nil, port = nil) - @http_proxyaddr = addr - @http_proxyport = port + default_options[:http_proxyaddr] = addr + default_options[:http_proxyport] = port end - def base_uri(base_uri=nil) - return @base_uri unless base_uri - @base_uri = normalize_base_uri(base_uri) + def base_uri(uri=nil) + return default_options[:base_uri] unless uri + default_options[:base_uri] = normalize_base_uri(uri) end - + # Warning: This is not thread safe most likely and # only works if you use one set of credentials. I # leave it because it is convenient on some occasions. def basic_auth(u, p) - @auth = {:username => u, :password => p} + default_options[:basic_auth] = {:username => u, :password => p} end # Updates the default query string parameters # that should be appended to each request. def default_params(h={}) raise ArgumentError, 'Default params must be a hash' unless h.is_a?(Hash) - @default_params ||= {} - return @default_params if h.blank? - @default_params.merge!(h) + default_options[:default_params] ||= {} + default_options[:default_params].merge!(h) end def headers(h={}) raise ArgumentError, 'Headers must be a hash' unless h.is_a?(Hash) - @headers ||= {} - return @headers if h.blank? - @headers.merge!(h) + default_options[:headers] ||= {} + default_options[:headers].merge!(h) end def format(f) raise UnsupportedFormat, "Must be one of: #{AllowedFormats.keys.join(', ')}" unless AllowedFormats.key?(f) - @format = f + default_options[:format] = f end + # TODO: spec out this def get(path, options={}) send_request Net::HTTP::Get, path, options @@ -84,72 +88,11 @@ module HTTParty def delete(path, options={}) send_request Net::HTTP::Delete, path, options end - - private - def http(uri) #:nodoc: - http = Net::HTTP.new(uri.host, uri.port, @http_proxyaddr, @http_proxyport) - http.use_ssl = (uri.port == 443) - http.verify_mode = OpenSSL::SSL::VERIFY_NONE - http - end - - # FIXME: this method is doing way to much and needs to be split up - # options can be any or all of: - # query => hash of keys/values or a query string (foo=bar&baz=poo) - # body => hash of keys/values or a query string (foo=bar&baz=poo) - # headers => hash of headers to send request with - # basic_auth => :username and :password to use as basic http authentication (overrides @auth class instance variable) - # Raises exception Net::XXX (http error code) if an http error occured - def send_request(klass, path, options={}) #:nodoc: - options = {:limit => 5}.merge(options) - options[:limit] = 0 if options.delete(:no_follow) - - raise HTTParty::RedirectionTooDeep, 'HTTP redirects too deep' if options[:limit].to_i <= 0 - raise ArgumentError, 'only get, post, put and delete methods are supported' unless SupportedHTTPMethods.include?(klass) - raise ArgumentError, ':headers must be a hash' if options[:headers] && !options[:headers].is_a?(Hash) - raise ArgumentError, ':basic_auth must be a hash' if options[:basic_auth] && !options[:basic_auth].is_a?(Hash) - - path = URI.parse(path) - uri = path.relative? ? URI.parse("#{base_uri}#{path}") : path - existing_query = uri.query ? "#{uri.query}&" : '' - uri.query = if options[:query].blank? - existing_query + default_params.to_query - else - existing_query + (options[:query].is_a?(Hash) ? default_params.merge(options[:query]).to_query : options[:query]) - end - - request = klass.new(uri.request_uri) - request.body = options[:body].is_a?(Hash) ? options[:body].to_query : options[:body] unless options[:body].blank? - basic_auth = options.delete(:basic_auth) || @auth - request.initialize_http_header headers.merge(options[:headers] || {}) - request.basic_auth(basic_auth[:username], basic_auth[:password]) if basic_auth - response = http(uri).request(request) - @format ||= format_from_mimetype(response['content-type']) - - case response - when Net::HTTPSuccess - parse_response(response.body) - when Net::HTTPRedirection - options[:limit] -= 1 - send_request(klass, response['location'], options) - else - response.instance_eval { class << self; attr_accessor :body_parsed; end } - begin; response.body_parsed = parse_response(response.body); rescue; end - response.error! # raises exception corresponding to http error Net::XXX - end - end - - def parse_response(body) #:nodoc: - return nil if body.nil? or body.empty? - case @format - when :xml - Hash.from_xml(body) - when :json - ActiveSupport::JSON.decode(body) - else - body - end + private + + def send_request(http_method, path, options) + Request.send_request(http_method, path, default_options.merge(options)) end # Makes it so uri is sure to parse stuff like google.com with the http @@ -159,11 +102,5 @@ module HTTParty url.gsub!(/^https?:\/\//i, '') "http#{'s' if use_ssl}://#{url}" end - - # Uses the HTTP Content-Type header to determine the format of the response - # It compares the MIME type returned to the types stored in the AllowedFormats hash - def format_from_mimetype(mimetype) #:nodoc: - AllowedFormats.each { |k, v| return k if mimetype.include?(v) } - end end end diff --git a/lib/httparty/request.rb b/lib/httparty/request.rb new file mode 100644 index 0000000..0e15aa7 --- /dev/null +++ b/lib/httparty/request.rb @@ -0,0 +1,100 @@ +module HTTParty + class Request + SupportedHTTPMethods = [Net::HTTP::Get, Net::HTTP::Post, Net::HTTP::Put, Net::HTTP::Delete] + + def self.send_request(http_method, path, options={}) + new(http_method, path, options).send_request + end + + attr_accessor :http_method, :path, :options + def initialize(http_method, path, options={}) + options = {:limit => 5}.merge(options) + options[:limit] = 0 if options.delete(:no_follow) + options[:default_params] ||= {} + + @http_method = http_method + + @path = URI.parse(path) + @options = options + end + + def path=(uri) + @path = URI.parse(uri) + end + + def uri + @uri ||= path.relative? ? URI.parse("#{options[:base_uri]}#{path}") : path + end + + # FIXME: this method is doing way to much and needs to be split up + # options can be any or all of: + # query => hash of keys/values or a query string (foo=bar&baz=poo) + # body => hash of keys/values or a query string (foo=bar&baz=poo) + # headers => hash of headers to send request with + # basic_auth => :username and :password to use as basic http authentication (overrides basic_auth setting) + # Raises exception Net::XXX (http error code) if an http error occured + def send_request #:nodoc: + raise HTTParty::RedirectionTooDeep, 'HTTP redirects too deep' if options[:limit].to_i <= 0 + raise ArgumentError, 'only get, post, put and delete methods are supported' unless SupportedHTTPMethods.include?(http_method) + raise ArgumentError, ':headers must be a hash' if options[:headers] && !options[:headers].is_a?(Hash) + raise ArgumentError, ':basic_auth must be a hash' if options[:basic_auth] && !options[:basic_auth].is_a?(Hash) + + existing_query = uri.query ? "#{uri.query}&" : '' + uri.query = if options[:query].blank? + existing_query + options[:default_params].to_query + else + existing_query + (options[:query].is_a?(Hash) ? options[:default_params].merge(options[:query]).to_query : options[:query]) + end + + request = http_method.new(uri.request_uri) + request.body = options[:body].is_a?(Hash) ? options[:body].to_query : options[:body] unless options[:body].blank? + basic_auth = options.delete(:basic_auth) + request.initialize_http_header options[:headers] + request.basic_auth(basic_auth[:username], basic_auth[:password]) if options[:basic_auth] + response = http(uri).request(request) + + options[:format] ||= format_from_mimetype(response['content-type']) + + case response + when Net::HTTPSuccess + parse_response(response.body) + when Net::HTTPRedirection + options[:limit] -= 1 + self.path = response['location'] + send_request + else + response.instance_eval { class << self; attr_accessor :body_parsed; end } + begin; response.body_parsed = parse_response(response.body); rescue; end + response.error! # raises exception corresponding to http error Net::XXX + end + + end + + private + + def http(uri) #:nodoc: + http = Net::HTTP.new(uri.host, uri.port, options[:http_proxyaddr], options[:http_proxyport]) + http.use_ssl = (uri.port == 443) + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + http + end + + def parse_response(body) #:nodoc: + return nil if body.nil? or body.empty? + case options[:format] + when :xml + Hash.from_xml(body) + when :json + ActiveSupport::JSON.decode(body) + else + body + end + end + + # Uses the HTTP Content-Type header to determine the format of the response + # It compares the MIME type returned to the types stored in the AllowedFormats hash + def format_from_mimetype(mimetype) #:nodoc: + AllowedFormats.each { |k, v| return k if mimetype.include?(v) } + end + end +end diff --git a/spec/httparty/request_spec.rb b/spec/httparty/request_spec.rb new file mode 100644 index 0000000..0b318e0 --- /dev/null +++ b/spec/httparty/request_spec.rb @@ -0,0 +1,106 @@ +require File.join(File.dirname(__FILE__), '..', 'spec_helper') + +describe HTTParty::Request do + before do + @request = HTTParty::Request.new(Net::HTTP::Get, 'http://api.foo.com/v1', :format => :xml) + end + + describe 'http' do + it "should use ssl for port 443" do + @request.send(:http, URI.parse('https://api.foo.com/v1:443')).use_ssl?.should == true + end + + it 'should not use ssl for port 80' do + @request.send(:http, URI.parse('http://foobar.com')).use_ssl?.should == false + end + end + + describe 'parsing responses' do + it 'should handle xml automatically' do + xml = %q[1234Foo Bar!] + @request.options[:format] = :xml + @request.send(:parse_response, xml).should == {'books' => {'book' => {'id' => '1234', 'name' => 'Foo Bar!'}}} + end + + it 'should handle json automatically' do + json = %q[{"books": {"book": {"name": "Foo Bar!", "id": "1234"}}}] + @request.options[:format] = :json + @request.send(:parse_response, json).should == {'books' => {'book' => {'id' => '1234', 'name' => 'Foo Bar!'}}} + end + end + + it "should not attempt to parse empty responses" do + http = Net::HTTP.new('localhost', 80) + @request.stub!(:http).and_return(http) + response = Net::HTTPNoContent.new("1.1", 204, "No content for you") + response.stub!(:body).and_return(nil) + http.stub!(:request).and_return(response) + + @request.options[:format] = :xml + @request.send_request.should be_nil + + response.stub!(:body).and_return("") + @request.send_request.should be_nil + end + + describe "that respond with redirects" do + def setup_redirect + @http = Net::HTTP.new('localhost', 80) + @request.stub!(:http).and_return(@http) + @redirect = Net::HTTPFound.new("1.1", 302, "") + @redirect['location'] = '/foo' + end + + def setup_ok_response + @ok = Net::HTTPOK.new("1.1", 200, "Content for you") + @ok.stub!(:body).and_return({"foo" => "bar"}.to_xml) + @http.should_receive(:request).and_return(@redirect, @ok) + @request.options[:format] = :xml + end + + def setup_redirect_response + @http.stub!(:request).and_return(@redirect) + end + + def setup_successful_redirect + setup_redirect + setup_ok_response + end + + def setup_infinite_redirect + setup_redirect + setup_redirect_response + end + + it "should handle redirects for GET transparently" do + setup_successful_redirect + @request.send_request.should == {"hash" => {"foo" => "bar"}} + end + + it "should handle redirects for POST transparently" do + setup_successful_redirect + @request.http_method = Net::HTTP::Post + @request.send_request.should == {"hash" => {"foo" => "bar"}} + end + + it "should handle redirects for DELETE transparently" do + setup_successful_redirect + @request.http_method = Net::HTTP::Delete + @request.send_request.should == {"hash" => {"foo" => "bar"}} + end + + it "should handle redirects for PUT transparently" do + setup_successful_redirect + @request.http_method = Net::HTTP::Put + @request.send_request.should == {"hash" => {"foo" => "bar"}} + end + + it "should prevent infinite loops" do + setup_infinite_redirect + + lambda do + @request.send_request + end.should raise_error(HTTParty::RedirectionTooDeep) + end + end +end diff --git a/spec/httparty_spec.rb b/spec/httparty_spec.rb index 0111763..d2193b8 100644 --- a/spec/httparty_spec.rb +++ b/spec/httparty_spec.rb @@ -13,6 +13,10 @@ end describe HTTParty do describe "base uri" do + before do + Foo.base_uri('api.foo.com/v1') + end + it "should have reader" do Foo.base_uri.should == 'http://api.foo.com/v1' end @@ -65,19 +69,19 @@ describe HTTParty do describe "basic http authentication" do it "should work" do Foo.basic_auth 'foobar', 'secret' - Foo.instance_variable_get("@auth").should == {:username => 'foobar', :password => 'secret'} + Foo.default_options[:basic_auth].should == {:username => 'foobar', :password => 'secret'} end end describe "format" do it "should allow xml" do Foo.format :xml - Foo.instance_variable_get("@format").should == :xml + Foo.default_options[:format].should == :xml end it "should allow json" do Foo.format :json - Foo.instance_variable_get("@format").should == :json + Foo.default_options[:format].should == :json end it 'should not allow funky format' do @@ -87,30 +91,6 @@ describe HTTParty do end end - describe 'http' do - it "should use ssl for port 443" do - FooWithHttps.send(:http, URI.parse('https://api.foo.com/v1:443')).use_ssl?.should == true - end - - it 'should not use ssl for port 80' do - Foo.send(:http, URI.parse('http://foobar.com')).use_ssl?.should == false - end - end - - describe 'parsing responses' do - it 'should handle xml automatically' do - xml = %q[1234Foo Bar!] - Foo.format :xml - Foo.send(:parse_response, xml).should == {'books' => {'book' => {'id' => '1234', 'name' => 'Foo Bar!'}}} - end - - it 'should handle json automatically' do - json = %q[{"books": {"book": {"name": "Foo Bar!", "id": "1234"}}}] - Foo.format :json - Foo.send(:parse_response, json).should == {'books' => {'book' => {'id' => '1234', 'name' => 'Foo Bar!'}}} - end - end - describe "sending requests" do it "should not work with request method other than get, post, put, delete" do lambda do @@ -126,96 +106,35 @@ describe HTTParty do it 'should require that :basic_auth is a hash if present' do lambda do - Foo.send(:send_request, 'get', '/foo', :basic_auth => 'string') + Foo.get('/foo', :basic_auth => 'string') end.should raise_error(ArgumentError) end + end - it "should not attempt to parse empty responses" do - http = Net::HTTP.new('localhost', 80) - Foo.stub!(:http).and_return(http) - response = Net::HTTPNoContent.new("1.1", 204, "No content for you") - response.stub!(:body).and_return(nil) - http.stub!(:request).and_return(response) + describe "with explicit override of automatic redirect handling" do - Foo.headers.clear # clear out bogus settings from other specs - Foo.format :xml - Foo.get('/bar').should be_nil - - response.stub!(:body).and_return("") - Foo.get('bar').should be_nil + it "should fail with redirected GET" do + lambda do + Foo.get('/foo', :no_follow => true) + end.should raise_error(HTTParty::RedirectionTooDeep) end - describe "that respond with redirects" do - def setup_http - @http = Net::HTTP.new('localhost', 80) - Foo.stub!(:http).and_return(@http) - @redirect = Net::HTTPFound.new("1.1", 302, "") - @redirect.stub!(:[]).with('location').and_return('/foo') - @ok = Net::HTTPOK.new("1.1", 200, "Content for you") - @ok.stub!(:body).and_return({"foo" => "bar"}.to_xml) - @http.should_receive(:request).and_return(@redirect, @ok) - Foo.headers.clear - Foo.format :xml - end + it "should fail with redirected POST" do + lambda do + Foo.post('/foo', :no_follow => true) + end.should raise_error(HTTParty::RedirectionTooDeep) + end - it "should handle redirects for GET transparently" do - setup_http - Foo.get('/foo/').should == {"hash" => {"foo" => "bar"}} - end + it "should fail with redirected DELETE" do + lambda do + Foo.delete('/foo', :no_follow => true) + end.should raise_error(HTTParty::RedirectionTooDeep) + end - it "should handle redirects for POST transparently" do - setup_http - Foo.post('/foo/', {:foo => :bar}).should == {"hash" => {"foo" => "bar"}} - end - - it "should handle redirects for DELETE transparently" do - setup_http - Foo.delete('/foo/').should == {"hash" => {"foo" => "bar"}} - end - - it "should handle redirects for PUT transparently" do - setup_http - Foo.put('/foo/').should == {"hash" => {"foo" => "bar"}} - end - - it "should prevent infinite loops" do - http = Net::HTTP.new('localhost', 80) - Foo.stub!(:http).and_return(http) - redirect = Net::HTTPFound.new("1.1", "302", "Look, over there!") - redirect.stub!(:[]).with('location').and_return('/foo') - http.stub!(:request).and_return(redirect) - - lambda do - Foo.get('/foo') - end.should raise_error(HTTParty::RedirectionTooDeep) - end - - describe "with explicit override of automatic redirect handling" do - - it "should fail with redirected GET" do - lambda do - Foo.get('/foo', :no_follow => true) - end.should raise_error(HTTParty::RedirectionTooDeep) - end - - it "should fail with redirected POST" do - lambda do - Foo.post('/foo', :no_follow => true) - end.should raise_error(HTTParty::RedirectionTooDeep) - end - - it "should fail with redirected DELETE" do - lambda do - Foo.delete('/foo', :no_follow => true) - end - end - - it "should fail with redirected PUT" do - lambda do - Foo.put('/foo', :no_follow => true) - end - end - end + it "should fail with redirected PUT" do + lambda do + Foo.put('/foo', :no_follow => true) + end.should raise_error(HTTParty::RedirectionTooDeep) end end end