From 502b69123a57e2df0fb5848b84fa535973a4a559 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Thu, 15 Sep 2016 13:46:08 -0400 Subject: [PATCH] begin playing with using JSON API for deserialization extract key_transfor chaneg to case_transform gem fix tests pass --- Gemfile | 3 + active_model_serializers.gemspec | 1 + lib/active_model_serializers/adapter/base.rb | 4 +- .../adapter/json_api/deserialization.rb | 58 +--- lib/active_model_serializers/key_transform.rb | 74 ----- .../json_api/transform_test.rb | 6 +- .../key_transform_test.rb | 297 ------------------ test/adapter/json_api/parse_test.rb | 28 +- test/benchmark/bm_transform.rb | 10 +- 9 files changed, 38 insertions(+), 443 deletions(-) delete mode 100644 lib/active_model_serializers/key_transform.rb delete mode 100644 test/active_model_serializers/key_transform_test.rb diff --git a/Gemfile b/Gemfile index e854a2048..a8324784f 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,9 @@ eval_gemfile local_gemfile if File.readable?(local_gemfile) # Specify your gem's dependencies in active_model_serializers.gemspec gemspec +# TODO: remove when @beauby re-publishes this gem +gem 'jsonapi', github: 'beauby/jsonapi' + version = ENV['RAILS_VERSION'] || '4.2' if version == 'master' diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index d8d988d6e..3ef8260d8 100644 --- a/active_model_serializers.gemspec +++ b/active_model_serializers.gemspec @@ -43,6 +43,7 @@ Gem::Specification.new do |spec| # 'thread_safe' spec.add_runtime_dependency 'jsonapi', '~> 0.1.1.beta2' + spec.add_runtime_dependency 'case_transform', '>= 0.2' spec.add_development_dependency 'activerecord', rails_versions # arel diff --git a/lib/active_model_serializers/adapter/base.rb b/lib/active_model_serializers/adapter/base.rb index 7b60db70b..851583285 100644 --- a/lib/active_model_serializers/adapter/base.rb +++ b/lib/active_model_serializers/adapter/base.rb @@ -1,4 +1,4 @@ -require 'active_model_serializers/key_transform' +require 'case_transform' module ActiveModelSerializers module Adapter @@ -31,7 +31,7 @@ def self.transform(options) # @param options [Object] serializable resource options # @return [Symbol] the default transform for the adapter def self.transform_key_casing!(value, options) - KeyTransform.send(transform(options), value) + CaseTransform.send(transform(options), value) end def self.cache_key diff --git a/lib/active_model_serializers/adapter/json_api/deserialization.rb b/lib/active_model_serializers/adapter/json_api/deserialization.rb index 2e0e531dd..a481e9a77 100644 --- a/lib/active_model_serializers/adapter/json_api/deserialization.rb +++ b/lib/active_model_serializers/adapter/json_api/deserialization.rb @@ -1,3 +1,5 @@ +require 'jsonapi' + module ActiveModelSerializers module Adapter class JsonApi @@ -5,8 +7,6 @@ class JsonApi # This is an experimental feature. Both the interface and internals could be subject # to changes. module Deserialization - InvalidDocument = Class.new(ArgumentError) - module_function # Transform a JSON API document, containing a single data object, @@ -74,21 +74,21 @@ module Deserialization # def parse!(document, options = {}) parse(document, options) do |invalid_payload, reason| - fail InvalidDocument, "Invalid payload (#{reason}): #{invalid_payload}" + fail JSONAPI::Parser::InvalidDocument, "Invalid payload (#{reason}): #{invalid_payload}" end end # Same as parse!, but returns an empty hash instead of raising InvalidDocument # on invalid payloads. def parse(document, options = {}) - document = document.dup.permit!.to_h if document.is_a?(ActionController::Parameters) - - validate_payload(document) do |invalid_document, reason| - yield invalid_document, reason if block_given? - return {} - end + document = JSONAPI.parse(document, options) + document = document.to_hash primary_data = document['data'] + + # null data is allowed, as per the JSON API Schema + return {} unless primary_data + attributes = primary_data['attributes'] || {} attributes['id'] = primary_data['id'] if primary_data['id'] relationships = primary_data['relationships'] || {} @@ -101,43 +101,11 @@ def parse(document, options = {}) hash.merge!(parse_relationships(relationships, options)) hash - end - - # Checks whether a payload is compliant with the JSON API spec. - # - # @api private - # rubocop:disable Metrics/CyclomaticComplexity - def validate_payload(payload) - unless payload.is_a?(Hash) - yield payload, 'Expected hash' - return - end - primary_data = payload['data'] - unless primary_data.is_a?(Hash) - yield payload, { data: 'Expected hash' } - return - end - - attributes = primary_data['attributes'] || {} - unless attributes.is_a?(Hash) - yield payload, { data: { attributes: 'Expected hash or nil' } } - return - end - - relationships = primary_data['relationships'] || {} - unless relationships.is_a?(Hash) - yield payload, { data: { relationships: 'Expected hash or nil' } } - return - end - - relationships.each do |(key, value)| - unless value.is_a?(Hash) && value.key?('data') - yield payload, { data: { relationships: { key => 'Expected hash with :data key' } } } - end - end + rescue JSONAPI::Parser::InvalidDocument + return {} unless block_given? + yield end - # rubocop:enable Metrics/CyclomaticComplexity # @api private def filter_fields(fields, options) @@ -205,7 +173,7 @@ def parse_relationships(relationships, options) # @api private def transform_keys(hash, options) transform = options[:key_transform] || :underscore - KeyTransform.send(transform, hash) + CaseTransform.send(transform, hash) end end end diff --git a/lib/active_model_serializers/key_transform.rb b/lib/active_model_serializers/key_transform.rb deleted file mode 100644 index d0e648e59..000000000 --- a/lib/active_model_serializers/key_transform.rb +++ /dev/null @@ -1,74 +0,0 @@ -require 'active_support/core_ext/hash/keys' - -module ActiveModelSerializers - module KeyTransform - module_function - - # Transforms values to UpperCamelCase or PascalCase. - # - # @example: - # "some_key" => "SomeKey", - # @see {https://github.com/rails/rails/blob/master/activesupport/lib/active_support/inflector/methods.rb#L66-L76 ActiveSupport::Inflector.camelize} - def camel(value) - case value - when Array then value.map { |item| camel(item) } - when Hash then value.deep_transform_keys! { |key| camel(key) } - when Symbol then camel(value.to_s).to_sym - when String then value.underscore.camelize - else value - end - end - - # Transforms values to camelCase. - # - # @example: - # "some_key" => "someKey", - # @see {https://github.com/rails/rails/blob/master/activesupport/lib/active_support/inflector/methods.rb#L66-L76 ActiveSupport::Inflector.camelize} - def camel_lower(value) - case value - when Array then value.map { |item| camel_lower(item) } - when Hash then value.deep_transform_keys! { |key| camel_lower(key) } - when Symbol then camel_lower(value.to_s).to_sym - when String then value.underscore.camelize(:lower) - else value - end - end - - # Transforms values to dashed-case. - # This is the default case for the JsonApi adapter. - # - # @example: - # "some_key" => "some-key", - # @see {https://github.com/rails/rails/blob/master/activesupport/lib/active_support/inflector/methods.rb#L185-L187 ActiveSupport::Inflector.dasherize} - def dash(value) - case value - when Array then value.map { |item| dash(item) } - when Hash then value.deep_transform_keys! { |key| dash(key) } - when Symbol then dash(value.to_s).to_sym - when String then value.underscore.dasherize - else value - end - end - - # Transforms values to underscore_case. - # This is the default case for deserialization in the JsonApi adapter. - # - # @example: - # "some-key" => "some_key", - # @see {https://github.com/rails/rails/blob/master/activesupport/lib/active_support/inflector/methods.rb#L89-L98 ActiveSupport::Inflector.underscore} - def underscore(value) - case value - when Array then value.map { |item| underscore(item) } - when Hash then value.deep_transform_keys! { |key| underscore(key) } - when Symbol then underscore(value.to_s).to_sym - when String then value.underscore - else value - end - end - - # Returns the value unaltered - def unaltered(value) - value - end - end -end diff --git a/test/action_controller/json_api/transform_test.rb b/test/action_controller/json_api/transform_test.rb index 492b606be..685a76bb5 100644 --- a/test/action_controller/json_api/transform_test.rb +++ b/test/action_controller/json_api/transform_test.rb @@ -3,8 +3,8 @@ module ActionController module Serialization class JsonApi - class KeyTransformTest < ActionController::TestCase - class KeyTransformTestController < ActionController::Base + class CaseTransformTest < ActionController::TestCase + class CaseTransformTestController < ActionController::Base class Post < ::Model; end class Author < ::Model; end class TopComment < ::Model; end @@ -69,7 +69,7 @@ def render_resource_with_transform_with_global_config end end - tests KeyTransformTestController + tests CaseTransformTestController def test_render_resource_with_transform get :render_resource_with_transform diff --git a/test/active_model_serializers/key_transform_test.rb b/test/active_model_serializers/key_transform_test.rb deleted file mode 100644 index b4ff4d311..000000000 --- a/test/active_model_serializers/key_transform_test.rb +++ /dev/null @@ -1,297 +0,0 @@ -require 'test_helper' - -module ActiveModelSerializers - class KeyTransformTest < ActiveSupport::TestCase - def test_camel - obj = Object.new - scenarios = [ - { - value: { :"some-key" => 'value' }, - expected: { SomeKey: 'value' } - }, - { - value: { someKey: 'value' }, - expected: { SomeKey: 'value' } - }, - { - value: { some_key: 'value' }, - expected: { SomeKey: 'value' } - }, - { - value: { 'some-key' => 'value' }, - expected: { 'SomeKey' => 'value' } - }, - { - value: { 'someKey' => 'value' }, - expected: { 'SomeKey' => 'value' } - }, - { - value: { 'some_key' => 'value' }, - expected: { 'SomeKey' => 'value' } - }, - { - value: :"some-value", - expected: :SomeValue - }, - { - value: :some_value, - expected: :SomeValue - }, - { - value: :someValue, - expected: :SomeValue - }, - { - value: 'some-value', - expected: 'SomeValue' - }, - { - value: 'someValue', - expected: 'SomeValue' - }, - { - value: 'some_value', - expected: 'SomeValue' - }, - { - value: obj, - expected: obj - }, - { - value: nil, - expected: nil - }, - { - value: [ - { some_value: 'value' } - ], - expected: [ - { SomeValue: 'value' } - ] - } - ] - scenarios.each do |s| - result = ActiveModelSerializers::KeyTransform.camel(s[:value]) - assert_equal s[:expected], result - end - end - - def test_camel_lower - obj = Object.new - scenarios = [ - { - value: { :"some-key" => 'value' }, - expected: { someKey: 'value' } - }, - { - value: { SomeKey: 'value' }, - expected: { someKey: 'value' } - }, - { - value: { some_key: 'value' }, - expected: { someKey: 'value' } - }, - { - value: { 'some-key' => 'value' }, - expected: { 'someKey' => 'value' } - }, - { - value: { 'SomeKey' => 'value' }, - expected: { 'someKey' => 'value' } - }, - { - value: { 'some_key' => 'value' }, - expected: { 'someKey' => 'value' } - }, - { - value: :"some-value", - expected: :someValue - }, - { - value: :SomeValue, - expected: :someValue - }, - { - value: :some_value, - expected: :someValue - }, - { - value: 'some-value', - expected: 'someValue' - }, - { - value: 'SomeValue', - expected: 'someValue' - }, - { - value: 'some_value', - expected: 'someValue' - }, - { - value: obj, - expected: obj - }, - { - value: nil, - expected: nil - }, - { - value: [ - { some_value: 'value' } - ], - expected: [ - { someValue: 'value' } - ] - } - ] - scenarios.each do |s| - result = ActiveModelSerializers::KeyTransform.camel_lower(s[:value]) - assert_equal s[:expected], result - end - end - - def test_dash - obj = Object.new - scenarios = [ - { - value: { some_key: 'value' }, - expected: { :"some-key" => 'value' } - }, - { - value: { 'some_key' => 'value' }, - expected: { 'some-key' => 'value' } - }, - { - value: { SomeKey: 'value' }, - expected: { :"some-key" => 'value' } - }, - { - value: { 'SomeKey' => 'value' }, - expected: { 'some-key' => 'value' } - }, - { - value: { someKey: 'value' }, - expected: { :"some-key" => 'value' } - }, - { - value: { 'someKey' => 'value' }, - expected: { 'some-key' => 'value' } - }, - { - value: :some_value, - expected: :"some-value" - }, - { - value: :SomeValue, - expected: :"some-value" - }, - { - value: 'SomeValue', - expected: 'some-value' - }, - { - value: :someValue, - expected: :"some-value" - }, - { - value: 'someValue', - expected: 'some-value' - }, - { - value: obj, - expected: obj - }, - { - value: nil, - expected: nil - }, - { - value: [ - { 'some_value' => 'value' } - ], - expected: [ - { 'some-value' => 'value' } - ] - } - ] - scenarios.each do |s| - result = ActiveModelSerializers::KeyTransform.dash(s[:value]) - assert_equal s[:expected], result - end - end - - def test_underscore - obj = Object.new - scenarios = [ - { - value: { :"some-key" => 'value' }, - expected: { some_key: 'value' } - }, - { - value: { 'some-key' => 'value' }, - expected: { 'some_key' => 'value' } - }, - { - value: { SomeKey: 'value' }, - expected: { some_key: 'value' } - }, - { - value: { 'SomeKey' => 'value' }, - expected: { 'some_key' => 'value' } - }, - { - value: { someKey: 'value' }, - expected: { some_key: 'value' } - }, - { - value: { 'someKey' => 'value' }, - expected: { 'some_key' => 'value' } - }, - { - value: :"some-value", - expected: :some_value - }, - { - value: :SomeValue, - expected: :some_value - }, - { - value: :someValue, - expected: :some_value - }, - { - value: 'some-value', - expected: 'some_value' - }, - { - value: 'SomeValue', - expected: 'some_value' - }, - { - value: 'someValue', - expected: 'some_value' - }, - { - value: obj, - expected: obj - }, - { - value: nil, - expected: nil - }, - { - value: [ - { 'some-value' => 'value' } - ], - expected: [ - { 'some_value' => 'value' } - ] - } - ] - scenarios.each do |s| - result = ActiveModelSerializers::KeyTransform.underscore(s[:value]) - assert_equal s[:expected], result - end - end - end -end diff --git a/test/adapter/json_api/parse_test.rb b/test/adapter/json_api/parse_test.rb index bee79c8c1..807b7478e 100644 --- a/test/adapter/json_api/parse_test.rb +++ b/test/adapter/json_api/parse_test.rb @@ -3,7 +3,7 @@ module ActiveModelSerializers module Adapter class JsonApi module Deserialization - class ParseTest < Minitest::Test + class ParseTest < ActiveSupport::TestCase def setup @hash = { 'data' => { @@ -41,21 +41,10 @@ def setup @illformed_payloads = [nil, {}, - { - 'data' => nil - }, { - 'data' => { 'attributes' => [] } - }, { - 'data' => { 'relationships' => [] } - }, { - 'data' => { - 'relationships' => { 'rel' => nil } - } - }, { - 'data' => { - 'relationships' => { 'rel' => {} } - } - }] + { 'data' => { 'attributes' => [] } }, + { 'data' => { 'relationships' => [] } }, + { 'data' => { 'relationships' => { 'rel' => nil } } }, + { 'data' => { 'relationships' => { 'rel' => {} } } }] end def test_hash @@ -76,9 +65,14 @@ def test_illformed_payloads_safe end end + test 'null data is allow per the JSON API Schema' do + parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!('data' => nil) + assert_equal({}, parsed_hash) + end + def test_illformed_payloads_unsafe @illformed_payloads.each do |p| - assert_raises(InvalidDocument) do + assert_raises(JSONAPI::Parser::InvalidDocument) do ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(p) end end diff --git a/test/benchmark/bm_transform.rb b/test/benchmark/bm_transform.rb index 8af5298e9..97c655c01 100644 --- a/test/benchmark/bm_transform.rb +++ b/test/benchmark/bm_transform.rb @@ -25,21 +25,21 @@ serialization = adapter.as_json Benchmark.ams('camel', time: time, disable_gc: disable_gc) do - ActiveModelSerializers::KeyTransform.camel(serialization) + CaseTransform.camel(serialization) end Benchmark.ams('camel_lower', time: time, disable_gc: disable_gc) do - ActiveModelSerializers::KeyTransform.camel_lower(serialization) + CaseTransform.camel_lower(serialization) end Benchmark.ams('dash', time: time, disable_gc: disable_gc) do - ActiveModelSerializers::KeyTransform.dash(serialization) + CaseTransform.dash(serialization) end Benchmark.ams('unaltered', time: time, disable_gc: disable_gc) do - ActiveModelSerializers::KeyTransform.unaltered(serialization) + CaseTransform.unaltered(serialization) end Benchmark.ams('underscore', time: time, disable_gc: disable_gc) do - ActiveModelSerializers::KeyTransform.underscore(serialization) + CaseTransform.underscore(serialization) end