From 26b089c8813d5d3b20581bd20d775e4f5ae76210 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 7 Oct 2015 05:28:12 -0500 Subject: [PATCH 1/3] Add serialization_scope example [ci skip] --- docs/general/serializers.md | 93 ++++++++++++++++++++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) diff --git a/docs/general/serializers.md b/docs/general/serializers.md index d4a9e757..76b26b5f 100644 --- a/docs/general/serializers.md +++ b/docs/general/serializers.md @@ -156,7 +156,98 @@ PR please :) #### #scope -PR please :) +Allows you to include in the serializer access to an external method. + +It's intended to provide an authorization context to the serializer, so that +you may e.g. show an admin all comments on a post, else only published comments. + +- `scope` is a method on the serializer instance that comes from `options[:scope]`. It may be nil. +- `scope_name` is an option passed to the new serializer (`options[:scope_name]`). The serializer + defines a method with that name that calls the `scope`, e.g. `def current_user; scope; end`. + Note: it does not define the method if the serializer instance responds to it. + +That's a lot of words, so here's some examples: + +First, let's assume the serializer is instantiated in the controller, since that's the usual scenario. +We'll refer to the serialization context as `controller`. + +| options | `Serializer#scope` | method definition | +|-------- | ------------------|--------------------| +| `scope: current_user, scope_name: :current_user` | `current_user` | `Serializer#current_user` calls `controller.current_user` +| `scope: view_context, scope_name: :view_context` | `view_context` | `Serializer#view_context` calls `controller.view_context` + +We can take advantage of the scope to customize the objects returned based +on the current user (scope). + +For example, we can limit the posts the current user sees to those they created: + +```ruby +class PostSerializer < ActiveModel::Serializer + attributes :id, :title, :body + + # scope comments to those created_by the current user + has_many :comments do + object.comments.where(created_by: current_user) + end +end +``` + +Whether you write the method as above or as `object.comments.where(created_by: scope)` +is a matter of preference (assuming `scope_name` has been set). + +##### Controller Authorization Context + +In the controller, the scope/scope_name options are equal to +the [`serialization_scope`method](https://github.com/rails-api/active_model_serializers/blob/d02cd30fe55a3ea85e1d351b6e039620903c1871/lib/action_controller/serialization.rb#L13-L20), +which is `:current_user`, by default. + +Specfically, the `scope_name` is defaulted to `:current_user`, and may be set as +`serialization_scope :view_context`. The `scope` is set to `send(scope_name)` when `scope_name` is +present and the controller responds to `scope_name`. + +Thus, in a serializer, the controller provides `current_user` as the +current authorization scope when you call `render :json`. + +**IMPORTANT**: Since the scope is set at render, you may want to customize it so that `current_user` isn't +called on every request. This was [also a problem](https://github.com/rails-api/active_model_serializers/pull/1252#issuecomment-159810477) +in [`0.9`](https://github.com/rails-api/active_model_serializers/tree/0-9-stable#customizing-scope). + +We can change the scope from `current_user` to `view_context`. + +```diff +class SomeController < ActionController::Base ++ serialization_scope :view_context + + def current_user + User.new(id: 2, name: 'Bob', admin: true) + end + + def edit + user = User.new(id: 1, name: 'Pete') + render json: user, serializer: AdminUserSerializer, adapter: :json_api + end +end +``` + +We could then use the controller method `view_context` in our serializer, like so: + +```diff +class AdminUserSerializer < ActiveModel::Serializer + attributes :id, :name, :can_edit + + def can_edit? ++ view_context.current_user.admin? + end +end +``` + +So that when we render the `#edit` action, we'll get + +```json +{"data":{"id":"1","type":"users","attributes":{"name":"Pete","can_edit":true}}} +``` + +Where `can_edit` is `view_context.current_user.admin?` (true). #### #read_attribute_for_serialization(key) From 85658c0230c07ac90fff9d6f393efe584594f8b6 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 10 Feb 2016 00:44:06 -0600 Subject: [PATCH 2/3] Add better serialization_scope tests; uncover bug --- lib/active_model/serializer.rb | 2 + .../serialization_scope_name_test.rb | 266 ++++++++++++++---- 2 files changed, 214 insertions(+), 54 deletions(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index d17e1879..6b3d4090 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -96,6 +96,8 @@ module ActiveModel _serializer_instance_methods.include?(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 end diff --git a/test/action_controller/serialization_scope_name_test.rb b/test/action_controller/serialization_scope_name_test.rb index 5969f522..1d49bddf 100644 --- a/test/action_controller/serialization_scope_name_test.rb +++ b/test/action_controller/serialization_scope_name_test.rb @@ -1,63 +1,221 @@ require 'test_helper' -require 'pathname' -class DefaultScopeNameTest < ActionController::TestCase - class UserSerializer < ActiveModel::Serializer +module SerializationScopeTesting + class User < ActiveModelSerializers::Model + attr_accessor :id, :name, :admin def admin? - current_user.admin - end - attributes :admin? - end - - class UserTestController < ActionController::Base - protect_from_forgery - - before_action { request.format = :json } - - def current_user - User.new(id: 1, name: 'Pete', admin: false) - end - - def render_new_user - render json: User.new(id: 1, name: 'Pete', admin: false), serializer: UserSerializer, adapter: :json_api + 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 - tests UserTestController + def body + "The 'scope' is the 'current_user': #{scope == current_user}" + end - def test_default_scope_name - get :render_new_user - assert_equal '{"data":{"id":"1","type":"users","attributes":{"admin?":false}}}', @response.body - end -end - -class SerializationScopeNameTest < ActionController::TestCase - class AdminUserSerializer < ActiveModel::Serializer - def admin? - current_admin.admin - end - attributes :admin? - end - - class AdminUserTestController < ActionController::Base - protect_from_forgery - - serialization_scope :current_admin - before_action { request.format = :json } - - def current_admin - User.new(id: 2, name: 'Bob', admin: true) - end - - def render_new_user - render json: User.new(id: 1, name: 'Pete', admin: false), serializer: AdminUserSerializer, adapter: :json_api - end - end - - tests AdminUserTestController - - def test_override_scope_name_with_controller - get :render_new_user - assert_equal '{"data":{"id":"1","type":"users","attributes":{"admin?":true}}}', @response.body + 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 + class PostTestController < ActionController::Base + attr_accessor :current_user + def render_post_by_non_admin + self.current_user = User.new(id: 3, name: 'Pete', admin: false) + render json: new_post, serializer: serializer, adapter: :json + end + + def render_post_by_admin + self.current_user = User.new(id: 3, name: 'Pete', admin: true) + render json: new_post, serializer: serializer, adapter: :json + end + + private + + def new_post + Post.new(id: 4, title: 'Title') + end + + def serializer + PostSerializer + end + end + class PostViewContextSerializer < PostSerializer + def body + "The 'scope' is the 'view_context': #{scope == view_context}" + end + + def comments + if view_context.controller.current_user.admin? + [Comment.new(id: 1, body: 'Admin')] + else + [Comment.new(id: 2, body: 'Scoped')] + end + end + end + class DefaultScopeTest < ActionController::TestCase + tests PostTestController + + def test_default_serialization_scope + assert_equal :current_user, @controller._serialization_scope + end + + def test_default_serialization_scope_object + assert_equal @controller.current_user, @controller.serialization_scope + end + + def test_default_scope_non_admin + get :render_post_by_non_admin + 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_default_scope_admin + get :render_post_by_admin + expected_json = { + post: { + id: 4, + title: 'Title', + body: "The 'scope' is the 'current_user': true", + comments: [ + { id: 1, body: 'Admin' } + ] + } + }.to_json + assert_equal expected_json, @response.body + end + end + class SerializationScopeTest < ActionController::TestCase + class PostViewContextTestController < PostTestController + serialization_scope :view_context + + private + + def serializer + PostViewContextSerializer + end + end + tests PostViewContextTestController + + def test_defined_serialization_scope + assert_equal :view_context, @controller._serialization_scope + end + + def test_defined_serialization_scope_object + assert_equal @controller.view_context.class, @controller.serialization_scope.class + end + + def test_serialization_scope_non_admin + get :render_post_by_non_admin + expected_json = { + post: { + id: 4, + title: 'Title', + body: "The 'scope' is the 'view_context': true", + comments: [ + { id: 2, body: 'Scoped' } + ] + } + }.to_json + assert_equal expected_json, @response.body + end + + def test_serialization_scope_admin + get :render_post_by_admin + expected_json = { + post: { + id: 4, + title: 'Title', + body: "The 'scope' is the 'view_context': true", + comments: [ + { id: 1, body: 'Admin' } + ] + } + }.to_json + assert_equal expected_json, @response.body + end + end + 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 + exception = assert_raises(NameError) do + get :render_post_with_no_scope + end + assert_match(/admin|current_user/, 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 end From 1b2f5ec774898e3589894f96f8a595c1de9944f8 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 10 Feb 2016 21:08:42 -0600 Subject: [PATCH 3/3] Differentiate exception behavior in Rails 4.0 vs. others NoMethodError is current_user is nil, so nil.admin? NameError is a superclass of NoMethodError (which Rails 4.0 won't allow) and means current_user might not be defined --- .../serialization_scope_name_test.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/action_controller/serialization_scope_name_test.rb b/test/action_controller/serialization_scope_name_test.rb index 1d49bddf..5ad7251b 100644 --- a/test/action_controller/serialization_scope_name_test.rb +++ b/test/action_controller/serialization_scope_name_test.rb @@ -158,6 +158,8 @@ module SerializationScopeTesting 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 @@ -195,10 +197,17 @@ module SerializationScopeTesting # TODO: change to NoMethodError and match 'admin?' when the # global state in Serializer._serializer_instance_methods is fixed def test_nil_scope - exception = assert_raises(NameError) do + 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(/admin|current_user/, exception.message) + assert_match exception_matcher, exception.message end # TODO: run test when