Check for valid configuration keys on initialization

[troessner]

Originated from here: #117

[jondecko]

I would have to dig into other gems to see a pattern... but is there any general agreement on how do handle keys that are not "handled"? Let's look at it from transitions point of view...

event :explode, :timestamp => true, :foo => true do
  transitions :from => :complete, :to => :exploded
end

In the above, we obviously use :timestamp as a valid key but :foo obviously references nothing. Do we raise or do we just do nothing? Let's consider the next example...

class Order < ActiveRecord::Base
  include ActiveModel::Transitions
  state_machine :auto_scopes => true, :invalid_option => true do    #introduced :invalid_option key
    state :pick_line_items
    state :picking_line_items
    event :move_cart do
      transitions to: :pick_line_items, from: :picking_line_items
    end
  end
end

In the next example we are not working on the event but another area where we build up the state machine. We would need to expand this type of error checking to other parts of the gem to be consistent.

[geoffgarside]

Rails uses Hash#assert_valid_keys() in quite a few places which would reject any invalid keys being provided to a method. As new options are supported in the underlying method implementation then the valid keys list will need to be expanded. It does also give a nice hint when reading the code what kind of options you can send to a method.

[jondecko]

@geoffgarside - Thanks for the heads up how rails handles it. I think if we add it into this gem we will have to be very consistent in adding this type of assertion across each interface where we are passing in keys.

[troessner]

@jondecko @geoffgarside sounds excellent to me. Personally I am all on the "raise hell" side when it comes to invalid keys since imho it's too easy to make a mistake you'll only notice in production 4 weeks later.

[jondecko]

You are correct. This change would potentially break code that is working through silent errors. We just have to properly communicate that when/if this gets merged.

[troessner]

We just have to properly communicate that when/if this gets merged.

Since we're following semantic versioning this shouldn't be a problem - we'd just bump up our gem version to a major version number which would already indicate that we break backwards compatibility.