Merge pull request #333 from stripe/brandur-remove-sort-hack

Remove sorting hack from parameter encoding

lib/stripe/util.rb

@@ -127,11 +127,7 @@ module Stripe
         end
       end
 
-      # The #sort_by call here is mostly so that we can get some stability in
+      result
-      # our 1.8.7 test suite where Hash key order is not preserved.
-      #
-      # https://www.igvita.com/2009/02/04/ruby-19-internals-ordered-hash/
-      result.sort_by { |(k, _)| k }
     end
 
     def self.flatten_params_array(value, calculated_key)

test/stripe/account_test.rb

@@ -68,7 +68,7 @@ module Stripe
 
       @mock.expects(:post).
         once.
-        with('https://api.stripe.com/v1/accounts/acct_foo', nil, 'legal_entity[address][line1]=2+Three+Four&legal_entity[first_name]=Bob').
+        with('https://api.stripe.com/v1/accounts/acct_foo', nil, 'legal_entity[first_name]=Bob&legal_entity[address][line1]=2+Three+Four').
         returns(make_response(resp))
 
       a = Stripe::Account.retrieve('acct_foo')

test/stripe/list_object_test.rb

@@ -80,8 +80,17 @@ module Stripe
     should "fetch a next page through #next_page and respect limit" do
       list = TestListObject.construct_from({ :data => [{ :id => 1 }], :has_more => true })
       list.filters = { :expand => ['data.source'], :limit => 3 }
-      @mock.expects(:get).once.with("#{Stripe.api_base}/things?expand[]=data.source&limit=3&starting_after=1", nil, nil).
+      @mock.expects(:get).with do |url, _, _|
-        returns(make_response({ :data => [{ :id => 2 }], :has_more => false }))
+        # apparently URI.parse in 1.8.7 doesn't support query parameters ...
+        url, query = url.split("?")
+        u = URI.parse(url)
+        params = CGI.parse(query)
+        u.host == URI.parse(Stripe.api_base).host && u.path == "/things" && params == {
+          "expand[]"       => ["data.source"],
+          "limit"          => ["3"],
+          "starting_after" => ["1"],
+        }
+      end.returns(make_response({ :data => [{ :id => 2 }], :has_more => false }))
       next_list = list.next_page
       assert_equal({ :expand => ['data.source'], :limit => 3 }, next_list.filters)
     end
@@ -107,8 +116,17 @@ module Stripe
     should "fetch a next page through #previous_page and respect limit" do
       list = TestListObject.construct_from({ :data => [{ :id => 2 }] })
       list.filters = { :expand => ['data.source'], :limit => 3 }
-      @mock.expects(:get).once.with("#{Stripe.api_base}/things?ending_before=2&expand[]=data.source&limit=3", nil, nil).
+      @mock.expects(:get).with do |url, _, _|
-        returns(make_response({ :data => [{ :id => 1 }] }))
+        # apparently URI.parse in 1.8.7 doesn't support query parameters ...
+        url, query = url.split("?")
+        u = URI.parse(url)
+        params = CGI.parse(query)
+        u.host == URI.parse(Stripe.api_base).host && u.path == "/things" && params == {
+          "ending_before" => ["2"],
+          "expand[]"      => ["data.source"],
+          "limit"         => ["3"],
+        }
+      end.returns(make_response({ :data => [{ :id => 1 }] }))
       next_list = list.previous_page
       assert_equal({ :expand => ['data.source'], :limit => 3 }, next_list.filters)
     end

test/stripe/metadata_test.rb

@@ -60,7 +60,7 @@ module Stripe
 
       if is_greater_than_ruby_1_9?
         check_metadata({:metadata => {:initial => 'true'}},
-                      'metadata[initial]=&metadata[uuid]=6735',
+                      'metadata[uuid]=6735&metadata[initial]=',
                       update_actions)
       end
     end

test/stripe/util_test.rb

@@ -3,13 +3,14 @@ require File.expand_path('../../test_helper', __FILE__)
 module Stripe
   class UtilTest < Test::Unit::TestCase
     should "#encode_parameters should prepare parameters for an HTTP request" do
-      params = {
+      # use array instead of hash for 1.8.7 ordering
-        :a => 3,
+      params = [
-        :b => "+foo?",
+        [:a, 3],
-        :c => "bar&baz",
+        [:b, "+foo?"],
-        :d => { :a => "a", :b => "b" },
+        [:c, "bar&baz"],
-        :e => [0, 1],
+        [:d, { :a => "a", :b => "b" }],
-      }
+        [:e, [0, 1]],
+      ]
       assert_equal(
         "a=3&b=%2Bfoo%3F&c=bar%26baz&d[a]=a&d[b]=b&e[]=0&e[]=1",
         Stripe::Util.encode_parameters(params)
@@ -25,13 +26,17 @@ module Stripe
     end
 
     should "#flatten_params should encode parameters according to Rails convention" do
-      params = {
+      params = [
-        :a => 3,
+        [:a, 3],
-        :b => "foo?",
+        [:b, "foo?"],
-        :c => "bar&baz",
+        [:c, "bar&baz"],
-        :d => { :a => "a", :b => "b" },
+        [:d, { :a => "a", :b => "b" }],
-        :e => [0, 1],
+        [:e, [0, 1]],
-      }
+        [:f, [
+          { :foo => "1", :bar => "2" },
+          { :foo => "3", :baz => "4" },
+        ]],
+      ]
       assert_equal([
         ["a",        3],
         ["b",        "foo?"],
@@ -40,6 +45,14 @@ module Stripe
         ["d[b]",     "b"],
         ["e[]",      0],
         ["e[]",      1],
+
+        # *The key here is the order*. In order to be properly interpreted as
+        # an array of hashes on the server, everything from a single hash must
+        # come in at once. A duplicate key in an array triggers a new element.
+        ["f[][foo]", "1"],
+        ["f[][bar]", "2"],
+        ["f[][foo]", "3"],
+        ["f[][baz]", "4"],
       ], Stripe::Util.flatten_params(params))
     end