diff --git a/lib/action_controller/serialization.rb b/lib/action_controller/serialization.rb index acafc0fd..b216f068 100644 --- a/lib/action_controller/serialization.rb +++ b/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) - @_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) + def get_serializer(resource, options = {}) + if ! use_adapter? + warn "ActionController::Serialization#use_adapter? has been removed. "\ + "Please pass 'adapter: false' or see ActiveSupport::SerializableResource#serialize" + options[:adapter] = false + end + ActiveModel::SerializableResource.serialize(resource, options) do |serializable_resource| + if serializable_resource.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 - - @_serializer 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 = - options.partition { |k, _| ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] } - - 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) + serializable_resource = get_serializer(resource, options) + super(serializable_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 diff --git a/lib/active_model/serializable_resource.rb b/lib/active_model/serializable_resource.rb new file mode 100644 index 00000000..fa3fbe03 --- /dev/null +++ b/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 diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index 451bd853..69bafdaf 100644 --- a/lib/active_model_serializers.rb +++ b/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' diff --git a/test/action_controller/rescue_from_test.rb b/test/action_controller/rescue_from_test.rb deleted file mode 100644 index 82689178..00000000 --- a/test/action_controller/rescue_from_test.rb +++ /dev/null @@ -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 diff --git a/test/action_controller/serialization_test.rb b/test/action_controller/serialization_test.rb index d7e3555e..c558e768 100644 --- a/test/action_controller/serialization_test.rb +++ b/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) - serializer = serializer_class.new(obj) - adapter = ActiveModel::Serializer.adapter.new(serializer) - adapter.to_json + ActiveModel::SerializableResource.new(obj).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 diff --git a/test/serializable_resource_test.rb b/test/serializable_resource_test.rb new file mode 100644 index 00000000..9f695a13 --- /dev/null +++ b/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 diff --git a/test/serializers/cache_test.rb b/test/serializers/cache_test.rb index 37068804..60c35194 100644 --- a/test/serializers/cache_test.rb +++ b/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) - serializer = serializer_class.new(obj) - adapter = ActiveModel::Serializer.adapter.new(serializer) - adapter.serializable_hash + ActiveModel::SerializableResource.new(obj).serializable_hash end end end diff --git a/test/serializers/meta_test.rb b/test/serializers/meta_test.rb index 4b501756..60e17384 100644 --- a/test/serializers/meta_test.rb +++ b/test/serializers/meta_test.rb @@ -113,11 +113,8 @@ module ActiveModel private def load_adapter(options) - adapter_opts, serializer_opts = - options.partition { |k, _| ActionController::Serialization::ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] } - - serializer = AlternateBlogSerializer.new(@blog, serializer_opts) - ActiveModel::Serializer::Adapter::FlattenJson.new(serializer, adapter_opts) + options = options.merge(adapter: :flatten_json, serializer: AlternateBlogSerializer) + ActiveModel::SerializableResource.new(@blog, options) end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 8afe49c0..68d844e6 100644 --- a/test/test_helper.rb +++ b/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