From 10b48505673080f79405a5718a655456e55efa85 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Thu, 15 Sep 2016 13:46:08 -0400 Subject: [PATCH 1/6] begin playing with using JSON API for deserialization extract key_transfor chaneg to case_transform gem fix tests pass add empty array test upgrade to jsonapi beta5, improve tests to be valid resource requests use github links in gemfile for now, due to unreleased code use github links in gemfile for now, due to unreleased code some clenaup deserialization would work now if it didn't depend on the existance of certain objects progress on implementing new deserializer -- just have some behavior differences with relationships tests pass address rubocop issue -- update dependencies... wonder if we need to move @beauby's jsonapi gems to rails-api, in order to faster turnaround time reduce to minimum dependencies. Now we need beauby to merge my jsonapi-rails branch :-) --- Gemfile | 2 + active_model_serializers.gemspec | 2 +- .../adapter/json_api/deserialization.rb | 148 ++++-------------- .../json_api/deserialization_test.rb | 112 ------------- .../json_api/transform_test.rb | 6 +- test/adapter/json_api/parse_test.rb | 137 ---------------- 6 files changed, 34 insertions(+), 373 deletions(-) delete mode 100644 test/action_controller/json_api/deserialization_test.rb delete mode 100644 test/adapter/json_api/parse_test.rb diff --git a/Gemfile b/Gemfile index e854a2048..9437eedd7 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,8 @@ eval_gemfile local_gemfile if File.readable?(local_gemfile) # Specify your gem's dependencies in active_model_serializers.gemspec gemspec +gem 'jsonapi-rails', github: 'beauby/jsonapi-rails', branch: 'initial-implementation' + version = ENV['RAILS_VERSION'] || '4.2' if version == 'master' diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index 3581ce408..fc3a6dad7 100644 --- a/active_model_serializers.gemspec +++ b/active_model_serializers.gemspec @@ -42,7 +42,7 @@ Gem::Specification.new do |spec| # 'minitest' # 'thread_safe' - spec.add_runtime_dependency 'jsonapi', '0.1.1.beta2' + spec.add_runtime_dependency 'jsonapi-deserializable', '~> 0.1.1.beta3' spec.add_runtime_dependency 'case_transform', '>= 0.2' spec.add_development_dependency 'activerecord', rails_versions diff --git a/lib/active_model_serializers/adapter/json_api/deserialization.rb b/lib/active_model_serializers/adapter/json_api/deserialization.rb index b79125ac4..a7c4caead 100644 --- a/lib/active_model_serializers/adapter/json_api/deserialization.rb +++ b/lib/active_model_serializers/adapter/json_api/deserialization.rb @@ -1,3 +1,6 @@ +require 'jsonapi/parser' +require 'jsonapi/rails' + module ActiveModelSerializers module Adapter class JsonApi @@ -5,8 +8,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, @@ -73,140 +74,47 @@ module Deserialization # # } # def parse!(document, options = {}) - parse(document, options) do |invalid_payload, reason| - fail InvalidDocument, "Invalid payload (#{reason}): #{invalid_payload}" + parse(document, options) do |exception| + fail exception 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 - - primary_data = document['data'] - attributes = primary_data['attributes'] || {} - attributes['id'] = primary_data['id'] if primary_data['id'] - relationships = primary_data['relationships'] || {} - - filter_fields(attributes, options) - filter_fields(relationships, options) - - hash = {} - hash.merge!(parse_attributes(attributes, 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 + # TODO: change to jsonapi-ralis to have default conversion to flat hashes + result = JSONAPI::Deserializable::ActiveRecord.new(document, options: options).to_hash + result = apply_options(result, options) + result + rescue JSONAPI::Parser::InvalidDocument => e + return {} unless block_given? + yield e end - # rubocop:enable Metrics/CyclomaticComplexity - - # @api private - def filter_fields(fields, options) - if (only = options[:only]) - fields.slice!(*Array(only).map(&:to_s)) - elsif (except = options[:except]) - fields.except!(*Array(except).map(&:to_s)) - end - end - - # @api private - def field_key(field, options) - (options[:keys] || {}).fetch(field.to_sym, field).to_sym - end - - # @api private - def parse_attributes(attributes, options) - transform_keys(attributes, options) - .map { |(k, v)| { field_key(k, options) => v } } - .reduce({}, :merge) - end - - # Given an association name, and a relationship data attribute, build a hash - # mapping the corresponding ActiveRecord attribute to the corresponding value. - # - # @example - # parse_relationship(:comments, [{ 'id' => '1', 'type' => 'comments' }, - # { 'id' => '2', 'type' => 'comments' }], - # {}) - # # => { :comment_ids => ['1', '2'] } - # parse_relationship(:author, { 'id' => '1', 'type' => 'users' }, {}) - # # => { :author_id => '1' } - # parse_relationship(:author, nil, {}) - # # => { :author_id => nil } - # @param [Symbol] assoc_name - # @param [Hash] assoc_data - # @param [Hash] options - # @return [Hash{Symbol, Object}] - # - # @api private - def parse_relationship(assoc_name, assoc_data, options) - prefix_key = field_key(assoc_name, options).to_s.singularize - hash = - if assoc_data.is_a?(Array) - { "#{prefix_key}_ids".to_sym => assoc_data.map { |ri| ri['id'] } } - else - { "#{prefix_key}_id".to_sym => assoc_data ? assoc_data['id'] : nil } - end - - polymorphic = (options[:polymorphic] || []).include?(assoc_name.to_sym) - if polymorphic - hash["#{prefix_key}_type".to_sym] = assoc_data.present? ? assoc_data['type'] : nil - end + def apply_options(hash, options) + hash = transform_keys(hash, options) if options[:key_transform] + hash = hash.deep_symbolize_keys + hash = rename_fields(hash, options) hash end - # @api private - def parse_relationships(relationships, options) - transform_keys(relationships, options) - .map { |(k, v)| parse_relationship(k, v['data'], options) } - .reduce({}, :merge) - end - + # TODO: transform the keys after parsing # @api private def transform_keys(hash, options) transform = options[:key_transform] || :underscore CaseTransform.send(transform, hash) end + + def rename_fields(hash, options) + return hash unless options[:keys] + + keys = options[:keys] + hash.each_with_object({}) do |(k, v), h| + k = keys.fetch(k, k) + h[k] = v + h + end + end end end end diff --git a/test/action_controller/json_api/deserialization_test.rb b/test/action_controller/json_api/deserialization_test.rb deleted file mode 100644 index 025f857b7..000000000 --- a/test/action_controller/json_api/deserialization_test.rb +++ /dev/null @@ -1,112 +0,0 @@ -require 'test_helper' - -module ActionController - module Serialization - class JsonApi - class DeserializationTest < ActionController::TestCase - class DeserializationTestController < ActionController::Base - def render_parsed_payload - parsed_hash = ActiveModelSerializers::Deserialization.jsonapi_parse(params) - render json: parsed_hash - end - - def render_polymorphic_parsed_payload - parsed_hash = ActiveModelSerializers::Deserialization.jsonapi_parse( - params, - polymorphic: [:restriction_for, :restricted_to] - ) - render json: parsed_hash - end - end - - tests DeserializationTestController - - def test_deserialization_of_relationship_only_object - hash = { - 'data' => { - 'type' => 'restraints', - 'relationships' => { - 'restriction_for' => { - 'data' => { - 'type' => 'discounts', - 'id' => '67' - } - }, - 'restricted_to' => { - 'data' => nil - } - } - }, - 'restraint' => {} - } - - post :render_polymorphic_parsed_payload, params: hash - - response = JSON.parse(@response.body) - expected = { - 'restriction_for_id' => '67', - 'restriction_for_type' => 'discounts', - 'restricted_to_id' => nil, - 'restricted_to_type' => nil - } - - assert_equal(expected, response) - end - - def test_deserialization - hash = { - 'data' => { - 'type' => 'photos', - 'id' => 'zorglub', - 'attributes' => { - 'title' => 'Ember Hamster', - 'src' => 'http://example.com/images/productivity.png', - 'image-width' => '200', - 'imageHeight' => '200', - 'ImageSize' => '1024' - }, - 'relationships' => { - 'author' => { - 'data' => nil - }, - 'photographer' => { - 'data' => { 'type' => 'people', 'id' => '9' } - }, - 'comments' => { - 'data' => [ - { 'type' => 'comments', 'id' => '1' }, - { 'type' => 'comments', 'id' => '2' } - ] - }, - 'related-images' => { - 'data' => [ - { 'type' => 'image', 'id' => '7' }, - { 'type' => 'image', 'id' => '8' } - ] - } - } - } - } - - post :render_parsed_payload, params: hash - - response = JSON.parse(@response.body) - expected = { - 'id' => 'zorglub', - 'title' => 'Ember Hamster', - 'src' => 'http://example.com/images/productivity.png', - 'image_width' => '200', - 'image_height' => '200', - 'image_size' => '1024', - 'author_id' => nil, - 'photographer_id' => '9', - 'comment_ids' => %w(1 2), - 'related_image_ids' => %w(7 8) - } - - assert_equal(expected, response) - end - end - end - end -end diff --git a/test/action_controller/json_api/transform_test.rb b/test/action_controller/json_api/transform_test.rb index 69212f324..d105eb942 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 attributes :title, :body, :publish_at associations :author, :top_comments @@ -77,7 +77,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/adapter/json_api/parse_test.rb b/test/adapter/json_api/parse_test.rb deleted file mode 100644 index bee79c8c1..000000000 --- a/test/adapter/json_api/parse_test.rb +++ /dev/null @@ -1,137 +0,0 @@ -require 'test_helper' -module ActiveModelSerializers - module Adapter - class JsonApi - module Deserialization - class ParseTest < Minitest::Test - def setup - @hash = { - 'data' => { - 'type' => 'photos', - 'id' => 'zorglub', - 'attributes' => { - 'title' => 'Ember Hamster', - 'src' => 'http://example.com/images/productivity.png' - }, - 'relationships' => { - 'author' => { - 'data' => nil - }, - 'photographer' => { - 'data' => { 'type' => 'people', 'id' => '9' } - }, - 'comments' => { - 'data' => [ - { 'type' => 'comments', 'id' => '1' }, - { 'type' => 'comments', 'id' => '2' } - ] - } - } - } - } - @params = ActionController::Parameters.new(@hash) - @expected = { - id: 'zorglub', - title: 'Ember Hamster', - src: 'http://example.com/images/productivity.png', - author_id: nil, - photographer_id: '9', - comment_ids: %w(1 2) - } - - @illformed_payloads = [nil, - {}, - { - 'data' => nil - }, { - 'data' => { 'attributes' => [] } - }, { - 'data' => { 'relationships' => [] } - }, { - 'data' => { - 'relationships' => { 'rel' => nil } - } - }, { - 'data' => { - 'relationships' => { 'rel' => {} } - } - }] - end - - def test_hash - parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(@hash) - assert_equal(@expected, parsed_hash) - end - - def test_actioncontroller_parameters - assert_equal(false, @params.permitted?) - parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(@params) - assert_equal(@expected, parsed_hash) - end - - def test_illformed_payloads_safe - @illformed_payloads.each do |p| - parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse(p) - assert_equal({}, parsed_hash) - end - end - - def test_illformed_payloads_unsafe - @illformed_payloads.each do |p| - assert_raises(InvalidDocument) do - ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(p) - end - end - end - - def test_filter_fields_only - parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(@hash, only: [:id, :title, :author]) - expected = { - id: 'zorglub', - title: 'Ember Hamster', - author_id: nil - } - assert_equal(expected, parsed_hash) - end - - def test_filter_fields_except - parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(@hash, except: [:id, :title, :author]) - expected = { - src: 'http://example.com/images/productivity.png', - photographer_id: '9', - comment_ids: %w(1 2) - } - assert_equal(expected, parsed_hash) - end - - def test_keys - parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(@hash, keys: { author: :user, title: :post_title }) - expected = { - id: 'zorglub', - post_title: 'Ember Hamster', - src: 'http://example.com/images/productivity.png', - user_id: nil, - photographer_id: '9', - comment_ids: %w(1 2) - } - assert_equal(expected, parsed_hash) - end - - def test_polymorphic - parsed_hash = ActiveModelSerializers::Adapter::JsonApi::Deserialization.parse!(@hash, polymorphic: [:photographer]) - expected = { - id: 'zorglub', - title: 'Ember Hamster', - src: 'http://example.com/images/productivity.png', - author_id: nil, - photographer_id: '9', - photographer_type: 'people', - comment_ids: %w(1 2) - } - assert_equal(expected, parsed_hash) - end - end - end - end - end -end From 90307f066a46588b36aadb06b4181dc6b3ecb342 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 9 Dec 2016 08:14:50 -0500 Subject: [PATCH 2/6] update deserializable --- Gemfile | 2 -- active_model_serializers.gemspec | 2 +- .../adapter/json_api/deserialization.rb | 3 ++- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 9437eedd7..e854a2048 100644 --- a/Gemfile +++ b/Gemfile @@ -7,8 +7,6 @@ eval_gemfile local_gemfile if File.readable?(local_gemfile) # Specify your gem's dependencies in active_model_serializers.gemspec gemspec -gem 'jsonapi-rails', github: 'beauby/jsonapi-rails', branch: 'initial-implementation' - version = ENV['RAILS_VERSION'] || '4.2' if version == 'master' diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index fc3a6dad7..a4273200c 100644 --- a/active_model_serializers.gemspec +++ b/active_model_serializers.gemspec @@ -42,7 +42,7 @@ Gem::Specification.new do |spec| # 'minitest' # 'thread_safe' - spec.add_runtime_dependency 'jsonapi-deserializable', '~> 0.1.1.beta3' + spec.add_runtime_dependency 'jsonapi-deserializable', '~> 0.1.1' spec.add_runtime_dependency 'case_transform', '>= 0.2' spec.add_development_dependency 'activerecord', rails_versions diff --git a/lib/active_model_serializers/adapter/json_api/deserialization.rb b/lib/active_model_serializers/adapter/json_api/deserialization.rb index a7c4caead..476bb9f81 100644 --- a/lib/active_model_serializers/adapter/json_api/deserialization.rb +++ b/lib/active_model_serializers/adapter/json_api/deserialization.rb @@ -83,7 +83,8 @@ def parse!(document, options = {}) # on invalid payloads. def parse(document, options = {}) # TODO: change to jsonapi-ralis to have default conversion to flat hashes - result = JSONAPI::Deserializable::ActiveRecord.new(document, options: options).to_hash + result = JSONAPI::Deserializable::Resource.call(document) + # result = JSONAPI::Deserializable::ActiveRecord.new(document, options: options).to_hash result = apply_options(result, options) result rescue JSONAPI::Parser::InvalidDocument => e From 2f9921993797a638ddd745e6a9d3cd927e7befa8 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 9 Dec 2016 08:27:08 -0500 Subject: [PATCH 3/6] need jsonapi-renderer for the include-directive --- active_model_serializers.gemspec | 1 + lib/active_model_serializers.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index a4273200c..75100ab8e 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-deserializable', '~> 0.1.1' + spec.add_runtime_dependency 'jsonapi-renderer', '~> 0.1.1' spec.add_runtime_dependency 'case_transform', '>= 0.2' spec.add_development_dependency 'activerecord', rails_versions diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index 18cdd9f70..3ad66d418 100644 --- a/lib/active_model_serializers.rb +++ b/lib/active_model_serializers.rb @@ -1,5 +1,6 @@ require 'active_model' require 'active_support' +require 'jsonapi/deserializable' require 'active_support/core_ext/object/with_options' require 'active_support/core_ext/string/inflections' require 'active_support/json' From d6e98fbeb70d8e0750ce05256da4fd0b24a524fc Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 9 Dec 2016 08:34:59 -0500 Subject: [PATCH 4/6] remove unneeded requires --- .../adapter/json_api/deserialization.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/active_model_serializers/adapter/json_api/deserialization.rb b/lib/active_model_serializers/adapter/json_api/deserialization.rb index 476bb9f81..fae3e162d 100644 --- a/lib/active_model_serializers/adapter/json_api/deserialization.rb +++ b/lib/active_model_serializers/adapter/json_api/deserialization.rb @@ -1,6 +1,3 @@ -require 'jsonapi/parser' -require 'jsonapi/rails' - module ActiveModelSerializers module Adapter class JsonApi From 34c7586048675e67dc6a63f8745c82817fba4610 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 9 Dec 2016 08:35:40 -0500 Subject: [PATCH 5/6] cleanup commented code --- .../adapter/json_api/deserialization.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/active_model_serializers/adapter/json_api/deserialization.rb b/lib/active_model_serializers/adapter/json_api/deserialization.rb index fae3e162d..fc376e94f 100644 --- a/lib/active_model_serializers/adapter/json_api/deserialization.rb +++ b/lib/active_model_serializers/adapter/json_api/deserialization.rb @@ -79,9 +79,7 @@ def parse!(document, options = {}) # Same as parse!, but returns an empty hash instead of raising InvalidDocument # on invalid payloads. def parse(document, options = {}) - # TODO: change to jsonapi-ralis to have default conversion to flat hashes result = JSONAPI::Deserializable::Resource.call(document) - # result = JSONAPI::Deserializable::ActiveRecord.new(document, options: options).to_hash result = apply_options(result, options) result rescue JSONAPI::Parser::InvalidDocument => e From 8ff403f3c0089f76bd5fb88e6a748f206d8442b7 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 9 Dec 2016 08:37:06 -0500 Subject: [PATCH 6/6] unrename key transfrom tests --- test/action_controller/json_api/transform_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/action_controller/json_api/transform_test.rb b/test/action_controller/json_api/transform_test.rb index d105eb942..69212f324 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 CaseTransformTest < ActionController::TestCase - class CaseTransformTestController < ActionController::Base + class KeyTransformTest < ActionController::TestCase + class KeyTransformTestController < ActionController::Base class Post < ::Model attributes :title, :body, :publish_at associations :author, :top_comments @@ -77,7 +77,7 @@ def render_resource_with_transform_with_global_config end end - tests CaseTransformTestController + tests KeyTransformTestController def test_render_resource_with_transform get :render_resource_with_transform