sharpetronics/stripe-ruby
1
0
Fork 0
mirror of https://github.com/stripe/stripe-ruby.git synced 2025-06-18 00:01:05 -04:00
Code Issues Projects Releases Wiki Activity

Special case the serialization of account's additional_owners

Browse Source 
We attempt to do a special encoding trick when serializing
`additional_owners` under an account: when updating a value, we actually
send the update parameters up as an integer-indexed hash rather than an
array. So instead of this:

    field[]=item1&field[]=item2&field[]=item3

We send this:

    field[0]=item1&field[1]=item2&field[2]=item3

The trouble is that this had previously been built into the core library
as the default handling for all arrays. Because of this, it was
impossible to resize a non-`additional_owners` array as described in
more detail in #340.

This patch special cases `additional_owners` and brings sane behavior
back to normal arrays along with a test suite so that we try to build
some better guarantees around both the general and non-general cases.
This commit is contained in:
Brandur 2015-10-27 08:55:16 -07:00
parent 1182539ae4
commit 431ef3b1f2
5 changed files with 299 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
lib/stripe/account.rb
View File

@ -29,6 +29,43 @@ module Stripe
super(id, opts)
end
# Somewhat unfortunately, we attempt to do a special encoding trick when
# serializing `additional_owners` under an account: when updating a value,
# we actually send the update parameters up as an integer-indexed hash
# rather than an array. So instead of this:
#
# field[]=item1&field[]=item2&field[]=item3
#
# We send this:
#
# field[0]=item1&field[1]=item2&field[2]=item3
#
# There are two major problems with this technique:
#
# * Entities are addressed by array index, which is not stable and can
# easily result in unexpected results between two different requests.
#
# * A replacement of the array's contents is ambiguous with setting a
# subset of the array. Because of this, the only way to shorten an
# array is to unset it completely by making sure it goes into the
# server as an empty string, then setting its contents again.
#
# We're trying to get this overturned on the server side, but for now,
# patch in a special allowance.
def self.serialize_params(obj, original_value=nil)
update_hash = StripeObject.serialize_params(obj, original_value)
case obj
when StripeObject
obj_values = obj.instance_variable_get(:@values)
obj_values.each do |k, v|
if k == :additional_owners && v.is_a?(Array)
update_hash[k] = serialize_additional_owners(obj, v)
end
end
end
update_hash
end
def protected_fields
[:legal_entity]
end
@ -49,5 +86,28 @@ module Stripe
end
ARGUMENT_NOT_PROVIDED = Object.new
private
def self.serialize_additional_owners(obj, value)
original_value = obj.instance_variable_get(:@original_values)[:additional_owners]
if original_value && original_value.length > value.length
# url params provide no mechanism for deleting an item in an array,
# just overwriting the whole array or adding new items. So let's not
# allow deleting without a full overwrite until we have a solution.
raise ArgumentError.new(
"You cannot delete an item from an array, you must instead set a new array"
)
end
update_hash = {}
value.each_with_index do |v, i|
update = StripeObject.serialize_params(v)
if update != {} && (!original_value || update != original_value[i])
update_hash[i.to_s] = update
end
end
update_hash
end
end
end

47
lib/stripe/stripe_object.rb
View File

@ -170,6 +170,13 @@ module Stripe
case obj
when nil
''
when Array
update = obj.map { |v| serialize_params(v) }
if original_value != update
update
else
nil
end
when StripeObject
unsaved_keys = obj.instance_variable_get(:@unsaved_values)
obj_values = obj.instance_variable_get(:@values)
@ -180,37 +187,23 @@ module Stripe
end
obj_values.each do |k, v|
if v.is_a?(StripeObject) || v.is_a?(Hash)
update_hash[k] = obj.serialize_nested_object(k)
elsif v.is_a?(Array)
original_value = obj.instance_variable_get(:@original_values)[k]
if original_value && original_value.length > v.length
# url params provide no mechanism for deleting an item in an array,
# just overwriting the whole array or adding new items. So let's not
# allow deleting without a full overwrite until we have a solution.
raise ArgumentError.new(
"You cannot delete an item from an array, you must instead set a new array"
)
end
update_hash[k] = serialize_params(v, original_value)
if v.is_a?(Array)
original_value = obj.instance_variable_get(:@original_values)[k]
# the conditional here tests whether the old and new values are
# different (and therefore needs an update), or the same (meaning
# we can leave it out of the request)
if updated = serialize_params(v, original_value)
update_hash[k] = updated
else
update_hash.delete(k)
end
elsif v.is_a?(StripeObject) || v.is_a?(Hash)
update_hash[k] = obj.serialize_nested_object(k)
end
end
update_hash
when Array
update_hash = {}
obj.each_with_index do |value, index|
update = serialize_params(value)
if update != {} && (!original_value || update != original_value[index])
update_hash[index] = update
end
end
if update_hash == {}
nil
else
update_hash
end
else
obj
end

97
test/stripe/account_test.rb
View File

