Merge pull request #449 from seanpdoyle/net-http-requests

Implement `Connection` in terms of `Net::HTTP` Request

Author: rafaelfranca

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,50 +89,52 @@ 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
       # Makes a request to the remote service.
       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)

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

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