Merge pull request #674 from stripe/brandur-integer-indexes

Integer-index encode all arrays

lib/stripe/invoice.rb

@@ -9,7 +9,6 @@ module Stripe
     OBJECT_NAME = "invoice".freeze
 
     def self.upcoming(params, opts = {})
-      params[:subscription_items] = Util.array_to_hash(params[:subscription_items]) if params[:subscription_items]
       resp, opts = request(:get, upcoming_url, params, opts)
       Util.convert_to_stripe_object(resp.data, opts)
     end

lib/stripe/order.rb

@@ -14,16 +14,10 @@ module Stripe
     end
 
     def return_order(params, opts = {})
-      params[:items] = Util.array_to_hash(params[:items]) if params[:items]
       resp, opts = request(:post, returns_url, params, opts)
       Util.convert_to_stripe_object(resp.data, opts)
     end
 
-    def self.create(params = {}, opts = {})
-      params[:items] = Util.array_to_hash(params[:items]) if params[:items]
-      super(params, opts)
-    end
-
     private
 
     def pay_url

lib/stripe/subscription.rb

@@ -16,21 +16,6 @@ module Stripe
       initialize_from({ discount: nil }, opts, true)
     end
 
-    def self.update(id, params = {}, opts = {})
-      params[:items] = Util.array_to_hash(params[:items]) if params[:items]
-      super(id, params, opts)
-    end
-
-    def self.create(params = {}, opts = {})
-      params[:items] = Util.array_to_hash(params[:items]) if params[:items]
-      super(params, opts)
-    end
-
-    def serialize_params(options = {})
-      @values[:items] = Util.array_to_hash(@values[:items]) if @values[:items]
-      super
-    end
-
     private
 
     def discount_url

lib/stripe/util.rb

@@ -190,20 +190,6 @@ module Stripe
           .map { |k, v| "#{url_encode(k)}=#{url_encode(v)}" }.join("&")
     end
 
-    # Transforms an array into a hash with integer keys. Used for a small
-    # number of API endpoints. If the argument is not an Array, return it
-    # unchanged. Example: [{foo: 'bar'}] => {"0" => {foo: "bar"}}
-    def self.array_to_hash(array)
-      case array
-      when Array
-        hash = {}
-        array.each_with_index { |v, i| hash[i.to_s] = v }
-        hash
-      else
-        array
-      end
-    end
-
     # Encodes a string in a way that makes it suitable for use in a set of
     # query parameters in a URI or in a set of form parameters in a request
     # body.
@@ -225,7 +211,6 @@ module Stripe
         if value.is_a?(Hash)
           result += flatten_params(value, calculated_key)
         elsif value.is_a?(Array)
-          check_array_of_maps_start_keys!(value)
           result += flatten_params_array(value, calculated_key)
         else
           result << [calculated_key, value]
@@ -237,13 +222,13 @@ module Stripe
 
     def self.flatten_params_array(value, calculated_key)
       result = []
-      value.each do |elem|
+      value.each_with_index do |elem, i|
         if elem.is_a?(Hash)
-          result += flatten_params(elem, "#{calculated_key}[]")
+          result += flatten_params(elem, "#{calculated_key}[#{i}]")
         elsif elem.is_a?(Array)
           result += flatten_params_array(elem, calculated_key)
         else
-          result << ["#{calculated_key}[]", elem]
+          result << ["#{calculated_key}[#{i}]", elem]
         end
       end
       result
@@ -337,44 +322,6 @@ module Stripe
     }.freeze
     private_constant :COLOR_CODES
 
-    # We use a pretty janky version of form encoding (Rack's) that supports
-    # more complex data structures like maps and arrays through the use of
-    # specialized syntax. To encode an array of maps like:
-    #
-    #     [{a: 1, b: 2}, {a: 3, b: 4}]
-    #
-    # We have to produce something that looks like this:
-    #
-    #     arr[][a]=1&arr[][b]=2&arr[][a]=3&arr[][b]=4
-    #
-    # The only way for the server to recognize that this is a two item array is
-    # that it notices the repetition of element "a", so it's key that these
-    # repeated elements are encoded first.
-    #
-    # This method is invoked for any arrays being encoded and checks that if
-    # the array contains all non-empty maps, that each of those maps must start
-    # with the same key so that their boundaries can be properly encoded.
-    def self.check_array_of_maps_start_keys!(arr)
-      expected_key = nil
-      arr.each do |item|
-        break unless item.is_a?(Hash)
-        break if item.count.zero?
-
-        first_key = item.first[0]
-
-        if expected_key
-          if expected_key != first_key
-            raise ArgumentError,
-                  "All maps nested in an array should start with the same key " \
-                  "(expected starting key '#{expected_key}', got '#{first_key}')"
-          end
-        else
-          expected_key = first_key
-        end
-      end
-    end
-    private_class_method :check_array_of_maps_start_keys!
-
     # Uses an ANSI escape code to colorize text if it's going to be sent to a
     # TTY.
     def self.colorize(val, color, isatty)

