From 975793f72442553733ab653245fd57b5846631c8 Mon Sep 17 00:00:00 2001 From: Joe Woodward Date: Sat, 1 Sep 2018 18:24:06 +0700 Subject: [PATCH 1/3] Simplify jsonapi_class configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds • JSONAPI::Rails::SerializableClassMapping class Overriding Hash’s lookup can be confusing without creating an descendent class. - Old behavior inferrer.class == Hash Doesn’t make it obvious that there’s custom behavior - New behavior inferrer.class == JSONAPI::Rails::SerializableClassMapping Now it’s obvious where to look for the unusual behavior This setup also allows us to define the default mappings and the lookup behavior in separate configuration options • configuration options for 1. jsonapi_class_mapper 2. jsonapi_class_mappings 3. jsonapi_errors_class_mapper (fallback to jsonapi_class_mapper if nil) 4. jsonapi_errors_class_mappings Removes • configration options for 1. jsonapi_class 2. jsonapi_errors_class --- .../initializer/templates/initializer.rb | 36 ++++++++++++------- lib/jsonapi/rails/configuration.rb | 27 +++++++------- lib/jsonapi/rails/controller/hooks.rb | 12 +++++-- .../rails/serializable_class_mapping.rb | 13 +++++++ 4 files changed, 61 insertions(+), 27 deletions(-) create mode 100644 lib/jsonapi/rails/serializable_class_mapping.rb diff --git a/lib/generators/jsonapi/initializer/templates/initializer.rb b/lib/generators/jsonapi/initializer/templates/initializer.rb index 36a9774..13d444e 100644 --- a/lib/generators/jsonapi/initializer/templates/initializer.rb +++ b/lib/generators/jsonapi/initializer/templates/initializer.rb @@ -1,19 +1,31 @@ JSONAPI::Rails.configure do |config| - # # Set a default serializable class mapping. - # config.jsonapi_class = Hash.new { |h, k| - # names = k.to_s.split('::') + # # Set a default serializable class mapper + # # Can be a Proc or any object that responds to #call(class_name) + # # e.g. MyCustomMapper.call('User::Article') => User::SerializableArticle + # config.jsonapi_class_mapper = -> (class_name) do + # names = class_name.to_s.split('::') # klass = names.pop - # h[k] = [*names, "Serializable#{klass}"].join('::').safe_constantize + # [*names, "Serializable#{klass}"].join('::').safe_constantize + # end + # + # # Set any default serializable class mappings + # config.jsonapi_class_mappings = { + # :Car => SerializableVehicle, + # :Boat => SerializableVehicle # } # - # # Set a default serializable class mapping for errors. - # config.jsonapi_errors_class = Hash.new { |h, k| - # names = k.to_s.split('::') - # klass = names.pop - # h[k] = [*names, "Serializable#{klass}"].join('::').safe_constantize - # }.tap { |h| - # h[:'ActiveModel::Errors'] = JSONAPI::Rails::SerializableActiveModelErrors - # h[:Hash] = JSONAPI::Rails::SerializableErrorHash + # # Set a default serializable class mapper for errors. + # # Can be a Proc or any object that responds to #call(class_name) + # # e.g. MyCustomMapper.call('PORO::Error') => PORO::SerializableError + # # If no jsonapi_errors_class_mapper is configured jsonapi_class_mapper will + # # be used + # config.jsonapi_errors_class_mapper = config.jsonapi_class_mapper.dup + # + # # Set any default serializable class errors mappings + # config.jsonapi_errors_class_mappings = { + # :'MyCustomModule::ErrorObject' => MyCustomModule::SerializableErrorObject, + # :'ActiveModel::Errors' => JSONAPI::Rails::SerializableActiveModelErrors, + # :Hash => JSONAPI::Rails::SerializableErrorHash # } # # # Set a default JSON API object. diff --git a/lib/jsonapi/rails/configuration.rb b/lib/jsonapi/rails/configuration.rb index 7a7f76f..6e5b8bb 100644 --- a/lib/jsonapi/rails/configuration.rb +++ b/lib/jsonapi/rails/configuration.rb @@ -7,22 +7,21 @@ class Configuration < ActiveSupport::InheritableOptions; end # @private module Configurable - DEFAULT_JSONAPI_CLASS = Hash.new do |h, k| - names = k.to_s.split('::') + DEFAULT_JSONAPI_CLASS_MAPPER = -> (class_name) do + names = class_name.to_s.split('::') klass = names.pop - h[k] = [*names, "Serializable#{klass}"].join('::').safe_constantize - end.freeze + [*names, "Serializable#{klass}"].join('::').safe_constantize + end - DEFAULT_JSONAPI_ERRORS_CLASS = DEFAULT_JSONAPI_CLASS.dup.merge!( - 'ActiveModel::Errors'.to_sym => - JSONAPI::Rails::SerializableActiveModelErrors, - 'Hash'.to_sym => JSONAPI::Rails::SerializableErrorHash - ).freeze + DEFAULT_JSONAPI_CLASS_MAPPINGS = {}.freeze - DEFAULT_JSONAPI_OBJECT = { - version: '1.0' + DEFAULT_JSONAPI_ERROR_CLASS_MAPPINGS = { + :'ActiveModel::Errors' => JSONAPI::Rails::SerializableActiveModelErrors, + :Hash => JSONAPI::Rails::SerializableErrorHash }.freeze + DEFAULT_JSONAPI_OBJECT = { version: '1.0' }.freeze + DEFAULT_JSONAPI_CACHE = ->() { nil } DEFAULT_JSONAPI_EXPOSE = lambda { @@ -42,8 +41,10 @@ module Configurable DEFAULT_LOGGER = Logger.new(STDERR) DEFAULT_CONFIG = { - jsonapi_class: DEFAULT_JSONAPI_CLASS, - jsonapi_errors_class: DEFAULT_JSONAPI_ERRORS_CLASS, + jsonapi_class_mapper: DEFAULT_JSONAPI_CLASS_MAPPER, + jsonapi_class_mappings: DEFAULT_JSONAPI_CLASS_MAPPINGS, + jsonapi_errors_class_mapper: nil, + jsonapi_errors_class_mappings: DEFAULT_JSONAPI_ERROR_CLASS_MAPPINGS, jsonapi_cache: DEFAULT_JSONAPI_CACHE, jsonapi_expose: DEFAULT_JSONAPI_EXPOSE, jsonapi_fields: DEFAULT_JSONAPI_FIELDS, diff --git a/lib/jsonapi/rails/controller/hooks.rb b/lib/jsonapi/rails/controller/hooks.rb index d18a6c6..32243d5 100644 --- a/lib/jsonapi/rails/controller/hooks.rb +++ b/lib/jsonapi/rails/controller/hooks.rb @@ -1,4 +1,5 @@ require 'jsonapi/rails/configuration' +require 'jsonapi/rails/serializable_class_mapping' module JSONAPI module Rails @@ -11,14 +12,21 @@ module Hooks # Overridden by the `class` renderer option. # @return [Hash{Symbol=>Class}] def jsonapi_class - JSONAPI::Rails.config[:jsonapi_class].dup + JSONAPI::Rails::SerializableClassMapping.new( + JSONAPI::Rails.config[:jsonapi_class_mapper], + JSONAPI::Rails.config[:jsonapi_class_mappings].dup + ) end # Hook for serializable class mapping (for errors). # Overridden by the `class` renderer option. # @return [Hash{Symbol=>Class}] def jsonapi_errors_class - JSONAPI::Rails.config[:jsonapi_errors_class].dup + mapper = JSONAPI::Rails.config[:jsonapi_errors_class_mapper] || + JSONAPI::Rails.config[:jsonapi_class_mapper] + JSONAPI::Rails::SerializableClassMapping.new( + mapper, JSONAPI::Rails.config[:jsonapi_errors_class_mappings].dup + ) end # Hook for the jsonapi object. diff --git a/lib/jsonapi/rails/serializable_class_mapping.rb b/lib/jsonapi/rails/serializable_class_mapping.rb new file mode 100644 index 0000000..70b2941 --- /dev/null +++ b/lib/jsonapi/rails/serializable_class_mapping.rb @@ -0,0 +1,13 @@ +module JSONAPI + module Rails + # @private + class SerializableClassMapping < Hash + def initialize(mapper, default_mappings = {}) + super() do |hash, class_name_sym| + hash[class_name_sym] = + mapper.call(class_name_sym.to_s) + end.merge!(default_mappings) + end + end + end +end From 0d1cddf1c1e9464273b22fdc57a22e9dca96577b Mon Sep 17 00:00:00 2001 From: Joe Woodward Date: Sun, 2 Sep 2018 13:16:45 +0700 Subject: [PATCH 2/3] Fix test suite and test against multiple ruby/rails versions --- .travis.yml | 23 +++++++++++++++---- Gemfile | 12 ++++++++++ .../initializers/new_framework_defaults.rb | 4 +++- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index aa9bb36..9be5387 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,22 +6,35 @@ env: global: - CC_TEST_REPORTER_ID=98c9b3070ea9ac0e8f7afb6570f181506c3a06372b1db5c7deb8e46089fdf132 - GIT_COMMITTED_AT=$(if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then git log -1 --pretty=format:%ct; else git log -1 --skip 1 --pretty=format:%ct; fi) + matrix: + - RAILS_VERSION=5.0.0 + - RAILS_VERSION=5.1.0 + - RAILS_VERSION=5.2.0 + - RAILS_VERSION=master rvm: - - 2.2.2 - - 2.3.3 + - 2.2.10 + - 2.3.7 + - 2.4.4 + - 2.5.1 - ruby-head matrix: allow_failures: - rvm: ruby-head + - env: RAILS_VERSION=master + exclude: + - rvm: 2.2.10 + env: RAILS_VERSION=master + - rvm: 2.3.7 + env: RAILS_VERSION=master before_script: - curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter - chmod +x ./cc-test-reporter - ./cc-test-reporter before-build after_script: - # Preferably you will run test-reporter on branch update events. But - # if you setup travis to build PR updates only, you don't need to run + # Preferably you will run test-reporter on branch update events. But + # if you setup travis to build PR updates only, you don't need to run # the line below - if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT; fi - # In the case where travis is setup to build PR updates only, + # In the case where travis is setup to build PR updates only, # uncomment the line below # - ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT diff --git a/Gemfile b/Gemfile index fa75df1..04a417a 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,15 @@ source 'https://rubygems.org' +rails_version = ENV['RAILS_VERSION'] || "default" +rails = case rails_version + when 'master' + { github: 'rails/rails' } + when 'default' + '>= 5.0' + else + "~> #{ENV['RAILS_VERSION']}" + end + +gem 'rails', rails + gemspec diff --git a/spec/dummy/config/initializers/new_framework_defaults.rb b/spec/dummy/config/initializers/new_framework_defaults.rb index 0706caf..b6c8df7 100644 --- a/spec/dummy/config/initializers/new_framework_defaults.rb +++ b/spec/dummy/config/initializers/new_framework_defaults.rb @@ -18,7 +18,9 @@ Rails.application.config.active_record.belongs_to_required_by_default = true # Do not halt callback chains when a callback returns false. Previous versions had true. -ActiveSupport.halt_callback_chains_on_return_false = false +if Rails.version.to_f < 5.2 + ActiveSupport.halt_callback_chains_on_return_false = false +end # Configure SSL options to enable HSTS with subdomains. Previous versions had false. Rails.application.config.ssl_options = { hsts: { subdomains: true } } From dfaf98a6958e11b3af673a2cc3bae60c21aa9ede Mon Sep 17 00:00:00 2001 From: Joe Woodward Date: Mon, 17 Sep 2018 13:56:45 +0700 Subject: [PATCH 3/3] Render error when unable to deserialize resource --- .../initializer/templates/initializer.rb | 14 ++++++ lib/jsonapi/rails/configuration.rb | 13 +++++- .../rails/controller/deserialization.rb | 1 + lib/jsonapi/rails/controller/hooks.rb | 6 +++ spec/deserialization_spec.rb | 43 +++++++++++++++++++ .../initializers/new_framework_defaults.rb | 4 +- 6 files changed, 79 insertions(+), 2 deletions(-) diff --git a/lib/generators/jsonapi/initializer/templates/initializer.rb b/lib/generators/jsonapi/initializer/templates/initializer.rb index 36a9774..d9c335c 100644 --- a/lib/generators/jsonapi/initializer/templates/initializer.rb +++ b/lib/generators/jsonapi/initializer/templates/initializer.rb @@ -69,6 +69,20 @@ # # Set a default pagination scheme. # config.jsonapi_pagination = ->(_) { {} } # + # # Set the default action when the payload cannot be deserialized + # config.jsonapi_payload_malformed = -> { + # render jsonapi_errors: { + # title: 'Non-compliant Request Body', + # detail: 'The request was not formatted in compliance with the application/vnd.api+json spec', + # links: { + # about: 'http://jsonapi.org/format/' + # } + # }, status: :bad_request + # } + # + # # Uncomment to take no action when the payload cannot be deserialized + # config.jsonapi_payload_malformed = nil + # # # Set a logger. # config.logger = Logger.new(STDOUT) # diff --git a/lib/jsonapi/rails/configuration.rb b/lib/jsonapi/rails/configuration.rb index 7a7f76f..57f1332 100644 --- a/lib/jsonapi/rails/configuration.rb +++ b/lib/jsonapi/rails/configuration.rb @@ -39,12 +39,22 @@ module Configurable DEFAULT_JSONAPI_PAGINATION = ->(_) { {} } + DEFAULT_JSONAPI_PAYLOAD_MALFORMED = -> { + render jsonapi_errors: { + title: 'Non-compliant Request Body', + detail: 'The request was not formatted in compliance with the application/vnd.api+json spec', + links: { + about: 'http://jsonapi.org/format/' + } + }, status: :bad_request + } + DEFAULT_LOGGER = Logger.new(STDERR) DEFAULT_CONFIG = { + jsonapi_cache: DEFAULT_JSONAPI_CACHE, jsonapi_class: DEFAULT_JSONAPI_CLASS, jsonapi_errors_class: DEFAULT_JSONAPI_ERRORS_CLASS, - jsonapi_cache: DEFAULT_JSONAPI_CACHE, jsonapi_expose: DEFAULT_JSONAPI_EXPOSE, jsonapi_fields: DEFAULT_JSONAPI_FIELDS, jsonapi_include: DEFAULT_JSONAPI_INCLUDE, @@ -52,6 +62,7 @@ module Configurable jsonapi_meta: DEFAULT_JSONAPI_META, jsonapi_object: DEFAULT_JSONAPI_OBJECT, jsonapi_pagination: DEFAULT_JSONAPI_PAGINATION, + jsonapi_payload_malformed: DEFAULT_JSONAPI_PAYLOAD_MALFORMED, logger: DEFAULT_LOGGER }.freeze diff --git a/lib/jsonapi/rails/controller/deserialization.rb b/lib/jsonapi/rails/controller/deserialization.rb index c89934e..33185c1 100644 --- a/lib/jsonapi/rails/controller/deserialization.rb +++ b/lib/jsonapi/rails/controller/deserialization.rb @@ -55,6 +55,7 @@ def deserializable_resource(key, options = {}, &block) "Unable to deserialize #{key} because no JSON API payload was" \ " found. (#{controller.controller_name}##{params[:action]})" end + controller.jsonapi_payload_malformed next end diff --git a/lib/jsonapi/rails/controller/hooks.rb b/lib/jsonapi/rails/controller/hooks.rb index d18a6c6..49370e3 100644 --- a/lib/jsonapi/rails/controller/hooks.rb +++ b/lib/jsonapi/rails/controller/hooks.rb @@ -69,6 +69,12 @@ def jsonapi_meta def jsonapi_pagination(resources) instance_exec(resources, &JSONAPI::Rails.config[:jsonapi_pagination]) end + + # Hook for rendering jsonapi_errors when no payload passed + def jsonapi_payload_malformed + return unless JSONAPI::Rails.config[:jsonapi_payload_malformed] + instance_exec(&JSONAPI::Rails.config[:jsonapi_payload_malformed]) + end end end end diff --git a/spec/deserialization_spec.rb b/spec/deserialization_spec.rb index f7a0454..90143d0 100644 --- a/spec/deserialization_spec.rb +++ b/spec/deserialization_spec.rb @@ -93,4 +93,47 @@ def create expect(controller.jsonapi_pointers).to eq(expected) end end + + context 'when unable to deserialize resource from params' do + controller do + deserializable_resource :user + + def create + render plain: :ok + end + end + + context 'with the default config' do + it 'makes the deserialized resource available in params' do + post :create + + expect(response.body).to eq( + { + :errors => [ + { + :links => { + :about => "http://jsonapi.org/format/" + }, + :title => "Non-compliant Request Body", + :detail => "The request was not formatted in compliance with the application/vnd.api+json spec" + } + ], + :jsonapi => { + :version=>"1.0" + } + }.to_json + ) + expect(response).to be_bad_request + end + end + + context 'when the config[:jsonapi_payload_malformed] == nil' do + it 'makes the deserialization mapping available via #jsonapi_pointers' do + with_config(jsonapi_payload_malformed: nil) { post :create } + + expect(response.body).to eq('ok') + expect(response).to be_success + end + end + end end diff --git a/spec/dummy/config/initializers/new_framework_defaults.rb b/spec/dummy/config/initializers/new_framework_defaults.rb index 0706caf..b6c8df7 100644 --- a/spec/dummy/config/initializers/new_framework_defaults.rb +++ b/spec/dummy/config/initializers/new_framework_defaults.rb @@ -18,7 +18,9 @@ Rails.application.config.active_record.belongs_to_required_by_default = true # Do not halt callback chains when a callback returns false. Previous versions had true. -ActiveSupport.halt_callback_chains_on_return_false = false +if Rails.version.to_f < 5.2 + ActiveSupport.halt_callback_chains_on_return_false = false +end # Configure SSL options to enable HSTS with subdomains. Previous versions had false. Rails.application.config.ssl_options = { hsts: { subdomains: true } }