Merge pull request #954 from bf4/encapsulate_serialization

Encapsulate serialization in ActiveModel::SerializableResource

lib/action_controller/serialization.rb

@@ -6,7 +6,8 @@ module ActionController
 
     include ActionController::Renderers
 
-    ADAPTER_OPTION_KEYS = [:include, :fields, :adapter]
+    # Deprecated
+    ADAPTER_OPTION_KEYS = ActiveModel::SerializableResource::ADAPTER_OPTION_KEYS
 
     included do
       class_attribute :_serialization_scope
@@ -18,50 +19,39 @@ module ActionController
         respond_to?(_serialization_scope, true)
     end
 
-    def get_serializer(resource)
+    def get_serializer(resource, options = {})
-      @_serializer ||= @_serializer_opts.delete(:serializer)
+      if ! use_adapter?
-      @_serializer ||= ActiveModel::Serializer.serializer_for(resource)
+        warn "ActionController::Serialization#use_adapter? has been removed. "\
-
+          "Please pass 'adapter: false' or see ActiveSupport::SerializableResource#serialize"
-      if @_serializer_opts.key?(:each_serializer)
+        options[:adapter] = false
-        @_serializer_opts[:serializer] = @_serializer_opts.delete(:each_serializer)
+      end
-      end
+      ActiveModel::SerializableResource.serialize(resource, options) do |serializable_resource|
-
+        if serializable_resource.serializer?
-      @_serializer
+          serializable_resource.serialization_scope ||= serialization_scope
+          serializable_resource.serialization_scope_name = _serialization_scope
+          begin
+            serializable_resource.adapter
+          rescue ActiveModel::Serializer::ArraySerializer::NoSerializerError
+            resource
+          end
+        else
+          resource
+        end
+      end
     end
 
+    # Deprecated
     def use_adapter?
-      !(@_adapter_opts.key?(:adapter) && !@_adapter_opts[:adapter])
+      true
     end
 
     [:_render_option_json, :_render_with_renderer_json].each do |renderer_method|
       define_method renderer_method do |resource, options|
-        @_adapter_opts, @_serializer_opts =
+        serializable_resource = get_serializer(resource, options)
-          options.partition { |k, _| ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] }
+        super(serializable_resource, options)
-
-        if use_adapter? && (serializer = get_serializer(resource))
-          @_serializer_opts[:scope] ||= serialization_scope
-          @_serializer_opts[:scope_name] = _serialization_scope
-
-          begin
-            serialized = serializer.new(resource, @_serializer_opts)
-          rescue ActiveModel::Serializer::ArraySerializer::NoSerializerError
-          else
-            resource = ActiveModel::Serializer::Adapter.create(serialized, @_adapter_opts)
       end
     end
 
-        super(resource, options)
-      end
-    end
-
-    def rescue_with_handler(exception)
-      @_serializer = nil
-      @_serializer_opts = nil
-      @_adapter_opts = nil
-
-      super(exception)
-    end
-
     module ClassMethods
       def serialization_scope(scope)
         self._serialization_scope = scope

lib/active_model/serializable_resource.rb

