From 6288203277653cb6f6255c0431688b9eb4453848 Mon Sep 17 00:00:00 2001 From: Ben Mills Date: Thu, 26 May 2016 11:17:32 -0600 Subject: [PATCH] Revert "Add Rails >= 5.0.beta3 JSON API params parsing" (#1751) --- CHANGELOG.md | 1 - .../register_jsonapi_renderer.rb | 57 +++---- .../action_controller/json_api/linked_test.rb | 47 +++--- ...register_jsonapi_renderer_test_isolated.rb | 143 ------------------ test/support/isolated_unit.rb | 1 - test/support/rails5_shims.rb | 10 +- 6 files changed, 46 insertions(+), 213 deletions(-) delete mode 100644 test/active_model_serializers/register_jsonapi_renderer_test_isolated.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 65c350460..f2a8e2bd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,6 @@ 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 813503f5c..4a532b445 100644 --- a/lib/active_model_serializers/register_jsonapi_renderer.rb +++ b/lib/active_model_serializers/register_jsonapi_renderer.rb @@ -22,54 +22,43 @@ # 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) do - options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(request) - end + options.fetch(:serialization_context) { options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(request) } get_serializer(json, options) end end end -ActiveModelSerializers::Jsonapi.install +# 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 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 6e59e4ea3..5a1e3bc36 100644 --- a/test/action_controller/json_api/linked_test.rb +++ b/test/action_controller/json_api/linked_test.rb @@ -3,8 +3,9 @@ module ActionController module Serialization class JsonApi - class LinkedTest < ActionDispatch::IntegrationTest + class LinkedTest < ActionController::TestCase 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') @@ -38,68 +39,62 @@ def setup_post def render_resource_without_include setup_post - render json: @post + render jsonapi: @post end def render_resource_with_include setup_post - render json: @post, adapter: :json_api, include: [:author] + render jsonapi: @post, include: [:author] end def render_resource_with_include_of_custom_key_by_original setup_post - render json: @post, adapter: :json_api, include: [:reviews], serializer: PostWithCustomKeysSerializer + render jsonapi: @post, include: [:reviews], serializer: PostWithCustomKeysSerializer end def render_resource_with_nested_include setup_post - render json: @post, adapter: :json_api, include: [comments: [:author]] + render jsonapi: @post, include: [comments: [:author]] end def render_resource_with_nested_has_many_include_wildcard setup_post - render json: @post, adapter: :json_api, include: 'author.*' + render jsonapi: @post, include: 'author.*' end def render_resource_with_missing_nested_has_many_include setup_post @post.author = @author2 # author2 has no roles. - render json: @post, adapter: :json_api, include: [author: [:roles]] + render jsonapi: @post, include: [author: [:roles]] end def render_collection_with_missing_nested_has_many_include setup_post @post.author = @author2 - render json: [@post, @post2], adapter: :json_api, include: [author: [:roles]] + render jsonapi: [@post, @post2], include: [author: [:roles]] end def render_collection_without_include setup_post - render json: [@post], adapter: :json_api + render jsonapi: [@post] end def render_collection_with_include setup_post - render json: [@post], adapter: :json_api, include: 'author, comments' + render jsonapi: [@post], include: 'author, comments' end end - setup do - @routes = Rails.application.routes.draw do - ActiveSupport::Deprecation.silence do - match ':action', :to => LinkedTestController, via: [:get, :post] - end - end - end + tests LinkedTestController 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 @@ -107,7 +102,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 = [ { @@ -149,7 +144,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' @@ -161,33 +156,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 deleted file mode 100644 index 14e32535f..000000000 --- a/test/active_model_serializers/register_jsonapi_renderer_test_isolated.rb +++ /dev/null @@ -1,143 +0,0 @@ -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 d1d18eb69..4adc96a15 100644 --- a/test/support/isolated_unit.rb +++ b/test/support/isolated_unit.rb @@ -41,7 +41,6 @@ # 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 03a036da6..0a5fa050d 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, :headers, :session, :flash, :method, :body, :xhr].freeze + REQUEST_KWARGS = [:params, :session, :flash, :method, :body, :xhr].freeze def get(path, *args) fold_kwargs!(args) @@ -30,12 +30,7 @@ def fold_kwargs!(args) return unless hash.respond_to?(:key) Rails5Shims::ControllerTests::REQUEST_KWARGS.each do |kwarg| next unless hash.key?(kwarg) - value = hash.delete(kwarg) - if value.is_a? String - args.insert(0, value) - else - hash.merge! value - end + hash.merge! hash.delete(kwarg) end end @@ -49,5 +44,4 @@ def fold_kwargs!(args) end if Rails::VERSION::MAJOR < 5 ActionController::TestCase.send :include, Rails5Shims::ControllerTests - ActionDispatch::IntegrationTest.send :include, Rails5Shims::ControllerTests end