Merge branch 'master' of git://github.com/rails/rails

6 files changed, 93 insertions, 46 deletions

diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb
index b9f6f6cbbf..7bc3f997b5 100644
--- a/activemodel/lib/active_model/serialization.rb
+++ b/activemodel/lib/active_model/serialization.rb
@@ -78,8 +78,11 @@ module ActiveModel
         attribute_names -= Array.wrap(except).map(&:to_s)
       end
 
+      hash = {}
+      attribute_names.each { |n| hash[n] = read_attribute_for_serialization(n) }
+
       method_names = Array.wrap(options[:methods]).select { |n| respond_to?(n) }
-      hash = Hash[(attribute_names + method_names).map { |n| [n, send(n)] }]
+      method_names.each { |n| hash[n] = send(n) }
 
       serializable_add_includes(options) do |association, records, opts|
         hash[association] = if records.is_a?(Enumerable)
@@ -93,13 +96,33 @@ module ActiveModel
     end
 
     private
+
+      # Hook method defining how an attribute value should be retrieved for
+      # serialization. By default this is assumed to be an instance named after
+      # the attribute. Override this method in subclasses should you need to
+      # retrieve the value for a given attribute differently:
+      #
+      #   class MyClass
+      #     include ActiveModel::Validations
+      #
+      #     def initialize(data = {})
+      #       @data = data
+      #     end
+      #
+      #     def read_attribute_for_serialization(key)
+      #       @data[key]
+      #     end
+      #   end
+      #
+      alias :read_attribute_for_serialization :send
+
       # Add associations specified via the <tt>:include</tt> option.
       #
       # Expects a block that takes as arguments:
       #   +association+ - name of the association
       #   +records+     - the association record(s) to be serialized
       #   +opts+        - options for the association records
-      def serializable_add_includes(options = {})
+      def serializable_add_includes(options = {}) #:nodoc:
         return unless include = options[:include]
 
         unless include.is_a?(Hash)
diff --git a/activemodel/lib/active_model/serializers/json.rb b/activemodel/lib/active_model/serializers/json.rb
index 4fbccd7419..885964633f 100644
--- a/activemodel/lib/active_model/serializers/json.rb
+++ b/activemodel/lib/active_model/serializers/json.rb
@@ -86,21 +86,15 @@ module ActiveModel
       #                    "title": "Welcome to the weblog"},
       #                   {"comments": [{"body": "Don't think too hard"}],
       #                    "title": "So I was thinking"}]}
-
       def as_json(options = nil)
-        hash = serializable_hash(options)
-
-        include_root = include_root_in_json
-        if options.try(:key?, :root)
-          include_root = options[:root]
+        root = include_root_in_json
+        root = options[:root] if options.try(:key?, :root)
+        if root
+          root = self.class.model_name.element if root == true
+          { root => serializable_hash(options) }
+        else
+          serializable_hash(options)
         end
-
-        if include_root
-          custom_root = options && options[:root]
-          hash = { custom_root || self.class.model_name.element => hash }
-        end
-
-        hash
       end
 
       def from_json(json, include_root=include_root_in_json)
diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb
index 64dda3bcee..d61d9d7119 100644
--- a/activemodel/lib/active_model/serializers/xml.rb
+++ b/activemodel/lib/active_model/serializers/xml.rb
@@ -15,10 +15,10 @@ module ActiveModel
         class Attribute #:nodoc:
           attr_reader :name, :value, :type
 
-          def initialize(name, serializable, raw_value=nil)
+          def initialize(name, serializable, value)
             @name, @serializable = name, serializable
-            raw_value = raw_value.in_time_zone if raw_value.respond_to?(:in_time_zone)
-            @value = raw_value || @serializable.send(name)
+            value  = value.in_time_zone if value.respond_to?(:in_time_zone)
+            @value = value
             @type  = compute_type
           end
 
@@ -49,40 +49,24 @@ module ActiveModel
         def initialize(serializable, options = nil)
           @serializable = serializable
           @options = options ? options.dup : {}
-
-          @options[:only] = Array.wrap(@options[:only]).map { |n| n.to_s }
-          @options[:except] = Array.wrap(@options[:except]).map { |n| n.to_s }
         end
 
-        # To replicate the behavior in ActiveRecord#attributes, <tt>:except</tt>
-        # takes precedence over <tt>:only</tt>. If <tt>:only</tt> is not set
-        # for a N level model but is set for the N+1 level models,
-        # then because <tt>:except</tt> is set to a default value, the second
-        # level model can have both <tt>:except</tt> and <tt>:only</tt> set. So if
-        # <tt>:only</tt> is set, always delete <tt>:except</tt>.
-        def attributes_hash
-          attributes = @serializable.attributes
-          if options[:only].any?
-            attributes.slice(*options[:only])
-          elsif options[:except].any?
-            attributes.except(*options[:except])
-          else
-            attributes
-          end
+        def serializable_hash
+          @serializable.serializable_hash(@options.except(:include))
         end
 