@@ -0,0 +1,82 @@
+require "set"
+module ActiveModel
+  class SerializableResource
+
+    ADAPTER_OPTION_KEYS = Set.new([:include, :fields, :adapter])
+
+    def initialize(resource, options = {})
+      @resource = resource
+      @adapter_opts, @serializer_opts =
+        options.partition { |k, _| ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] }
+    end
+
+    delegate :serializable_hash, :as_json, :to_json, to: :adapter
+
+    # Primary interface to building a serializer (with adapter)
+    # If no block is given,
+    # returns the serializable_resource, ready for #as_json/#to_json/#serializable_hash.
+    # Otherwise, yields the serializable_resource and
+    # returns the contents of the block
+    def self.serialize(resource, options = {})
+      serializable_resource = SerializableResource.new(resource, options)
+      if block_given?
+        yield serializable_resource
+      else
+        serializable_resource
+      end
+    end
+
+    def serialization_scope=(scope)
+      serializer_opts[:scope] = scope
+    end
+
+    def serialization_scope
+      serializer_opts[:scope]
+    end
+
+    def serialization_scope_name=(scope_name)
+      serializer_opts[:scope_name] = scope_name
+    end
+
+    def adapter
+      @adapter ||= ActiveModel::Serializer::Adapter.create(serializer_instance, adapter_opts)
+    end
+    alias_method :adapter_instance, :adapter
+
+    def serializer_instance
+      @serializer_instance ||= serializer.new(resource, serializer_opts)
+    end
+
+    # Get serializer either explicitly :serializer or implicitly from resource
+    # Remove :serializer key from serializer_opts
+    # Replace :serializer key with :each_serializer if present
+    def serializer
+      @serializer ||=
+        begin
+          @serializer = serializer_opts.delete(:serializer)
+          @serializer ||= ActiveModel::Serializer.serializer_for(resource)
+
+          if serializer_opts.key?(:each_serializer)
+            serializer_opts[:serializer] = serializer_opts.delete(:each_serializer)
+          end
+          @serializer
+        end
+    end
+    alias_method :serializer_class, :serializer
+
+    # True when no explicit adapter given, or explicit appear is truthy (non-nil)
+    # False when explicit adapter is falsy (nil or false)
+    def use_adapter?
+      !(adapter_opts.key?(:adapter) && !adapter_opts[:adapter])
+    end
+
+    def serializer?
+      use_adapter? && !!(serializer)
+    end
+
+    private
+
+    attr_reader :resource, :adapter_opts, :serializer_opts
+
+  end
+end

lib/active_model_serializers.rb

@@ -3,6 +3,7 @@ require 'active_model/serializer/version'
 require 'active_model/serializer'
 require 'active_model/serializer/fieldset'
 require 'active_model/serializer/railtie'
+require 'active_model/serializable_resource'
 
 begin
   require 'action_controller'

test/action_controller/rescue_from_test.rb

@@ -1,32 +0,0 @@
-require 'test_helper'
-
-module ActionController
-  module Serialization
-    class RescueFromTest < ActionController::TestCase
-      class RescueFromTestController < ActionController::Base
-        rescue_from Exception, with: :handle_error
-
-        def render_using_raise_error_serializer
-          @profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' })
-          render json: [@profile], serializer: RaiseErrorSerializer
-        end
-
-        def handle_error(exception)
-          render json: { errors: ['Internal Server Error'] }, status: :internal_server_error
-        end
-      end
-
-      tests RescueFromTestController
-
-      def test_rescue_from
-        get :render_using_raise_error_serializer
-
-        expected = {
-          errors: ['Internal Server Error']
-        }.to_json
-
-        assert_equal expected, @response.body
-      end
-    end
-  end
-end

test/action_controller/serialization_test.rb

@@ -4,6 +4,7 @@ require 'test_helper'
 module ActionController
   module Serialization
     class ImplicitSerializerTest < ActionController::TestCase
+      include ActiveSupport::Testing::Stream
       class ImplicitSerializationTestController < ActionController::Base
         def render_using_implicit_serializer
           @profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' })
@@ -140,10 +141,7 @@ module ActionController
 
         private
         def generate_cached_serializer(obj)
-          serializer_class = ActiveModel::Serializer.serializer_for(obj)
+          ActiveModel::SerializableResource.new(obj).to_json
-          serializer = serializer_class.new(obj)
-          adapter = ActiveModel::Serializer.adapter.new(serializer)
-          adapter.to_json
         end
 
         def with_adapter(adapter)
@@ -400,6 +398,28 @@ module ActionController
         assert_equal 'application/json', @response.content_type
         assert_equal expected.to_json, @response.body
       end
