Merge pull request #1754 from bf4/handle_missing_serialization_context_when_paginating

Fix #1759, Grape integration, adds serialization_context

CHANGELOG.md

@@ -9,6 +9,10 @@ Features:
 - [#1426](https://github.com/rails-api/active_model_serializers/pull/1426) Add ActiveModelSerializers.config.default_includes (@empact)
 
 Fixes:
+- [#1754](https://github.com/rails-api/active_model_serializers/pull/1754) Fixes #1759, Grape integration, improves serialization_context
+  missing error message on pagination. Document overriding CollectionSerializer#paginated?. (@bf4)
+  Moved serialization_context creation to Grape formatter, so resource serialization works without explicit calls to the `render` helper method.
+  Added Grape collection tests. (@onomated)
 - [#1287](https://github.com/rails-api/active_model_serializers/pull/1287) Pass `fields` options from adapter to serializer. (@vasilakisfil)
 - [#1710](https://github.com/rails-api/active_model_serializers/pull/1710) Prevent association loading when `include_data` option
   is set to `false`. (@groyoh)

lib/active_model_serializers/adapter/json_api/pagination_links.rb

@@ -2,6 +2,7 @@ module ActiveModelSerializers
   module Adapter
     class JsonApi < Base
       class PaginationLinks
+        MissingSerializationContextError = Class.new(KeyError)
         FIRST_PAGE = 1
 
         attr_reader :collection, :context
@@ -9,7 +10,13 @@ module ActiveModelSerializers
         def initialize(collection, adapter_options)
           @collection = collection
           @adapter_options = adapter_options
-          @context = adapter_options.fetch(:serialization_context)
+          @context = adapter_options.fetch(:serialization_context) do
+            fail MissingSerializationContextError, <<-EOF.freeze
+ JsonApi::PaginationLinks requires a ActiveModelSerializers::SerializationContext.
+ Please pass a ':serialization_context' option or
+ override CollectionSerializer#paginated? to return 'false'.
+             EOF
+          end
         end
 
         def as_json

lib/active_model_serializers/serialization_context.rb

@@ -1,3 +1,4 @@
+require 'active_support/core_ext/array/extract_options'
 module ActiveModelSerializers
   class SerializationContext
     class << self
@@ -22,9 +23,15 @@ module ActiveModelSerializers
 
     attr_reader :request_url, :query_parameters, :key_transform
 
-    def initialize(request, options = {})
-      @request_url = request.original_url[/\A[^?]+/]
-      @query_parameters = request.query_parameters
+    def initialize(*args)
+      options = args.extract_options!
+      if args.size == 1
+        request = args.pop
+        options[:request_url] = request.original_url[/\A[^?]+/]
+        options[:query_parameters] = request.query_parameters
+      end
+      @request_url = options.delete(:request_url)
+      @query_parameters = options.delete(:query_parameters)
       @url_helpers = options.delete(:url_helpers) || self.class.url_helpers
       @default_url_options = options.delete(:default_url_options) || self.class.default_url_options
     end

lib/grape/formatters/active_model_serializers.rb

@@ -2,14 +2,31 @@
 #
 # Serializer options can be passed as a hash from your Grape endpoint using env[:active_model_serializer_options],
 # or better yet user the render helper in Grape::Helpers::ActiveModelSerializers
+
+require 'active_model_serializers/serialization_context'
+
 module Grape
   module Formatters
     module ActiveModelSerializers
       def self.call(resource, env)
-        serializer_options = {}
-        serializer_options.merge!(env[:active_model_serializer_options]) if env[:active_model_serializer_options]
+        serializer_options = build_serializer_options(env)
         ::ActiveModelSerializers::SerializableResource.new(resource, serializer_options).to_json
       end
+
+      def self.build_serializer_options(env)
+        ams_options = env[:active_model_serializer_options] || {}
+
+        # Add serialization context
+        ams_options.fetch(:serialization_context) do
+          request = env['grape.request']
+          ams_options[:serialization_context] = ::ActiveModelSerializers::SerializationContext.new(
+            request_url: request.url[/\A[^?]+/],
+            query_parameters: request.params
+          )
+        end
+
+        ams_options
+      end
     end
   end
 end

lib/grape/helpers/active_model_serializers.rb

@@ -1,4 +1,5 @@
 # Helpers can be included in your Grape endpoint as: helpers Grape::Helpers::ActiveModelSerializers
+
 module Grape
   module Helpers
     module ActiveModelSerializers

test/active_model_serializers/serialization_context_test_isolated.rb

@@ -5,13 +5,19 @@ require 'minitest/mock'
 class SerializationContextTest < ActiveSupport::TestCase
   include ActiveSupport::Testing::Isolation
 
-  def create_request
-    request = Minitest::Mock.new
-    request.expect(:original_url, 'original_url')
-    request.expect(:query_parameters, 'query_parameters')
-  end
-
   class WithRails < SerializationContextTest
+    def create_request
+      request = ActionDispatch::Request.new({})
+      def request.original_url
+        'http://example.com/articles?page=2'
+      end
+
+      def request.query_parameters
+        { 'page' => 2 }
+      end
+      request
+    end
+
     setup do
       require 'rails'
       require 'active_model_serializers'
@@ -20,8 +26,8 @@ class SerializationContextTest < ActiveSupport::TestCase
     end
 
     test 'create context with request url and query parameters' do
-      assert_equal @context.request_url, 'original_url'
-      assert_equal @context.query_parameters, 'query_parameters'
+      assert_equal @context.request_url, 'http://example.com/articles'
+      assert_equal @context.query_parameters, 'page' => 2
     end
 
     test 'url_helpers is set up for Rails url_helpers' do
@@ -36,14 +42,21 @@ class SerializationContextTest < ActiveSupport::TestCase
   end
 
   class WithoutRails < SerializationContextTest
+    def create_request
+      {
+        request_url: 'http://example.com/articles',
+        query_parameters: { 'page' => 2 }
+      }
+    end
+
     setup do
       require 'active_model_serializers/serialization_context'
       @context = ActiveModelSerializers::SerializationContext.new(create_request)
     end
 
     test 'create context with request url and query parameters' do
-      assert_equal @context.request_url, 'original_url'
-      assert_equal @context.query_parameters, 'query_parameters'
+      assert_equal @context.request_url, 'http://example.com/articles'
+      assert_equal @context.query_parameters, 'page' => 2
     end
 
     test 'url_helpers is a module when Rails is not present' do

test/adapter/json_api/pagination_links_test.rb

@@ -161,6 +161,17 @@ module ActiveModelSerializers
 
           assert_equal expected_response_without_pagination_links, adapter.serializable_hash
         end
+
+        def test_raises_descriptive_error_when_serialization_context_unset
+          render_options = { adapter: :json_api }
+          adapter = serializable(using_kaminari, render_options)
+          exception = assert_raises do
+            adapter.as_json
+          end
+          exception_class = ActiveModelSerializers::Adapter::JsonApi::PaginationLinks::MissingSerializationContextError
+          assert_equal exception_class, exception.class
+          assert_match(/CollectionSerializer#paginated\?/, exception.message)
+        end
       end
     end
   end

test/grape_test.rb

@@ -1,6 +1,9 @@
 require 'test_helper'
 require 'grape'
 require 'grape/active_model_serializers'
+require 'kaminari'
+require 'kaminari/hooks'
+::Kaminari::Hooks.init
 
 class ActiveModelSerializers::GrapeTest < ActiveSupport::TestCase
   include Rack::Test::Methods
@@ -21,6 +24,30 @@ class ActiveModelSerializers::GrapeTest < ActiveSupport::TestCase
           ARModels::Post.all
         end
     end
+
+    def self.reset_all
+      ARModels::Post.delete_all
+      @all = nil
+    end
+
+    def self.collection_per
+      2
+    end
+
+    def self.collection
+      @collection ||=
+        begin
+          Kaminari.paginate_array(
+            [
+                  Profile.new(id: 1, name: 'Name 1', description: 'Description 1', comments: 'Comments 1'),
+                  Profile.new(id: 2, name: 'Name 2', description: 'Description 2', comments: 'Comments 2'),
+                  Profile.new(id: 3, name: 'Name 3', description: 'Description 3', comments: 'Comments 3'),
+                  Profile.new(id: 4, name: 'Name 4', description: 'Description 4', comments: 'Comments 4'),
+                  Profile.new(id: 5, name: 'Name 5', description: 'Description 5', comments: 'Comments 5')
+              ]
+          ).page(1).per(collection_per)
+        end
+    end
   end
 
   class GrapeTest < Grape::API
@@ -41,11 +68,28 @@ class ActiveModelSerializers::GrapeTest < ActiveSupport::TestCase
         posts = Models.all
         render posts, adapter: :json_api
       end
+
+      get '/render_collection_with_json_api' do
+        posts = Models.collection
+        render posts, adapter: :json_api
+      end
+
+      get '/render_with_implicit_formatter' do
+        Models.model1
+      end
+
+      get '/render_array_with_implicit_formatter' do
+        Models.all
+      end
+
+      get '/render_collection_with_implicit_formatter' do
+        Models.collection
+      end
     end
   end
 
   def app
-    GrapeTest.new
+    Grape::Middleware::Globals.new(GrapeTest.new)
   end
 
   def test_formatter_returns_json
@@ -77,6 +121,56 @@ class ActiveModelSerializers::GrapeTest < ActiveSupport::TestCase
     assert last_response.ok?
     assert_equal serializable_resource.to_json, last_response.body
   ensure
-    ARModels::Post.delete_all
+    Models.reset_all
+  end
+
+  def test_formatter_handles_collections
+    get '/grape/render_collection_with_json_api'
+    assert last_response.ok?
+
+    representation = JSON.parse(last_response.body)
+    assert representation.include?('data')
+    assert representation['data'].count == Models.collection_per
+    assert representation.include?('links')
+    assert representation['links'].count > 0
+  end
+
+  def test_implicit_formatter
+    post = Models.model1
+    serializable_resource = serializable(post, adapter: :json_api)
+
+    with_adapter :json_api do
+      get '/grape/render_with_implicit_formatter'
+    end
+
+    assert last_response.ok?
+    assert_equal serializable_resource.to_json, last_response.body
+  end
+
+  def test_implicit_formatter_handles_arrays
+    posts = Models.all
+    serializable_resource = serializable(posts, adapter: :json_api)
+
+    with_adapter :json_api do
+      get '/grape/render_array_with_implicit_formatter'
+    end
+
+    assert last_response.ok?
+    assert_equal serializable_resource.to_json, last_response.body
+  ensure
+    Models.reset_all
+  end
+
+  def test_implicit_formatter_handles_collections
+    with_adapter :json_api do
+      get '/grape/render_collection_with_implicit_formatter'
+    end
+
+    representation = JSON.parse(last_response.body)
+    assert last_response.ok?
+    assert representation.include?('data')
+    assert representation['data'].count == Models.collection_per
+    assert representation.include?('links')
+    assert representation['links'].count > 0
   end
 end