Allow empty strings in API invocation parameters

Currently, with a normal API resource, you can unset fields by
specifying a `nil` to that field's setter:

``` ruby
c = Charge.retrieve('ch_123')
c.customer = nil
c.save
```

This actually gets serialized as the form `customer=` (i.e. an empty
string), but we had to use the empty string to handle unsets because
form encoding has no concept of a `nil`/`null`.

To try and prevent usage errors, we actually prevent you from setting
fields with an empty string:

``` ruby
c = Charge.retrieve('ch_123')
c.customer = '' # error! use nil instead
```

When specifying parameters though, this doesn't work anywhere nearly as
well because usage patterns like this are very common in Ruby:

``` ruby
charge_opts = {
  params[:amount],
  params[:currency],
  params[:customer],
}
charge = Charge.create(charge_opts)
```

Each one of `params` above may or may not be `nil`, so we've
traditionally filtered those fields out during the invocation of
`Charge.create`.

Recently, I suggested to Slava that we may be able to change this
behavior, and we ended up putting in a patch as part of #557. Users
brought to my attention that this would be far too disruptive of a
change in #560 though, and having thought about it more carefully, I
agree. There's also an argument that filtered `nil` values are just a
better API, especially in Ruby where patterns like the one above are
frequently in effect.

So the best thing I can think of currently is to leave things as they
were before #557, and just require that users use an explicit empty
string when passes in parameter hashes:

``` ruby
Charge.update(customer: '') # will try to unset customer
```

Empty strings will continue to error for `StripeObject` fields like they
always have.

I don't think this is a perfect solution by any means (the different
between values on `StripeObject` versus using parameters is weird), but
it's the least disruptive thing that I can think of right now that gets
us the functionality that we need for endpoints like
`/v1/invoices/upcoming`.

Fixes #560.
This commit is contained in:
Brandur 2017-07-27 13:35:12 -07:00
parent b0918b9317
commit 1417cb5bd1
3 changed files with 25 additions and 10 deletions

View File

@ -117,11 +117,6 @@ module Stripe
check_api_key!(api_key)
# there's no concept of a nil in form encoding, so convert any explicitly
# provided nils to an empty string which the API will handle
# appropriately
params = params.merge(params) {|_, v| v.nil? ? '' : v}
params = Util.objects_to_ids(params)
url = api_url(url, api_base)

View File

@ -74,6 +74,19 @@ module Stripe
}
assert invoice.kind_of?(Stripe::Invoice)
end
should "be callable with an empty string" do
invoice = Stripe::Invoice.upcoming(
coupon: '',
customer: API_FIXTURES[:customer][:id]
)
assert_requested :get, "#{Stripe.api_base}/v1/invoices/upcoming",
query: {
coupon: '',
customer: API_FIXTURES[:customer][:id]
}
assert invoice.kind_of?(Stripe::Invoice)
end
end
end
end

View File

@ -493,17 +493,24 @@ module Stripe
end
context "params serialization" do
should 'convert nil params to empty string' do
should 'allows empty strings in params' do
client = StripeClient.new
client.execute_request(:get, '/v1/invoices/upcoming',
params: { customer: API_FIXTURES[:customer][:id],
subscription: API_FIXTURES[:subscription][:id],
coupon: nil})
coupon: '' })
assert_requested(:get, "#{Stripe.api_base}/v1/invoices/upcoming?",
query: { customer: API_FIXTURES[:customer][:id],
subscription: API_FIXTURES[:subscription][:id],
coupon: '' })
end
should 'filter nils in params' do
client = StripeClient.new
client.execute_request(:get, '/v1/invoices/upcoming',
params: { customer: API_FIXTURES[:customer][:id],
coupon: nil })
assert_requested(:get, "#{Stripe.api_base}/v1/invoices/upcoming?",
query: { customer: API_FIXTURES[:customer][:id] })
end
end
end