+
+      def test_warn_overridding_use_adapter_as_falsy_on_controller_instance
+        controller = Class.new(ImplicitSerializationTestController) {
+          def use_adapter?
+            false
+          end
+        }.new
+        assert_match /adapter: false/, (capture(:stderr) {
+          controller.get_serializer(@profile)
+        })
+      end
+
+      def test_dont_warn_overridding_use_adapter_as_truthy_on_controller_instance
+        controller = Class.new(ImplicitSerializationTestController) {
+          def use_adapter?
+            true
+          end
+        }.new
+        assert_equal "", (capture(:stderr) {
+          controller.get_serializer(@profile)
+        })
+      end
     end
   end
 end

test/serializable_resource_test.rb

@@ -0,0 +1,27 @@
+require 'test_helper'
+
+module ActiveModel
+  class SerializableResourceTest < Minitest::Test
+    def setup
+      @resource = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' })
+      @serializer = ProfileSerializer.new(@resource)
+      @adapter = ActiveModel::Serializer::Adapter.create(@serializer)
+      @serializable_resource = ActiveModel::SerializableResource.new(@resource)
+    end
+
+    def test_serializable_resource_delegates_serializable_hash_to_the_adapter
+      options = nil
+      assert_equal @adapter.serializable_hash(options), @serializable_resource.serializable_hash(options)
+    end
+
+    def test_serializable_resource_delegates_to_json_to_the_adapter
+      options = nil
+      assert_equal @adapter.to_json(options), @serializable_resource.to_json(options)
+    end
+
+    def test_serializable_resource_delegates_as_json_to_the_adapter
+      options = nil
+      assert_equal @adapter.as_json(options), @serializable_resource.as_json(options)
+    end
+  end
+end

test/serializers/cache_test.rb

@@ -131,10 +131,7 @@ module ActiveModel
 
       private
       def render_object_with_cache(obj)
-        serializer_class = ActiveModel::Serializer.serializer_for(obj)
+        ActiveModel::SerializableResource.new(obj).serializable_hash
-        serializer = serializer_class.new(obj)
-        adapter = ActiveModel::Serializer.adapter.new(serializer)
-        adapter.serializable_hash
       end
     end
   end

test/serializers/meta_test.rb

@@ -113,11 +113,8 @@ module ActiveModel
       private
 
       def load_adapter(options)
-        adapter_opts, serializer_opts =
+        options = options.merge(adapter: :flatten_json, serializer: AlternateBlogSerializer)
-          options.partition { |k, _| ActionController::Serialization::ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] }
+        ActiveModel::SerializableResource.new(@blog, options)
-
-        serializer = AlternateBlogSerializer.new(@blog, serializer_opts)
-        ActiveModel::Serializer::Adapter::FlattenJson.new(serializer, adapter_opts)
       end
     end
   end

test/test_helper.rb

@@ -19,6 +19,55 @@ at_exit do
 end
 require 'active_model_serializers'
 
+# Use cleaner stream testing interface from Rails 5 if available
+# see https://github.com/rails/rails/blob/29959eb59d/activesupport/lib/active_support/testing/stream.rb
+begin
+  require "active_support/testing/stream"
+rescue LoadError
+  module ActiveSupport
+    module Testing
+      module Stream #:nodoc:
+        private
+
+        def silence_stream(stream)
+          old_stream = stream.dup
+          stream.reopen(IO::NULL)
+          stream.sync = true
+          yield
+        ensure
+          stream.reopen(old_stream)
+          old_stream.close
+        end
+
+        def quietly
+          silence_stream(STDOUT) do
+            silence_stream(STDERR) do
+              yield
+            end
+          end
+        end
+
+        def capture(stream)
+          stream = stream.to_s
+          captured_stream = Tempfile.new(stream)
+          stream_io = eval("$#{stream}")
+          origin_stream = stream_io.dup
+          stream_io.reopen(captured_stream)
+
+          yield
+
+          stream_io.rewind
+          return captured_stream.read
+        ensure
+          captured_stream.close
+          captured_stream.unlink
+          stream_io.reopen(origin_stream)
+        end
+      end
+    end
+  end
+end
+
 class Foo < Rails::Application
   if Rails::VERSION::MAJOR >= 4
     config.eager_load = false