diff --git a/lib/active_resource/connection.rb b/lib/active_resource/connection.rb index 59d7f19957..7a5e917e3b 100644 --- a/lib/active_resource/connection.rb +++ b/lib/active_resource/connection.rb @@ -11,13 +11,22 @@ module ActiveResource # This class is used by ActiveResource::Base to interface with REST # services. class Connection - HTTP_FORMAT_HEADER_NAMES = { get: "Accept", + HTTP_FORMAT_HEADER_NAMES = { + get: "Accept", put: "Content-Type", post: "Content-Type", patch: "Content-Type", delete: "Accept", head: "Accept" } + HTTP_METHODS = { + get: Net::HTTP::Get, + put: Net::HTTP::Put, + post: Net::HTTP::Post, + patch: Net::HTTP::Patch, + delete: Net::HTTP::Delete, + head: Net::HTTP::Head + }.freeze # :nodoc: attr_reader :site, :user, :password, :bearer_token, :auth_type, :timeout, :open_timeout, :read_timeout, :proxy, :ssl_options attr_accessor :format, :logger @@ -80,37 +89,37 @@ def auth_type=(auth_type) # Executes a GET request. # Used to get (find) resources. def get(path, headers = {}) - with_auth { request(:get, path, build_request_headers(headers, :get, self.site.merge(path))) } + with_auth { request(:get, path, headers) } end # Executes a DELETE request (see HTTP protocol documentation if unfamiliar). # Used to delete resources. def delete(path, headers = {}) - with_auth { request(:delete, path, build_request_headers(headers, :delete, self.site.merge(path))) } + with_auth { request(:delete, path, headers) } end # Executes a PATCH request (see HTTP protocol documentation if unfamiliar). # Used to update resources. def patch(path, body = "", headers = {}) - with_auth { request(:patch, path, body.to_s, build_request_headers(headers, :patch, self.site.merge(path))) } + with_auth { request(:patch, path, body.to_s, headers) } end # Executes a PUT request (see HTTP protocol documentation if unfamiliar). # Used to update resources. def put(path, body = "", headers = {}) - with_auth { request(:put, path, body.to_s, build_request_headers(headers, :put, self.site.merge(path))) } + with_auth { request(:put, path, body.to_s, headers) } end # Executes a POST request. # Used to create new resources. def post(path, body = "", headers = {}) - with_auth { request(:post, path, body.to_s, build_request_headers(headers, :post, self.site.merge(path))) } + with_auth { request(:post, path, body.to_s, headers) } end # Executes a HEAD request. # Used to obtain meta-information about resources, such as whether they exist and their size (via response headers). def head(path, headers = {}) - with_auth { request(:head, path, build_request_headers(headers, :head, self.site.merge(path))) } + with_auth { request(:head, path, headers) } end private @@ -118,12 +127,14 @@ def head(path, headers = {}) def request(method, path, *arguments) body, headers = arguments headers, body = body, nil if headers.nil? + request = build_request(method, path, headers) + request.body = body result = ActiveSupport::Notifications.instrument("request.active_resource") do |payload| payload[:method] = method - payload[:request_uri] = "#{site.scheme}://#{site.host}:#{site.port}#{path}" - payload[:headers] = headers + payload[:request_uri] = request.uri.to_s + payload[:headers] = request.each_capitalized.to_h payload[:body] = body - payload[:result] = http.send(method, path, *arguments) + payload[:result] = http.request(request) end handle_response(result) rescue Timeout::Error => e @@ -218,6 +229,15 @@ def default_header @default_header ||= {} end + def build_request(http_method, path, headers) + HTTP_METHODS[http_method].new(site.merge(path)).tap do |request| + headers = build_request_headers(headers, http_method, request.uri) + + request.each_name { |name| request.delete(name) } + headers.each_pair { |name, value| request[name] = value } + end + end + # Builds headers for request to remote service. def build_request_headers(headers, http_method, uri) authorization_header(http_method, uri).update(default_header).update(http_format_header(http_method)).update(headers.to_hash) diff --git a/lib/active_resource/http_mock.rb b/lib/active_resource/http_mock.rb index ed5f53caf6..bb832808d8 100644 --- a/lib/active_resource/http_mock.rb +++ b/lib/active_resource/http_mock.rb @@ -75,6 +75,8 @@ class InvalidRequestError < StandardError; end # :nodoc: # # When a block is passed to the mock, it ignores the +body+, +status+, and +response_headers+ arguments. class HttpMock + HTTP_METHODS = Connection::HTTP_METHODS.invert.freeze # :nodoc: + class Responder # :nodoc: def initialize(responses) @responses = responses @@ -270,33 +272,38 @@ def net_connection_disabled? methods.each do |method| # def post(path, body, headers, options = {}) # request = ActiveResource::Request.new(:post, path, body, headers, options) - # self.class.requests << request - # if response = self.class.responses.assoc(request) - # response = response[1] - # response = response.call(request) if response.respond_to?(:call) # - # Response.wrap(response) - # else - # raise InvalidRequestError.new("Could not find a response recorded for #{request.to_s} - Responses recorded are: - #{inspect_responses}") - # end + # process(request) # end module_eval <<-EOE, __FILE__, __LINE__ + 1 def #{method}(path, #{'body, ' if has_body}headers, options = {}) request = ActiveResource::Request.new(:#{method}, path, #{has_body ? 'body, ' : 'nil, '}headers, options) - self.class.requests << request - if response = self.class.responses.assoc(request) - response = response[1] - response = response.call(request) if response.respond_to?(:call) - - Response.wrap(response) - else - raise InvalidRequestError.new("Could not find a response recorded for \#{request.to_s} - Responses recorded are: \#{inspect_responses}") - end + + process(request) end EOE end end + def request(http) # :nodoc: + request = Request.new(HTTP_METHODS[http.class], http.path, nil, http.each_capitalized.to_h) + request.body = http.body if http.request_body_permitted? + + process(request) + end + + def process(request) # :nodoc: + self.class.requests << request + if response = self.class.responses.assoc(request) + response = response[1] + response = response.call(request) if response.respond_to?(:call) + + Response.wrap(response) + else + raise InvalidRequestError.new("Could not find a response recorded for #{request} - Responses recorded are: #{inspect_responses}") + end + end + def initialize(site) # :nodoc: @site = site end @@ -310,7 +317,7 @@ class Request attr_accessor :path, :method, :body, :headers def initialize(method, path, body = nil, headers = {}, options = {}) - @method, @path, @body, @headers, @options = method, path, body, headers, options + @method, @path, @body, @headers, @options = method, path, body, headers.transform_keys(&:downcase), options end def ==(req) @@ -339,7 +346,7 @@ def same_path?(req) def headers_match?(req) # Ignore format header on equality if it's not defined - format_header = ActiveResource::Connection::HTTP_FORMAT_HEADER_NAMES[method] + format_header = ActiveResource::Connection::HTTP_FORMAT_HEADER_NAMES[method].downcase if headers[format_header].present? || req.headers[format_header].blank? headers == req.headers else diff --git a/test/cases/connection_test.rb b/test/cases/connection_test.rb index 941b477698..9be07f4917 100644 --- a/test/cases/connection_test.rb +++ b/test/cases/connection_test.rb @@ -242,7 +242,7 @@ def test_delete_with_header def test_timeout @http = mock("new Net::HTTP") @conn.expects(:http).returns(@http) - @http.expects(:get).raises(Timeout::Error, "execution expired") + @http.expects(:request).raises(Timeout::Error, "execution expired") assert_raise(ActiveResource::TimeoutError) { @conn.get("/people_timeout.json") } end @@ -261,7 +261,7 @@ def test_accept_http_header @http = mock("new Net::HTTP") @conn.expects(:http).returns(@http) path = "/people/1.xml" - @http.expects(:get).with(path, { "Accept" => "application/xhtml+xml" }).returns(ActiveResource::Response.new(@matz, 200, "Content-Type" => "text/xhtml")) + @http.expects(:request).with { |get| get.path == path && get["Accept"] == "application/xhtml+xml" }.returns(ActiveResource::Response.new(@matz, 200, "Content-Type" => "text/xhtml")) assert_nothing_raised { @conn.get(path, "Accept" => "application/xhtml+xml") } end @@ -284,21 +284,21 @@ def test_ssl_options_get_applied_to_https_urls_without_explicitly_setting_ssl_op def test_ssl_error http = Net::HTTP.new("") @conn.expects(:http).returns(http) - http.expects(:get).raises(OpenSSL::SSL::SSLError, "Expired certificate") + http.expects(:request).raises(OpenSSL::SSL::SSLError, "Expired certificate") assert_raise(ActiveResource::SSLError) { @conn.get("/people/1.json") } end def test_handle_econnrefused http = Net::HTTP.new("") @conn.expects(:http).returns(http) - http.expects(:get).raises(Errno::ECONNREFUSED, "Failed to open TCP connection") + http.expects(:request).raises(Errno::ECONNREFUSED, "Failed to open TCP connection") assert_raise(ActiveResource::ConnectionRefusedError) { @conn.get("/people/1.json") } end def test_handle_econnrefused_with_backwards_compatible_error http = Net::HTTP.new("") @conn.expects(:http).returns(http) - http.expects(:get).raises(Errno::ECONNREFUSED, "Failed to open TCP connection") + http.expects(:request).raises(Errno::ECONNREFUSED, "Failed to open TCP connection") assert_raise(Errno::ECONNREFUSED) { @conn.get("/people/1.json") } end