refactor query params hash handling

- Connection#params is now an instance of Faraday::Utils::ParamsHash
 - get rid of `merge_params`, `merge_headers` methods
 - don't mix in Faraday::Utils into Connection

Fixes #48

lib/faraday/connection.rb

@@ -4,7 +4,7 @@ require 'base64'
 
 module Faraday
   class Connection
-    include Addressable, Faraday::Utils
+    include Addressable
 
     METHODS = Set.new [:get, :post, :put, :delete, :head]
     METHODS_WITH_BODIES = Set.new [:post, :put]
@@ -22,15 +22,17 @@ module Faraday
         options = url
         url     = options[:url]
       end
-      @headers          = Headers.new
-      @params           = {}
+      @headers          = Utils::Headers.new
+      @params           = Utils::ParamsHash.new
       @options          = options[:request] || {}
       @ssl              = options[:ssl]     || {}
       @parallel_manager = options[:parallel]
+
       self.url_prefix = url if url
       proxy(options[:proxy])
-      merge_params  @params,  options[:params]  if options[:params]
-      merge_headers @headers, options[:headers] if options[:headers]
+
+      @params.update options[:params]   if options[:params]
+      @headers.update options[:headers] if options[:headers]
 
       if block_given?
         @builder = Builder.create { |b| yield b }
@@ -145,12 +147,11 @@ module Faraday
       self.host        = uri.host
       self.port        = uri.port
       self.path_prefix = uri.path
-      if uri.query && !uri.query.empty?
-        merge_params @params, parse_query(uri.query)
-      end
-      if uri.user && uri.password
-        basic_auth(uri.user, uri.password)
-      end
+
+      @params.merge_query(uri.query)
+      basic_auth(uri.user, uri.password) if uri.user && uri.password
+
+      uri
     end
 
     # Ensures that the path prefix always has a leading / and no trailing /
@@ -186,7 +187,7 @@ module Faraday
     #   conn.build_url("nigiri?page=2")      # => https://sushi.com/api/nigiri?token=abc&page=2
     #   conn.build_url("nigiri", :page => 2) # => https://sushi.com/api/nigiri?token=abc&page=2
     #
-    def build_url(url, params = nil)
+    def build_url(url, extra_params = nil)
       uri          = URI.parse(url.to_s)
       if @path_prefix && uri.path !~ /^\//
         uri.path = "#{@path_prefix.size > 1 ? @path_prefix : nil}/#{uri.path}"
@@ -194,7 +195,11 @@ module Faraday
       uri.host   ||= @host
       uri.port   ||= @port
       uri.scheme ||= @scheme
-      replace_query(uri, params)
+
+      params = @params.dup.merge_query(uri.query)
+      params.update extra_params if extra_params
+      uri.query = params.empty? ? nil : params.to_query
+
       uri
     end
 
@@ -202,18 +207,6 @@ module Faraday
       self.class.new(build_url(''), :headers => headers.dup, :params => params.dup, :builder => builder.dup)
     end
 
-    def replace_query(uri, params)
-      url_params = @params.dup
-      if uri.query && !uri.query.empty?
-        merge_params(url_params, parse_query(uri.query))
-      end
-      if params && !params.empty?
-        merge_params(url_params, params)
-      end
-      uri.query = url_params.empty? ? nil : build_query(url_params)
-      uri
-    end
-
     def proxy_arg_to_uri(arg)
       case arg
         when String then URI.parse(arg)

lib/faraday/request.rb

@@ -64,10 +64,8 @@ module Faraday
     #     :password   - Proxy server password
     # :ssl - Hash of options for configuring SSL requests.
     def to_env_hash(connection, request_method)