@ -151,5 +151,102 @@ module Stripe
a = Stripe::Account.retrieve
assert_equal(BankAccount, a.external_accounts.data[0].class)
end
should "#serialize_params an a new additional_owners" do
obj = Stripe::Util.convert_to_stripe_object({
:object => "account",
:legal_entity => {
},
}, {})
obj.legal_entity.additional_owners = [
{ :first_name => "Joe" },
{ :first_name => "Jane" },
]
expected = {
:legal_entity => {
:additional_owners => {
"0" => { :first_name => "Joe" },
"1" => { :first_name => "Jane" },
}
}
}
assert_equal(expected, obj.class.serialize_params(obj))
end
should "#serialize_params on an partially changed additional_owners" do
obj = Stripe::Util.convert_to_stripe_object({
:object => "account",
:legal_entity => {
:additional_owners => [
Stripe::StripeObject.construct_from({
:first_name => "Joe"
}),
Stripe::StripeObject.construct_from({
:first_name => "Jane"
}),
]
}
}, {})
obj.legal_entity.additional_owners[1].first_name = "Stripe"
expected = {
:legal_entity => {
:additional_owners => {
"1" => { :first_name => "Stripe" }
}
}
}
assert_equal(expected, obj.class.serialize_params(obj))
end
should "#serialize_params on an unchanged additional_owners" do
obj = Stripe::Util.convert_to_stripe_object({
:object => "account",
:legal_entity => {
:additional_owners => [
Stripe::StripeObject.construct_from({
:first_name => "Joe"
}),
Stripe::StripeObject.construct_from({
:first_name => "Jane"
}),
]
}
}, {})
expected = {
:legal_entity => {
:additional_owners => {}
}
}
assert_equal(expected, obj.class.serialize_params(obj))
end
# Note that the empty string that we send for this one has a special
# meaning for the server, which interprets it as an array unset.
should "#serialize_params on an unset additional_owners" do
obj = Stripe::Util.convert_to_stripe_object({
:object => "account",
:legal_entity => {
:additional_owners => [
Stripe::StripeObject.construct_from({
:first_name => "Joe"
}),
Stripe::StripeObject.construct_from({
:first_name => "Jane"
}),
]
}
}, {})
obj.legal_entity.additional_owners = nil
expected = {
:legal_entity => {
:additional_owners => ""
}
}
assert_equal(expected, obj.class.serialize_params(obj))
end
end
end

93
test/stripe/stripe_object_test.rb
View File

@ -99,5 +99,98 @@ module Stripe
# it to something more useful).
assert_equal opts, source.instance_variable_get(:@opts)
end
should "#serialize_params on an empty object" do
obj = Stripe::StripeObject.construct_from({})
assert_equal({}, Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on a basic object" do
obj = Stripe::StripeObject.construct_from({ :foo => nil })
obj.update_attributes(:foo => "bar")
assert_equal({ :foo => "bar" }, Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on a more complex object" do
obj = Stripe::StripeObject.construct_from({
:foo => Stripe::StripeObject.construct_from({
:bar => nil,
:baz => nil,
}),
})
obj.foo.bar = "newbar"
assert_equal({ :foo => { :bar => "newbar" } },
Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on an array" do
obj = Stripe::StripeObject.construct_from({
:foo => nil,
})
obj.foo = ["new-value"]
assert_equal({ :foo => ["new-value"] },
Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on an array that shortens" do
obj = Stripe::StripeObject.construct_from({
:foo => ["0-index", "1-index", "2-index"],
})
obj.foo = ["new-value"]
assert_equal({ :foo => ["new-value"] },
Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on an array that lengthens" do
obj = Stripe::StripeObject.construct_from({
:foo => ["0-index", "1-index", "2-index"],
})
obj.foo = ["new-value"] * 4
assert_equal({ :foo => ["new-value"] * 4 },
Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on an array of hashes" do
obj = Stripe::StripeObject.construct_from({
:foo => nil,
})
obj.foo = [
Stripe::StripeObject.construct_from({
:bar => nil
})
]
obj.foo[0].bar = "baz"
assert_equal({ :foo => [{ :bar => "baz" }] },
Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on an partially changed array of hashes" do
obj = Stripe::StripeObject.construct_from({
:foo => [
Stripe::StripeObject.construct_from({
:bar => nil
}),
Stripe::StripeObject.construct_from({
:bar => nil
}),
]
})
obj.foo[1].bar = "baz"
assert_equal({ :foo => [{}, { :bar => "baz" }] },
Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params doesn't include unchanged values" do
obj = Stripe::StripeObject.construct_from({ :foo => nil })
assert_equal({}, Stripe::StripeObject.serialize_params(obj))
end
should "#serialize_params on an array that is unchanged" do
obj = Stripe::StripeObject.construct_from({
:foo => ["0-index", "1-index", "2-index"],
})
obj.foo = ["0-index", "1-index", "2-index"]
assert_equal({}, Stripe::StripeObject.serialize_params(obj))
end
end
end

29
test/stripe/util_test.rb
View File

@ -10,6 +10,9 @@ module Stripe
[:c, "bar&baz"],
[:d, { :a => "a", :b => "b" }],
[:e, [0, 1]],
# note the empty hash won't even show up in the request
[:f, []]
]
assert_equal(
"a=3&b=%2Bfoo%3F&c=bar%26baz&d[a]=a&d[b]=b&e[]=0&e[]=1",
@ -84,5 +87,31 @@ module Stripe
assert_raise { Stripe::Util.normalize_opts(nil) }
assert_raise { Stripe::Util.normalize_opts(:api_key => nil) }
end
should "#convert_to_stripe_object should pass through unknown types" do
obj = Util.convert_to_stripe_object(7, {})
assert_equal 7, obj
end
should "#convert_to_stripe_object should turn hashes into StripeObjects" do
obj = Util.convert_to_stripe_object({ :foo => "bar" }, {})
assert obj.is_a?(StripeObject)
assert_equal "bar", obj.foo
end
should "#convert_to_stripe_object should turn lists into ListObjects" do
obj = Util.convert_to_stripe_object({ :object => "list" }, {})
assert obj.is_a?(ListObject)
end
should "#convert_to_stripe_object should marshal other classes" do
obj = Util.convert_to_stripe_object({ :object => "account" }, {})
assert obj.is_a?(Account)
end
should "#convert_to_stripe_object should marshal arrays" do
obj = Util.convert_to_stripe_object([1, 2, 3], {})
assert_equal [1, 2, 3], obj
end
end
end