-        def serializable_attributes
-          attributes_hash.map do |name, value|
-            self.class::Attribute.new(name, @serializable, value)
+        def serializable_collection
+          methods = Array.wrap(options[:methods]).map(&:to_s)
+          serializable_hash.map do |name, value|
+            name = name.to_s
+            if methods.include?(name)
+              self.class::MethodAttribute.new(name, @serializable, value)
+            else
+              self.class::Attribute.new(name, @serializable, value)
+            end
           end
         end
 
-        def serializable_methods
-          Array.wrap(options[:methods]).map do |name|
-            self.class::MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s)
-          end.compact
-        end
-
         def serialize
           require 'builder' unless defined? ::Builder
 
@@ -114,7 +98,7 @@ module ActiveModel
         end
 
         def add_attributes_and_methods
-          (serializable_attributes + serializable_methods).each do |attribute|
+          serializable_collection.each do |attribute|
             key = ActiveSupport::XmlMini.rename_key(attribute.name, options)
             ActiveSupport::XmlMini.to_tag(key, attribute.value,
               options.merge(attribute.decorations))
diff --git a/activemodel/test/cases/serialization_test.rb b/activemodel/test/cases/serialization_test.rb
index 071cb9ff4e..1ec915d245 100644
--- a/activemodel/test/cases/serialization_test.rb
+++ b/activemodel/test/cases/serialization_test.rb
@@ -77,6 +77,15 @@ class SerializationTest < ActiveModel::TestCase
     assert_equal expected , @user.serializable_hash(:methods => [:bar])
   end
 
+  def test_should_use_read_attribute_for_serialization
+    def @user.read_attribute_for_serialization(n)
+      "Jon"
+    end
+
+    expected = { "name" => "Jon" }
+    assert_equal expected, @user.serializable_hash(:only => :name)
+  end
+
   def test_include_option_with_singular_association
     expected =  {"name"=>"David", "gender"=>"male", "email"=>"david@example.com",
                  :address=>{"street"=>"123 Lane", "city"=>"Springfield", "state"=>"CA", "zip"=>11111}}
diff --git a/activemodel/test/cases/serializers/json_serialization_test.rb b/activemodel/test/cases/serializers/json_serialization_test.rb
index 5e1e7d897a..a754d610b9 100644
--- a/activemodel/test/cases/serializers/json_serialization_test.rb
+++ b/activemodel/test/cases/serializers/json_serialization_test.rb
@@ -56,6 +56,16 @@ class JsonSerializationTest < ActiveModel::TestCase
     end
   end
 
+  test "should include root in json (option) even if the default is set to false" do
+    begin
+      Contact.include_root_in_json = false
+      json = @contact.to_json(:root => true)
+      assert_match %r{^\{"contact":\{}, json
+    ensure
+      Contact.include_root_in_json = true
+    end
+  end
+
   test "should not include root in json (option)" do
 
     json = @contact.to_json(:root => false)
@@ -196,4 +206,14 @@ class JsonSerializationTest < ActiveModel::TestCase
     assert_no_match %r{"preferences":}, json
   end
 
+  test "custom as_json options should be extendible" do
+    def @contact.as_json(options = {}); super(options.merge(:only => [:name])); end
+    json = @contact.to_json
+
+    assert_match %r{"name":"Konata Izumi"}, json
+    assert_no_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
diff --git a/activemodel/test/cases/serializers/xml_serialization_test.rb b/activemodel/test/cases/serializers/xml_serialization_test.rb
index a38ef8e223..fc73d9dcd8 100644
--- a/activemodel/test/cases/serializers/xml_serialization_test.rb
+++ b/activemodel/test/cases/serializers/xml_serialization_test.rb
@@ -33,6 +33,12 @@ class Address
   end
 end
 
+class SerializableContact < Contact
+  def serializable_hash(options={})
+    super(options.merge(:only => [:name, :age]))
+  end
+end
+
 class XmlSerializationTest < ActiveModel::TestCase
   def setup
     @contact = Contact.new
@@ -96,6 +102,17 @@ class XmlSerializationTest < ActiveModel::TestCase
     assert_match %r{<createdAt},     @xml
   end
 
+  test "should use serialiable hash" do
+    @contact = SerializableContact.new
+    @contact.name = 'aaron stack'
+    @contact.age = 25
+
+    @xml = @contact.to_xml
+    assert_match %r{<name>aaron stack</name>}, @xml
+    assert_match %r{<age type="integer">25</age>}, @xml
+    assert_no_match %r{<awesome>}, @xml
+  end
+
   test "should allow skipped types" do
     @xml = @contact.to_xml :skip_types => true
     assert_match %r{<age>25</age>}, @xml