Revert "Add Rails >= 5.0.beta3 JSON API params parsing" (#1751)

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)

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.fetch(:serialization_context) { options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(request) }
-        options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(request)
-      end
       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

test/action_controller/json_api/linked_test.rb

@@ -3,8 +3,9 @@ require 'test_helper'
 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 @@ module ActionController
 
           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
+        tests LinkedTestController
-          @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
@@ -107,7 +102,7 @@ module ActionController
         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 @@ module ActionController
         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 @@ module ActionController
         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')

test/active_model_serializers/register_jsonapi_renderer_test_isolated.rb

@@ -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

test/support/isolated_unit.rb

@@ -41,7 +41,6 @@ require 'active_support/core_ext/string/access'
 
 # 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

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 @@ module Rails5Shims
       return unless hash.respond_to?(:key)
       Rails5Shims::ControllerTests::REQUEST_KWARGS.each do |kwarg|
         next unless hash.key?(kwarg)
-        value = hash.delete(kwarg)
+        hash.merge! hash.delete(kwarg)
-        if value.is_a? String
-          args.insert(0, value)
-        else
-          hash.merge! value
-        end
       end
     end
 
@@ -49,5 +44,4 @@ module Rails5Shims
 end
 if Rails::VERSION::MAJOR < 5
   ActionController::TestCase.send :include, Rails5Shims::ControllerTests
-  ActionDispatch::IntegrationTest.send :include, Rails5Shims::ControllerTests
 end