Bug in `serialization_scope nil` scope methods existing on the serializer

[bf4]

Discovered in https://github.com/rails-api/active_model_serializers/pull/1252/files#r52560934

Follows is the failing test:

Summary:

It may be either of

• [:body, :comments, :json_key, :current_user, :_reflections, :_reflections?, :attributes, :_links, :_links?, :_meta, :_meta?, :_type, :_type?, :object, :object=, :root, :root=, :scope, :scope=, :read_attribute_for_serialization, :associations, :config]
• [:_reflections, :_reflections?, :attributes, :_links, :_links?, :_meta, :_meta?, :_type, :_type?, :object, :object=, :root, :root=, :scope, :scope=, :json_key, :read_attribute_for_serialization, :associations, :config]

And has to do, I think, with a method on the serializer such as comments calling the scope name current_user which may not be set.

One problem is that the scope_name and scope options aren't being passed through to the serializer. The controller overwrites the :scope and :scope_name options passed to render with the values of serialization_scope and _serialization_scope respectively. But, even with that fixed, this still ocurrs.

If we change Serializer#read_attribute_for_serialization to check for respond_to?(attr); send(attr) the problem persists. i.e. not used ._serializer_instance_methods at all

When scope is present but scope_name is nil, current_user won't exist.

Given:

  class User < ActiveModelSerializers::Model
    attr_accessor :id, :name, :admin
    def admin?
      admin
    end
  end
  class Comment < ActiveModelSerializers::Model
    attr_accessor :id, :body
  end
  class Post < ActiveModelSerializers::Model
    attr_accessor :id, :title, :body, :comments
  end
  class PostSerializer < ActiveModel::Serializer
    attributes :id, :title, :body, :comments

    def body
      "The 'scope' is the 'current_user': #{scope == current_user}"
    end

    def comments
      if current_user.admin?
        [Comment.new(id: 1, body: 'Admin')]
      else
        [Comment.new(id: 2, body: 'Scoped')]
      end
    end

    def json_key
      'post'
    end
  end

then

  class NilSerializationScopeTest < ActionController::TestCase

    class PostViewContextTestController < ActionController::Base
      serialization_scope nil

      attr_accessor :current_user

      def render_post_with_no_scope
        self.current_user = User.new(id: 3, name: 'Pete', admin: false)
        render json: new_post, serializer: PostSerializer, adapter: :json
      end

      # TODO: run test when
      # global state in Serializer._serializer_instance_methods is fixed
      # def render_post_with_passed_in_scope
      #   self.current_user = User.new(id: 3, name: 'Pete', admin: false)
      #   render json: new_post, serializer: PostSerializer, adapter: :json, scope: current_user, scope_name: :current_user
      # end

      private

      def new_post
        Post.new(id: 4, title: 'Title')
      end
    end
    tests PostViewContextTestController

    def test_nil_serialization_scope
      assert_nil @controller._serialization_scope
    end

    def test_nil_serialization_scope_object
      assert_nil @controller.serialization_scope
    end

    # TODO: change to NoMethodError and match 'admin?' when the
    # global state in Serializer._serializer_instance_methods is fixed
    def test_nil_scope
      if Rails.version.start_with?('4.0')
        exception_class = NoMethodError
        exception_matcher = 'admin?'
      else
        exception_class = NameError
        exception_matcher = /admin|current_user/
      end
      exception = assert_raises(exception_class) do
        get :render_post_with_no_scope
      end
      assert_match exception_matcher, exception.message
    end

    # TODO: run test when
    # global state in Serializer._serializer_instance_methods is fixed
    # def test_nil_scope_passed_in_current_user
    #   get :render_post_with_passed_in_scope
    #   expected_json = {
    #     post: {
    #       id: 4,
    #       title: 'Title',
    #       body: "The 'scope' is the 'current_user': true",
    #       comments: [
    #         { id: 2, body: 'Scoped' }
    #       ]
    #     }
    #   }.to_json
    #   assert_equal expected_json, @response.body
    # end
  end

It can be reproduced by running bundle exec ruby -Ilib:test -rtest_helper test/action_controller/serialization_scope_name_test.rb -n "/^(test_default_scope_admin|test_default_scope_non_admin|test_default_serialization_scope|test_default_serialization_scope_object|test_defined_serialization_scope|test_defined_serialization_scope_object|test_serialization_scope_admin|test_serialization_scope_non_admin|test_nil_serialization_scope|test_nil_scope_passed_in_current_user|test_nil_serialization_scope_object|test_nil_scope)$/" --seed 41462 at bf4@47669f5

[bf4]

possible solution is rather than create dynamic methods, to just create a hash of method name to lambda that is used in the read_attribute_for_serialization

e.g. something like

- attr_accessor :scope
+ attr_accessor :scope, :scope_name

def initialize(object, options = {})
   self.scope = instance_options[:scope]
+  self.scope_name = instance_options[:scope_name]
-  scope_name = instance_options[:scope_name]
-  if scope_name && !respond_to?(scope_name)
-    self.class.class_eval do
-      define_method scope_name, lambda { scope }
-    end
-  end
end

def read_attribute_for_serialization(attr)
   if self.class._serializer_instance_method_defined?(attr)
     send(attr)
+  elsif scope_name && !respond_to?(scope_name) && attr == scope_name
+    scope.call
   elsif self.class._fragmented
     self.class._fragmented.read_attribute_for_serialization(attr)
   else
    object.read_attribute_for_serialization(attr)
  end
end