StripeObject#method_missing doesn't agree with StripeObject#respond_to_missing?

[bloopletech]

Example:

> account = Stripe::Account.new
 => #<Stripe::Account:0x4f9e6c0> JSON: {
}
> account.respond_to?('email=')
 => false
> account.email = '123@example.com'
 => "123@example.com"
> account.email
 => "123@example.com"

The #method_missing implementation on StripeObject allows the user to assign arbitrary key/values into the @values hash. However, #respond_to_missing? only returns true if the method name passed in matches a key that is already in the @values hash.

This has follow on effects which I'm going to write up into a separate issue, but an example of the other issues this introduces is:

> account = Stripe::Account.new
 => #<Stripe::Account:0x4f8f6e8> JSON: {
}
> account.update_attributes({ email: '123@example.com' })
ArgumentError: email is not an attribute that can be assigned on this object
> account.email = '123@example.com'
 => "123@example.com"
> account.email
 => "123@example.com"

Because #update_attributes_with_options calls #respond_to? to check whether to allow the assignment, it will fail until something either adds the key to @values, or adds the accessor method to the object.

[brandur]

Thanks for writing in @bloopletech!

So although a discrepancy around #respond_to? helps cause this problem, I think the actual lapse in functionality that we're concerned with is that #update_attributes is more heavy-handed about what it allows to be set than direct assignment (i..e account.email = ...).

The most obvious thing to do here is to loosen its constraints so that they fall in-line with direct assignment. In a way I hate to do this though, because we'd be giving up one of the very, very few places in this gem where we've managed to put any kind of safety in. Another possibility is that we could add #update_attributes_unsafe which would set anything but strongly imply that the operation may produce unexpected results, and then aim to deprecate that version of the method once we have the ability to do so (i.e. a schema).

I'm probably going to lean toward the latter solution unless some other ideas are presented here. /cc @kyleconroy

[kyleconroy]

Hmm, I think update_attributes should probably have the same affect as direct assignment. In the future, we can then make assignment and attribute setting both much safer.

[brandur]

Hmm, I think update_attributes should probably have the same affect as direct assignment. In the future, we can then make assignment and attribute setting both much safer.

Yeah, good call. Dialed down safety to fix this issue in #329.

[bloopletech]

Awesome! Thanks for looking at this issue.