Prefer custom #to_json implementations over #serializable_hash

[dnd]

As discussed on the mailing list, this is the change for json formatting to prefer #to_json over #serializable_hash when a custom implementation is found.

I had mentioned on the mailing list that a comparison to Object looked to be all that was needed. It turns out, however, that it's a bit muddier than that. Object was appearing as the method owner due to the object I was using being a Mongoid document.

The solution that worked ended up being a regex comparison for the owner matching /^JSON::Ext::Generator::GeneratorMethods/ as there are multiple GeneratorMethods(Object, Array, Hash, etc...). It feels dirty, but I can't come up with a better solution at the moment.

I could possibly change it to just check for the presence of more than one ancestor as well, but I'm not entirely sure how foolproof that is.

[dblock]

Thanks. I am going to leave this open for comments :)

[dblock]

As mentioned in this PR the implementation is a bit scary. Anyone please suggest something better? Maybe we can get away from guessing whether to call serializable_hash or something else in a more explicit manner?

[mbleigh]

This definitely seems a bit too specific and scary to me. Maybe a better way would be to have an optional flag on the object, e.g. prefer_direct_serialization? that, if defined and true, would call to_json preferentially. I don't think I'm comfortable merging this as-is.

[dnd]

Maybe a different question would be, what is the purpose or reason for calling #serializable_hash? Grape's Entity class just aliases to_json and serializable_hash. If you have an actual hash it has functional ability to convert using its to_json method. ActiveRecord, and Mongoid models can do their own JSON conversions. It would seem pretty much anything that wants to be convertable to JSON already implements its own method for that.

[dblock]

@dnd is right, of course. Want to try killing #serializable_hash, see what specs break?

[dnd]

All that breaks are the three specs that specifically test for the serialization of an object implementing serializable_hash.

1. Grape::Middleware::Formatter serialization should serialize the #serializable_hash if that is available
2. Grape::Middleware::Formatter serialization should serialize multiple objects that respond to #serializable_hash
3. Grape::Middleware::Formatter serialization should serialize objects that respond to #serializable_hash if there is a root element

Unfortunately this doesn't tell us at all why serializable_hash would be preferred to to_json. So in my mind it's both good and bad.

[dblock]

I pulled #282, refactoring the formatters and parsers into something more sane and where serializable hash can be turned on via format :serializable_hash for anyone who needs it. @dnd, could you do a code review for me pls?

[dblock]

I am going to close this in favor of #282. Appreciate everybody's patience.