diff --git a/CHANGELOG.md b/CHANGELOG.md index f2a8e2bd2..65c350460 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Features: Fixes: - [#1710](https://github.com/rails-api/active_model_serializers/pull/1710) Prevent association loading when `include_data` option is set to `false`. (@groyoh) +- [#1747](https://github.com/rails-api/active_model_serializers/pull/1747) Improve jsonapi mime type registration for Rails 5 (@remear) Misc: - [#1734](https://github.com/rails-api/active_model_serializers/pull/1734) Adds documentation for conditional attribute (@lambda2) diff --git a/lib/active_model_serializers/register_jsonapi_renderer.rb b/lib/active_model_serializers/register_jsonapi_renderer.rb index 4a532b445..813503f5c 100644 --- a/lib/active_model_serializers/register_jsonapi_renderer.rb +++ b/lib/active_model_serializers/register_jsonapi_renderer.rb @@ -22,43 +22,54 @@ # render jsonapi: model # # No wrapper format needed as it does not apply (i.e. no `wrap_parameters format: [jsonapi]`) - module ActiveModelSerializers::Jsonapi MEDIA_TYPE = 'application/vnd.api+json'.freeze HEADERS = { response: { 'CONTENT_TYPE'.freeze => MEDIA_TYPE }, request: { 'ACCEPT'.freeze => MEDIA_TYPE } }.freeze + + def self.install + # actionpack/lib/action_dispatch/http/mime_types.rb + Mime::Type.register MEDIA_TYPE, :jsonapi + + if Rails::VERSION::MAJOR >= 5 + ActionDispatch::Request.parameter_parsers[:jsonapi] = parser + else + ActionDispatch::ParamsParser::DEFAULT_PARSERS[Mime[:jsonapi]] = parser + end + + # ref https://github.com/rails/rails/pull/21496 + ActionController::Renderers.add :jsonapi do |json, options| + json = serialize_jsonapi(json, options).to_json(options) unless json.is_a?(String) + self.content_type ||= Mime[:jsonapi] + self.response_body = json + end + end + + # Proposal: should actually deserialize the JSON API params + # to the hash format expected by `ActiveModel::Serializers::JSON` + # actionpack/lib/action_dispatch/http/parameters.rb + def self.parser + lambda do |body| + data = JSON.parse(body) + data = { :_json => data } unless data.is_a?(Hash) + data.with_indifferent_access + end + end + module ControllerSupport def serialize_jsonapi(json, options) options[:adapter] = :json_api - options.fetch(:serialization_context) { options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(request) } + options.fetch(:serialization_context) do + options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(request) + end get_serializer(json, options) end end end -# actionpack/lib/action_dispatch/http/mime_types.rb -Mime::Type.register ActiveModelSerializers::Jsonapi::MEDIA_TYPE, :jsonapi - -parsers = Rails::VERSION::MAJOR >= 5 ? ActionDispatch::Http::Parameters : ActionDispatch::ParamsParser -media_type = Mime::Type.lookup(ActiveModelSerializers::Jsonapi::MEDIA_TYPE) - -# Proposal: should actually deserialize the JSON API params -# to the hash format expected by `ActiveModel::Serializers::JSON` -# actionpack/lib/action_dispatch/http/parameters.rb -parsers::DEFAULT_PARSERS[media_type] = lambda do |body| - data = JSON.parse(body) - data = { :_json => data } unless data.is_a?(Hash) - data.with_indifferent_access -end - -# ref https://github.com/rails/rails/pull/21496 -ActionController::Renderers.add :jsonapi do |json, options| - json = serialize_jsonapi(json, options).to_json(options) unless json.is_a?(String) - self.content_type ||= media_type - self.response_body = json -end +ActiveModelSerializers::Jsonapi.install ActiveSupport.on_load(:action_controller) do include ActiveModelSerializers::Jsonapi::ControllerSupport diff --git a/test/action_controller/json_api/linked_test.rb b/test/action_controller/json_api/linked_test.rb index 5a1e3bc36..6e59e4ea3 100644 --- a/test/action_controller/json_api/linked_test.rb +++ b/test/action_controller/json_api/linked_test.rb @@ -3,9 +3,8 @@ module ActionController module Serialization class JsonApi - class LinkedTest < ActionController::TestCase + class LinkedTest < ActionDispatch::IntegrationTest class LinkedTestController < ActionController::Base - require 'active_model_serializers/register_jsonapi_renderer' def setup_post ActionController::Base.cache_store.clear @role1 = Role.new(id: 1, name: 'admin') @@ -39,62 +38,68 @@ def setup_post def render_resource_without_include setup_post - render jsonapi: @post + render json: @post end def render_resource_with_include setup_post - render jsonapi: @post, include: [:author] + render json: @post, adapter: :json_api, include: [:author] end def render_resource_with_include_of_custom_key_by_original setup_post - render jsonapi: @post, include: [:reviews], serializer: PostWithCustomKeysSerializer + render json: @post, adapter: :json_api, include: [:reviews], serializer: PostWithCustomKeysSerializer end def render_resource_with_nested_include setup_post - render jsonapi: @post, include: [comments: [:author]] + render json: @post, adapter: :json_api, include: [comments: [:author]] end def render_resource_with_nested_has_many_include_wildcard setup_post - render jsonapi: @post, include: 'author.*' + render json: @post, adapter: :json_api, include: 'author.*' end def render_resource_with_missing_nested_has_many_include setup_post @post.author = @author2 # author2 has no roles. - render jsonapi: @post, include: [author: [:roles]] + render json: @post, adapter: :json_api, include: [author: [:roles]] end def render_collection_with_missing_nested_has_many_include setup_post @post.author = @author2 - render jsonapi: [@post, @post2], include: [author: [:roles]] + render json: [@post, @post2], adapter: :json_api, include: [author: [:roles]] end def render_collection_without_include setup_post - render jsonapi: [@post] + render json: [@post], adapter: :json_api end def render_collection_with_include setup_post - render jsonapi: [@post], include: 'author, comments' + render json: [@post], adapter: :json_api, include: 'author, comments' end end - tests LinkedTestController + setup do + @routes = Rails.application.routes.draw do + ActiveSupport::Deprecation.silence do + match ':action', :to => LinkedTestController, via: [:get, :post] + end + end + end def test_render_resource_without_include - get :render_resource_without_include + get '/render_resource_without_include' response = JSON.parse(@response.body) refute response.key? 'included' end def test_render_resource_with_include - get :render_resource_with_include + get '/render_resource_with_include' response = JSON.parse(@response.body) assert response.key? 'included' assert_equal 1, response['included'].size @@ -102,7 +107,7 @@ def test_render_resource_with_include end def test_render_resource_with_nested_has_many_include - get :render_resource_with_nested_has_many_include_wildcard + get '/render_resource_with_nested_has_many_include_wildcard' response = JSON.parse(@response.body) expected_linked = [ { @@ -144,7 +149,7 @@ def test_render_resource_with_nested_has_many_include end def test_render_resource_with_include_of_custom_key_by_original - get :render_resource_with_include_of_custom_key_by_original + get '/render_resource_with_include_of_custom_key_by_original' response = JSON.parse(@response.body) assert response.key? 'included' @@ -156,33 +161,33 @@ def test_render_resource_with_include_of_custom_key_by_original end def test_render_resource_with_nested_include - get :render_resource_with_nested_include + get '/render_resource_with_nested_include' response = JSON.parse(@response.body) assert response.key? 'included' assert_equal 3, response['included'].size end def test_render_collection_without_include - get :render_collection_without_include + get '/render_collection_without_include' response = JSON.parse(@response.body) refute response.key? 'included' end def test_render_collection_with_include - get :render_collection_with_include + get '/render_collection_with_include' response = JSON.parse(@response.body) assert response.key? 'included' end def test_render_resource_with_nested_attributes_even_when_missing_associations - get :render_resource_with_missing_nested_has_many_include + get '/render_resource_with_missing_nested_has_many_include' response = JSON.parse(@response.body) assert response.key? 'included' refute has_type?(response['included'], 'roles') end def test_render_collection_with_missing_nested_has_many_include - get :render_collection_with_missing_nested_has_many_include + get '/render_collection_with_missing_nested_has_many_include' response = JSON.parse(@response.body) assert response.key? 'included' assert has_type?(response['included'], 'roles') diff --git a/test/active_model_serializers/register_jsonapi_renderer_test_isolated.rb b/test/active_model_serializers/register_jsonapi_renderer_test_isolated.rb new file mode 100644 index 000000000..14e32535f --- /dev/null +++ b/test/active_model_serializers/register_jsonapi_renderer_test_isolated.rb @@ -0,0 +1,143 @@ +require 'support/isolated_unit' +require 'minitest/mock' +require 'action_dispatch' +require 'action_controller' + +class JsonApiRendererTest < ActionDispatch::IntegrationTest + include ActiveSupport::Testing::Isolation + + class TestController < ActionController::Base + class << self + attr_accessor :last_request_parameters + end + + def render_with_jsonapi_renderer + author = Author.new(params[:data][:attributes]) + render jsonapi: author + end + + def parse + self.class.last_request_parameters = request.request_parameters + head :ok + end + end + + def teardown + TestController.last_request_parameters = nil + end + + def assert_parses(expected, actual, headers = {}) + post '/parse', params: actual, headers: headers + assert_response :ok + assert_equal(expected, TestController.last_request_parameters) + end + + class WithoutRenderer < JsonApiRendererTest + setup do + require 'rails' + require 'active_record' + require 'support/rails5_shims' + require 'active_model_serializers' + require 'fixtures/poro' + + make_basic_app + + Rails.application.routes.draw do + ActiveSupport::Deprecation.silence do + match ':action', :to => TestController, via: [:get, :post] + end + end + end + + def test_jsonapi_parser_not_registered + parsers = if Rails::VERSION::MAJOR >= 5 + ActionDispatch::Request.parameter_parsers + else + ActionDispatch::ParamsParser::DEFAULT_PARSERS + end + assert_nil parsers[Mime[:jsonapi]] + end + + def test_jsonapi_renderer_not_registered + expected = { + 'data' => { + 'attributes' => { + 'name' => 'Johnny Rico' + }, + 'type' => 'users' + } + } + payload = '{"data": {"attributes": {"name": "Johnny Rico"}, "type": "authors"}}' + headers = { 'CONTENT_TYPE' => 'application/vnd.api+json' } + post '/render_with_jsonapi_renderer', params: payload, headers: headers + assert expected, response.body + end + + def test_jsonapi_parser + assert_parses( + {}, + '', + 'CONTENT_TYPE' => 'application/vnd.api+json' + ) + end + end + + class WithRenderer < JsonApiRendererTest + setup do + require 'rails' + require 'active_record' + require 'support/rails5_shims' + require 'active_model_serializers' + require 'fixtures/poro' + require 'active_model_serializers/register_jsonapi_renderer' + + make_basic_app + + Rails.application.routes.draw do + ActiveSupport::Deprecation.silence do + match ':action', :to => TestController, via: [:get, :post] + end + end + end + + def test_jsonapi_parser_registered + if Rails::VERSION::MAJOR >= 5 + parsers = ActionDispatch::Request.parameter_parsers + assert_equal Proc, parsers[:jsonapi].class + else + parsers = ActionDispatch::ParamsParser::DEFAULT_PARSERS + assert_equal Proc, parsers[Mime[:jsonapi]].class + end + end + + def test_jsonapi_renderer_registered + expected = { + 'data' => { + 'attributes' => { + 'name' => 'Johnny Rico' + }, + 'type' => 'users' + } + } + payload = '{"data": {"attributes": {"name": "Johnny Rico"}, "type": "authors"}}' + headers = { 'CONTENT_TYPE' => 'application/vnd.api+json' } + post '/render_with_jsonapi_renderer', params: payload, headers: headers + assert expected, response.body + end + + def test_jsonapi_parser + assert_parses( + { + 'data' => { + 'attributes' => { + 'name' => 'John Doe' + }, + 'type' => 'users' + } + }, + '{"data": {"attributes": {"name": "John Doe"}, "type": "users"}}', + 'CONTENT_TYPE' => 'application/vnd.api+json' + ) + end + end +end diff --git a/test/support/isolated_unit.rb b/test/support/isolated_unit.rb index 4adc96a15..d1d18eb69 100644 --- a/test/support/isolated_unit.rb +++ b/test/support/isolated_unit.rb @@ -41,6 +41,7 @@ # These files do not require any others and are needed # to run the tests +require 'active_support/testing/autorun' require 'active_support/testing/isolation' module TestHelpers diff --git a/test/support/rails5_shims.rb b/test/support/rails5_shims.rb index 0a5fa050d..03a036da6 100644 --- a/test/support/rails5_shims.rb +++ b/test/support/rails5_shims.rb @@ -1,7 +1,7 @@ module Rails5Shims module ControllerTests # https://github.com/rails/rails/blob/b217354/actionpack/lib/action_controller/test_case.rb - REQUEST_KWARGS = [:params, :session, :flash, :method, :body, :xhr].freeze + REQUEST_KWARGS = [:params, :headers, :session, :flash, :method, :body, :xhr].freeze def get(path, *args) fold_kwargs!(args) @@ -30,7 +30,12 @@ def fold_kwargs!(args) return unless hash.respond_to?(:key) Rails5Shims::ControllerTests::REQUEST_KWARGS.each do |kwarg| next unless hash.key?(kwarg) - hash.merge! hash.delete(kwarg) + value = hash.delete(kwarg) + if value.is_a? String + args.insert(0, value) + else + hash.merge! value + end end end @@ -44,4 +49,5 @@ def fold_kwargs!(args) end if Rails::VERSION::MAJOR < 5 ActionController::TestCase.send :include, Rails5Shims::ControllerTests + ActionDispatch::IntegrationTest.send :include, Rails5Shims::ControllerTests end