fixed some issues with JSON encoding

- as_json in ActiveModel should return a hash
  and handle :only/:except/:methods options
- Array and Hash should call as_json on their elements
- json methods should not modify options argument

[#5374 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

3 files changed, 43 insertions, 16 deletions

diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb
index 542cff3aaa..8f90ef64ba 100644
--- a/activemodel/lib/active_model/serialization.rb
+++ b/activemodel/lib/active_model/serialization.rb
@@ -54,12 +54,14 @@ module ActiveModel
   #
   #   person = Person.new
   #   person.serializable_hash   # => {"name"=>nil}
-  #   person.as_json             # => "{\"name\":null}"
+  #   person.as_json             # => {"name"=>nil}
+  #   person.to_json             # => "{\"name\":null}"
   #   person.to_xml              # => "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<serial-person...
   #
   #   person.name = "Bob"
   #   person.serializable_hash   # => {"name"=>"Bob"}
-  #   person.as_json             # => "{\"name\":\"Bob\"}"
+  #   person.as_json             # => {"name"=>"Bob"}
+  #   person.to_json             # => "{\"name\":\"Bob\"}"
   #   person.to_xml              # => "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<serial-person...
   #
   # Valid options are <tt>:only</tt>, <tt>:except</tt> and <tt>:methods</tt> .
@@ -67,14 +69,14 @@ module ActiveModel
     def serializable_hash(options = nil)
       options ||= {}
 
-      options[:only]   = Array.wrap(options[:only]).map { |n| n.to_s }
-      options[:except] = Array.wrap(options[:except]).map { |n| n.to_s }
+      only   = Array.wrap(options[:only]).map(&:to_s)
+      except = Array.wrap(options[:except]).map(&:to_s)
 
       attribute_names = attributes.keys.sort
-      if options[:only].any?
-        attribute_names &= options[:only]
-      elsif options[:except].any?
-        attribute_names -= options[:except]
+      if only.any?
+        attribute_names &= only
+      elsif except.any?
+        attribute_names -= except
       end
 
       method_names = Array.wrap(options[:methods]).inject([]) do |methods, name|
diff --git a/activemodel/lib/active_model/serializers/json.rb b/activemodel/lib/active_model/serializers/json.rb
index c9271ed9b4..0bfbf2aa06 100644
--- a/activemodel/lib/active_model/serializers/json.rb
+++ b/activemodel/lib/active_model/serializers/json.rb
@@ -79,18 +79,16 @@ module ActiveModel
       #                    "title": "Welcome to the weblog"},
       #                   {"comments": [{"body": "Don't think too hard"}],
       #                    "title": "So I was thinking"}]}
-      def encode_json(encoder)
-        hash = serializable_hash(encoder.options)
+
+      def as_json(options = nil)
+        hash = serializable_hash(options)
+
         if include_root_in_json
-          custom_root = encoder.options && encoder.options[:root]
+          custom_root = options && options[:root]
           hash = { custom_root || self.class.model_name.element => hash }
         end
 
-        ActiveSupport::JSON.encode(hash)
-      end
-
-      def as_json(options = nil)
-        self
+        hash
       end
 
       def from_json(json)
diff --git a/activemodel/test/cases/serializeration/json_serialization_test.rb b/activemodel/test/cases/serializeration/json_serialization_test.rb
index 3bc39bb06d..20d123ef0b 100644
--- a/activemodel/test/cases/serializeration/json_serialization_test.rb
+++ b/activemodel/test/cases/serializeration/json_serialization_test.rb
@@ -116,5 +116,32 @@ class JsonSerializationTest < ActiveModel::TestCase
     assert_equal hash.to_json, car.errors.to_json
   end
 
+  test "serializable_hash should not modify options passed in argument" do
+    options = { :except => :name }
+    @contact.serializable_hash(options)
+
+    assert_nil options[:only]
+    assert_equal :name, options[:except]
+  end
+
+  test "as_json should return a hash" do
+    json = @contact.as_json
+
+    assert_kind_of Hash, json
+    assert_kind_of Hash, json['contact']
+    %w(name age created_at awesome preferences).each do |field|
+      assert_equal @contact.send(field), json['contact'][field]
+    end
+  end
+
+  test "custom as_json should be honored when generating json" do
+    def @contact.as_json(options); { :name => name, :created_at => created_at }; end
+    json = @contact.to_json
+
+    assert_match %r{"name":"Konata Izumi"}, json
+    assert_match %r{"created_at":#{ActiveSupport::JSON.encode(Time.utc(2006, 8, 1))}}, json
+    assert_no_match %r{"awesome":}, json
+    assert_no_match %r{"preferences":}, json
+  end
 
 end