From 8794387ac34d4b612de9e5cee4f2e2f397870853 Mon Sep 17 00:00:00 2001 From: Ben Rosenblum Date: Fri, 1 Jun 2012 23:17:28 -0400 Subject: [PATCH 1/6] Correctly serializes nested hash elements when a root element is used with an Entity. Previously, when a root element is used with an Entity, the presenter wraps the array of Entity object in a plain hash, and the Entities end up serialized as plain strings of the class name (i.e., {"root":["#","#"]) The new code recursively checks for any elements of the hash that respond to serializable_hash and calls it if present. --- lib/grape/middleware/base.rb | 21 +++++++++++++++------ spec/grape/middleware/formatter_spec.rb | 12 ++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index be7de3864..cd80827ae 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -110,17 +110,26 @@ def decode_json(object) MultiJson.load(object) end + def serialize_recurse(object) + if object.respond_to? :serializable_hash + object.serializable_hash + elsif object.kind_of?(Array) && !object.map {|o| o.respond_to? :serializable_hash }.include?(false) + object.map {|o| o.serializable_hash } + elsif object.kind_of?(Hash) + object.inject({}) { |h,(k,v)| h[k] = serialize_recurse(v); h } + else + object + end + end + def encode_json(object) return object if object.is_a?(String) - if object.respond_to? :serializable_hash - MultiJson.dump(object.serializable_hash) - elsif object.kind_of?(Array) && !object.map {|o| o.respond_to? :serializable_hash }.include?(false) - MultiJson.dump(object.map {|o| o.serializable_hash }) - elsif object.respond_to? :to_json + serialized_hash = serialize_recurse(object) + if (object == serialized_hash) && (object.respond_to? :to_json) object.to_json else - MultiJson.dump(object) + MultiJson.dump(serialized_hash) end end diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 4461b1ff7..fa58a4dc0 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -48,6 +48,18 @@ def serializable_hash subject.call({'PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json'}).last.each{|b| b.should == '{"abc":"def"}'} end + it 'should serialize objects that respond to #serializable_hash if there is a root element' do + class SimpleExample + def serializable_hash + {:abc => 'def'} + end + end + + @body = {"root" => SimpleExample.new} + + subject.call({'PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json'}).last.each{|b| b.should == '{"root":{"abc":"def"}}'} + end + it 'should call #to_xml if the content type is xml' do @body = "string" @body.instance_eval do From eb49952a646efd04f5276812059f6346a8b0bd34 Mon Sep 17 00:00:00 2001 From: Ben Rosenblum Date: Wed, 13 Jun 2012 13:22:50 -0400 Subject: [PATCH 2/6] update CHANGELOG with info from pull request 181 (fix JSON serialization for nested hashes of presenters) --- CHANGELOG.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.markdown b/CHANGELOG.markdown index e17c4a01a..889f25038 100644 --- a/CHANGELOG.markdown +++ b/CHANGELOG.markdown @@ -1,6 +1,7 @@ Next Release ============ +* [#181](https://github.com/intridea/grape/pull/181): Fix: Corrected JSON serialization of nested hashes containing `Grape::Entity` instances - [@benrosenblum](https://github.com/benrosenblum). * [#64](https://github.com/intridea/grape/issues/64), [#180](https://github.com/intridea/grape/pull/180): Added support to get request bodies as parameters - [@bobbytables](https://github.com/bobbytables). * [#175](https://github.com/intridea/grape/pull/175): Added support for API versioning based on a request parameter - [@jackcasey](https://github.com/jackcasey). * [#168](https://github.com/intridea/grape/pull/168): Fix: Formatter can parse symbol keys in the headers hash - [@netmask](https://github.com/netmask). From a1de0e2e202409f737f3f8a7dcb1a4055b68d2ac Mon Sep 17 00:00:00 2001 From: "Kyle S. Goodwin" Date: Fri, 22 Jun 2012 13:16:41 -0400 Subject: [PATCH 3/6] Grape::Entity#serializable_hash should call #serializable_hash if the data returned from #value_for responds to that method so that nested Grape::Entity presentation works correctly. --- lib/grape/entity.rb | 5 ++++- spec/grape/entity_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/grape/entity.rb b/lib/grape/entity.rb index 46a38a9b8..f1752dda2 100644 --- a/lib/grape/entity.rb +++ b/lib/grape/entity.rb @@ -232,7 +232,10 @@ def serializable_hash(runtime_options = {}) return nil if object.nil? opts = options.merge(runtime_options || {}) exposures.inject({}) do |output, (attribute, exposure_options)| - output[key_for(attribute)] = value_for(attribute, opts) if conditions_met?(exposure_options, opts) + partial_output = value_for(attribute, opts) if conditions_met?(exposure_options, opts) + partial_output = partial_output.serializable_hash(runtime_options) if partial_output.respond_to? :serializable_hash + output[key_for(attribute)] = partial_output + output end end diff --git a/spec/grape/entity_spec.rb b/spec/grape/entity_spec.rb index e80bc4167..58c6b35d4 100644 --- a/spec/grape/entity_spec.rb +++ b/spec/grape/entity_spec.rb @@ -257,6 +257,28 @@ fresh_class.expose :name expect{ fresh_class.new(nil).serializable_hash }.not_to raise_error end + + it 'should serialize embedded objects which respond to #serializable_hash' do + class EmbeddedExample + def serializable_hash(opts = {}) + {:abc => 'def'} + end + end + + class SimpleExample + def name + "abc" + end + + def embedded + EmbeddedExample.new + end + end + + fresh_class.expose :name, :embedded + presenter = fresh_class.new(SimpleExample.new) + presenter.serializable_hash.should == {:name => "abc", :embedded => {:abc => "def"}} + end end describe '#value_for' do From 04149becfa44e183521aa72be15781e1b2aee81e Mon Sep 17 00:00:00 2001 From: "Kyle S. Goodwin" Date: Tue, 26 Jun 2012 07:36:13 -0400 Subject: [PATCH 4/6] Grape::Entity#serializable_hash should call #serializable_hash elementwise if the data returned from #value_for is an array for which each element responds to that method so that nested Grape::Entity presentation works correctly with arrays of values. --- lib/grape/entity.rb | 11 +++++++++-- spec/grape/entity_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/grape/entity.rb b/lib/grape/entity.rb index f1752dda2..0bd78f159 100644 --- a/lib/grape/entity.rb +++ b/lib/grape/entity.rb @@ -233,8 +233,15 @@ def serializable_hash(runtime_options = {}) opts = options.merge(runtime_options || {}) exposures.inject({}) do |output, (attribute, exposure_options)| partial_output = value_for(attribute, opts) if conditions_met?(exposure_options, opts) - partial_output = partial_output.serializable_hash(runtime_options) if partial_output.respond_to? :serializable_hash - output[key_for(attribute)] = partial_output + + output[key_for(attribute)] = + if partial_output.respond_to? :serializable_hash + partial_output.serializable_hash(runtime_options) + elsif partial_output.kind_of?(Array) && !partial_output.map {|o| o.respond_to? :serializable_hash}.include?(false) + partial_output.map {|o| o.serializable_hash} + else + partial_output + end output end diff --git a/spec/grape/entity_spec.rb b/spec/grape/entity_spec.rb index 58c6b35d4..ea937bf5c 100644 --- a/spec/grape/entity_spec.rb +++ b/spec/grape/entity_spec.rb @@ -279,6 +279,28 @@ def embedded presenter = fresh_class.new(SimpleExample.new) presenter.serializable_hash.should == {:name => "abc", :embedded => {:abc => "def"}} end + + it 'should serialize embedded arrays of objects which respond to #serializable_hash' do + class EmbeddedExample + def serializable_hash(opts = {}) + {:abc => 'def'} + end + end + + class SimpleExample + def name + "abc" + end + + def embedded + [EmbeddedExample.new, EmbeddedExample.new] + end + end + + fresh_class.expose :name, :embedded + presenter = fresh_class.new(SimpleExample.new) + presenter.serializable_hash.should == {:name => "abc", :embedded => [{:abc => "def"}, {:abc => "def"}]} + end end describe '#value_for' do From 67250db4363d344975a77f367ed860698a4520c3 Mon Sep 17 00:00:00 2001 From: Ben Rosenblum Date: Mon, 16 Jul 2012 15:48:09 -0400 Subject: [PATCH 5/6] cleanup recursive json serialization code per comments from dblock --- lib/grape/middleware/base.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index cd80827ae..9413c3c59 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -110,13 +110,19 @@ def decode_json(object) MultiJson.load(object) end - def serialize_recurse(object) + 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) if object.respond_to? :serializable_hash object.serializable_hash elsif object.kind_of?(Array) && !object.map {|o| o.respond_to? :serializable_hash }.include?(false) object.map {|o| o.serializable_hash } elsif object.kind_of?(Hash) - object.inject({}) { |h,(k,v)| h[k] = serialize_recurse(v); h } + object.inject({}) { |h,(k,v)| h[k] = serialize(v); h } else object end @@ -124,13 +130,10 @@ def serialize_recurse(object) def encode_json(object) return object if object.is_a?(String) + return MultiJson.dump(serialize(object)) if serializable?(object) + return object.to_json if object.respond_to?(:to_json) - serialized_hash = serialize_recurse(object) - if (object == serialized_hash) && (object.respond_to? :to_json) - object.to_json - else - MultiJson.dump(serialized_hash) - end + MultiJson.dump(object) end def encode_txt(object) From 18419c4892a18028e0fe1211628040285e1d8bc1 Mon Sep 17 00:00:00 2001 From: Ben Rosenblum Date: Mon, 16 Jul 2012 16:21:00 -0400 Subject: [PATCH 6/6] fix changelog info for #181 under wrong section --- CHANGELOG.markdown | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.markdown b/CHANGELOG.markdown index 01ff3382d..8428c00ba 100644 --- a/CHANGELOG.markdown +++ b/CHANGELOG.markdown @@ -1,7 +1,10 @@ +Next Release +================= +* [#181](https://github.com/intridea/grape/pull/181): Fix: Corrected JSON serialization of nested hashes containing `Grape::Entity` instances - [@benrosenblum](https://github.com/benrosenblum). + 0.2.1 (7/11/2012) ================= -* [#181](https://github.com/intridea/grape/pull/181): Fix: Corrected JSON serialization of nested hashes containing `Grape::Entity` instances - [@benrosenblum](https://github.com/benrosenblum). * [#186](https://github.com/intridea/grape/issues/186): Fix: helpers allow multiple calls with modules and blocks - [@ppadron](https://github.com/ppadron). * [#188](https://github.com/intridea/grape/pull/188): Fix: multi-method routes append '(.:format)' only once - [@kainosnoema](https://github.com/kainosnoema). * [#64](https://github.com/intridea/grape/issues/64), [#180](https://github.com/intridea/grape/pull/180): Added support to get request bodies as parameters - [@bobbytables](https://github.com/bobbytables).