test/stripe/subscription_test.rb

@@ -56,51 +56,5 @@ module Stripe
         assert subscription.is_a?(Stripe::Subscription)
       end
     end
-
-    context "#serialize_params" do
-      should "serialize when items is set to an Array" do
-        obj = Stripe::Util.convert_to_stripe_object({
-          object: "subscription",
-          items: Stripe::Util.convert_to_stripe_object(
-            object: "list",
-            data: []
-          ),
-        }, {})
-        obj.items = [
-          { id: "si_foo", deleted: true },
-          { plan: "plan_bar" },
-        ]
-
-        expected = {
-          items: {
-            :"0" => { id: "si_foo", deleted: true },
-            :"1" => { plan: "plan_bar" },
-          },
-        }
-        assert_equal(expected, obj.serialize_params)
-      end
-
-      should "serialize when items is set to a Hash" do
-        obj = Stripe::Util.convert_to_stripe_object({
-          object: "subscription",
-          items: Stripe::Util.convert_to_stripe_object(
-            object: "list",
-            data: []
-          ),
-        }, {})
-        obj.items = {
-          "0" => { id: "si_foo", deleted: true },
-          "1" => { plan: "plan_bar" },
-        }
-
-        expected = {
-          items: {
-            :"0" => { id: "si_foo", deleted: true },
-            :"1" => { plan: "plan_bar" },
-          },
-        }
-        assert_equal(expected, obj.serialize_params)
-      end
-    end
   end
 end

test/stripe/util_test.rb

@@ -33,39 +33,11 @@ module Stripe
         g: [],
       }
       assert_equal(
-        "a=3&b=%2Bfoo%3F&c=bar%26baz&d[a]=a&d[b]=b&e[]=0&e[]=1&f=",
+        "a=3&b=%2Bfoo%3F&c=bar%26baz&d[a]=a&d[b]=b&e[0]=0&e[1]=1&f=",
         Stripe::Util.encode_parameters(params)
       )
     end
 
-    should "#encode_params should throw an error on an array of maps that cannot be encoded" do
-      params = {
-        a: [
-          { a: 1, b: 2 },
-          { c: 3, a: 4 },
-        ],
-      }
-      e = assert_raises(ArgumentError) do
-        Stripe::Util.encode_parameters(params)
-      end
-      expected = "All maps nested in an array should start with the same key " \
-                 "(expected starting key 'a', got 'c')"
-      assert_equal expected, e.message
-
-      # Make sure the check is recursive by taking our original params and
-      # nesting it into yet another map and array. Should throw exactly the
-      # same error because it's still the in inner array of maps that's wrong.
-      params = {
-        x: [
-          params,
-        ],
-      }
-      e = assert_raises(ArgumentError) do
-        Stripe::Util.encode_parameters(params)
-      end
-      assert_equal expected, e.message
-    end
-
     should "#url_encode should prepare strings for HTTP" do
       assert_equal "foo",      Stripe::Util.url_encode("foo")
       assert_equal "foo",      Stripe::Util.url_encode(:foo)
@@ -92,16 +64,16 @@ module Stripe
         ["c",        "bar&baz"],
         ["d[a]",     "a"],
         ["d[b]",     "b"],
-        ["e[]",      0],
+        ["e[0]",      0],
-        ["e[]",      1],
+        ["e[1]",      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[0][foo]", "1"],
-        ["f[][ghi]", "2"],
+        ["f[0][ghi]", "2"],
-        ["f[][foo]", "3"],
+        ["f[1][foo]", "3"],
-        ["f[][bar]", "4"],
+        ["f[1][bar]", "4"],
       ], Stripe::Util.flatten_params(params))
     end
 
@@ -160,10 +132,6 @@ module Stripe
       assert_equal [1, 2, 3], obj
     end
 
-    should "#array_to_hash should convert an array into a hash with integer keys" do
-      assert_equal({ "0" => 1, "1" => 2, "2" => 3 }, Util.array_to_hash([1, 2, 3]))
-    end
-
     context ".request_id_dashboard_url" do
       should "generate a livemode URL" do
         assert_equal "https://dashboard.stripe.com/live/logs/request-id",