Fix json serialization of nested hash when using root elements with a Grape::Entity presenter

[benrosenblum]

Previously, when a root element is used with an Entity, the presenter
wraps the array of Entity objects in a plain hash, and the Entities end
up serialized as plain strings of the class name
(i.e., {"root":["#<EntityFoo...>","#<EntityBar...>"]})

The new code recursively checks for any elements of the hash that respond
to serializable_hash and calls it if present.

[dblock]

Could you please update the new CHANGELOG.markdown with this? I'll merge it. Thanks!

[benrosenblum]

There you go. Let me know if you need any changes!

[benrosenblum]

Kyle added some additional changes that will also correctly serialize nested Entity objects in addition to nested Hashes.

[dblock]

Could you explain the comparison here? I am also afraid that seriazlie_recurse, which is quite hefty for large JSONs, is going to run for every JSON output, then get thrown out.

[benrosenblum]

The idea is that if the objects supports serializable_hash, or is an array or hash of those objects, then we need to build a new array or hash of the results of the serializable_hash calls. Otherwise, we want the results of to_json as our final output. The comparison checks if the unmodified object has been returned, and is an equivalent to the final condition in the previous code. You're right that this is unclear, and I'll refactor it to have everything inside a single serialize_object method or similar.

It is hefty for deeply nested hashes, but how else can we ensure they are properly serialized?

[dblock]

I think my biggest problem with this is that we call serialize_recurse every time, then compare the object to the result of the serialization. So I am not arguing the purpose, but the implementation has to be unwrapped into something that looks less obscure. Maybe something in this genre?

def serializable?(object)
   object.respond_to? :serializable_hash
     || object.kind_of?(Array) && !object.map {|o| o.respond_to? :serializable_hash }.include?(false)
     || object.kind_of?(Hash)
end

def serialize(object)
 # just like now
end

def encode_json(object)
 return object if object.is_a?(String)
 return MultiJson.dump(serialize(object)) if  serializable?(object)
 ...
end

I can take a stab at this if you don't find anything workable.

[benrosenblum]

Right, I agree. Your implementation is much cleaner :) I'll try to find some time in the next day or two to tidy this up.

[twoism-dev]

Is there any update on this one yet - I'm happy to give a patch a go, but if you are close I won't bother ;-)

[benrosenblum]

Yeah, I'll try and get the last update to this pushed up tomorrow.

[benrosenblum]

There we go. Let me know if you need any other changes. Thanks!

[dblock]

You should know that these definitions become global in Ruby, and notably pollute other tests. Either use subject that gets cleared every time or move the classes into spec/support with a better defined name.

[dblock]

This looks like it's in the middle of #203, could you also please take a look at that one and comment? Appreciated.

[dblock]

I merged this with some minor spec changes. Thanks everybody.

[tobert]

I was looking at adding pretty-print support for JSON and noticed:

It looks like :serializable? always returns true, which will force all hashes to be copied before serialization. It's not a huge deal, but I sometimes serve up multi-megabyte JSON docs where that would have a measurable cost.

https://github.com/intridea/grape/blob/master/lib/grape/middleware/base.rb#L116

Maybe I'm missing something? In any case that long boolean expression should have some grouping to make it easier to read.

[dblock]

@tobert => You are probably right about the serializable issue and aren't missing anything - the cost may be very high here. We've been increasingly calling to_json in our API to make results strings and avoiding automatic serialization. It also plays better with caching implementations, such as garner.

Feel free to make pull requests that make this better, including noop changes for readability.