-      env_headers = connection.headers.dup
-      env_params  = connection.params.dup
-      connection.merge_headers(env_headers, headers)
-      connection.merge_params(env_params,  params)
+      env_params  = connection.params.merge(params)
+      env_headers = connection.headers.merge(headers)
 
       { :method           => request_method,
         :body             => body,

lib/faraday/utils.rb

@@ -1,4 +1,5 @@
 require 'rack/utils'
+
 module Faraday
   module Utils
     include Rack::Utils
@@ -44,8 +45,65 @@ module Faraday
       end
     end
 
-    # Make Rack::Utils build_query method public.
-    public :build_query
+    # hash with stringified keys
+    class ParamsHash < Hash
+      def [](key)
+        super(convert_key(key))
+      end
+
+      def []=(key, value)
+        super(convert_key(key), value)
+      end
+
+      def delete(key)
+        super(convert_key(key))
+      end
+
+      def include?(key)
+        super(convert_key(key))
+      end
+
+      alias_method :has_key?, :include?
+      alias_method :member?, :include?
+      alias_method :key?, :include?
+
+      def update(params)
+        params.each do |key, value|
+          self[key] = value
+        end
+        self
+      end
+      alias_method :merge!, :update
+
+      def merge(params)
+        dup.update(params)
+      end
+
+      def replace(other)
+        clear
+        update(other)
+      end
+
+      def merge_query(query)
+        if query && !query.empty?
+          update Utils.parse_query(query)
+        end
+        self
+      end
+
+      def to_query
+        Utils.build_query(self)
+      end
+
+      private
+
+      def convert_key(key)
+        key.to_s
+      end
+    end
+
+    # Make Rack::Utils methods public.
+    public :build_query, :parse_query
 
     # Override Rack's version since it doesn't handle non-String values
     def build_nested_query(value, prefix = nil)
@@ -72,20 +130,6 @@ module Faraday
       end
     end
 
-    # Turns param keys into strings
-    def merge_params(existing_params, new_params)
-      new_params.each do |key, value|
-        existing_params[key.to_s] = value
-      end
-    end
-
-    # Turns headers keys and values into strings
-    def merge_headers(existing_headers, new_headers)
-      new_headers.each do |key, value|
-        existing_headers[key] = value.to_s
-      end
-    end
-
     # Receives a URL and returns just the path with the query string sorted.
     def normalize_path(url)
       (url.path != "" ? url.path : "/") +

test/connection_test.rb

@@ -38,18 +38,22 @@ class TestConnection < Faraday::TestCase
 
   def test_initialize_stores_default_params_from_options
     conn = Faraday::Connection.new :params => {:a => 1}
-    assert_equal 1, conn.params['a']
+    assert_equal({'a' => 1}, conn.params)
   end
 
   def test_initialize_stores_default_params_from_uri
-    conn = Faraday::Connection.new "http://sushi.com/fish?a=1", :params => {'b' => '2'}
-    assert_equal '1', conn.params['a']
-    assert_equal '2', conn.params['b']
+    conn = Faraday::Connection.new "http://sushi.com/fish?a=1"
+    assert_equal({'a' => '1'}, conn.params)
+  end
+
+  def test_initialize_stores_default_params_from_uri_and_options
+    conn = Faraday::Connection.new "http://sushi.com/fish?a=1&b=2", :params => {'a' => 3}
+    assert_equal({'a' => 3, 'b' => '2'}, conn.params)
   end
 
   def test_initialize_stores_default_headers_from_options
-    conn = Faraday::Connection.new :headers => {:a => 1}
-    assert_equal '1', conn.headers['A']
+    conn = Faraday::Connection.new :headers => {:user_agent => 'Faraday'}
+    assert_equal 'Faraday', conn.headers['User-agent']
   end
 
   def test_basic_auth_sets_authorization_header
@@ -152,21 +156,21 @@ class TestConnection < Faraday::TestCase
   def test_build_url_mashes_default_and_given_params_together
     conn = Faraday::Connection.new 'http://sushi.com/api?token=abc', :params => {'format' => 'json'}
     url = conn.build_url("nigiri?page=1", :limit => 5)
-    assert_match /limit=5/,      url.query
-    assert_match /page=1/,       url.query
-    assert_match /format=json/,  url.query
-    assert_match /token=abc/,    url.query
+    assert_equal %w[format=json limit=5 page=1 token=abc], url.query.split('&').sort
   end
 
   def test_build_url_overrides_default_params_with_given_params
     conn = Faraday::Connection.new 'http://sushi.com/api?token=abc', :params => {'format' => 'json'}
     url = conn.build_url("nigiri?page=1", :limit => 5, :token => 'def', :format => 'xml')
-    assert_match /limit=5/,        url.query
-    assert_match /page=1/,         url.query
-    assert_match /format=xml/,     url.query
-    assert_match /token=def/,      url.query
-    assert_no_match /format=json/, url.query
-    assert_no_match /token=abc/,   url.query
+    assert_equal %w[format=xml limit=5 page=1 token=def], url.query.split('&').sort
+  end
+
+  def test_default_params_hash_has_indifferent_access
+    conn = Faraday::Connection.new :params => {'format' => 'json'}
+    assert conn.params.has_key?(:format)
+    conn.params[:format] = 'xml'
+    url = conn.build_url("")
+    assert_equal %w[format=xml], url.query.split('&').sort
   end
 
   def test_build_url_parses_url
@@ -222,10 +226,8 @@ class TestConnection < Faraday::TestCase
 
   def test_params_to_query_converts_hash_of_params_to_uri_escaped_query_string
     conn = Faraday::Connection.new
-    class << conn
-      public :build_query
-    end
-    assert_equal "a%5Bb%5D=1%20%2B%202", conn.build_query('a[b]' => '1 + 2')
+    url = conn.build_url('', 'a[b]' => '1 + 2')
+    assert_equal "a%5Bb%5D=1%20%2B%202", url.query
   end
 
   def test_dups_connection_object