From 6e7d7b10f1b28e279c42940ad2abcca50269b6a0 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Fri, 18 Oct 2019 06:14:58 -0700 Subject: [PATCH 01/15] WIP Validate request structure when writing --- lib/graphiti/railtie.rb | 10 +- lib/graphiti/request_validator.rb | 169 +++++++++++++-------- lib/graphiti/resource.rb | 2 +- lib/graphiti/resource_proxy.rb | 87 ++++++----- spec/integration/rails/persistence_spec.rb | 24 ++- 5 files changed, 186 insertions(+), 106 deletions(-) diff --git a/lib/graphiti/railtie.rb b/lib/graphiti/railtie.rb index 347a11c5..c7c96712 100644 --- a/lib/graphiti/railtie.rb +++ b/lib/graphiti/railtie.rb @@ -79,8 +79,14 @@ def register_renderers ::ActionController::Renderers.add(:jsonapi_errors) do |proxy, options| self.content_type ||= Mime[:jsonapi] - validation = GraphitiErrors::Validation::Serializer.new \ - proxy.data, proxy.payload.relationships + if proxy.errors.kind_of?(Graphiti::Util::SimpleErrors) + validation = proxy + else + validation = GraphitiErrors::Validation::Serializer.new( + proxy.data, + proxy.payload.relationships + ) + end render \ json: {errors: validation.errors}, diff --git a/lib/graphiti/request_validator.rb b/lib/graphiti/request_validator.rb index 4f318738..e6b65e9c 100644 --- a/lib/graphiti/request_validator.rb +++ b/lib/graphiti/request_validator.rb @@ -1,94 +1,139 @@ module Graphiti class RequestValidator - attr_reader :errors + delegate :vaildate, + :validate!, + :errors, + :deserialized_payload, + to: :@validator def initialize(root_resource, raw_params) - @root_resource = root_resource - @raw_params = raw_params - @errors = Graphiti::Util::SimpleErrors.new(raw_params) + @validator = ValidatorFactory.create(root_resource, raw_params) end - def validate - resource = @root_resource - if (meta_type = deserialized_payload.meta[:type].try(:to_sym)) - if @root_resource.type != meta_type && @root_resource.polymorphic? - resource = @root_resource.class.resource_for_type(meta_type).new - end + class ValidatorFactory + def self.create(root_resource, raw_params) + case raw_params["action"] + when "update" then + UpdateValidator + else + Validator + end.new(root_resource, raw_params) end - - typecast_attributes(resource, deserialized_payload.attributes, deserialized_payload.meta[:payload_path]) - process_relationships(resource, deserialized_payload.relationships, deserialized_payload.meta[:payload_path]) - - errors.blank? end - def validate! - unless validate - raise Graphiti::Errors::InvalidRequest, self.errors + class Validator + attr_reader :root_resource, :errors + def initialize(root_resource, raw_params) + @root_resource = root_resource + @raw_params = raw_params + @errors = Graphiti::Util::SimpleErrors.new(raw_params) end - true - end + # FIXME: set resource in initializer and use the attr_reader instead of passing it around + def validate + resource = @root_resource + if (meta_type = deserialized_payload.meta[:type].try(:to_sym)) + if @root_resource.type != meta_type && @root_resource.polymorphic? + resource = @root_resource.class.resource_for_type(meta_type).new + end + end - def deserialized_payload - @deserialized_payload ||= begin - payload = normalized_params - if payload[:data] && payload[:data][:type] - Graphiti::Deserializer.new(payload) - else - Graphiti::Deserializer.new({}) + typecast_attributes(resource, deserialized_payload.attributes, deserialized_payload.meta[:payload_path]) + process_relationships(resource, deserialized_payload.relationships, deserialized_payload.meta[:payload_path]) + + errors.blank? + end + + def validate! + unless validate + raise Graphiti::Errors::InvalidRequest, self.errors end + + true end - end - private + def deserialized_payload + @deserialized_payload ||= begin + payload = normalized_params + if payload[:data] && payload[:data][:type] + Graphiti::Deserializer.new(payload) + else + Graphiti::Deserializer.new({}) + end + end + end + + private - def process_relationships(resource, relationships, payload_path) - opts = { - resource: resource, - relationships: relationships, - } + def process_relationships(resource, relationships, payload_path) + opts = { + resource: resource, + relationships: relationships, + } - Graphiti::Util::RelationshipPayload.iterate(opts) do |x| - sideload_def = x[:sideload] + Graphiti::Util::RelationshipPayload.iterate(opts) do |x| + sideload_def = x[:sideload] - unless sideload_def.writable? - full_key = fully_qualified_key(sideload_def.name, payload_path, :relationships) - unless @errors.added?(full_key, :unwritable_relationship) - @errors.add(full_key, :unwritable_relationship) + unless sideload_def.writable? + full_key = fully_qualified_key(sideload_def.name, payload_path, :relationships) + unless @errors.added?(full_key, :unwritable_relationship) + @errors.add(full_key, :unwritable_relationship) + end + next end - next + + typecast_attributes(x[:resource], x[:attributes], x[:meta][:payload_path]) + process_relationships(x[:resource], x[:relationships], x[:meta][:payload_path]) end + end - typecast_attributes(x[:resource], x[:attributes], x[:meta][:payload_path]) - process_relationships(x[:resource], x[:relationships], x[:meta][:payload_path]) + def typecast_attributes(resource, attributes, payload_path) + attributes.each_pair do |key, value| + begin + attributes[key] = resource.typecast(key, value, :writable) + rescue Graphiti::Errors::UnknownAttribute + resource.typecast(key, value, :writable) + @errors.add(fully_qualified_key(key, payload_path), :unknown_attribute, message: "is an unknown attribute") + rescue Graphiti::Errors::InvalidAttributeAccess + @errors.add(fully_qualified_key(key, payload_path), :unwritable_attribute, message: "cannot be written") + rescue Graphiti::Errors::TypecastFailed => e + @errors.add(fully_qualified_key(key, payload_path), :type_error, message: "should be type #{e.type_name}") + end + end end - end - def typecast_attributes(resource, attributes, payload_path) - attributes.each_pair do |key, value| - begin - attributes[key] = resource.typecast(key, value, :writable) - rescue Graphiti::Errors::UnknownAttribute - @errors.add(fully_qualified_key(key, payload_path), :unknown_attribute, message: "is an unknown attribute") - rescue Graphiti::Errors::InvalidAttributeAccess - @errors.add(fully_qualified_key(key, payload_path), :unwritable_attribute, message: "cannot be written") - rescue Graphiti::Errors::TypecastFailed => e - @errors.add(fully_qualified_key(key, payload_path), :type_error, message: "should be type #{e.type_name}") + def normalized_params + normalized = @raw_params + if normalized.respond_to?(:to_unsafe_h) + normalized = normalized.to_unsafe_h.deep_symbolize_keys end + normalized + end + + def fully_qualified_key(key, path, attributes_or_relationships = :attributes) + (path + [attributes_or_relationships, key]).join(".") end - end - def normalized_params - normalized = @raw_params - if normalized.respond_to?(:to_unsafe_h) - normalized = normalized.to_unsafe_h.deep_symbolize_keys + def missing_required_attribute(attr_path) + @errors.add( + attr_path.join("."), + :missing_required_attribute, + message: "is missing from the payload" + ) end - normalized end - def fully_qualified_key(key, path, attributes_or_relationships = :attributes) - (path + [attributes_or_relationships, key]).join(".") + class UpdateValidator < Validator + def validate + [ + [:data], + [:data, :type], + [:data, :id] + ].each do |required_attr| + missing_required_attribute(required_attr) unless @raw_params.dig(*required_attr) + end + super + end end end end diff --git a/lib/graphiti/resource.rb b/lib/graphiti/resource.rb index 1e9ad7dd..74badda9 100644 --- a/lib/graphiti/resource.rb +++ b/lib/graphiti/resource.rb @@ -140,7 +140,7 @@ def transaction response = yield end rescue Errors::ValidationError => e - response = {result: e.validation_response} + response = { result: e.validation_response } end response end diff --git a/lib/graphiti/resource_proxy.rb b/lib/graphiti/resource_proxy.rb index 472f7354..24130709 100644 --- a/lib/graphiti/resource_proxy.rb +++ b/lib/graphiti/resource_proxy.rb @@ -5,9 +5,9 @@ class ResourceProxy attr_reader :resource, :query, :scope, :payload def initialize(resource, scope, query, - payload: nil, - single: false, - raise_on_missing: false) + payload: nil, + single: false, + raise_on_missing: false) @resource = resource @scope = scope @query = query @@ -53,13 +53,13 @@ def to_xml(options = {}) def data @data ||= begin - records = @scope.resolve - if records.empty? && raise_on_missing? - raise Graphiti::Errors::RecordNotFound - end - records = records[0] if single? - records - end + records = @scope.resolve + if records.empty? && raise_on_missing? + raise Graphiti::Errors::RecordNotFound + end + records = records[0] if single? + records + end end alias to_a data @@ -73,14 +73,14 @@ def each(&blk) def stats @stats ||= if @query.hash[:stats] - payload = Stats::Payload.new @resource, - @query, - @scope.unpaginated_object, - data - payload.generate - else - {} - end + payload = Stats::Payload.new @resource, + @query, + @scope.unpaginated_object, + data + payload.generate + else + {} + end end def pagination @@ -95,9 +95,9 @@ def save(action: :create) Graphiti.context[:namespace] = action validator = persist { @resource.persist_with_relationships \ - @payload.meta(action: action), - @payload.attributes, - @payload.relationships + @payload.meta(action: action), + @payload.attributes, + @payload.relationships } ensure Graphiti.context[:namespace] = original @@ -123,7 +123,7 @@ def destroy model.instance_variable_set(:@__serializer_klass, @resource.serializer) @resource.after_graph_persist(model, metadata) validator = ::Graphiti::Util::ValidationResponse.new \ - model, @payload + model, @payload validator.validate! @resource.before_commit(model, metadata) @@ -139,9 +139,9 @@ def update_attributes def include_hash @include_hash ||= begin - base = @payload ? @payload.include_hash : {} - base.deep_merge(@query.include_hash) - end + base = @payload ? @payload.include_hash : {} + base.deep_merge(@query.include_hash) + end end def fields @@ -159,25 +159,32 @@ def debug_requested? private def persist - transaction_response = @resource.transaction do - ::Graphiti::Util::TransactionHooksRecorder.record do - model = yield - ::Graphiti::Util::TransactionHooksRecorder.run_graph_persist_hooks - validator = ::Graphiti::Util::ValidationResponse.new \ - model, @payload - validator.validate! - validator + model = nil + begin + transaction_response = @resource.transaction do + ::Graphiti::Util::TransactionHooksRecorder.record do + model = yield + ::Graphiti::Util::TransactionHooksRecorder.run_graph_persist_hooks + + ::Graphiti::RequestValidator.new(@resource, @payload.params).validate! + validator = ::Graphiti::Util::ValidationResponse.new \ + model, @payload + validator.validate! + validator + end end - end - _data, success = transaction_response[:result].to_a - if success - transaction_response[:after_commit_hooks].each do |hook| - hook.call + _data, success = transaction_response[:result].to_a + if success + transaction_response[:after_commit_hooks].each do |hook| + hook.call + end end - end - transaction_response[:result] + transaction_response[:result] + rescue Graphiti::Errors::InvalidRequest => e + [e, false] + end end end end diff --git a/spec/integration/rails/persistence_spec.rb b/spec/integration/rails/persistence_spec.rb index 12f142bd..da390420 100644 --- a/spec/integration/rails/persistence_spec.rb +++ b/spec/integration/rails/persistence_spec.rb @@ -126,6 +126,27 @@ ) end end + + context "when there is an invalid request payload" do + before do + payload[:data] = nil + end + + it "responds with error" do + make_request + expect(json["errors"].first).to match( + "code" => "unprocessable_entity", + "status" => "422", + "source" => {"pointer" => "/data/attributes/first_name"}, + "detail" => "First name can't be blank", + "title" => "Validation Error", + "meta" => hash_including( + "attribute" => "first_name", + "message" => "can't be blank" + ) + ) + end + end end describe "basic destroy" do @@ -1328,7 +1349,8 @@ def self.model_name it "associates workspace as home office" do make_request - + puts payload + binding.pry employee = Employee.first expect(employee.workspace).to be_a(HomeOffice) end From b0edf50d6db832fc63ded60fd40c3ccaf6c1e75a Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Fri, 18 Oct 2019 18:04:59 -0700 Subject: [PATCH 02/15] WIP attempting to get raise ConflictRequest to work --- Appraisals | 5 +++ Gemfile | 2 +- gemfiles/rails_4.gemfile | 1 + gemfiles/rails_5.gemfile | 1 + gemfiles/rails_5_graphiti_rails.gemfile | 1 + gemfiles/rails_6.gemfile | 3 +- gemfiles/rails_6_graphiti_rails.gemfile | 3 +- graphiti.gemspec | 2 +- lib/graphiti/errors.rb | 3 ++ lib/graphiti/request_validator.rb | 46 ++++++++++++++++++++-- lib/graphiti/resource_proxy.rb | 2 +- spec/integration/rails/persistence_spec.rb | 25 ++++-------- 12 files changed, 69 insertions(+), 25 deletions(-) diff --git a/Appraisals b/Appraisals index 05345c52..05171dc4 100644 --- a/Appraisals +++ b/Appraisals @@ -3,6 +3,7 @@ appraise "rails-4" do gem "rspec-rails" gem "sqlite3", "~> 1.3.6" gem "database_cleaner" + gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-5" do @@ -10,6 +11,7 @@ appraise "rails-5" do gem "rspec-rails" gem "sqlite3", "~> 1.3.6" gem "database_cleaner" + gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-5-graphiti-rails" do @@ -19,6 +21,7 @@ appraise "rails-5-graphiti-rails" do gem "database_cleaner" gem "rescue_registry", git: "https://github.com/wagenet/rescue_registry.git", branch: "master" gem "graphiti-rails", git: "https://github.com/wagenet/graphiti-rails.git", branch: "master" + gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-6" do @@ -26,6 +29,7 @@ appraise "rails-6" do gem "rspec-rails" gem "sqlite3", "~> 1.4.0" gem "database_cleaner" + gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-6-graphiti-rails" do @@ -35,4 +39,5 @@ appraise "rails-6-graphiti-rails" do gem "database_cleaner" gem "rescue_registry", git: "https://github.com/wagenet/rescue_registry.git", branch: "master" gem "graphiti-rails", git: "https://github.com/wagenet/graphiti-rails.git", branch: "master" + gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end diff --git a/Gemfile b/Gemfile index 3317b886..4e7aebbd 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" # Specify your gem's dependencies in graphiti.gemspec gemspec - +gem "graphiti_errors", path: "~/src/github.com/sideshowbandana/graphiti_errors" # "~> 1.1.0" # group :test do gem "pry" gem "pry-byebug", platform: [:mri] diff --git a/gemfiles/rails_4.gemfile b/gemfiles/rails_4.gemfile index a7f9a657..16103088 100644 --- a/gemfiles/rails_4.gemfile +++ b/gemfiles/rails_4.gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" +gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 4.1" gem "rspec-rails" gem "sqlite3", "~> 1.3.6" diff --git a/gemfiles/rails_5.gemfile b/gemfiles/rails_5.gemfile index d3fdb42f..78e40f31 100644 --- a/gemfiles/rails_5.gemfile +++ b/gemfiles/rails_5.gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" +gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 5.2" gem "rspec-rails" gem "sqlite3", "~> 1.3.6" diff --git a/gemfiles/rails_5_graphiti_rails.gemfile b/gemfiles/rails_5_graphiti_rails.gemfile index 3b5aeb82..3a0e1a12 100644 --- a/gemfiles/rails_5_graphiti_rails.gemfile +++ b/gemfiles/rails_5_graphiti_rails.gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" +gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 5.2" gem "rspec-rails" gem "sqlite3", "~> 1.3.6" diff --git a/gemfiles/rails_6.gemfile b/gemfiles/rails_6.gemfile index 54aac8b2..21a5bdcf 100644 --- a/gemfiles/rails_6.gemfile +++ b/gemfiles/rails_6.gemfile @@ -2,7 +2,8 @@ source "https://rubygems.org" -gem "rails", "~> 6.0.0" +gem "graphiti_errors", path: "../../graphiti_errors" +gem "rails", "~> 6.0.0.rc1" gem "rspec-rails" gem "sqlite3", "~> 1.4.0" gem "database_cleaner" diff --git a/gemfiles/rails_6_graphiti_rails.gemfile b/gemfiles/rails_6_graphiti_rails.gemfile index 518a4453..c9f5e6db 100644 --- a/gemfiles/rails_6_graphiti_rails.gemfile +++ b/gemfiles/rails_6_graphiti_rails.gemfile @@ -2,7 +2,8 @@ source "https://rubygems.org" -gem "rails", "~> 6.0.0" +gem "graphiti_errors", path: "../../graphiti_errors" +gem "rails", "~> 6.0.0.rc1" gem "rspec-rails" gem "sqlite3", "~> 1.4.0" gem "database_cleaner" diff --git a/graphiti.gemspec b/graphiti.gemspec index b73c7906..06fea488 100644 --- a/graphiti.gemspec +++ b/graphiti.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |spec| spec.add_dependency "jsonapi-serializable", "~> 0.3.0" spec.add_dependency "jsonapi-renderer", "0.2.0" spec.add_dependency "dry-types", ">= 0.15.0", "< 2.0" - spec.add_dependency "graphiti_errors", "~> 1.1.0" + # spec.add_dependency "graphiti_errors" # "~> 1.1.0" # spec.add_dependency "concurrent-ruby", "~> 1.0" spec.add_dependency "activesupport", ">= 4.1" diff --git a/lib/graphiti/errors.rb b/lib/graphiti/errors.rb index a473cdfd..99233ad6 100644 --- a/lib/graphiti/errors.rb +++ b/lib/graphiti/errors.rb @@ -754,5 +754,8 @@ def message MSG end end + + class ConflictRequest < InvalidRequest + end end end diff --git a/lib/graphiti/request_validator.rb b/lib/graphiti/request_validator.rb index e6b65e9c..00608978 100644 --- a/lib/graphiti/request_validator.rb +++ b/lib/graphiti/request_validator.rb @@ -46,7 +46,7 @@ def validate def validate! unless validate - raise Graphiti::Errors::InvalidRequest, self.errors + raise @error_class || Graphiti::Errors::InvalidRequest, self.errors end true @@ -121,18 +121,58 @@ def missing_required_attribute(attr_path) message: "is missing from the payload" ) end + + def attribute_mismatch(attr_path) + @error_class = Graphiti::Errors::ConflictRequest + @errors.add( + attr_path.join("."), + :attribute_mismatch, + message: "does not match the server endpoint" + ) + end end class UpdateValidator < Validator def validate + if required_payload? && payload_matches_endpoint? + super + else + return false + end + end + + private + + def required_payload? [ [:data], [:data, :type], [:data, :id] ].each do |required_attr| - missing_required_attribute(required_attr) unless @raw_params.dig(*required_attr) + attribute_mismatch(required_attr) unless @raw_params.dig(*required_attr) end - super + errors.blank? + end + + def payload_matches_endpoint? + unless @raw_params.dig(:data, :id) == @raw_params.dig(:filter, :id) + attribute_mismatch([:data, :id]) + end + + meta_type = @raw_params.dig(:data, :type) + if @root_resource.type != meta_type + if @root_resource.polymorphic? + begin + @root_resource.class.resource_for_type(meta_type).new + rescue Errors::PolymorphicResourceChildNotFound + attribute_mismatch([:data, :type]) + end + else + attribute_mismatch([:data, :type]) + end + end + + errors.blank? end end end diff --git a/lib/graphiti/resource_proxy.rb b/lib/graphiti/resource_proxy.rb index 24130709..86699d38 100644 --- a/lib/graphiti/resource_proxy.rb +++ b/lib/graphiti/resource_proxy.rb @@ -93,6 +93,7 @@ def save(action: :create) original = Graphiti.context[:namespace] begin Graphiti.context[:namespace] = action + ::Graphiti::RequestValidator.new(@resource, @payload.params).validate! validator = persist { @resource.persist_with_relationships \ @payload.meta(action: action), @@ -166,7 +167,6 @@ def persist model = yield ::Graphiti::Util::TransactionHooksRecorder.run_graph_persist_hooks - ::Graphiti::RequestValidator.new(@resource, @payload.params).validate! validator = ::Graphiti::Util::ValidationResponse.new \ model, @payload validator.validate! diff --git a/spec/integration/rails/persistence_spec.rb b/spec/integration/rails/persistence_spec.rb index da390420..16678260 100644 --- a/spec/integration/rails/persistence_spec.rb +++ b/spec/integration/rails/persistence_spec.rb @@ -129,22 +129,13 @@ context "when there is an invalid request payload" do before do - payload[:data] = nil + payload[:data][:type] = "" end - it "responds with error" do - make_request - expect(json["errors"].first).to match( - "code" => "unprocessable_entity", - "status" => "422", - "source" => {"pointer" => "/data/attributes/first_name"}, - "detail" => "First name can't be blank", - "title" => "Validation Error", - "meta" => hash_including( - "attribute" => "first_name", - "message" => "can't be blank" - ) - ) + it "raises a Graphiti::Errors::ConflictRequest" do + expect{ + make_request + }.to raise_error(Graphiti::Errors::ConflictRequest) end end end @@ -1349,8 +1340,6 @@ def self.model_name it "associates workspace as home office" do make_request - puts payload - binding.pry employee = Employee.first expect(employee.workspace).to be_a(HomeOffice) end @@ -1367,7 +1356,9 @@ def self.model_name end describe "delete nested item" do - subject(:make_request) { do_update(payload) } + subject(:make_request) { + do_update(payload) + } let!(:employee) { Employee.create!(first_name: "original", positions: [position1, position2, position3]) } let!(:position1) { Position.create!(title: "pos1") } From cd49d3b2cbf331d91e6254a68eaa6f84c997e89e Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 10:10:15 -0700 Subject: [PATCH 03/15] Remove local dependencies - Fix string & symbol comparison - Add a note about string and symbol comparison --- Appraisals | 5 ----- Gemfile | 2 +- gemfiles/rails_4.gemfile | 1 - gemfiles/rails_5.gemfile | 1 - gemfiles/rails_5_graphiti_rails.gemfile | 1 - gemfiles/rails_6.gemfile | 1 - gemfiles/rails_6_graphiti_rails.gemfile | 1 - graphiti.gemspec | 2 +- lib/graphiti/request_validator.rb | 7 ++++++- 9 files changed, 8 insertions(+), 13 deletions(-) diff --git a/Appraisals b/Appraisals index 05171dc4..05345c52 100644 --- a/Appraisals +++ b/Appraisals @@ -3,7 +3,6 @@ appraise "rails-4" do gem "rspec-rails" gem "sqlite3", "~> 1.3.6" gem "database_cleaner" - gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-5" do @@ -11,7 +10,6 @@ appraise "rails-5" do gem "rspec-rails" gem "sqlite3", "~> 1.3.6" gem "database_cleaner" - gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-5-graphiti-rails" do @@ -21,7 +19,6 @@ appraise "rails-5-graphiti-rails" do gem "database_cleaner" gem "rescue_registry", git: "https://github.com/wagenet/rescue_registry.git", branch: "master" gem "graphiti-rails", git: "https://github.com/wagenet/graphiti-rails.git", branch: "master" - gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-6" do @@ -29,7 +26,6 @@ appraise "rails-6" do gem "rspec-rails" gem "sqlite3", "~> 1.4.0" gem "database_cleaner" - gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end appraise "rails-6-graphiti-rails" do @@ -39,5 +35,4 @@ appraise "rails-6-graphiti-rails" do gem "database_cleaner" gem "rescue_registry", git: "https://github.com/wagenet/rescue_registry.git", branch: "master" gem "graphiti-rails", git: "https://github.com/wagenet/graphiti-rails.git", branch: "master" - gem "graphiti_errors", path: "../graphiti_errors" # "~> 1.1.0" # end diff --git a/Gemfile b/Gemfile index 4e7aebbd..3317b886 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" # Specify your gem's dependencies in graphiti.gemspec gemspec -gem "graphiti_errors", path: "~/src/github.com/sideshowbandana/graphiti_errors" # "~> 1.1.0" # + group :test do gem "pry" gem "pry-byebug", platform: [:mri] diff --git a/gemfiles/rails_4.gemfile b/gemfiles/rails_4.gemfile index 16103088..a7f9a657 100644 --- a/gemfiles/rails_4.gemfile +++ b/gemfiles/rails_4.gemfile @@ -2,7 +2,6 @@ source "https://rubygems.org" -gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 4.1" gem "rspec-rails" gem "sqlite3", "~> 1.3.6" diff --git a/gemfiles/rails_5.gemfile b/gemfiles/rails_5.gemfile index 78e40f31..d3fdb42f 100644 --- a/gemfiles/rails_5.gemfile +++ b/gemfiles/rails_5.gemfile @@ -2,7 +2,6 @@ source "https://rubygems.org" -gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 5.2" gem "rspec-rails" gem "sqlite3", "~> 1.3.6" diff --git a/gemfiles/rails_5_graphiti_rails.gemfile b/gemfiles/rails_5_graphiti_rails.gemfile index 3a0e1a12..3b5aeb82 100644 --- a/gemfiles/rails_5_graphiti_rails.gemfile +++ b/gemfiles/rails_5_graphiti_rails.gemfile @@ -2,7 +2,6 @@ source "https://rubygems.org" -gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 5.2" gem "rspec-rails" gem "sqlite3", "~> 1.3.6" diff --git a/gemfiles/rails_6.gemfile b/gemfiles/rails_6.gemfile index 21a5bdcf..3c1e1f70 100644 --- a/gemfiles/rails_6.gemfile +++ b/gemfiles/rails_6.gemfile @@ -2,7 +2,6 @@ source "https://rubygems.org" -gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 6.0.0.rc1" gem "rspec-rails" gem "sqlite3", "~> 1.4.0" diff --git a/gemfiles/rails_6_graphiti_rails.gemfile b/gemfiles/rails_6_graphiti_rails.gemfile index c9f5e6db..8a501758 100644 --- a/gemfiles/rails_6_graphiti_rails.gemfile +++ b/gemfiles/rails_6_graphiti_rails.gemfile @@ -2,7 +2,6 @@ source "https://rubygems.org" -gem "graphiti_errors", path: "../../graphiti_errors" gem "rails", "~> 6.0.0.rc1" gem "rspec-rails" gem "sqlite3", "~> 1.4.0" diff --git a/graphiti.gemspec b/graphiti.gemspec index 06fea488..5f93e439 100644 --- a/graphiti.gemspec +++ b/graphiti.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |spec| spec.add_dependency "jsonapi-serializable", "~> 0.3.0" spec.add_dependency "jsonapi-renderer", "0.2.0" spec.add_dependency "dry-types", ">= 0.15.0", "< 2.0" - # spec.add_dependency "graphiti_errors" # "~> 1.1.0" # + spec.add_dependency "graphiti_errors", "~> 1.1.0" spec.add_dependency "concurrent-ruby", "~> 1.0" spec.add_dependency "activesupport", ">= 4.1" diff --git a/lib/graphiti/request_validator.rb b/lib/graphiti/request_validator.rb index 00608978..647912a1 100644 --- a/lib/graphiti/request_validator.rb +++ b/lib/graphiti/request_validator.rb @@ -159,8 +159,13 @@ def payload_matches_endpoint? attribute_mismatch([:data, :id]) end + meta_type = @raw_params.dig(:data, :type) - if @root_resource.type != meta_type + + # NOTE: calling #to_s and comparing 2 strings is slower than + # calling #to_sym and comparing 2 symbols. But pre ruby-2.2 + # #to_sym on user supplied data would lead to a memory leak. + if @root_resource.type.to_s != meta_type if @root_resource.polymorphic? begin @root_resource.class.resource_for_type(meta_type).new From bf5f61019ae2d67f66fb953f55c4a02bd71f7362 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 10:18:19 -0700 Subject: [PATCH 04/15] Revert resource_proxy changes except for request validator change --- lib/graphiti/resource_proxy.rb | 86 ++++++++++++++++------------------ 1 file changed, 40 insertions(+), 46 deletions(-) diff --git a/lib/graphiti/resource_proxy.rb b/lib/graphiti/resource_proxy.rb index 86699d38..ba69f4c0 100644 --- a/lib/graphiti/resource_proxy.rb +++ b/lib/graphiti/resource_proxy.rb @@ -5,9 +5,9 @@ class ResourceProxy attr_reader :resource, :query, :scope, :payload def initialize(resource, scope, query, - payload: nil, - single: false, - raise_on_missing: false) + payload: nil, + single: false, + raise_on_missing: false) @resource = resource @scope = scope @query = query @@ -53,13 +53,13 @@ def to_xml(options = {}) def data @data ||= begin - records = @scope.resolve - if records.empty? && raise_on_missing? - raise Graphiti::Errors::RecordNotFound - end - records = records[0] if single? - records - end + records = @scope.resolve + if records.empty? && raise_on_missing? + raise Graphiti::Errors::RecordNotFound + end + records = records[0] if single? + records + end end alias to_a data @@ -73,14 +73,14 @@ def each(&blk) def stats @stats ||= if @query.hash[:stats] - payload = Stats::Payload.new @resource, - @query, - @scope.unpaginated_object, - data - payload.generate - else - {} - end + payload = Stats::Payload.new @resource, + @query, + @scope.unpaginated_object, + data + payload.generate + else + {} + end end def pagination @@ -96,9 +96,9 @@ def save(action: :create) ::Graphiti::RequestValidator.new(@resource, @payload.params).validate! validator = persist { @resource.persist_with_relationships \ - @payload.meta(action: action), - @payload.attributes, - @payload.relationships + @payload.meta(action: action), + @payload.attributes, + @payload.relationships } ensure Graphiti.context[:namespace] = original @@ -124,7 +124,7 @@ def destroy model.instance_variable_set(:@__serializer_klass, @resource.serializer) @resource.after_graph_persist(model, metadata) validator = ::Graphiti::Util::ValidationResponse.new \ - model, @payload + model, @payload validator.validate! @resource.before_commit(model, metadata) @@ -140,9 +140,9 @@ def update_attributes def include_hash @include_hash ||= begin - base = @payload ? @payload.include_hash : {} - base.deep_merge(@query.include_hash) - end + base = @payload ? @payload.include_hash : {} + base.deep_merge(@query.include_hash) + end end def fields @@ -160,31 +160,25 @@ def debug_requested? private def persist - model = nil - begin - transaction_response = @resource.transaction do - ::Graphiti::Util::TransactionHooksRecorder.record do - model = yield - ::Graphiti::Util::TransactionHooksRecorder.run_graph_persist_hooks - - validator = ::Graphiti::Util::ValidationResponse.new \ - model, @payload - validator.validate! - validator - end + transaction_response = @resource.transaction do + ::Graphiti::Util::TransactionHooksRecorder.record do + model = yield + ::Graphiti::Util::TransactionHooksRecorder.run_graph_persist_hooks + validator = ::Graphiti::Util::ValidationResponse.new \ + model, @payload + validator.validate! + validator end + end - _data, success = transaction_response[:result].to_a - if success - transaction_response[:after_commit_hooks].each do |hook| - hook.call - end + _data, success = transaction_response[:result].to_a + if success + transaction_response[:after_commit_hooks].each do |hook| + hook.call end - - transaction_response[:result] - rescue Graphiti::Errors::InvalidRequest => e - [e, false] end + + transaction_response[:result] end end end From 60e598225d5eef8bcbd05ca6be1ef47d785c54c7 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 11:21:26 -0700 Subject: [PATCH 05/15] Break up RequestValidator into multiple files - add request_validators directory - move request_validator code into request_validators/validator - add the ability to set the error class but default to `InvalidRequest` - add attribute_mismatch method to `UpdateValidator` --- lib/graphiti.rb | 2 + lib/graphiti/request_validator.rb | 164 +----------------- .../request_validators/update_validator.rb | 61 +++++++ lib/graphiti/request_validators/validator.rb | 96 ++++++++++ 4 files changed, 161 insertions(+), 162 deletions(-) create mode 100644 lib/graphiti/request_validators/update_validator.rb create mode 100644 lib/graphiti/request_validators/validator.rb diff --git a/lib/graphiti.rb b/lib/graphiti.rb index 9096d722..c4806b27 100644 --- a/lib/graphiti.rb +++ b/lib/graphiti.rb @@ -124,6 +124,8 @@ def self.setup! require "graphiti/resource" require "graphiti/resource_proxy" require "graphiti/request_validator" +require "graphiti/request_validators/validator" +require "graphiti/request_validators/update_validator" require "graphiti/query" require "graphiti/scope" require "graphiti/deserializer" diff --git a/lib/graphiti/request_validator.rb b/lib/graphiti/request_validator.rb index 647912a1..c4ad15b9 100644 --- a/lib/graphiti/request_validator.rb +++ b/lib/graphiti/request_validator.rb @@ -14,171 +14,11 @@ class ValidatorFactory def self.create(root_resource, raw_params) case raw_params["action"] when "update" then - UpdateValidator + RequestValidators::UpdateValidator else - Validator + RequestValidators::Validator end.new(root_resource, raw_params) end end - - class Validator - attr_reader :root_resource, :errors - def initialize(root_resource, raw_params) - @root_resource = root_resource - @raw_params = raw_params - @errors = Graphiti::Util::SimpleErrors.new(raw_params) - end - - # FIXME: set resource in initializer and use the attr_reader instead of passing it around - def validate - resource = @root_resource - if (meta_type = deserialized_payload.meta[:type].try(:to_sym)) - if @root_resource.type != meta_type && @root_resource.polymorphic? - resource = @root_resource.class.resource_for_type(meta_type).new - end - end - - typecast_attributes(resource, deserialized_payload.attributes, deserialized_payload.meta[:payload_path]) - process_relationships(resource, deserialized_payload.relationships, deserialized_payload.meta[:payload_path]) - - errors.blank? - end - - def validate! - unless validate - raise @error_class || Graphiti::Errors::InvalidRequest, self.errors - end - - true - end - - def deserialized_payload - @deserialized_payload ||= begin - payload = normalized_params - if payload[:data] && payload[:data][:type] - Graphiti::Deserializer.new(payload) - else - Graphiti::Deserializer.new({}) - end - end - end - - private - - def process_relationships(resource, relationships, payload_path) - opts = { - resource: resource, - relationships: relationships, - } - - Graphiti::Util::RelationshipPayload.iterate(opts) do |x| - sideload_def = x[:sideload] - - unless sideload_def.writable? - full_key = fully_qualified_key(sideload_def.name, payload_path, :relationships) - unless @errors.added?(full_key, :unwritable_relationship) - @errors.add(full_key, :unwritable_relationship) - end - next - end - - typecast_attributes(x[:resource], x[:attributes], x[:meta][:payload_path]) - process_relationships(x[:resource], x[:relationships], x[:meta][:payload_path]) - end - end - - def typecast_attributes(resource, attributes, payload_path) - attributes.each_pair do |key, value| - begin - attributes[key] = resource.typecast(key, value, :writable) - rescue Graphiti::Errors::UnknownAttribute - resource.typecast(key, value, :writable) - @errors.add(fully_qualified_key(key, payload_path), :unknown_attribute, message: "is an unknown attribute") - rescue Graphiti::Errors::InvalidAttributeAccess - @errors.add(fully_qualified_key(key, payload_path), :unwritable_attribute, message: "cannot be written") - rescue Graphiti::Errors::TypecastFailed => e - @errors.add(fully_qualified_key(key, payload_path), :type_error, message: "should be type #{e.type_name}") - end - end - end - - def normalized_params - normalized = @raw_params - if normalized.respond_to?(:to_unsafe_h) - normalized = normalized.to_unsafe_h.deep_symbolize_keys - end - normalized - end - - def fully_qualified_key(key, path, attributes_or_relationships = :attributes) - (path + [attributes_or_relationships, key]).join(".") - end - - def missing_required_attribute(attr_path) - @errors.add( - attr_path.join("."), - :missing_required_attribute, - message: "is missing from the payload" - ) - end - - def attribute_mismatch(attr_path) - @error_class = Graphiti::Errors::ConflictRequest - @errors.add( - attr_path.join("."), - :attribute_mismatch, - message: "does not match the server endpoint" - ) - end - end - - class UpdateValidator < Validator - def validate - if required_payload? && payload_matches_endpoint? - super - else - return false - end - end - - private - - def required_payload? - [ - [:data], - [:data, :type], - [:data, :id] - ].each do |required_attr| - attribute_mismatch(required_attr) unless @raw_params.dig(*required_attr) - end - errors.blank? - end - - def payload_matches_endpoint? - unless @raw_params.dig(:data, :id) == @raw_params.dig(:filter, :id) - attribute_mismatch([:data, :id]) - end - - - meta_type = @raw_params.dig(:data, :type) - - # NOTE: calling #to_s and comparing 2 strings is slower than - # calling #to_sym and comparing 2 symbols. But pre ruby-2.2 - # #to_sym on user supplied data would lead to a memory leak. - if @root_resource.type.to_s != meta_type - if @root_resource.polymorphic? - begin - @root_resource.class.resource_for_type(meta_type).new - rescue Errors::PolymorphicResourceChildNotFound - attribute_mismatch([:data, :type]) - end - else - attribute_mismatch([:data, :type]) - end - end - - errors.blank? - end - end end end diff --git a/lib/graphiti/request_validators/update_validator.rb b/lib/graphiti/request_validators/update_validator.rb new file mode 100644 index 00000000..a0da5f71 --- /dev/null +++ b/lib/graphiti/request_validators/update_validator.rb @@ -0,0 +1,61 @@ +module Graphiti + module RequestValidators + class UpdateValidator < Validator + def validate + if required_payload? && payload_matches_endpoint? + super + else + return false + end + end + + private + + def attribute_mismatch(attr_path) + @error_class = Graphiti::Errors::ConflictRequest + @errors.add( + attr_path.join("."), + :attribute_mismatch, + message: "does not match the server endpoint" + ) + end + + def required_payload? + [ + [:data], + [:data, :type], + [:data, :id] + ].each do |required_attr| + attribute_mismatch(required_attr) unless @raw_params.dig(*required_attr) + end + errors.blank? + end + + def payload_matches_endpoint? + unless @raw_params.dig(:data, :id) == @raw_params.dig(:filter, :id) + attribute_mismatch([:data, :id]) + end + + + meta_type = @raw_params.dig(:data, :type) + + # NOTE: calling #to_s and comparing 2 strings is slower than + # calling #to_sym and comparing 2 symbols. But pre ruby-2.2 + # #to_sym on user supplied data would lead to a memory leak. + if @root_resource.type.to_s != meta_type + if @root_resource.polymorphic? + begin + @root_resource.class.resource_for_type(meta_type).new + rescue Errors::PolymorphicResourceChildNotFound + attribute_mismatch([:data, :type]) + end + else + attribute_mismatch([:data, :type]) + end + end + + errors.blank? + end + end + end +end diff --git a/lib/graphiti/request_validators/validator.rb b/lib/graphiti/request_validators/validator.rb new file mode 100644 index 00000000..aa3e86cf --- /dev/null +++ b/lib/graphiti/request_validators/validator.rb @@ -0,0 +1,96 @@ +module Graphiti + module RequestValidators + class Validator + attr_reader :errors + + def initialize(root_resource, raw_params) + @root_resource = root_resource + @raw_params = raw_params + @errors = Graphiti::Util::SimpleErrors.new(raw_params) + end + + def validate + resource = @root_resource + if (meta_type = deserialized_payload.meta[:type].try(:to_sym)) + if @root_resource.type != meta_type && @root_resource.polymorphic? + resource = @root_resource.class.resource_for_type(meta_type).new + end + end + + typecast_attributes(resource, deserialized_payload.attributes, deserialized_payload.meta[:payload_path]) + process_relationships(resource, deserialized_payload.relationships, deserialized_payload.meta[:payload_path]) + + errors.blank? + end + + def validate! + unless validate + raise @error_class || Graphiti::Errors::InvalidRequest, self.errors + end + + true + end + + def deserialized_payload + @deserialized_payload ||= begin + payload = normalized_params + if payload[:data] && payload[:data][:type] + Graphiti::Deserializer.new(payload) + else + Graphiti::Deserializer.new({}) + end + end + end + + private + + def process_relationships(resource, relationships, payload_path) + opts = { + resource: resource, + relationships: relationships, + } + + Graphiti::Util::RelationshipPayload.iterate(opts) do |x| + sideload_def = x[:sideload] + + unless sideload_def.writable? + full_key = fully_qualified_key(sideload_def.name, payload_path, :relationships) + unless @errors.added?(full_key, :unwritable_relationship) + @errors.add(full_key, :unwritable_relationship) + end + next + end + + typecast_attributes(x[:resource], x[:attributes], x[:meta][:payload_path]) + process_relationships(x[:resource], x[:relationships], x[:meta][:payload_path]) + end + end + + def typecast_attributes(resource, attributes, payload_path) + attributes.each_pair do |key, value| + begin + attributes[key] = resource.typecast(key, value, :writable) + rescue Graphiti::Errors::UnknownAttribute + @errors.add(fully_qualified_key(key, payload_path), :unknown_attribute, message: "is an unknown attribute") + rescue Graphiti::Errors::InvalidAttributeAccess + @errors.add(fully_qualified_key(key, payload_path), :unwritable_attribute, message: "cannot be written") + rescue Graphiti::Errors::TypecastFailed => e + @errors.add(fully_qualified_key(key, payload_path), :type_error, message: "should be type #{e.type_name}") + end + end + end + + def normalized_params + normalized = @raw_params + if normalized.respond_to?(:to_unsafe_h) + normalized = normalized.to_unsafe_h.deep_symbolize_keys + end + normalized + end + + def fully_qualified_key(key, path, attributes_or_relationships = :attributes) + (path + [attributes_or_relationships, key]).join(".") + end + end + end +end From 83f9362a3f571f270b3b4f0c4e54df84bce38034 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 12:20:58 -0700 Subject: [PATCH 06/15] fix typo --- lib/graphiti/request_validator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/graphiti/request_validator.rb b/lib/graphiti/request_validator.rb index c4ad15b9..a0e07180 100644 --- a/lib/graphiti/request_validator.rb +++ b/lib/graphiti/request_validator.rb @@ -1,6 +1,6 @@ module Graphiti class RequestValidator - delegate :vaildate, + delegate :validate, :validate!, :errors, :deserialized_payload, From d7d0ed668fa7b0ae41896cca6a025ca0c1205067 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 12:21:47 -0700 Subject: [PATCH 07/15] revert gemfiles changes --- gemfiles/rails_6.gemfile | 2 +- gemfiles/rails_6_graphiti_rails.gemfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gemfiles/rails_6.gemfile b/gemfiles/rails_6.gemfile index 3c1e1f70..54aac8b2 100644 --- a/gemfiles/rails_6.gemfile +++ b/gemfiles/rails_6.gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" -gem "rails", "~> 6.0.0.rc1" +gem "rails", "~> 6.0.0" gem "rspec-rails" gem "sqlite3", "~> 1.4.0" gem "database_cleaner" diff --git a/gemfiles/rails_6_graphiti_rails.gemfile b/gemfiles/rails_6_graphiti_rails.gemfile index 8a501758..518a4453 100644 --- a/gemfiles/rails_6_graphiti_rails.gemfile +++ b/gemfiles/rails_6_graphiti_rails.gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" -gem "rails", "~> 6.0.0.rc1" +gem "rails", "~> 6.0.0" gem "rspec-rails" gem "sqlite3", "~> 1.4.0" gem "database_cleaner" From ca84476abd4d94af4e250529bfd9d5d09f876691 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 12:22:34 -0700 Subject: [PATCH 08/15] revert gemspec changes --- graphiti.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphiti.gemspec b/graphiti.gemspec index 5f93e439..b73c7906 100644 --- a/graphiti.gemspec +++ b/graphiti.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |spec| spec.add_dependency "jsonapi-serializable", "~> 0.3.0" spec.add_dependency "jsonapi-renderer", "0.2.0" spec.add_dependency "dry-types", ">= 0.15.0", "< 2.0" - spec.add_dependency "graphiti_errors", "~> 1.1.0" + spec.add_dependency "graphiti_errors", "~> 1.1.0" spec.add_dependency "concurrent-ruby", "~> 1.0" spec.add_dependency "activesupport", ">= 4.1" From 706b53c6335e75359a293b9b07e1ff30c9f282ce Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 12:26:19 -0700 Subject: [PATCH 09/15] Revert railtie changes --- lib/graphiti/railtie.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/graphiti/railtie.rb b/lib/graphiti/railtie.rb index c7c96712..347a11c5 100644 --- a/lib/graphiti/railtie.rb +++ b/lib/graphiti/railtie.rb @@ -79,14 +79,8 @@ def register_renderers ::ActionController::Renderers.add(:jsonapi_errors) do |proxy, options| self.content_type ||= Mime[:jsonapi] - if proxy.errors.kind_of?(Graphiti::Util::SimpleErrors) - validation = proxy - else - validation = GraphitiErrors::Validation::Serializer.new( - proxy.data, - proxy.payload.relationships - ) - end + validation = GraphitiErrors::Validation::Serializer.new \ + proxy.data, proxy.payload.relationships render \ json: {errors: validation.errors}, From 0d65f7e665497a0118db469abe78c05ee65bcae9 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 12:28:28 -0700 Subject: [PATCH 10/15] revert newlines --- spec/integration/rails/persistence_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/integration/rails/persistence_spec.rb b/spec/integration/rails/persistence_spec.rb index 16678260..a2f3dc71 100644 --- a/spec/integration/rails/persistence_spec.rb +++ b/spec/integration/rails/persistence_spec.rb @@ -1356,9 +1356,7 @@ def self.model_name end describe "delete nested item" do - subject(:make_request) { - do_update(payload) - } + subject(:make_request) { do_update(payload) } let!(:employee) { Employee.create!(first_name: "original", positions: [position1, position2, position3]) } let!(:position1) { Position.create!(title: "pos1") } From 248d59c8fadd463860196eba5eb6b4334509d809 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Sat, 19 Oct 2019 12:41:40 -0700 Subject: [PATCH 11/15] Use Ruby Standard Style (https://github.com/testdouble/standard) - Use `find` instead of `select.first`. --- spec/integration/rails/persistence_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/integration/rails/persistence_spec.rb b/spec/integration/rails/persistence_spec.rb index a2f3dc71..9c225d5f 100644 --- a/spec/integration/rails/persistence_spec.rb +++ b/spec/integration/rails/persistence_spec.rb @@ -705,9 +705,9 @@ def self.model_name expect { make_request }.to_not(change { Employee.count + Position.count + Department.count }) - error = json["errors"].select do |err| + error = json["errors"].find do |err| err.fetch("meta", {}).fetch("relationship", {}).fetch("type", nil) == "positions" - end.first + end expect(error).to match( "code" => "unprocessable_entity", @@ -746,9 +746,9 @@ def self.model_name expect { make_request }.to_not(change { Employee.count + Position.count + Department.count }) - error = json["errors"].select do |err| + error = json["errors"].find do |err| err.fetch("meta", {}).fetch("relationship", {}).fetch("type", nil) == "departments" - end.first + end expect(error).to match( "code" => "unprocessable_entity", From aaf9cc1f64992b3de874db28c0bcb3ab7fca9615 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Mon, 21 Oct 2019 07:57:45 -0700 Subject: [PATCH 12/15] revert spacing change --- lib/graphiti/resource.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/graphiti/resource.rb b/lib/graphiti/resource.rb index 74badda9..1e9ad7dd 100644 --- a/lib/graphiti/resource.rb +++ b/lib/graphiti/resource.rb @@ -140,7 +140,7 @@ def transaction response = yield end rescue Errors::ValidationError => e - response = { result: e.validation_response } + response = {result: e.validation_response} end response end From e0d205509bcd743016de7254ff922d1434f885f5 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Mon, 21 Oct 2019 08:00:06 -0700 Subject: [PATCH 13/15] Add back removed newline --- spec/integration/rails/persistence_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/integration/rails/persistence_spec.rb b/spec/integration/rails/persistence_spec.rb index 9c225d5f..7143083c 100644 --- a/spec/integration/rails/persistence_spec.rb +++ b/spec/integration/rails/persistence_spec.rb @@ -1330,6 +1330,7 @@ def self.model_name it "associates workspace as office" do make_request + employee = Employee.first expect(employee.workspace).to be_a(Office) end From f06173f83584f64f2bbbfaedeafac1b44f54983c Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Mon, 21 Oct 2019 08:01:47 -0700 Subject: [PATCH 14/15] Revert "Add back removed newline" This reverts commit e0d205509bcd743016de7254ff922d1434f885f5. --- spec/integration/rails/persistence_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/integration/rails/persistence_spec.rb b/spec/integration/rails/persistence_spec.rb index 7143083c..9c225d5f 100644 --- a/spec/integration/rails/persistence_spec.rb +++ b/spec/integration/rails/persistence_spec.rb @@ -1330,7 +1330,6 @@ def self.model_name it "associates workspace as office" do make_request - employee = Employee.first expect(employee.workspace).to be_a(Office) end From cee6991e7e7be3633a6961cb5cb43b2357af2407 Mon Sep 17 00:00:00 2001 From: Kyle Barton Date: Mon, 21 Oct 2019 08:02:11 -0700 Subject: [PATCH 15/15] Add back newline --- spec/integration/rails/persistence_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/integration/rails/persistence_spec.rb b/spec/integration/rails/persistence_spec.rb index 9c225d5f..bb517e5b 100644 --- a/spec/integration/rails/persistence_spec.rb +++ b/spec/integration/rails/persistence_spec.rb @@ -1340,6 +1340,7 @@ def self.model_name it "associates workspace as home office" do make_request + employee = Employee.first expect(employee.workspace).to be_a(HomeOffice) end