Merge pull request #3928 from rmosolgo/validation-clean-up

Validation clean up

Author: rmosolgo

guides/type_definitions/scalars.md

@@ -97,5 +97,4 @@ Your class must define two class methods:
 
 When incoming data is incorrect, the method may raise {{ "GraphQL::CoercionError" | api_doc }}, which will be returned to the client in the `"errors"` key.
 
-
 Scalar classes are never initialized; only their `.coerce_*` methods are called at runtime.

lib/graphql/query.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 require "graphql/query/context"
 require "graphql/query/fingerprint"
-require "graphql/query/literal_input"
 require "graphql/query/null_context"
 require "graphql/query/result"
 require "graphql/query/variables"

lib/graphql/query/input_validation_result.rb

@@ -4,6 +4,12 @@ class Query
     class InputValidationResult
       attr_accessor :problems
 
+      def self.from_problem(explanation, path = nil, extensions: nil, message: nil)
+        result = self.new
+        result.add_problem(explanation, path, extensions: extensions, message: message)
+        result
+      end
+
       def initialize(valid: true, problems: nil)
         @valid = valid
         @problems = problems
@@ -27,7 +33,7 @@ def add_problem(explanation, path = nil, extensions: nil, message: nil)
       end
 
       def merge_result!(path, inner_result)
-        return if inner_result.valid?
+        return if inner_result.nil? || inner_result.valid?
 
         if inner_result.problems
           inner_result.problems.each do |p|
@@ -38,6 +44,9 @@ def merge_result!(path, inner_result)
         # It could have been explicitly set on inner_result (if it had no problems)
         @valid = false
       end
+
+      VALID = self.new
+      VALID.freeze
     end
   end
 end

lib/graphql/query/literal_input.rb

@@ -123,16 +123,14 @@ def kind
         end
 
         def validate_non_null_input(value_name, ctx)
-          result = GraphQL::Query::InputValidationResult.new
-
           allowed_values = ctx.warden.enum_values(self)
           matching_value = allowed_values.find { |v| v.graphql_name == value_name }
 
           if matching_value.nil?
-            result.add_problem("Expected #{GraphQL::Language.serialize(value_name)} to be one of: #{allowed_values.map(&:graphql_name).join(', ')}")
+            GraphQL::Query::InputValidationResult.from_problem("Expected #{GraphQL::Language.serialize(value_name)} to be one of: #{allowed_values.map(&:graphql_name).join(', ')}")
+          else
+            nil
           end
-
-          result
         end
 
         def coerce_result(value, ctx)

lib/graphql/schema/enum.rb

@@ -127,19 +127,15 @@ def kind
         INVALID_OBJECT_MESSAGE = "Expected %{object} to be a key-value object responding to `to_h` or `to_unsafe_h`."
 
         def validate_non_null_input(input, ctx)
-          result = GraphQL::Query::InputValidationResult.new
-
           warden = ctx.warden
 
           if input.is_a?(Array)
-            result.add_problem(INVALID_OBJECT_MESSAGE % { object: JSON.generate(input, quirks_mode: true) })
-            return result
+            return GraphQL::Query::InputValidationResult.from_problem(INVALID_OBJECT_MESSAGE % { object: JSON.generate(input, quirks_mode: true) })
           end
 
           if !(input.respond_to?(:to_h) || input.respond_to?(:to_unsafe_h))
             # We're not sure it'll act like a hash, so reject it:
-            result.add_problem(INVALID_OBJECT_MESSAGE % { object: JSON.generate(input, quirks_mode: true) })
-            return result
+            return GraphQL::Query::InputValidationResult.from_problem(INVALID_OBJECT_MESSAGE % { object: JSON.generate(input, quirks_mode: true) })
           end
 
           # Inject missing required arguments
@@ -151,18 +147,22 @@ def validate_non_null_input(input, ctx)
             m
           end
 
-
+          result = nil
           [input, missing_required_inputs].each do |args_to_validate|
             args_to_validate.each do |argument_name, value|
               argument = warden.get_argument(self, argument_name)
               # Items in the input that are unexpected
-              unless argument
+              if argument.nil?
+                result ||= Query::InputValidationResult.new
                 result.add_problem("Field is not defined on #{self.graphql_name}", [argument_name])
-                next
+              else
+                # Items in the input that are expected, but have invalid values
+                argument_result = argument.type.validate_input(value, ctx)
+                result ||= Query::InputValidationResult.new
+                if !argument_result.valid?
+                  result.merge_result!(argument_name, argument_result)
+                end
               end
-              # Items in the input that are expected, but have invalid values
-              argument_result = argument.type.validate_input(value, ctx)
-              result.merge_result!(argument_name, argument_result) unless argument_result.valid?
             end
           end
 

lib/graphql/schema/input_object.rb

@@ -46,10 +46,11 @@ def coerce_input(value, ctx)
       end
 
       def validate_non_null_input(value, ctx)
-        result = GraphQL::Query::InputValidationResult.new
+        result = nil
         ensure_array(value).each_with_index do |item, index|
           item_result = of_type.validate_input(item, ctx)
           if !item_result.valid?
+            result ||= GraphQL::Query::InputValidationResult.new
             result.merge_result!(index, item_result)
           end
         end

lib/graphql/schema/list.rb

@@ -10,9 +10,9 @@ def valid_input?(val, ctx)
 
         def validate_input(val, ctx)
           if val.nil?
-            GraphQL::Query::InputValidationResult.new
+            Query::InputValidationResult::VALID
           else
-            validate_non_null_input(val, ctx)
+            validate_non_null_input(val, ctx) || Query::InputValidationResult::VALID
           end
         end
 

lib/graphql/schema/member/validates_input.rb

@@ -41,7 +41,6 @@ def default_scalar?
         end
 
         def validate_non_null_input(value, ctx)
-          result = Query::InputValidationResult.new
           coerced_result = begin
             ctx.query.with_error_handling do
               coerce_input(value, ctx)
@@ -56,11 +55,12 @@ def validate_non_null_input(value, ctx)
             else
               " #{GraphQL::Language.serialize(value)}"
             end
-            result.add_problem("Could not coerce value#{str_value} to #{graphql_name}")
+            Query::InputValidationResult.from_problem("Could not coerce value#{str_value} to #{graphql_name}")
           elsif coerced_result.is_a?(GraphQL::CoercionError)
-            result.add_problem(coerced_result.message, message: coerced_result.message, extensions: coerced_result.extensions)
+            Query::InputValidationResult.from_problem(coerced_result.message, message: coerced_result.message, extensions: coerced_result.extensions)
+          else
+            nil
           end
-          result
         end
       end
     end