diff --git a/.travis.yml b/.travis.yml index dcef22c..75335d0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,16 @@ language: ruby rvm: -- 2.2.3 + - 2.2.3 before_install: gem install bundler -v 1.11.2 +script: "bundle exec rake" + +install: bundle install --retry=3 --jobs=3 + +gemfile: + - gemfiles/rails_4.gemfile + - gemfiles/rails_5.gemfile + deploy: provider: rubygems api_key: $RUBYGEMS_API_KEY diff --git a/Rakefile b/Rakefile index b7e9ed5..2f8ccc1 100644 --- a/Rakefile +++ b/Rakefile @@ -1,6 +1,11 @@ require "bundler/gem_tasks" require "rspec/core/rake_task" +require "appraisal" RSpec::Core::RakeTask.new(:spec) -task :default => :spec +if !ENV["APPRAISAL_INITIALIZED"] && !ENV["TRAVIS"] + task :default => :appraisal +else + task :default => :spec +end diff --git a/gemfiles/rails_4.gemfile.lock b/gemfiles/rails_4.gemfile.lock index bcf0738..22b5d36 100644 --- a/gemfiles/rails_4.gemfile.lock +++ b/gemfiles/rails_4.gemfile.lock @@ -1,9 +1,8 @@ PATH - remote: ../ + remote: .. specs: - jsonapi_errorable (0.1.1) - active_model_serializers (~> 0.10) - rails (>= 4.1, < 6) + jsonapi_errorable (0.7.0) + jsonapi-serializable (~> 0.1) GEM remote: https://rubygems.org/ @@ -27,11 +26,6 @@ GEM erubis (~> 2.7.0) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - active_model_serializers (0.10.2) - actionpack (>= 4.1, < 6) - activemodel (>= 4.1, < 6) - jsonapi (~> 0.1.1.beta2) - railties (>= 4.1, < 6) activejob (4.2.6) activesupport (= 4.2.6) globalid (>= 0.3.0) @@ -63,8 +57,9 @@ GEM activesupport (>= 4.1.0) i18n (0.7.0) json (1.8.3) - jsonapi (0.1.1.beta2) - json (~> 1.8) + jsonapi-renderer (0.2.0) + jsonapi-serializable (0.3.0) + jsonapi-renderer (~> 0.2.0) jsonapi_spec_helpers (0.2.0) loofah (2.0.3) nokogiri (>= 1.5.9) @@ -162,4 +157,4 @@ DEPENDENCIES sqlite3 BUNDLED WITH - 1.12.5 + 1.15.4 diff --git a/gemfiles/rails_5.gemfile.lock b/gemfiles/rails_5.gemfile.lock index 4781269..d3f581d 100644 --- a/gemfiles/rails_5.gemfile.lock +++ b/gemfiles/rails_5.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - jsonapi_errorable (0.6.2) + jsonapi_errorable (0.7.0) jsonapi-serializable (~> 0.1) GEM @@ -58,9 +58,9 @@ GEM globalid (0.3.7) activesupport (>= 4.1.0) i18n (0.7.0) - jsonapi-renderer (0.1.2) - jsonapi-serializable (0.1.3) - jsonapi-renderer (~> 0.1) + jsonapi-renderer (0.2.0) + jsonapi-serializable (0.3.0) + jsonapi-renderer (~> 0.2.0) jsonapi_spec_helpers (0.2.0) loofah (2.0.3) nokogiri (>= 1.5.9) @@ -161,4 +161,4 @@ DEPENDENCIES sqlite3 BUNDLED WITH - 1.15.0 + 1.15.4 diff --git a/jsonapi_errorable.gemspec b/jsonapi_errorable.gemspec index 518e4ab..87330a3 100644 --- a/jsonapi_errorable.gemspec +++ b/jsonapi_errorable.gemspec @@ -20,6 +20,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'jsonapi-serializable', '~> 0.1' + # Rails is added in Appraisals spec.add_development_dependency "bundler", "~> 1.11" spec.add_development_dependency "rake", "~> 10.0" spec.add_development_dependency "rspec-rails", "~> 3.0" diff --git a/lib/jsonapi_errorable/serializers/validation.rb b/lib/jsonapi_errorable/serializers/validation.rb index fb67037..ba0bbd9 100644 --- a/lib/jsonapi_errorable/serializers/validation.rb +++ b/lib/jsonapi_errorable/serializers/validation.rb @@ -3,85 +3,146 @@ module Serializers class Validation attr_reader :object - def initialize(object, relationship_params = {}, relationship_message = {}) + def initialize(object, relationship_payloads = {}, relationship_meta = {}) @object = object - @relationship_params = relationship_params || {} - @relationship_message = relationship_message + @relationship_payloads = relationship_payloads + @relationship_meta = relationship_meta end - def errors - return [] unless object.respond_to?(:errors) - - all_errors = object.errors.to_hash.map do |attribute, messages| - messages.map do |message| - meta = { attribute: attribute, message: message }.merge(@relationship_message) - meta = { relationship: meta } if @relationship_message.present? - - detail = object.errors.full_message(attribute, message) - detail = message if attribute.to_s.downcase == 'base' - - { + def attribute_errors + [].tap do |errors| + each_error do |attribute, message, code| + error = { code: 'unprocessable_entity', status: '422', title: 'Validation Error', - detail: detail, + detail: detail_for(attribute, message), source: { pointer: pointer_for(object, attribute) }, - meta: meta + meta: meta_for(attribute, message, code, @relationship_meta) } + + errors << error end - end.flatten - all_errors << relationship_errors(@relationship_params) - all_errors.flatten! - all_errors.compact! + end + end + + def errors + return [] unless object.respond_to?(:errors) + + all_errors = attribute_errors + all_errors |= relationship_errors(@relationship_payloads) all_errors end + private + + def each_error + object.errors.messages.each_pair do |attribute, messages| + details = if Rails::VERSION::MAJOR >= 5 + object.errors.details.find { |k,v| k == attribute }[1] + end + + messages.each_with_index do |message, index| + code = details[index][:error] if details + yield attribute, message, code + end + end + end + def relationship?(name) - return false unless activemodel? + relationship_names = [] + if activerecord? + relationship_names = object.class + .reflect_on_all_associations.map(&:name) + elsif object.respond_to?(:relationship_names) + relationship_names = object.relationship_names + end - relation_names = object.class.reflect_on_all_associations.map(&:name) - relation_names.include?(name) + relationship_names.include?(name) end def attribute?(name) object.respond_to?(name) end - private + def meta_for(attribute, message, code, relationship_meta) + meta = { + attribute: attribute, + message: message + } + meta.merge!(code: code) if Rails::VERSION::MAJOR >= 5 + + unless relationship_meta.empty? + meta = { + relationship: meta.merge(relationship_meta) + } + end + + meta + end + def detail_for(attribute, message) + detail = object.errors.full_message(attribute, message) + detail = message if attribute.to_s.downcase == 'base' + detail + end + + # @richmolj: Keeping this to support ember-data, but I hate the concept. def pointer_for(object, name) if relationship?(name) "/data/relationships/#{name}" elsif attribute?(name) "/data/attributes/#{name}" + elsif name == :base + nil else # Probably a nested relation, like post.comments "/data/relationships/#{name}" end end - def activemodel? + def activerecord? object.class.respond_to?(:reflect_on_all_associations) end - def relationship_errors(relationship_params) - errors = [] + def traverse_relationships(relationship_params) + return unless relationship_params + relationship_params.each_pair do |name, payload| - related = Array(@object.send(name)) - related.each do |r| + relationship_objects = Array(@object.send(name)) + + relationship_objects.each do |relationship_object| + related_payload = payload if payload.is_a?(Array) - related_payload = payload.find { |p| p[:meta][:temp_id] === r.instance_variable_get(:@_jsonapi_temp_id) || p[:meta][:id] == r.id } - else - related_payload = payload + related_payload = payload.find do |p| + temp_id = relationship_object + .instance_variable_get(:@_jsonapi_temp_id) + p[:meta][:temp_id] === temp_id || + p[:meta][:id] == relationship_object.id.to_s + end end - relationship_message = { - name: name, - id: r.id, - :'temp-id' => r.instance_variable_get(:@_jsonapi_temp_id) - } - errors << Validation.new(r, related_payload[:relationships], relationship_message).errors + yield name, relationship_object, related_payload + traverse_relationships(related_payload[:relationships]) + end + end + end + + def relationship_errors(relationship_payloads) + errors = [] + traverse_relationships(relationship_payloads) do |name, model, payload| + meta = {}.tap do |hash| + hash[:name] = name + hash[:type] = payload[:meta][:jsonapi_type] + if temp_id = model.instance_variable_get(:@_jsonapi_temp_id) + hash[:'temp-id'] = temp_id + else + hash[:id] = model.id + end end + + serializer = self.class.new(model, payload[:relationships], meta) + errors |= serializer.errors end errors end diff --git a/spec/serializers/serializable_validation_spec.rb b/spec/serializers/serializable_validation_spec.rb index c728b8d..295ead8 100644 --- a/spec/serializers/serializable_validation_spec.rb +++ b/spec/serializers/serializable_validation_spec.rb @@ -1,211 +1,314 @@ require 'spec_helper' +require 'active_model' RSpec.describe JsonapiErrorable::Serializers::Validation do - let(:attribute) { :username } - let(:message) { "can't be blank" } - let(:errors_hash) { { attribute => [message] } } + let(:klass) do + Class.new do + def self.name;'Dummy';end # required for anonymous class + # users + attr_accessor :id, :username, :pets + # pets + attr_accessor :name, :favorite_toy + # toys + attr_accessor :cost + include ActiveModel::Validations - let(:object) { double(id: 123).as_null_object } + def initialize(attrs = {}) + attrs.each_pair { |k,v| send("#{k}=", v) } + end + end + end + + let(:object) { klass.new } let(:instance) { described_class.new(object) } - before do - allow(instance).to receive(:activemodel?) { true } - allow(object.class) - .to receive(:reflect_on_all_associations) - .and_return([double(name: :pets)]) - allow(object).to receive_message_chain(:errors, :to_hash) { errors_hash } - allow(object).to receive_message_chain(:errors, :full_message).with(attribute, message) { "#{attribute.capitalize} #{message}" } + def assert_error(actual, expected) + if Rails::VERSION::MAJOR < 5 + expected[:meta].delete(:code) + + if relationship = expected[:meta][:relationship] + relationship.delete(:code) + end + end + + expect(actual).to eq(expected) + end + + def assert_errors(actual, expected) + expect(actual.length).to eq(expected.length) + actual.each_with_index do |a, index| + assert_error(a, expected[index]) + end end describe '#errors' do subject { instance.errors } - before do - allow(object).to receive(:respond_to?).with(:errors) { true } - end - context 'when the error is on an attribute' do before do - allow(object).to receive(:respond_to?).with(attribute) { true } + object.errors.add(:username, :blank, message: "can't be blank") end it 'renders valid JSONAPI error format' do - expect(subject).to eq( - [ - { - code: 'unprocessable_entity', - status: '422', - title: "Validation Error", - detail: "Username can't be blank", - source: { pointer: '/data/attributes/username' }, - meta: { - attribute: :username, - message: "can't be blank" - } + assert_errors(subject, [ + { + code: 'unprocessable_entity', + status: '422', + title: "Validation Error", + detail: "Username can't be blank", + source: { pointer: '/data/attributes/username' }, + meta: { + attribute: :username, + message: "can't be blank", + code: :blank } - ] - ) + } + ]) end + end - context 'when the error attribute is "base"' do - let(:attribute) { :base } - let(:message) { "Model is invalid" } + context 'when the error attribute is "base"' do + before do + object.errors.add(:base, :invalid, message: 'Model is invalid') + end + + it 'should not render the attribute in the message detail' do + assert_errors(subject, [{ + code: 'unprocessable_entity', + status: '422', + title: "Validation Error", + detail: "Model is invalid", + source: { pointer: nil }, + meta: { + attribute: :base, + message: "Model is invalid", + code: :invalid + } + }]) + end + end + context 'when the error is on a relationship' do + before do + klass.class_eval do + attr_accessor :pets + validates :pets, presence: true + end + object.errors.add(:pets, :invalid, message: 'is invalid') + end + + context 'and the object is activerecord' do before do - allow(object).to receive(:respond_to?).with(attribute) { true } + allow(object.class).to receive(:reflect_on_all_associations) + .and_return([double(name: :pets)]) end - it 'should not render the attribute in the message detail' do - expect(subject).to eq( - [ - { - code: 'unprocessable_entity', - status: '422', - title: "Validation Error", - detail: "Model is invalid", - source: { pointer: '/data/attributes/base' }, - meta: { - attribute: :base, - message: "Model is invalid" - } - } - ] - ) + it 'puts the source pointer on the relationship' do + assert_errors(subject, [{ + code: 'unprocessable_entity', + status: '422', + title: 'Validation Error', + detail: 'Pets is invalid', + source: { pointer: '/data/relationships/pets' }, + meta: { + attribute: :pets, + message: 'is invalid', + code: :invalid + } + }]) end end - end - context 'when the error is on a relationship' do - let(:attribute) { :pets } - let(:message) { "is invalid" } + context 'and the object is not activerecord' do + context 'but it defines #relationship_names' do + before do + klass.class_eval do + def relationship_names + [:pets] + end + end + end - it 'puts the source pointer on relationships' do - expect(subject).to eq( - [ - { + it 'puts the source pointer on the relationship' do + assert_errors(subject, [{ code: 'unprocessable_entity', status: '422', title: 'Validation Error', detail: 'Pets is invalid', source: { pointer: '/data/relationships/pets' }, - meta: { attribute: :pets, message: 'is invalid' } - } - ] - ) - end - - context 'but the object is not activerecord' do - before do - allow(instance).to receive(:activemodel?) { false } - allow(object).to receive(:respond_to?).with(attribute) { true } - end - - it 'places the error on attribute' do - expect(subject).to eq( - [ - { - code: 'unprocessable_entity', - status: '422', - title: 'Validation Error', - detail: 'Pets is invalid', - source: { pointer: '/data/attributes/pets' }, - meta: { attribute: :pets, message: 'is invalid' } + meta: { + attribute: :pets, + message: 'is invalid', + code: :invalid } - ] - ) - end - - context 'but the object does not respond to this property' do - before do - allow(object).to receive(:respond_to?).with(attribute) { false } + }]) end + end - it 'defaults to relationship' do - expect(subject).to eq( - [ - { - code: 'unprocessable_entity', - status: '422', - title: 'Validation Error', - detail: 'Pets is invalid', - source: { pointer: '/data/relationships/pets' }, - meta: { attribute: :pets, message: 'is invalid' } - } - ] - ) + # We have no way to tell this is a relationship :( + context 'and it does not define #relationship_names' do + it 'puts the source pointer on attributes' do + assert_errors(subject, [{ + code: 'unprocessable_entity', + status: '422', + title: 'Validation Error', + detail: 'Pets is invalid', + source: { pointer: '/data/attributes/pets' }, + meta: { + attribute: :pets, + message: 'is invalid', + code: :invalid + } + }]) end end end end - context 'when the error is neither a relationship or attribute of the object' do - let(:attribute) { :'foo.bar' } - let(:message) { "is invalid" } - + context 'when the object does not respond to the attribute' do before do - allow(object).to receive(:respond_to?).with(attribute) { false } + object.errors.add(:"foo.bar", 'is invalid') end - it 'puts the source pointer on relationships' do - expect(subject).to eq( - [ - { - code: 'unprocessable_entity', - status: '422', - title: 'Validation Error', - detail: 'Foo.bar is invalid', - source: { pointer: '/data/relationships/foo.bar' }, - meta: { attribute: :'foo.bar', message: 'is invalid' } - } - ] - ) + it 'is treated as a nested relationship' do + assert_errors(subject, [{ + code: 'unprocessable_entity', + status: '422', + title: 'Validation Error', + detail: 'Foo bar is invalid', + source: { pointer: '/data/relationships/foo.bar' }, + meta: { + attribute: :"foo.bar", + message: 'is invalid', + code: 'is invalid' + } + }]) end end - end - - describe '#relationship?' do - subject { instance.relationship?(:pets) } - context 'when activerecord' do - context 'and is a valid relation' do - it { is_expected.to be(true) } + context 'when the error is on a sideposted object' do + let(:relationship_params) do + { + pets: [{ + meta: { + id: '444', + jsonapi_type: 'pets' + }, + relationships: { + favorite_toy: { + meta: { + id: '555', + jsonapi_type: 'toys' + } + } + } + }] + } end - context 'but not a valid relation' do - before do - allow(object.class).to receive(:reflect_on_all_associations) { [] } - end + let(:instance) { described_class.new(object, relationship_params) } - it { is_expected.to be(false) } - end - end + let(:toy) { klass.new(id: '555') } + let(:pet) { klass.new(id: '444', favorite_toy: toy) } - context 'when not activerecord' do before do - allow(instance).to receive(:activemodel?) { false } + toy.errors.add(:cost, :too_high, message: "is too high") + pet.errors.add(:name, :blank, message: "can't be blank") + object.pets = [pet] end - it { is_expected.to be(false) } - end - end - - describe '#attribute?' do - subject { instance.attribute?(:foo) } + it 'stores the error under meta > relationship' do + assert_error(subject[0], { + code: 'unprocessable_entity', + status: '422', + title: 'Validation Error', + detail: "Name can't be blank", + source: { pointer: '/data/attributes/name' }, + meta: { + relationship: { + attribute: :name, + message: "can't be blank", + code: :blank, + name: :pets, + id: '444', + type: 'pets' + } + } + }) - context 'when object responds to name' do - before do - allow(object).to receive(:respond_to?).with(:foo) { true } + assert_error(subject[1], { + code: 'unprocessable_entity', + status: '422', + title: 'Validation Error', + detail: 'Cost is too high', + source: { pointer: '/data/attributes/cost' }, + meta: { + relationship: { + attribute: :cost, + message: 'is too high', + code: :too_high, + name: :favorite_toy, + id: '555', + type: 'toys' + } + } + }) end - it { is_expected.to be(true) } - end + context 'that is 3 levels deep, with no error at second level' do + before do + pet.errors.clear + end - context 'when object does not respond to name' do - before do - allow(object).to receive(:respond_to?).with(:foo) { false } + it 'still renders the error correctly' do + assert_errors(subject, [{ + code: 'unprocessable_entity', + status: '422', + title: 'Validation Error', + detail: 'Cost is too high', + source: { pointer: '/data/attributes/cost' }, + meta: { + relationship: { + attribute: :cost, + message: 'is too high', + code: :too_high, + name: :favorite_toy, + id: '555', + type: 'toys' + } + } + }]) + end end - it { is_expected.to be(false) } + context 'and the sideposted object has a temp id' do + before do + toy.errors.clear + pet.id = nil + pet.instance_variable_set(:@_jsonapi_temp_id, 't3mp-1d') + relationship_params[:pets][0][:meta][:temp_id] = 't3mp-1d' + end + + it 'is returned instead of id' do + assert_errors(subject, [{ + code: 'unprocessable_entity', + status: '422', + title: 'Validation Error', + detail: "Name can't be blank", + source: { pointer: '/data/attributes/name' }, + meta: { + relationship: { + attribute: :name, + message: "can't be blank", + code: :blank, + name: :pets, + :'temp-id' => 't3mp-1d', + type: 'pets' + } + } + }]) + end + end end end end