diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eb04825d..84cc350eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Breaking changes: Features: +- [#1650](https://github.com/rails-api/active_model_serializers/pull/1650) Fix serialization scope options `scope`, `scope_name` + take precedence over `serialization_scope` in the controller. + Fix tests that required tearing down dynamic methods. (@bf4) - [#1644](https://github.com/rails-api/active_model_serializers/pull/1644) Include adapter name in cache key so that the same serializer can be cached per adapter. (@bf4 via #1346 by @kevintyll) - [#1642](https://github.com/rails-api/active_model_serializers/pull/1642) Prefer object.cache_key over the generated diff --git a/lib/action_controller/serialization.rb b/lib/action_controller/serialization.rb index 3097cdc40..e8d5a420d 100644 --- a/lib/action_controller/serialization.rb +++ b/lib/action_controller/serialization.rb @@ -30,8 +30,8 @@ def get_serializer(resource, options = {}) options[:adapter] = false end serializable_resource = ActiveModelSerializers::SerializableResource.new(resource, options) - serializable_resource.serialization_scope ||= serialization_scope - serializable_resource.serialization_scope_name = _serialization_scope + serializable_resource.serialization_scope ||= options.fetch(:scope) { serialization_scope } + serializable_resource.serialization_scope_name = options.fetch(:scope_name) { _serialization_scope } # For compatibility with the JSON renderer: `json.to_json(options) if json.is_a?(String)`. # Otherwise, since `serializable_resource` is not a string, the renderer would call # `to_json` on a String and given odd results, such as `"".to_json #=> '""'` diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index e86078456..c3193120d 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -96,16 +96,18 @@ def self.get_serializer_for(klass) end end - def self._serializer_instance_method_defined?(name) - _serializer_instance_methods.include?(name) + # rubocop:disable Style/ClassVars + def self.method_added(method_name) + @@_serializer_instance_methods ||= Hash.new { |h, k| h[k] = Set.new } + @@_serializer_instance_methods[self] << method_name end - # TODO: Fix load-order failures when different serializer instances define different - # scope methods - def self._serializer_instance_methods - @_serializer_instance_methods ||= (public_instance_methods - Object.public_instance_methods).to_set + def self._serializer_instance_method_defined?(name) + @_serializer_instance_methods ||= (ActiveModel::Serializer.public_instance_methods - Object.public_instance_methods).to_set + @_serializer_instance_methods.include?(name) || + @@_serializer_instance_methods[self].include?(name) end - private_class_method :_serializer_instance_methods + # rubocop:enable Style/ClassVars attr_accessor :object, :root, :scope diff --git a/test/action_controller/serialization_scope_name_test.rb b/test/action_controller/serialization_scope_name_test.rb index 5ad7251b5..1ef980583 100644 --- a/test/action_controller/serialization_scope_name_test.rb +++ b/test/action_controller/serialization_scope_name_test.rb @@ -1,6 +1,17 @@ require 'test_helper' module SerializationScopeTesting + def self.undef_serializer_dynamic_scope_methods + [PostSerializer, PostViewContextSerializer]. each do |serializer_class| + instance_method_cache = serializer_class.instance_variable_get(:@_serializer_instance_methods) + class_method_cache = serializer_class.class_variable_get(:@@_serializer_instance_methods)[serializer_class] + [:view_context, :current_user].each do |scope_method| + serializer_class.send(:undef_method, scope_method) if serializer_class.method_defined?(scope_method) + instance_method_cache && instance_method_cache.delete(scope_method) + class_method_cache && class_method_cache.delete(scope_method) + end + end + end class User < ActiveModelSerializers::Model attr_accessor :id, :name, :admin def admin? @@ -70,6 +81,10 @@ def comments class DefaultScopeTest < ActionController::TestCase tests PostTestController + teardown do + SerializationScopeTesting.undef_serializer_dynamic_scope_methods + end + def test_default_serialization_scope assert_equal :current_user, @controller._serialization_scope end @@ -120,6 +135,10 @@ def serializer end tests PostViewContextTestController + teardown do + SerializationScopeTesting.undef_serializer_dynamic_scope_methods + end + def test_defined_serialization_scope assert_equal :view_context, @controller._serialization_scope end @@ -158,8 +177,6 @@ def test_serialization_scope_admin assert_equal expected_json, @response.body end end - # FIXME: Has bugs. See comments below and - # https://github.com/rails-api/active_model_serializers/issues/1509 class NilSerializationScopeTest < ActionController::TestCase class PostViewContextTestController < ActionController::Base serialization_scope nil @@ -171,12 +188,15 @@ def render_post_with_no_scope 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 + 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 + + def render_post_with_passed_in_scope_without_scope_name + self.current_user = User.new(id: 3, name: 'Pete', admin: false) + render json: new_post, serializer: PostSerializer, adapter: :json, scope: current_user + end private @@ -186,6 +206,10 @@ def new_post end tests PostViewContextTestController + teardown do + SerializationScopeTesting.undef_serializer_dynamic_scope_methods + end + def test_nil_serialization_scope assert_nil @controller._serialization_scope end @@ -194,37 +218,35 @@ 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 + exception_matcher = /current_user/ + exception = assert_raises(NameError) 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 + def test_serialization_scope_is_and_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 + + def test_serialization_scope_is_nil_and_scope_passed_in_current_user_without_scope_name + exception_matcher = /current_user/ + exception = assert_raises(NameError) do + get :render_post_with_passed_in_scope_without_scope_name + end + assert_match exception_matcher, exception.message + end end end