Skip calling to_hash for nil

[ashkan18]

Problem

Using stripe and representable where they both override to_hash method, depending on load order of the gems we randomly ran into issues like:

 NoMethodError:
       undefined method `definitions' for NilClass:Class
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/representable-3.0.4/lib/representable.rb:96:in `representable_attrs'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/representable-3.0.4/lib/representable.rb:74:in `representable_bindings_for'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/representable-3.0.4/lib/representable.rb:64:in `representable_map'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/representable-3.0.4/lib/representable.rb:70:in `representable_map!'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/representable-3.0.4/lib/representable.rb:45:in `create_representation_with'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/representable-3.0.4/lib/representable/hash.rb:34:in `to_hash'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/roar-1.1.0/lib/roar/json/hal.rb:58:in `to_hash'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:121:in `block in to_hash'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:129:in `block in to_hash'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:124:in `each'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:124:in `inject'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:124:in `to_hash'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:121:in `block in to_hash'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:129:in `block in to_hash'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:124:in `each'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:124:in `inject'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/stripe-1.58.0/lib/stripe/stripe_object.rb:124:in `to_hash'
     # ./spec/notifications/events/invoice_transaction_representer_spec.rb:23:in `block (2 levels) in <top (required)>'
     # ./spec/notifications/events/invoice_transaction_representer_spec.rb:27:in `block (3 levels) in <top (required)>'
     # ./spec/rails_helper.rb:43:in `block (3 levels) in <top (required)>'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/database_cleaner-1.6.1/lib/database_cleaner/generic/base.rb:16:in `cleaning'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/database_cleaner-1.6.1/lib/database_cleaner/base.rb:98:in `cleaning'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/database_cleaner-1.6.1/lib/database_cleaner/configuration.rb:86:in `block (2 levels) in cleaning'
     # /Users/ashkinas/.rvm/gems/ruby-2.4.1/gems/database_cleaner-1.6.1/lib/database_cleaner/configuration.rb:87:in `cleaning'
     # ./spec/rails_helper.rb:43:in `block (2 levels) in <top (required)>'

When calling to_hash on an Stripe::Event object.

Solution

While this seems like something that need to be fixed also on representable we noticed above issue only happens when trying to call to_hash on nil attributes of Stripe::Event so adding a simple check to see if value is not nil then call to_hash should fix the problem.

[brandur-stripe]

Agreed that this is something that should be fixed in representable, but I'm fine with a change like this because it's not really going to hurt anything.

@ashkan18 A word of warning though: this is the type of minor code change that without a comment or test is in serious danger of regressing as someone refactors in the future. Do you think a test to check the behavior would be in order?

[ob-stripe]

Maybe change this to !value.nil? instead of just value to avoid changing the behavior for non-nil falsey values.

[ashkan18]

Thanks :) I thought about that, the thing is in case of false assertion of this statement we end up just using the value and basically not call to_hash on it which should be fine for any non-nil falsey values but let me know if you can think of any issues 🙌

[ob-stripe]

I always forget that false and nil are the only falsey values in Ruby. Should be fine then, as FalseClass does not have a to_hash method anyway!

[ashkan18]

Sure @brandur-stripe, your warning makes total sense, I thought about adding a test but it would be too specific to a scenario where nil has implemented to_hash, Im happy to come up with a test or maybe more comments around it, whichever you prefer :)

[ob-stripe]

@ashkan18 Sorry for the late reply. It looks like your new test is failing on Ruby 2.0.0 because include is private in that Ruby version. Given the context, it should be safe to call the method via send(:include, ...). Do you mind updating the PR?

[ob-stripe]

Handled in #625.