Break up concerns for choosing what attributes should be serialized and the actual serializer

5 files changed, 87 insertions, 99 deletions

diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb
index b24a929ff5..244f3a546e 100644
--- a/activemodel/lib/active_model.rb
+++ b/activemodel/lib/active_model.rb
@@ -35,7 +35,7 @@ module ActiveModel
   autoload :Naming, 'active_model/naming'
   autoload :Observer, 'active_model/observing'
   autoload :Observing, 'active_model/observing'
-  autoload :Serializer, 'active_model/serializer'
+  autoload :Serialization, 'active_model/serialization'
   autoload :StateMachine, 'active_model/state_machine'
   autoload :TestCase, 'active_model/test_case'
   autoload :Validations, 'active_model/validations'
diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb
new file mode 100644
index 0000000000..4c0073f687
--- /dev/null
+++ b/activemodel/lib/active_model/serialization.rb
@@ -0,0 +1,30 @@
+require 'active_support/core_ext/hash/except'
+require 'active_support/core_ext/hash/slice'
+
+module ActiveModel
+  module Serialization
+    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 }
+
+      attribute_names = attributes.keys.sort
+      if options[:only].any?
+        attribute_names &= options[:only]
+      elsif options[:except].any?
+        attribute_names -= options[:except]
+      end
+
+      method_names = Array.wrap(options[:methods]).inject([]) do |methods, name|
+        methods << name if respond_to?(name.to_s)
+        methods
+      end
+
+      (attribute_names + method_names).inject({}) { |hash, name|
+        hash[name] = send(name)
+        hash
+      }
+    end
+  end
+end
diff --git a/activemodel/lib/active_model/serializer.rb b/activemodel/lib/active_model/serializer.rb
deleted file mode 100644
index 5b603bdbd7..0000000000
--- a/activemodel/lib/active_model/serializer.rb
+++ /dev/null
@@ -1,60 +0,0 @@
-require 'active_support/core_ext/hash/except'
-require 'active_support/core_ext/hash/slice'
-
-module ActiveModel
-  class Serializer
-    attr_reader :options
-
-    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
-
-    def serialize
-      raise NotImplemented
-    end
-
-    def to_s(&block)
-      serialize(&block)
-    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 serializable_attribute_names
-      attribute_names = @serializable.attributes.keys.sort
-
-      if options[:only].any?
-        attribute_names &= options[:only]
-      elsif options[:except].any?
-        attribute_names -= options[:except]
-      end
-
-      attribute_names
-    end
-
-    def serializable_method_names
-      Array.wrap(options[:methods]).inject([]) do |methods, name|
-        methods << name if @serializable.respond_to?(name.to_s)
-        methods
-      end
-    end
-
-    def serializable_names
-      serializable_attribute_names + serializable_method_names
-    end
-
-    def serializable_hash
-      serializable_names.inject({}) { |hash, name|
-        hash[name] = @serializable.send(name)
-        hash
-      }
-    end
-  end
-end
diff --git a/activemodel/lib/active_model/serializers/json.rb b/activemodel/lib/active_model/serializers/json.rb
index e94512fd64..ee6d48bfc6 100644
--- a/activemodel/lib/active_model/serializers/json.rb
+++ b/activemodel/lib/active_model/serializers/json.rb
@@ -5,6 +5,7 @@ module ActiveModel
   module Serializers
     module JSON
       extend ActiveSupport::Concern
+      include ActiveModel::Serialization
 
       included do
         extend ActiveModel::Naming
@@ -12,19 +13,6 @@ module ActiveModel
         cattr_accessor :include_root_in_json, :instance_writer => false
       end
 
-      class Serializer < ActiveModel::Serializer
-        def serializable_hash
-          model = super
-          @serializable.include_root_in_json ?
-            { @serializable.class.model_name.element => model } :
-            model
-        end
-
-        def serialize
-          ActiveSupport::JSON.encode(serializable_hash)
-        end
-      end
-
       # Returns a JSON string representing the model. Some configuration is
       # available through +options+.
       #
@@ -92,7 +80,9 @@ module ActiveModel
       #                   {"comments": [{"body": "Don't think too hard"}],
       #                    "title": "So I was thinking"}]}
       def encode_json(encoder)
-        Serializer.new(self, encoder.options).to_s
+        hash = serializable_hash(encoder.options)
+        hash = { self.class.model_name.element => hash } if include_root_in_json
+        ActiveSupport::JSON.encode(hash)
       end
 
       def as_json(options = nil)
diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb
index 4508a39347..86149f1e5f 100644
--- a/activemodel/lib/active_model/serializers/xml.rb
+++ b/activemodel/lib/active_model/serializers/xml.rb
@@ -5,8 +5,9 @@ module ActiveModel
   module Serializers
     module Xml
       extend ActiveSupport::Concern
+      include ActiveModel::Serialization
 
-      class Serializer < ActiveModel::Serializer #:nodoc:
+      class Serializer #:nodoc:
         class Attribute #:nodoc:
           attr_reader :name, :value, :type
 
@@ -74,32 +75,32 @@ module ActiveModel
             end
         end
 
-        def builder
-          @builder ||= begin
-            require 'builder' unless defined? ::Builder
-            options[:indent] ||= 2
-            builder = options[:builder] ||= ::Builder::XmlMarkup.new(:indent => options[:indent])
+        attr_reader :options
 
-            unless options[:skip_instruct]
-              builder.instruct!
-              options[:skip_instruct] = true
-            end
+        def initialize(serializable, options = nil)
+          @serializable = serializable
+          @options = options ? options.dup : {}
 
-            builder
-          end
+          @options[:only] = Array.wrap(@options[:only]).map { |n| n.to_s }
+          @options[:except] = Array.wrap(@options[:except]).map { |n| n.to_s }
         end
 
-        def root
-          root = (options[:root] || @serializable.class.model_name.singular).to_s
-          reformat_name(root)
-        end
-
-        def dasherize?
-          !options.has_key?(:dasherize) || options[:dasherize]
-        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 serializable_attribute_names
+          attribute_names = @serializable.attributes.keys.sort
+
+          if options[:only].any?
+            attribute_names &= options[:only]
+          elsif options[:except].any?
+            attribute_names -= options[:except]
+          end
 
-        def camelize?
-          options.has_key?(:camelize) && options[:camelize]
+          attribute_names
         end
 
         def serializable_attributes
@@ -134,6 +135,34 @@ module ActiveModel
         end
 
         private
+          def builder
+            @builder ||= begin
+              require 'builder' unless defined? ::Builder
+              options[:indent] ||= 2
+              builder = options[:builder] ||= ::Builder::XmlMarkup.new(:indent => options[:indent])
+
+              unless options[:skip_instruct]
+                builder.instruct!
+                options[:skip_instruct] = true
+              end
+
+              builder
+            end
+          end
+
+          def root
+            root = (options[:root] || @serializable.class.model_name.singular).to_s
+            reformat_name(root)
+          end
+
+          def dasherize?
+            !options.has_key?(:dasherize) || options[:dasherize]
+          end
+
+          def camelize?
+            options.has_key?(:camelize) && options[:camelize]
+          end
+
           def reformat_name(name)
             name = name.camelize if camelize?
             dasherize? ? name.dasherize : name
@@ -163,8 +192,7 @@ module ActiveModel
       end
 
       def to_xml(options = {}, &block)
-        serializer = Serializer.new(self, options)
-        block_given? ? serializer.to_s(&block) : serializer.to_s
+        Serializer.new(self, options).serialize(&block)
       end
 
       def from_xml(xml)