diff --git a/CHANGELOG.md b/CHANGELOG.md index a40d777b0..6a5370f48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Misc: Breaking changes: - [#1662](https://github.com/rails-api/active_model_serializers/pull/1662) Drop support for Rails 4.0 and Ruby 2.0.0. (@remear) +- [#1720](https://github.com/rails-api/active_model_serializers/pull/1720) Block relationships must explicitly use `load_data` instead of the return value to customize relationship contents. Features: - [#1677](https://github.com/rails-api/active_model_serializers/pull/1677) Add `assert_schema`, `assert_request_schema`, `assert_request_response_schema`. (@bf4) @@ -31,6 +32,7 @@ Features: - [#1687](https://github.com/rails-api/active_model_serializers/pull/1687) Only calculate `_cache_digest` (in `cache_key`) when `skip_digest` is false. (@bf4) - [#1647](https://github.com/rails-api/active_model_serializers/pull/1647) Restrict usage of `serializable_hash` options to the ActiveModel::Serialization and ActiveModel::Serializers::JSON interface. (@bf4) +- [#1720](https://github.com/rails-api/active_model_serializers/pull/1720) Support JSONAPI 'include` parameter with links/loading. Fixes: - [#1700](https://github.com/rails-api/active_model_serializers/pull/1700) Support pagination link for Kaminari when no data is returned. (@iamnader) diff --git a/docs/general/adapters.md b/docs/general/adapters.md index 6ae1d27a5..dbc91f102 100644 --- a/docs/general/adapters.md +++ b/docs/general/adapters.md @@ -194,6 +194,43 @@ The user could pass in `include=**`. We recommend filtering any user-supplied includes appropriately. +#### Include Parameters + +You may wish to include related resources based on the `include` request +parameter (See [fetching_includes](http://jsonapi.org/format/#fetching-includes). You can opt-in to this pattern: + +```ruby +class PostSerializer < ActiveModel::Serializer + associations_via_include_param(true) +end +``` + +The serializer will now behave as follows: + +* If the relationship is in the `include` parameter, both + `relationships` and `included` will be populated. +* If the relationship is not in the `include` parameter, but a `link` + has been specified, `relationships` will contain the link but not +`id/type`s. This avoids an unnecessary database hit. +* If the relationship is not in the `include` parameter, and there is no + `link`, the relationship is rendered without `data`. This specifies +that the relationship exists, but we aren't saying anything about the +contents of that relationship (nil, empty, or otherwise). + +To sync your URL `include` parameter with the `include` option passed to +`render`, you'll probably also want to add something like this: + +```ruby +class PeopleController < ApplicationController + def index + render json: Person.all, include: params[:include] + end +end +``` + +Remember, you probably want to [whitelist your includes for security +purposes](https://github.com/rails-api/active_model_serializers/blob/master/docs/general/adapters.md#security-considerations). + ## Advanced adapter configuration ### Registering an adapter diff --git a/docs/general/serializers.md b/docs/general/serializers.md index 91d558a8c..015d4c7f3 100644 --- a/docs/general/serializers.md +++ b/docs/general/serializers.md @@ -112,7 +112,7 @@ has_many :comments, key: :reviews has_many :comments, serializer: CommentPreviewSerializer has_many :reviews, virtual_value: [{ id: 1 }, { id: 2 }] has_many :comments, key: :last_comments do - last(1) + load_data { last(1) } end ``` @@ -252,7 +252,7 @@ class PostSerializer < ActiveModel::Serializer # scope comments to those created_by the current user has_many :comments do - object.comments.where(created_by: current_user) + load_data { object.comments.where(created_by: current_user) } end end ``` @@ -365,7 +365,7 @@ To override an association, call `has_many`, `has_one` or `belongs_to` with a bl ```ruby class PostSerializer < ActiveModel::Serializer has_many :comments do - object.comments.active + load_data { object.comments.active } end end ``` diff --git a/lib/active_model/serializer/associations.rb b/lib/active_model/serializer/associations.rb index dcad7ea7f..1977a3d40 100644 --- a/lib/active_model/serializer/associations.rb +++ b/lib/active_model/serializer/associations.rb @@ -13,6 +13,7 @@ module Associations included do with_options instance_writer: false, instance_reader: true do |serializer| serializer.class_attribute :_reflections + serializer.class_attribute :_associations_via_include_param self._reflections ||= [] end @@ -65,6 +66,10 @@ def has_one(name, options = {}, &block) associate(HasOneReflection.new(name, options, block)) end + def associations_via_include_param(val) + self._associations_via_include_param = val + end + private # Add reflection and define {name} accessor. @@ -82,7 +87,8 @@ def associate(reflection) # +default_include_directive+ config value when not provided) # @return [Enumerator] # - def associations(include_directive = ActiveModelSerializers.default_include_directive) + def associations(include_directive = ActiveModelSerializers.default_include_directive, current_include_directive = nil) + current_include_directive ||= include_directive return unless object Enumerator.new do |y| @@ -90,7 +96,8 @@ def associations(include_directive = ActiveModelSerializers.default_include_dire next if reflection.excluded?(self) key = reflection.options.fetch(:key, reflection.name) next unless include_directive.key?(key) - y.yield reflection.build_association(self, instance_options) + + y.yield reflection.build_association(self, instance_options, current_include_directive) end end end diff --git a/lib/active_model/serializer/reflection.rb b/lib/active_model/serializer/reflection.rb index fbc421f73..051e79e55 100644 --- a/lib/active_model/serializer/reflection.rb +++ b/lib/active_model/serializer/reflection.rb @@ -38,6 +38,7 @@ def initialize(*) super @_links = {} @_include_data = true + @_load_data = false @_meta = nil end @@ -56,6 +57,11 @@ def include_data(value = true) :nil end + def load_data(&blk) + @_load_data = blk + :nil + end + # @param serializer [ActiveModel::Serializer] # @yield [ActiveModel::Serializer] # @return [:nil, associated resource or resource collection] @@ -69,19 +75,23 @@ def include_data(value = true) # Blog.find(object.blog_id) # end # end - def value(serializer) + def value(serializer, current_include_directive) @object = serializer.object @scope = serializer.scope if block block_value = instance_exec(serializer, &block) - if block_value != :nil - block_value - elsif @_include_data - serializer.read_attribute_for_serialization(name) + load_data { block_value } if block_value != :nil + + if include_data?(serializer, current_include_directive) + if @_load_data + @_load_data.call(current_include_directive) + else + include_data_for(serializer, current_include_directive) + end end else - serializer.read_attribute_for_serialization(name) + include_data_for(serializer, current_include_directive) end end @@ -106,11 +116,11 @@ def value(serializer) # # @api private # - def build_association(subject, parent_serializer_options) - association_value = value(subject) + def build_association(subject, parent_serializer_options, current_include_directive = {}) + association_value = value(subject, current_include_directive) reflection_options = options.dup serializer_class = subject.class.serializer_for(association_value, reflection_options) - reflection_options[:include_data] = @_include_data + reflection_options[:include_data] = include_data?(subject, current_include_directive) if serializer_class begin @@ -134,6 +144,26 @@ def build_association(subject, parent_serializer_options) private + def include_data_for(serializer, current_include_directive) + return unless include_data?(serializer, current_include_directive) + + if serializer.class._associations_via_include_param + if current_include_directive.key?(name) + serializer.read_attribute_for_serialization(name) + end + else + serializer.read_attribute_for_serialization(name) + end + end + + def include_data?(serializer, current_include_directive) + if serializer.class._associations_via_include_param + current_include_directive.key?(name) + else + @_include_data + end + end + def serializer_options(subject, parent_serializer_options, reflection_options) serializer = reflection_options.fetch(:serializer, nil) diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index 9b516f8dc..da5b57375 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -235,17 +235,17 @@ def resource_objects_for(serializers) @primary = [] @included = [] @resource_identifiers = Set.new - serializers.each { |serializer| process_resource(serializer, true) } + serializers.each { |serializer| process_resource(serializer, true, @include_directive) } serializers.each { |serializer| process_relationships(serializer, @include_directive) } [@primary, @included] end - def process_resource(serializer, primary) + def process_resource(serializer, primary, include_directive = {}) resource_identifier = ResourceIdentifier.new(serializer, instance_options).as_json return false unless @resource_identifiers.add?(resource_identifier) - resource_object = resource_object_for(serializer) + resource_object = resource_object_for(serializer, include_directive) if primary @primary << resource_object else @@ -267,7 +267,7 @@ def process_relationship(serializer, include_directive) return end return unless serializer && serializer.object - return unless process_resource(serializer, false) + return unless process_resource(serializer, false, include_directive) process_relationships(serializer, include_directive) end @@ -293,7 +293,7 @@ def attributes_for(serializer, fields) end # {http://jsonapi.org/format/#document-resource-objects Document Resource Objects} - def resource_object_for(serializer) + def resource_object_for(serializer, include_directive = {}) resource_object = serializer.fetch(self) do resource_object = ResourceIdentifier.new(serializer, instance_options).as_json @@ -304,7 +304,7 @@ def resource_object_for(serializer) end requested_associations = fieldset.fields_for(resource_object[:type]) || '*' - relationships = relationships_for(serializer, requested_associations) + relationships = relationships_for(serializer, requested_associations, include_directive) resource_object[:relationships] = relationships if relationships.any? links = links_for(serializer) @@ -432,12 +432,12 @@ def resource_object_for(serializer) # id: 'required-id', # meta: meta # }.reject! {|_,v| v.nil? } - def relationships_for(serializer, requested_associations) - include_directive = JSONAPI::IncludeDirective.new( + def relationships_for(serializer, requested_associations, current_include_directive) + full_include_directive = JSONAPI::IncludeDirective.new( requested_associations, allow_wildcard: true ) - serializer.associations(include_directive).each_with_object({}) do |association, hash| + serializer.associations(full_include_directive, current_include_directive).each_with_object({}) do |association, hash| hash[association.key] = Relationship.new( serializer, association.serializer, diff --git a/test/adapter/json_api/include_param_test.rb b/test/adapter/json_api/include_param_test.rb new file mode 100644 index 000000000..9d8b11c54 --- /dev/null +++ b/test/adapter/json_api/include_param_test.rb @@ -0,0 +1,187 @@ +require 'test_helper' + +module ActiveModel + class Serializer + module Adapter + class JsonApi + class IncludeParamTest < ActiveSupport::TestCase + IncludeParamAuthor = Class.new(::Model) + + class CustomCommentLoader + def all + [{ foo: 'bar' }] + end + end + + class TagSerializer < ActiveModel::Serializer + attributes :id, :name + end + + class IncludeParamAuthorSerializer < ActiveModel::Serializer + class_attribute :comment_loader + + associations_via_include_param(true) + + has_many :tags, serializer: TagSerializer do + link :self, '//example.com/link_author/relationships/tags' + end + + has_many :unlinked_tags, serializer: TagSerializer + + has_many :posts, serializer: PostWithTagsSerializer + has_many :locations + has_many :comments do + load_data { IncludeParamAuthorSerializer.comment_loader.all } + end + + has_many :inline_comments do + [{ im: 'inline' }] + end + end + + def setup + IncludeParamAuthorSerializer.comment_loader = Class.new(CustomCommentLoader).new + @tag = Tag.new(id: 1337, name: 'mytag') + @author = IncludeParamAuthor.new( + id: 1337, + tags: [@tag] + ) + end + + def test_relationship_not_loaded_when_not_included + expected = { + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + @author.define_singleton_method(:read_attribute_for_serialization) do |attr| + fail 'should not be called' if attr == :tags + super(attr) + end + + assert_relationship(:tags, expected) + end + + def test_relationship_included + expected = { + data: [ + { + id: '1337', + type: 'tags' + } + ], + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + assert_relationship(:tags, expected, include: :tags) + end + + def test_sideloads_included + expected = [ + { + id: '1337', + type: 'tags', + attributes: { name: 'mytag' } + } + ] + hash = result(include: :tags) + assert_equal(expected, hash[:included]) + end + + def test_nested_relationship + expected = { + data: [ + { + id: '1337', + type: 'tags' + } + ], + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + expected_no_data = { + links: { + self: '//example.com/link_author/relationships/tags' + } + } + + assert_relationship(:tags, expected, include: [:tags, { posts: :tags }]) + + @author.define_singleton_method(:read_attribute_for_serialization) do |attr| + fail 'should not be called' if attr == :tags + super(attr) + end + + assert_relationship(:tags, expected_no_data, include: { posts: :tags }) + end + + def test_include_params_with_no_block + @author.define_singleton_method(:read_attribute_for_serialization) do |attr| + fail 'should not be called' if attr == :locations + super(attr) + end + + expected = {} + + assert_relationship(:locations, expected) + end + + def test_block_relationship + expected = { + data: [ + { 'foo' => 'bar' } + ] + } + + assert_relationship(:comments, expected, include: [:comments]) + end + + def test_block_relationship_not_included + expected = {} + + IncludeParamAuthorSerializer.comment_loader.define_singleton_method(:all) do + fail 'should not be called' + end + + assert_relationship(:comments, expected) + end + + # TODO: This is currently testing for backwards-compatibility + # It can be removed when a decision is reached in + # https://github.com/rails-api/active_model_serializers/pull/1720 + def test_block_value_without_load_data + expected = { + data: [ + { 'im' => 'inline' } + ] + } + + assert_relationship(:'inline-comments', expected, include: [:inline_comments]) + end + + def test_node_not_included_when_no_link + expected = nil + assert_relationship(:unlinked_tags, expected) + end + + private + + def result(opts) + opts = { adapter: :json_api }.merge(opts) + serializable(@author, opts).serializable_hash + end + + def assert_relationship(relationship_name, expected, opts = {}) + hash = result(opts) + assert_equal(expected, hash[:data][:relationships][relationship_name]) + end + end + end + end + end +end diff --git a/test/adapter/json_api/relationships_test.rb b/test/adapter/json_api/relationships_test.rb index c0656cf1b..3204d43f9 100644 --- a/test/adapter/json_api/relationships_test.rb +++ b/test/adapter/json_api/relationships_test.rb @@ -41,7 +41,7 @@ class RelationshipAuthorSerializer < ActiveModel::Serializer has_many :roles do |serializer| meta count: object.posts.count - serializer.cached_roles + load_data { serializer.cached_roles } end has_one :blog do diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index 218e0d727..47150fb83 100644 --- a/test/serializers/associations_test.rb +++ b/test/serializers/associations_test.rb @@ -129,7 +129,7 @@ def test_associations_custom_keys class InlineAssociationTestPostSerializer < ActiveModel::Serializer has_many :comments has_many :comments, key: :last_comments do - object.comments.last(1) + load_data { object.comments.last(1) } end end