From 33f3a88ba0327a0991fbea081fb61ed5d52771b4 Mon Sep 17 00:00:00 2001 From: Mateo Murphy Date: Fri, 20 Mar 2015 16:04:33 -0400 Subject: [PATCH] Implement `included` and `id` and `type` as per spec --- lib/active_model/serializer.rb | 10 +++ .../serializer/adapter/json_api.rb | 27 +++---- .../adapter_selector_test.rb | 12 ++- .../action_controller/json_api_linked_test.rb | 47 ++++++----- .../serialization_scope_name_test.rb | 16 ++-- test/action_controller/serialization_test.rb | 81 ++++++++++++++----- test/adapter/json_api/belongs_to_test.rb | 27 ++++--- test/adapter/json_api/collection_test.rb | 10 ++- .../has_many_explicit_serializer_test.rb | 25 +++--- test/adapter/json_api/has_many_test.rb | 8 +- test/adapter/json_api/has_one_test.rb | 3 +- test/adapter/json_api/linked_test.rb | 67 ++++++++------- 12 files changed, 203 insertions(+), 130 deletions(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index b6193400..588abc99 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -164,6 +164,14 @@ module ActiveModel end end + def id + object.id if object + end + + def type + object.class.to_s.demodulize.underscore.pluralize + end + def attributes(options = {}) attributes = if options[:fields] @@ -172,6 +180,8 @@ module ActiveModel self.class._attributes.dup end + attributes += options[:required_fields] if options[:required_fields] + attributes.each_with_object({}) do |name, hash| hash[name] = send(name) end diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb index ef51ea14..69e69028 100644 --- a/lib/active_model/serializer/adapter/json_api.rb +++ b/lib/active_model/serializer/adapter/json_api.rb @@ -35,11 +35,9 @@ module ActiveModel private def add_links(resource, name, serializers) - type = serialized_object_type(serializers) resource[:links] ||= {} - resource[:links][name] ||= { linkage: [] } - resource[:links][name][:linkage] += serializers.map { |serializer| { type: type, id: serializer.id.to_s } } + resource[:links][name][:linkage] += serializers.map { |serializer| { type: serializer.type, id: serializer.id.to_s } } end def add_link(resource, name, serializer) @@ -47,9 +45,7 @@ module ActiveModel resource[:links][name] = { linkage: nil } if serializer && serializer.object - type = serialized_object_type(serializer) - - resource[:links][name][:linkage] = { type: type, id: serializer.id.to_s } + resource[:links][name][:linkage] = { type: serializer.type, id: serializer.id.to_s } end end @@ -58,17 +54,15 @@ module ActiveModel resource_path = [parent, resource_name].compact.join('.') - if include_assoc?(resource_path) && resource_type = serialized_object_type(serializers) - plural_name = resource_type.pluralize.to_sym - @top[:linked] ||= {} - @top[:linked][plural_name] ||= [] + if include_assoc?(resource_path) + @top[:included] ||= [] serializers.each do |serializer| attrs = attributes_for_serializer(serializer, @options) add_resource_links(attrs, serializer, add_included: false) - @top[:linked][plural_name].push(attrs) unless @top[:linked][plural_name].include?(attrs) + @top[:included].push(attrs) unless @top[:included].include?(attrs) end end @@ -85,14 +79,16 @@ module ActiveModel result = [] serializer.each do |object| options[:fields] = @fieldset && @fieldset.fields_for(serializer) + options[:required_fields] = [:id, :type] attributes = object.attributes(options) - attributes[:id] = attributes[:id].to_s if attributes[:id] + attributes[:id] = attributes[:id].to_s result << attributes end else options[:fields] = @fieldset && @fieldset.fields_for(serializer) + options[:required_fields] = [:id, :type] result = serializer.attributes(options) - result[:id] = result[:id].to_s if result[:id] + result[:id] = result[:id].to_s end result @@ -116,11 +112,6 @@ module ActiveModel end end - def serialized_object_type(serializer) - return false unless Array(serializer).first - Array(serializer).first.object.class.to_s.demodulize.underscore.pluralize - end - def add_resource_links(attrs, serializer, options = {}) options[:add_included] = options.fetch(:add_included, true) diff --git a/test/action_controller/adapter_selector_test.rb b/test/action_controller/adapter_selector_test.rb index ea266881..efa61f7b 100644 --- a/test/action_controller/adapter_selector_test.rb +++ b/test/action_controller/adapter_selector_test.rb @@ -29,7 +29,17 @@ module ActionController def test_render_using_adapter_override get :render_using_adapter_override - assert_equal '{"data":{"name":"Name 1","description":"Description 1"}}', response.body + + expected = { + data: { + name: "Name 1", + description: "Description 1", + id: assigns(:profile).id.to_s, + type: "profiles" + } + } + + assert_equal expected.to_json, response.body end def test_render_skipping_adapter diff --git a/test/action_controller/json_api_linked_test.rb b/test/action_controller/json_api_linked_test.rb index 3f8b45eb..321974e3 100644 --- a/test/action_controller/json_api_linked_test.rb +++ b/test/action_controller/json_api_linked_test.rb @@ -83,80 +83,87 @@ module ActionController def test_render_resource_without_include get :render_resource_without_include response = JSON.parse(@response.body) - refute response.key? 'linked' + refute response.key? 'included' end def test_render_resource_with_include get :render_resource_with_include response = JSON.parse(@response.body) - assert response.key? 'linked' - assert_equal 1, response['linked']['authors'].size - assert_equal 'Steve K.', response['linked']['authors'].first['name'] + assert response.key? 'included' + assert_equal 1, response['included'].size + assert_equal 'Steve K.', response['included'].first['name'] end def test_render_resource_with_nested_has_many_include get :render_resource_with_nested_has_many_include response = JSON.parse(@response.body) - expected_linked = { - "authors" => [{ + expected_linked = [ + { "id" => "1", + "type" => "authors", "name" => "Steve K.", "links" => { "posts" => { "linkage" => [] }, "roles" => { "linkage" => [{ "type" =>"roles", "id" => "1" }, { "type" =>"roles", "id" => "2" }] }, "bio" => { "linkage" => nil } } - }], - "roles"=>[{ + }, { "id" => "1", + "type" => "roles", "name" => "admin", "links" => { "author" => { "linkage" => { "type" =>"authors", "id" => "1" } } } }, { "id" => "2", + "type" => "roles", "name" => "colab", "links" => { "author" => { "linkage" => { "type" =>"authors", "id" => "1" } } } - }] - } - assert_equal expected_linked, response['linked'] + } + ] + assert_equal expected_linked, response['included'] end def test_render_resource_with_nested_include get :render_resource_with_nested_include response = JSON.parse(@response.body) - assert response.key? 'linked' - assert_equal 1, response['linked']['authors'].size - assert_equal 'Anonymous', response['linked']['authors'].first['name'] + assert response.key? 'included' + assert_equal 1, response['included'].size + assert_equal 'Anonymous', response['included'].first['name'] end def test_render_collection_without_include get :render_collection_without_include response = JSON.parse(@response.body) - refute response.key? 'linked' + refute response.key? 'included' end def test_render_collection_with_include get :render_collection_with_include response = JSON.parse(@response.body) - assert response.key? 'linked' + assert response.key? 'included' end def test_render_resource_with_nested_attributes_even_when_missing_associations get :render_resource_with_missing_nested_has_many_include response = JSON.parse(@response.body) - assert response.key? 'linked' - refute response['linked'].key? 'roles' + assert response.key? 'included' + refute has_type?(response['included'], 'roles') end def test_render_collection_with_missing_nested_has_many_include get :render_collection_with_missing_nested_has_many_include response = JSON.parse(@response.body) - assert response.key? 'linked' - assert response['linked'].key? 'roles' + assert response.key? 'included' + assert has_type?(response['included'], 'roles') end + + def has_type?(collection, value) + collection.detect { |i| i['type'] == value} + end + end end end diff --git a/test/action_controller/serialization_scope_name_test.rb b/test/action_controller/serialization_scope_name_test.rb index df77231a..65f44c00 100644 --- a/test/action_controller/serialization_scope_name_test.rb +++ b/test/action_controller/serialization_scope_name_test.rb @@ -2,7 +2,7 @@ require 'test_helper' require 'pathname' class DefaultScopeNameTest < ActionController::TestCase - TestUser = Struct.new(:name, :admin) + TestUser = Struct.new(:id, :name, :admin) class UserSerializer < ActiveModel::Serializer attributes :admin? @@ -17,11 +17,11 @@ class DefaultScopeNameTest < ActionController::TestCase before_filter { request.format = :json } def current_user - TestUser.new('Pete', false) + TestUser.new(1, 'Pete', false) end def render_new_user - render json: TestUser.new('pete', false), serializer: UserSerializer, adapter: :json_api + render json: TestUser.new(1, 'pete', false), serializer: UserSerializer, adapter: :json_api end end @@ -29,12 +29,12 @@ class DefaultScopeNameTest < ActionController::TestCase def test_default_scope_name get :render_new_user - assert_equal '{"data":{"admin?":false}}', @response.body + assert_equal '{"data":{"admin?":false,"id":"1","type":"test_users"}}', @response.body end end class SerializationScopeNameTest < ActionController::TestCase - TestUser = Struct.new(:name, :admin) + TestUser = Struct.new(:id, :name, :admin) class AdminUserSerializer < ActiveModel::Serializer attributes :admin? @@ -50,11 +50,11 @@ class SerializationScopeNameTest < ActionController::TestCase before_filter { request.format = :json } def current_admin - TestUser.new('Bob', true) + TestUser.new(1, 'Bob', true) end def render_new_user - render json: TestUser.new('pete', false), serializer: AdminUserSerializer, adapter: :json_api + render json: TestUser.new(1, 'pete', false), serializer: AdminUserSerializer, adapter: :json_api end end @@ -62,6 +62,6 @@ class SerializationScopeNameTest < ActionController::TestCase def test_override_scope_name_with_controller get :render_new_user - assert_equal '{"data":{"admin?":true}}', @response.body + assert_equal '{"data":{"admin?":true,"id":"1","type":"test_users"}}', @response.body end end \ No newline at end of file diff --git a/test/action_controller/serialization_test.rb b/test/action_controller/serialization_test.rb index 28325ede..b47db521 100644 --- a/test/action_controller/serialization_test.rb +++ b/test/action_controller/serialization_test.rb @@ -15,13 +15,11 @@ module ActionController end def render_using_default_adapter_root - old_adapter = ActiveModel::Serializer.config.adapter - # JSON-API adapter sets root by default - ActiveModel::Serializer.config.adapter = ActiveModel::Serializer::Adapter::JsonApi - @profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }) - render json: @profile - ensure - ActiveModel::Serializer.config.adapter = old_adapter + with_adapter ActiveModel::Serializer::Adapter::JsonApi do + # JSON-API adapter sets root by default + @profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }) + render json: @profile + end end def render_using_custom_root_in_adapter_with_a_default @@ -39,15 +37,14 @@ module ActionController end def render_array_using_implicit_serializer_and_meta - old_adapter = ActiveModel::Serializer.config.adapter + with_adapter ActiveModel::Serializer::Adapter::JsonApi do - ActiveModel::Serializer.config.adapter = ActiveModel::Serializer::Adapter::JsonApi - array = [ - Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }) - ] - render json: array, meta: { total: 10 } - ensure - ActiveModel::Serializer.config.adapter = old_adapter + @profiles = [ + Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }) + ] + + render json: @profiles, meta: { total: 10 } + end end def render_object_with_cache_enabled @@ -88,6 +85,15 @@ module ActionController adapter = ActiveModel::Serializer.adapter.new(serializer) adapter.to_json end + + def with_adapter(adapter) + old_adapter = ActiveModel::Serializer.config.adapter + # JSON-API adapter sets root by default + ActiveModel::Serializer.config.adapter = adapter + yield + ensure + ActiveModel::Serializer.config.adapter = old_adapter + end end tests MyController @@ -96,8 +102,13 @@ module ActionController def test_render_using_implicit_serializer get :render_using_implicit_serializer + expected = { + name: "Name 1", + description: "Description 1" + } + assert_equal 'application/json', @response.content_type - assert_equal '{"name":"Name 1","description":"Description 1"}', @response.body + assert_equal expected.to_json, @response.body end def test_render_using_custom_root @@ -110,15 +121,33 @@ module ActionController def test_render_using_default_root get :render_using_default_adapter_root + expected = { + data: { + name: "Name 1", + description: "Description 1", + id: assigns(:profile).id.to_s, + type: "profiles" + } + } + assert_equal 'application/json', @response.content_type - assert_equal '{"data":{"name":"Name 1","description":"Description 1"}}', @response.body + assert_equal expected.to_json, @response.body end def test_render_using_custom_root_in_adapter_with_a_default get :render_using_custom_root_in_adapter_with_a_default + expected = { + data: { + name: "Name 1", + description: "Description 1", + id: assigns(:profile).id.to_s, + type: "profiles" + } + } + assert_equal 'application/json', @response.content_type - assert_equal '{"data":{"name":"Name 1","description":"Description 1"}}', @response.body + assert_equal expected.to_json, @response.body end def test_render_array_using_implicit_serializer @@ -142,8 +171,22 @@ module ActionController def test_render_array_using_implicit_serializer_and_meta get :render_array_using_implicit_serializer_and_meta + expected = { + data: [ + { + name: "Name 1", + description: "Description 1", + id: assigns(:profiles).first.id.to_s, + type: "profiles" + } + ], + meta: { + total: 10 + } + } + assert_equal 'application/json', @response.content_type - assert_equal '{"data":[{"name":"Name 1","description":"Description 1"}],"meta":{"total":10}}', @response.body + assert_equal expected.to_json, @response.body end def test_render_with_cache_enable diff --git a/test/adapter/json_api/belongs_to_test.rb b/test/adapter/json_api/belongs_to_test.rb index 38f80800..3ce8074e 100644 --- a/test/adapter/json_api/belongs_to_test.rb +++ b/test/adapter/json_api/belongs_to_test.rb @@ -41,6 +41,7 @@ module ActiveModel @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, include: 'post') expected = [{ id: "42", + type: "posts", title: 'New Post', body: 'Body', links: { @@ -49,12 +50,14 @@ module ActiveModel author: { linkage: { type: "authors", id: "1" } } } }] - assert_equal expected, @adapter.serializable_hash[:linked][:posts] + assert_equal expected, @adapter.serializable_hash[:included] end def test_limiting_linked_post_fields @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, include: 'post', fields: {post: [:title]}) expected = [{ + id: "42", + type: "posts", title: 'New Post', links: { comments: { linkage: [ { type: "comments", id: "1" } ] }, @@ -62,7 +65,7 @@ module ActiveModel author: { linkage: { type: "authors", id: "1" } } } }] - assert_equal expected, @adapter.serializable_hash[:linked][:posts] + assert_equal expected, @adapter.serializable_hash[:included] end def test_include_nil_author @@ -102,37 +105,39 @@ module ActiveModel def test_include_linked_resources_with_type_name serializer = BlogSerializer.new(@blog) adapter = ActiveModel::Serializer::Adapter::JsonApi.new(serializer, include: ['writer', 'articles']) - linked = adapter.serializable_hash[:linked] - expected = { - authors: [{ + linked = adapter.serializable_hash[:included] + expected = [ + { id: "1", + type: "authors", name: "Steve K.", links: { posts: { linkage: [] }, roles: { linkage: [] }, bio: { linkage: nil } } - }], - posts: [{ + },{ + id: "42", + type: "posts", title: "New Post", body: "Body", - id: "42", links: { comments: { linkage: [ { type: "comments", id: "1" } ] }, blog: { linkage: { type: "blogs", id: "999" } }, author: { linkage: { type: "authors", id: "1" } } } }, { + id: "43", + type: "posts", title: "Hello!!", body: "Hello, world!!", - id: "43", links: { comments: { linkage: [] }, blog: { linkage: { type: "blogs", id: "999" } }, author: { linkage: nil } } - }] - } + } + ] assert_equal expected, linked end end diff --git a/test/adapter/json_api/collection_test.rb b/test/adapter/json_api/collection_test.rb index c0216c36..36fb68e2 100644 --- a/test/adapter/json_api/collection_test.rb +++ b/test/adapter/json_api/collection_test.rb @@ -27,9 +27,10 @@ module ActiveModel def test_include_multiple_posts expected = [ { + id: "1", + type: "posts", title: "Hello!!", body: "Hello, world!!", - id: "1", links: { comments: { linkage: [] }, blog: { linkage: { type: "blogs", id: "999" } }, @@ -37,9 +38,10 @@ module ActiveModel } }, { + id: "2", + type: "posts", title: "New Post", body: "Body", - id: "2", links: { comments: { linkage: [] }, blog: { linkage: { type: "blogs", id: "999" } }, @@ -56,6 +58,8 @@ module ActiveModel expected = [ { + id: "1", + type: "posts", title: "Hello!!", links: { comments: { linkage: [] }, @@ -64,6 +68,8 @@ module ActiveModel } }, { + id: "2", + type: "posts", title: "New Post", links: { comments: { linkage: [] }, diff --git a/test/adapter/json_api/has_many_explicit_serializer_test.rb b/test/adapter/json_api/has_many_explicit_serializer_test.rb index ecebbaa7..4ff53728 100644 --- a/test/adapter/json_api/has_many_explicit_serializer_test.rb +++ b/test/adapter/json_api/has_many_explicit_serializer_test.rb @@ -39,25 +39,33 @@ module ActiveModel assert_equal(expected, @adapter.serializable_hash[:data][:links][:comments]) end - def test_includes_linked_comments + def test_includes_linked_data # If CommentPreviewSerializer is applied correctly the body text will not be present in the output expected = [ { id: '1', + type: 'comments', links: { post: { linkage: { type: 'posts', id: @post.id.to_s } } } }, { id: '2', + type: 'comments', links: { post: { linkage: { type: 'posts', id: @post.id.to_s } } } + }, + { + id: @author.id.to_s, + type: "authors", + links: { + posts: { linkage: [ {type: "posts", id: @post.id.to_s } ] } + } } ] - assert_equal(expected, - @adapter.serializable_hash[:linked][:comments]) + assert_equal(expected, @adapter.serializable_hash[:included]) end def test_includes_author_id @@ -68,17 +76,6 @@ module ActiveModel assert_equal(expected, @adapter.serializable_hash[:data][:links][:author]) end - def test_includes_linked_authors - expected = [{ - id: @author.id.to_s, - links: { - posts: { linkage: [ { type: "posts", id: @post.id.to_s } ] } - } - }] - - assert_equal(expected, @adapter.serializable_hash[:linked][:authors]) - end - def test_explicit_serializer_with_null_resource @post.author = nil diff --git a/test/adapter/json_api/has_many_test.rb b/test/adapter/json_api/has_many_test.rb index b520e5b0..cdd4bf3e 100644 --- a/test/adapter/json_api/has_many_test.rb +++ b/test/adapter/json_api/has_many_test.rb @@ -42,6 +42,7 @@ module ActiveModel @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, include: 'comments') expected = [{ id: "1", + type: "comments", body: 'ZOMG A COMMENT', links: { post: { linkage: { type: "posts", id: "1" } }, @@ -49,31 +50,34 @@ module ActiveModel } }, { id: "2", + type: "comments", body: 'ZOMG ANOTHER COMMENT', links: { post: { linkage: { type: "posts", id: "1" } }, author: { linkage: nil } } }] - assert_equal expected, @adapter.serializable_hash[:linked][:comments] + assert_equal expected, @adapter.serializable_hash[:included] end def test_limit_fields_of_linked_comments @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, include: 'comments', fields: {comment: [:id]}) expected = [{ id: "1", + type: "comments", links: { post: { linkage: { type: "posts", id: "1" } }, author: { linkage: nil } } }, { id: "2", + type: "comments", links: { post: { linkage: { type: "posts", id: "1" } }, author: { linkage: nil } } }] - assert_equal expected, @adapter.serializable_hash[:linked][:comments] + assert_equal expected, @adapter.serializable_hash[:included] end def test_no_include_linked_if_comments_is_empty diff --git a/test/adapter/json_api/has_one_test.rb b/test/adapter/json_api/has_one_test.rb index 267ec4e5..8847e020 100644 --- a/test/adapter/json_api/has_one_test.rb +++ b/test/adapter/json_api/has_one_test.rb @@ -41,6 +41,7 @@ module ActiveModel expected = [ { id: "43", + type: "bios", content:"AMS Contributor", links: { author: { linkage: { type: "authors", id: "1" } } @@ -48,7 +49,7 @@ module ActiveModel } ] - assert_equal(expected, @adapter.serializable_hash[:linked][:bios]) + assert_equal(expected, @adapter.serializable_hash[:included]) end end end diff --git a/test/adapter/json_api/linked_test.rb b/test/adapter/json_api/linked_test.rb index e85c4b7d..c6fada87 100644 --- a/test/adapter/json_api/linked_test.rb +++ b/test/adapter/json_api/linked_test.rb @@ -142,42 +142,41 @@ module ActiveModel include: 'author,author.posts' ) - expected = { - authors: [ - { - id: "1", - name: "Steve K.", - links: { - posts: { linkage: [ { type: "posts", id: "10"}, { type: "posts", id: "30" }] }, - roles: { linkage: [] }, - bio: { linkage: { type: "bios", id: "1" }} - } + expected = [ + { + id: "1", + type: "authors", + name: "Steve K.", + links: { + posts: { linkage: [ { type: "posts", id: "10"}, { type: "posts", id: "30" }] }, + roles: { linkage: [] }, + bio: { linkage: { type: "bios", id: "1" }} } - ], - posts: [ - { - id: "10", - title: "Hello!!", - body: "Hello, world!!", - links: { - comments: { linkage: [ { type: "comments", id: "1"}, { type: "comments", id: "2" }] }, - blog: { linkage: { type: "blogs", id: "999" } }, - author: { linkage: { type: "authors", id: "1" } } - } - }, { - id: "30", - title: "Yet Another Post", - body: "Body", - links: { - comments: { linkage: [] }, - blog: { linkage: { type: "blogs", id: "999" } }, - author: { linkage: { type: "authors", id: "1" } } - } + }, { + id: "10", + type: "posts", + title: "Hello!!", + body: "Hello, world!!", + links: { + comments: { linkage: [ { type: "comments", id: "1"}, { type: "comments", id: "2" }] }, + blog: { linkage: { type: "blogs", id: "999" } }, + author: { linkage: { type: "authors", id: "1" } } } - ] - } - assert_equal expected, adapter.serializable_hash[:linked] - assert_equal expected, alt_adapter.serializable_hash[:linked] + }, { + id: "30", + type: "posts", + title: "Yet Another Post", + body: "Body", + links: { + comments: { linkage: [] }, + blog: { linkage: { type: "blogs", id: "999" } }, + author: { linkage: { type: "authors", id: "1" } } + } + } + ] + + assert_equal expected, adapter.serializable_hash[:included] + assert_equal expected, alt_adapter.serializable_hash[:included] end def test_ignore_model_namespace_for_linked_resource_type