From b6493b2ae3672eea5fb3717d98a9dc0d1d702118 Mon Sep 17 00:00:00 2001 From: Lee Richmond Date: Mon, 9 May 2016 09:43:19 -0400 Subject: [PATCH] Support JSONAPI include params with links/loading The current implementation does support conditionally sideloading relationships based on the 'include' URL param. However, omitting the relationship still loads the relationship (to populate the type/id 'relationships' payload), somewhat defeating the purpose. Instead, this changes the flow to: 1. If the relationship is included, load it and include it the response. 2. If the relationship is not included but there is a JSONAPI link, include the link in the response but do not load the relationship or include data. 3. If the relationship is not in the URL param and there is no link, do not include this node in the 'relationships' response. The `current_include_tree` edits in json_api.rb are to pass the current nested includes. This is to support when multiple entities have the same relationship, e.g. `/blogs/?include=posts.tags,tags` should include both blog tags and post tags, but `/blogs/?include=posts.tags` should only include post tags. This API is opt-in to support users who always want to load `relationships` data. To opt-in: ```ruby class BlogSerializer < ActiveModel::Serializer associations_via_include_param(true) # opt-in to this pattern has_many :tags has_many :posts do link :self, '//example.com/blogs/relationships/posts' end end ``` JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes). Fixes https://github.com/rails-api/active_model_serializers/issues/1707 Fixes https://github.com/rails-api/active_model_serializers/issues/1555 --- CHANGELOG.md | 2 + docs/general/serializers.md | 6 +- lib/active_model/serializer/associations.rb | 11 +- lib/active_model/serializer/reflection.rb | 49 +++-- .../adapter/json_api.rb | 18 +- test/adapter/json_api/include_param_test.rb | 170 ++++++++++++++++++ test/adapter/json_api/relationships_test.rb | 2 +- test/serializers/associations_test.rb | 2 +- 8 files changed, 234 insertions(+), 26 deletions(-) create mode 100644 test/adapter/json_api/include_param_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb573f24..9198ec45b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,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) @@ -29,6 +30,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/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..6dc1a12fe 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,22 @@ 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) + instance_exec(serializer, &block) + + 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 +115,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 +143,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 a62caab49..6c85af38b 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -231,17 +231,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 @@ -263,7 +263,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 @@ -289,7 +289,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 = cache_check(serializer) do resource_object = ResourceIdentifier.new(serializer, instance_options).as_json @@ -300,7 +300,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) @@ -428,11 +428,11 @@ 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..0dd06eb1b --- /dev/null +++ b/test/adapter/json_api/include_param_test.rb @@ -0,0 +1,170 @@ +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 + 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 + + 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