Reserve some critical field names when adding StripeObject
accessors
#970
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When populating
StripeObject
s, we add accessors to them so that peoplecan access fields like
obj.currency
.This was probably only meant to apply to API resources, but through
what might have been an accident of history, we've also traditionally
unmarshaled any hash that comes back from the API as a
StripeObject
,including
metadata
fields. This allows some convenience because userscan access values like
obj.metadata.my_field
, but is also obviously aminefield for potential problems.
In issue #969, what's essentially happening is that because there's a
metadata field named
class
, we've overwritten the object's normalclass
method with our own custom one that accesses the metadata value.Amazingly, the object can still marshal/unmarshal mostly properly, but
fails on this line as we try to access
obj.class
and that turns out tobe a metadata value instead of a class:
Here I solve the problem by banning accessors added with the name
class
. This has a slight risk of backward incompatibility in thatusers that previously had metadata named "class" will now have to use
square bracket accessors instead like
obj.metadata[:class]
, buthonestly, I just can't see anything good in allowing "class" to be used
as an accessor.
An alternative solution might be to alias
class
inStripeObject
andthen make sure we always use that in places like
initialize_from
anddeep_copy
.The best long term solution would be to stop add accessors to metadata
objects. This just seems like a bad idea given that there are still
myriads of Ruby built-ins that could potentially be overwritten. This is
definitely a considerably-sized breaking change though, so we'd have to
do it on a major.
Fixes #969.
r? @richardm-stripe @remi-stripe