Fix flattening of array params

[sedouard]

Consider the following Java snippet using the Stripe java SDK:

final Map<String, Object> params=newLinkedHashMap<String, Object>();
finalMap<String, Object> legal_entity=newLinkedHashMap<String, Object>();
finalList<Map> additional_owners=newArrayList<Map>();
finalMap<String, Object> owner_0=newLinkedHashMap<String, Object>();

owner_0.put("first_name", "FirstOwner"); finalMap<String, Object> owner_0_address=newLinkedHashMap<String, Object>();
owner_0_address.put("line1", "First Owner Address"); 

finalMap<String, Object> owner_1=newLinkedHashMap<String, Object>();
owner_1.put("first_name", "SecondOwner");
finalMap<String, Object> owner_1_address=newLinkedHashMap<String, Object>(); owner_1_address.put("line1", "Second Owner Address");

params.put("legal_entity", legal_entity);
legal_entity.put("additional_owners", additional_owners);
owner_0.put("address", owner_0_address);
owner_1.put("address", owner_1_address);
additional_owners.add(owner_0);
additional_owners.add(owner_1);

System.out.println(LiveStripeResponseGetter.createQuery(params))

This code outputs the url query parameters:

legal_entity[additional_owners][][first_name]=FirstOwner&legal_entity[additional_owners][][address][line1]=First+Owner+Address&legal_entity[additional_owners][][first_name]=SecondOwner&legal_entity[additional_owners][][address][line1]=Second+Owner+Address

When in actuality Stripe's API cannot interpret the hashes within the array elements correctly without index numbers assigned to each empty []. Instead with this PR the output looks like:

legal_entity[additional_owners][0][first_name]=FirstOwner&legal_entity[additional_owners][0][address][line1]=First+Owner+Address&legal_entity[additional_owners][1][first_name]=SecondOwner&legal_entity[additional_owners][1][address][line1]=Second+Owner+Address

I've modified the existing tests to accommodate for this change however new tests do not seem to be required.

[enugent-stripe]

LGTM

Exciting seeing another client library getting this resolved.

[brandur-stripe]

Nice!

The one problem may be that traditionally, we've actually required both the integer-indexed and non-indexed versions of arrays, depending on the endpoint. Certain special-cased ones can support the integer-indexed style (e.g., update account), but others couldn't.

We're now fully converted over to the new AbstractAPIMethod, which in theory should give any endpoint the ability to tolerate integer indexes, but we should probably do a little more in the way of testing before fully assuming that.

If you look at how this is implemented in other SDKs, we usually have special encoding calls in places we know we'll need them. e.g.:

module Stripe
  class Subscription < APIResource

    def self.update(id, params = {}, opts = {})
      params[:items] = Util.array_to_hash(params[:items]) if params[:items]
      super(id, params, opts)
    end

  ...
end

[brandur-stripe]

Left a minor comment, but otherwise LGTM. Thanks again @stevene-stripe!

ptal @stevene-stripe

[brandur-stripe]

Would you mind updating this comment to something like this (now that we expect to have integer indexes everywhere):

	// Because application/x-www-form-urlencoded cannot represent an empty
	// list, convention is to take the list parameter and just set it to an
	// empty string. (e.g. A regular list might look like `a[]=1&b[]=2` or
	// `a[0]=1&b[1]=2` when encoded with Stripe's style. Emptying it would look
	// like `a=`.)

[brandur-stripe]

There's also a duplicate comment in flattenParamsArray that should be updated.

[sedouard]

fixed!

[brandur-stripe]

We can fix this in a different PR, but just wanted to flag that having both flattenParamsArray and flattenParamsList is pretty silly — these should be merged into one method that just takes an Iterator.

[sedouard]

Sigh agreed.

[brandur-stripe]

And just to address my comment above: we discussed and tested this offline, and it seems that we're now safe to use integer-indexed arrays everywhere, so ignore my previous objection.

[brandur-stripe]

Thanks! LGTM.

[brandur-stripe]

Released as 6.4.0.

[sedouard]

@brandur-stripe can we get this version pushed to maven central?

[brandur-stripe]

@sedouard Sorry, something must have gone wrong with the release process. I've manually pruned Sontatype and confirmed a successful release. It should bubble out to Maven Central within the next few hours.