From b439fe69c6d82b2a736489d762166ce87afe3f6e Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 31 Oct 2017 14:26:05 -0500 Subject: [PATCH 1/6] Refactor jsonapi type/id tests to be more explicit --- .../json_api/resource_identifier_test.rb | 59 +++++++++---------- test/adapter/json_api/type_test.rb | 12 +--- test/support/serialization_testing.rb | 8 +++ 3 files changed, 39 insertions(+), 40 deletions(-) diff --git a/test/adapter/json_api/resource_identifier_test.rb b/test/adapter/json_api/resource_identifier_test.rb index 62b7d93b..1e93b26b 100644 --- a/test/adapter/json_api/resource_identifier_test.rb +++ b/test/adapter/json_api/resource_identifier_test.rb @@ -28,15 +28,25 @@ module ActiveModelSerializers end def test_defined_type - test_type(WithDefinedTypeSerializer, 'with-defined-type') + actual = actual_resource_identifier_object(WithDefinedTypeSerializer) + expected = { id: expected_model_id, type: 'with-defined-type' } + assert_equal actual, expected end def test_singular_type - test_type_inflection(AuthorSerializer, 'author', :singular) + actual = with_jsonapi_inflection :singular do + actual_resource_identifier_object(AuthorSerializer) + end + expected = { id: expected_model_id, type: 'author' } + assert_equal actual, expected end def test_plural_type - test_type_inflection(AuthorSerializer, 'authors', :plural) + actual = with_jsonapi_inflection :plural do + actual_resource_identifier_object(AuthorSerializer) + end + expected = { id: expected_model_id, type: 'authors' } + assert_equal actual, expected end def test_type_with_namespace @@ -60,49 +70,38 @@ module ActiveModelSerializers end def test_id_defined_on_object - test_id(AuthorSerializer, @model.id.to_s) + actual = actual_resource_identifier_object(AuthorSerializer) + expected = { id: @model.id.to_s, type: expected_model_type } + assert_equal actual, expected end def test_id_defined_on_serializer - test_id(WithDefinedIdSerializer, 'special_id') + actual = actual_resource_identifier_object(WithDefinedIdSerializer) + expected = { id: 'special_id', type: expected_model_type } + assert_equal actual, expected end def test_id_defined_on_fragmented - test_id(FragmentedSerializer, 'special_id') + actual = actual_resource_identifier_object(FragmentedSerializer) + expected = { id: 'special_id', type: expected_model_type } + assert_equal actual, expected end private - def test_type_inflection(serializer_class, expected_type, inflection) - original_inflection = ActiveModelSerializers.config.jsonapi_resource_type - ActiveModelSerializers.config.jsonapi_resource_type = inflection - test_type(serializer_class, expected_type) - ensure - ActiveModelSerializers.config.jsonapi_resource_type = original_inflection - end - - def test_type(serializer_class, expected_type) + def actual_resource_identifier_object(serializer_class) serializer = serializer_class.new(@model) resource_identifier = ResourceIdentifier.new(serializer, nil) - expected = { - id: @model.id.to_s, - type: expected_type - } - - assert_equal(expected, resource_identifier.as_json) + resource_identifier.as_json end - def test_id(serializer_class, id) - serializer = serializer_class.new(@model) - resource_identifier = ResourceIdentifier.new(serializer, nil) + def expected_model_type inflection = ActiveModelSerializers.config.jsonapi_resource_type - type = @model.class.model_name.send(inflection) - expected = { - id: id, - type: type - } + @model.class.model_name.send(inflection) + end - assert_equal(expected, resource_identifier.as_json) + def expected_model_id + @model.id.to_s end end end diff --git a/test/adapter/json_api/type_test.rb b/test/adapter/json_api/type_test.rb index 40b84cf2..8d857dd8 100644 --- a/test/adapter/json_api/type_test.rb +++ b/test/adapter/json_api/type_test.rb @@ -20,13 +20,13 @@ module ActiveModel end def test_config_plural - with_jsonapi_resource_type :plural do + with_jsonapi_inflection :plural do assert_type(@author, 'authors') end end def test_config_singular - with_jsonapi_resource_type :singular do + with_jsonapi_inflection :singular do assert_type(@author, 'author') end end @@ -46,14 +46,6 @@ module ActiveModel hash = serializable(resource, opts).serializable_hash assert_equal(expected_type, hash.fetch(:data).fetch(:type)) end - - def with_jsonapi_resource_type(inflection) - old_inflection = ActiveModelSerializers.config.jsonapi_resource_type - ActiveModelSerializers.config.jsonapi_resource_type = inflection - yield - ensure - ActiveModelSerializers.config.jsonapi_resource_type = old_inflection - end end end end diff --git a/test/support/serialization_testing.rb b/test/support/serialization_testing.rb index 524a3297..4a957976 100644 --- a/test/support/serialization_testing.rb +++ b/test/support/serialization_testing.rb @@ -47,6 +47,14 @@ module SerializationTesting ActiveModelSerializers.config.replace(old_config) end + def with_jsonapi_inflection(inflection) + original_inflection = ActiveModelSerializers.config.jsonapi_resource_type + ActiveModelSerializers.config.jsonapi_resource_type = inflection + yield + ensure + ActiveModelSerializers.config.jsonapi_resource_type = original_inflection + end + def with_serializer_lookup_disabled original_serializer_lookup = ActiveModelSerializers.config.serializer_lookup_enabled ActiveModelSerializers.config.serializer_lookup_enabled = false From 82e90091fdaa02bf30dbaae2387272cccceb40e1 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 31 Oct 2017 14:27:07 -0500 Subject: [PATCH 2/6] Put the similar jsonapi type tests together --- .../json_api/resource_identifier_test.rb | 109 ------------ test/adapter/json_api/type_test.rb | 165 ++++++++++++++---- 2 files changed, 132 insertions(+), 142 deletions(-) delete mode 100644 test/adapter/json_api/resource_identifier_test.rb diff --git a/test/adapter/json_api/resource_identifier_test.rb b/test/adapter/json_api/resource_identifier_test.rb deleted file mode 100644 index 1e93b26b..00000000 --- a/test/adapter/json_api/resource_identifier_test.rb +++ /dev/null @@ -1,109 +0,0 @@ -require 'test_helper' - -module ActiveModelSerializers - module Adapter - class JsonApi - class ResourceIdentifierTest < ActiveSupport::TestCase - class WithDefinedTypeSerializer < ActiveModel::Serializer - type 'with_defined_type' - end - - class WithDefinedIdSerializer < ActiveModel::Serializer - def id - 'special_id' - end - end - - class FragmentedSerializer < ActiveModel::Serializer - cache only: :id - - def id - 'special_id' - end - end - - setup do - @model = Author.new(id: 1, name: 'Steve K.') - ActionController::Base.cache_store.clear - end - - def test_defined_type - actual = actual_resource_identifier_object(WithDefinedTypeSerializer) - expected = { id: expected_model_id, type: 'with-defined-type' } - assert_equal actual, expected - end - - def test_singular_type - actual = with_jsonapi_inflection :singular do - actual_resource_identifier_object(AuthorSerializer) - end - expected = { id: expected_model_id, type: 'author' } - assert_equal actual, expected - end - - def test_plural_type - actual = with_jsonapi_inflection :plural do - actual_resource_identifier_object(AuthorSerializer) - end - expected = { id: expected_model_id, type: 'authors' } - assert_equal actual, expected - end - - def test_type_with_namespace - Object.const_set(:Admin, Module.new) - model = Class.new(::Model) - Admin.const_set(:PowerUser, model) - serializer = Class.new(ActiveModel::Serializer) - Admin.const_set(:PowerUserSerializer, serializer) - with_namespace_separator '--' do - admin_user = Admin::PowerUser.new - serializer = Admin::PowerUserSerializer.new(admin_user) - expected = { - id: admin_user.id, - type: 'admin--power-users' - } - - identifier = ResourceIdentifier.new(serializer, {}) - actual = identifier.as_json - assert_equal(expected, actual) - end - end - - def test_id_defined_on_object - actual = actual_resource_identifier_object(AuthorSerializer) - expected = { id: @model.id.to_s, type: expected_model_type } - assert_equal actual, expected - end - - def test_id_defined_on_serializer - actual = actual_resource_identifier_object(WithDefinedIdSerializer) - expected = { id: 'special_id', type: expected_model_type } - assert_equal actual, expected - end - - def test_id_defined_on_fragmented - actual = actual_resource_identifier_object(FragmentedSerializer) - expected = { id: 'special_id', type: expected_model_type } - assert_equal actual, expected - end - - private - - def actual_resource_identifier_object(serializer_class) - serializer = serializer_class.new(@model) - resource_identifier = ResourceIdentifier.new(serializer, nil) - resource_identifier.as_json - end - - def expected_model_type - inflection = ActiveModelSerializers.config.jsonapi_resource_type - @model.class.model_name.send(inflection) - end - - def expected_model_id - @model.id.to_s - end - end - end - end -end diff --git a/test/adapter/json_api/type_test.rb b/test/adapter/json_api/type_test.rb index 8d857dd8..1b4d64d8 100644 --- a/test/adapter/json_api/type_test.rb +++ b/test/adapter/json_api/type_test.rb @@ -1,51 +1,150 @@ require 'test_helper' -module ActiveModel - class Serializer - module Adapter - class JsonApi - class TypeTest < ActiveSupport::TestCase - class StringTypeSerializer < ActiveModel::Serializer - attribute :name - type 'profile' - end +module ActiveModelSerializers + module Adapter + class JsonApi + class TypeTest < ActiveSupport::TestCase + class StringTypeSerializer < ActiveModel::Serializer + attribute :name + type 'profile' + end - class SymbolTypeSerializer < ActiveModel::Serializer - attribute :name - type :profile - end + class SymbolTypeSerializer < ActiveModel::Serializer + attribute :name + type :profile + end - setup do - @author = Author.new(id: 1, name: 'Steve K.') - end + setup do + @author = Author.new(id: 1, name: 'Steve K.') + end - def test_config_plural - with_jsonapi_inflection :plural do - assert_type(@author, 'authors') - end + def test_config_plural + with_jsonapi_inflection :plural do + assert_type(@author, 'authors') end + end - def test_config_singular - with_jsonapi_inflection :singular do - assert_type(@author, 'author') - end + def test_config_singular + with_jsonapi_inflection :singular do + assert_type(@author, 'author') end + end - def test_explicit_string_type_value - assert_type(@author, 'profile', serializer: StringTypeSerializer) + def test_explicit_string_type_value + assert_type(@author, 'profile', serializer: StringTypeSerializer) + end + + def test_explicit_symbol_type_value + assert_type(@author, 'profile', serializer: SymbolTypeSerializer) + end + + private + + def assert_type(resource, expected_type, opts = {}) + opts = opts.reverse_merge(adapter: :json_api) + hash = serializable(resource, opts).serializable_hash + assert_equal(expected_type, hash.fetch(:data).fetch(:type)) + end + end + class ResourceIdentifierTest < ActiveSupport::TestCase + class WithDefinedTypeSerializer < ActiveModel::Serializer + type 'with_defined_type' + end + + class WithDefinedIdSerializer < ActiveModel::Serializer + def id + 'special_id' end + end - def test_explicit_symbol_type_value - assert_type(@author, 'profile', serializer: SymbolTypeSerializer) + class FragmentedSerializer < ActiveModel::Serializer + cache only: :id + + def id + 'special_id' end + end - private + setup do + @model = Author.new(id: 1, name: 'Steve K.') + ActionController::Base.cache_store.clear + end - def assert_type(resource, expected_type, opts = {}) - opts = opts.reverse_merge(adapter: :json_api) - hash = serializable(resource, opts).serializable_hash - assert_equal(expected_type, hash.fetch(:data).fetch(:type)) + def test_defined_type + actual = actual_resource_identifier_object(WithDefinedTypeSerializer) + expected = { id: expected_model_id, type: 'with-defined-type' } + assert_equal actual, expected + end + + def test_singular_type + actual = with_jsonapi_inflection :singular do + actual_resource_identifier_object(AuthorSerializer) end + expected = { id: expected_model_id, type: 'author' } + assert_equal actual, expected + end + + def test_plural_type + actual = with_jsonapi_inflection :plural do + actual_resource_identifier_object(AuthorSerializer) + end + expected = { id: expected_model_id, type: 'authors' } + assert_equal actual, expected + end + + def test_type_with_namespace + Object.const_set(:Admin, Module.new) + model = Class.new(::Model) + Admin.const_set(:PowerUser, model) + serializer = Class.new(ActiveModel::Serializer) + Admin.const_set(:PowerUserSerializer, serializer) + with_namespace_separator '--' do + admin_user = Admin::PowerUser.new + serializer = Admin::PowerUserSerializer.new(admin_user) + expected = { + id: admin_user.id, + type: 'admin--power-users' + } + + identifier = ResourceIdentifier.new(serializer, {}) + actual = identifier.as_json + assert_equal(expected, actual) + end + end + + def test_id_defined_on_object + actual = actual_resource_identifier_object(AuthorSerializer) + expected = { id: @model.id.to_s, type: expected_model_type } + assert_equal actual, expected + end + + def test_id_defined_on_serializer + actual = actual_resource_identifier_object(WithDefinedIdSerializer) + expected = { id: 'special_id', type: expected_model_type } + assert_equal actual, expected + end + + def test_id_defined_on_fragmented + actual = actual_resource_identifier_object(FragmentedSerializer) + expected = { id: 'special_id', type: expected_model_type } + assert_equal actual, expected + end + + private + + def actual_resource_identifier_object(serializer_class) + serializer = serializer_class.new(@model) + resource_identifier = ResourceIdentifier.new(serializer, nil) + resource_identifier.as_json + end + + def expected_model_type + inflection = ActiveModelSerializers.config.jsonapi_resource_type + @model.class.model_name.send(inflection) + end + + def expected_model_id + @model.id.to_s end end end From 92dde58f5fcb6de03182700088ff7f2e20acc385 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 31 Oct 2017 14:31:33 -0500 Subject: [PATCH 3/6] Assert serializer-defined types are not inflected --- test/adapter/json_api/type_test.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/adapter/json_api/type_test.rb b/test/adapter/json_api/type_test.rb index 1b4d64d8..84bfe494 100644 --- a/test/adapter/json_api/type_test.rb +++ b/test/adapter/json_api/type_test.rb @@ -48,7 +48,7 @@ module ActiveModelSerializers end class ResourceIdentifierTest < ActiveSupport::TestCase class WithDefinedTypeSerializer < ActiveModel::Serializer - type 'with_defined_type' + type 'with_defined_types' end class WithDefinedIdSerializer < ActiveModel::Serializer @@ -71,8 +71,18 @@ module ActiveModelSerializers end def test_defined_type - actual = actual_resource_identifier_object(WithDefinedTypeSerializer) - expected = { id: expected_model_id, type: 'with-defined-type' } + actual = with_jsonapi_inflection :plural do + actual_resource_identifier_object(WithDefinedTypeSerializer) + end + expected = { id: expected_model_id, type: 'with-defined-types' } + assert_equal actual, expected + end + + def test_defined_type_not_inflected + actual = with_jsonapi_inflection :singular do + actual_resource_identifier_object(WithDefinedTypeSerializer) + end + expected = { id: expected_model_id, type: 'with-defined-types' } assert_equal actual, expected end From 5916014b4849d5a97e3e7ebed075bc68d6f2d60b Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 31 Oct 2017 14:34:30 -0500 Subject: [PATCH 4/6] Fix: resource object identifier with nil id excludes id --- .../adapter/json_api/resource_identifier.rb | 7 +++++-- test/adapter/json_api/type_test.rb | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/active_model_serializers/adapter/json_api/resource_identifier.rb b/lib/active_model_serializers/adapter/json_api/resource_identifier.rb index 3ff7118d..f7a8e2e9 100644 --- a/lib/active_model_serializers/adapter/json_api/resource_identifier.rb +++ b/lib/active_model_serializers/adapter/json_api/resource_identifier.rb @@ -38,8 +38,11 @@ module ActiveModelSerializers end def as_json - return nil if id.blank? - { id: id, type: type } + if id.blank? + { type: type } + else + { id: id.to_s, type: type } + end end protected diff --git a/test/adapter/json_api/type_test.rb b/test/adapter/json_api/type_test.rb index 84bfe494..53c4ca75 100644 --- a/test/adapter/json_api/type_test.rb +++ b/test/adapter/json_api/type_test.rb @@ -128,6 +128,13 @@ module ActiveModelSerializers assert_equal actual, expected end + def test_blank_id + @model.id = nil + actual = actual_resource_identifier_object(AuthorSerializer) + expected = { type: expected_model_type } + assert_equal actual, expected + end + def test_id_defined_on_serializer actual = actual_resource_identifier_object(WithDefinedIdSerializer) expected = { id: 'special_id', type: expected_model_type } From 9745a2f735b515a3f65b8993fa953b034a05dede Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 31 Oct 2017 14:47:48 -0500 Subject: [PATCH 5/6] Fix: ResourceIdentifier.for_type_with_id can serialize unpersisted resources --- .../adapter/json_api/resource_identifier.rb | 11 +-- test/adapter/json_api/type_test.rb | 72 ++++++++++++------- 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/lib/active_model_serializers/adapter/json_api/resource_identifier.rb b/lib/active_model_serializers/adapter/json_api/resource_identifier.rb index f7a8e2e9..4c1f56ba 100644 --- a/lib/active_model_serializers/adapter/json_api/resource_identifier.rb +++ b/lib/active_model_serializers/adapter/json_api/resource_identifier.rb @@ -8,12 +8,13 @@ module ActiveModelSerializers end def self.for_type_with_id(type, id, options) - return nil if id.blank? type = inflect_type(type) - { - id: id.to_s, - type: type_for(:no_class_needed, type, options) - } + type = type_for(:no_class_needed, type, options) + if id.blank? + { type: type } + else + { id: id.to_s, type: type } + end end def self.raw_type_from_serializer_object(object) diff --git a/test/adapter/json_api/type_test.rb b/test/adapter/json_api/type_test.rb index 53c4ca75..8e7fc259 100644 --- a/test/adapter/json_api/type_test.rb +++ b/test/adapter/json_api/type_test.rb @@ -72,33 +72,33 @@ module ActiveModelSerializers def test_defined_type actual = with_jsonapi_inflection :plural do - actual_resource_identifier_object(WithDefinedTypeSerializer) + actual_resource_identifier_object(WithDefinedTypeSerializer, @model) end - expected = { id: expected_model_id, type: 'with-defined-types' } + expected = { id: expected_model_id(@model), type: 'with-defined-types' } assert_equal actual, expected end def test_defined_type_not_inflected actual = with_jsonapi_inflection :singular do - actual_resource_identifier_object(WithDefinedTypeSerializer) + actual_resource_identifier_object(WithDefinedTypeSerializer, @model) end - expected = { id: expected_model_id, type: 'with-defined-types' } + expected = { id: expected_model_id(@model), type: 'with-defined-types' } assert_equal actual, expected end def test_singular_type actual = with_jsonapi_inflection :singular do - actual_resource_identifier_object(AuthorSerializer) + actual_resource_identifier_object(AuthorSerializer, @model) end - expected = { id: expected_model_id, type: 'author' } + expected = { id: expected_model_id(@model), type: 'author' } assert_equal actual, expected end def test_plural_type actual = with_jsonapi_inflection :plural do - actual_resource_identifier_object(AuthorSerializer) + actual_resource_identifier_object(AuthorSerializer, @model) end - expected = { id: expected_model_id, type: 'authors' } + expected = { id: expected_model_id(@model), type: 'authors' } assert_equal actual, expected end @@ -123,45 +123,69 @@ module ActiveModelSerializers end def test_id_defined_on_object - actual = actual_resource_identifier_object(AuthorSerializer) - expected = { id: @model.id.to_s, type: expected_model_type } + actual = actual_resource_identifier_object(AuthorSerializer, @model) + expected = { id: @model.id.to_s, type: expected_model_type(@model) } assert_equal actual, expected end def test_blank_id - @model.id = nil - actual = actual_resource_identifier_object(AuthorSerializer) - expected = { type: expected_model_type } + model = Author.new(id: nil, name: 'Steve K.') + actual = actual_resource_identifier_object(AuthorSerializer, model) + expected = { type: expected_model_type(model) } + assert_equal actual, expected + end + + def test_for_type_with_id + id = 1 + actual = ResourceIdentifier.for_type_with_id('admin_user', id, {}) + expected = { id: "1", type: 'admin-users' } + assert_equal actual, expected + end + + def test_for_type_with_id_given_blank_id + id = "" + actual = ResourceIdentifier.for_type_with_id('admin_user', id, {}) + expected = { type: 'admin-users' } + assert_equal actual, expected + end + + def test_for_type_with_id_inflected + id = 2 + actual = with_jsonapi_inflection :singular do + ResourceIdentifier.for_type_with_id('admin_users', id, {}) + end + expected = { id: "2", type: 'admin-user' } assert_equal actual, expected end def test_id_defined_on_serializer - actual = actual_resource_identifier_object(WithDefinedIdSerializer) - expected = { id: 'special_id', type: expected_model_type } + actual = actual_resource_identifier_object(WithDefinedIdSerializer, @model) + expected = { id: 'special_id', type: expected_model_type(@model) } assert_equal actual, expected end def test_id_defined_on_fragmented - actual = actual_resource_identifier_object(FragmentedSerializer) - expected = { id: 'special_id', type: expected_model_type } + actual = actual_resource_identifier_object(FragmentedSerializer, @model) + expected = { id: 'special_id', type: expected_model_type(@model) } assert_equal actual, expected end private - def actual_resource_identifier_object(serializer_class) - serializer = serializer_class.new(@model) + def actual_resource_identifier_object(serializer_class, model) + serializer = serializer_class.new(model) resource_identifier = ResourceIdentifier.new(serializer, nil) resource_identifier.as_json end - def expected_model_type - inflection = ActiveModelSerializers.config.jsonapi_resource_type - @model.class.model_name.send(inflection) + def expected_model_type(model, inflection = ActiveModelSerializers.config.jsonapi_resource_type) + with_jsonapi_inflection inflection do + model.class.model_name.send(inflection) + end end - def expected_model_id - @model.id.to_s + def expected_model_id(model) + model.id.to_s end end end From 51f2643f407b90ba7014760ca02a12137cbecc62 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 31 Oct 2017 14:48:27 -0500 Subject: [PATCH 6/6] Style --- test/adapter/json_api/type_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/adapter/json_api/type_test.rb b/test/adapter/json_api/type_test.rb index 8e7fc259..32074e0d 100644 --- a/test/adapter/json_api/type_test.rb +++ b/test/adapter/json_api/type_test.rb @@ -138,12 +138,12 @@ module ActiveModelSerializers def test_for_type_with_id id = 1 actual = ResourceIdentifier.for_type_with_id('admin_user', id, {}) - expected = { id: "1", type: 'admin-users' } + expected = { id: '1', type: 'admin-users' } assert_equal actual, expected end def test_for_type_with_id_given_blank_id - id = "" + id = '' actual = ResourceIdentifier.for_type_with_id('admin_user', id, {}) expected = { type: 'admin-users' } assert_equal actual, expected @@ -154,7 +154,7 @@ module ActiveModelSerializers actual = with_jsonapi_inflection :singular do ResourceIdentifier.for_type_with_id('admin_users', id, {}) end - expected = { id: "2", type: 'admin-user' } + expected = { id: '2', type: 'admin-user' } assert_equal actual, expected end