Merge pull request #7 from dgraph-io/jatin/GraphQl-789

Fixes GRAPHQL-789
This PR allows coercing of single values in variables to list.

Author: JatinDev543

validator/testdata/vars.graphql

@@ -12,6 +12,7 @@ type Query {
     intArrayArg(i: [Int]): Boolean!
     stringArrayArg(i: [String]): Boolean!
     boolArrayArg(i: [Boolean]): Boolean!
+    typeArrayArg(i: [CustomType]): Boolean!
 }
 
 input InputType {
@@ -26,8 +27,12 @@ input Embedded {
     name: String!
 }
 
+input CustomType {
+    and: [Int!]
+}
+
 enum Enum {
     A
 }
 
-scalar Custom
+scalar Custom

validator/vars.go

@@ -54,17 +54,16 @@ func VariableValues(schema *ast.Schema, op *ast.OperationDefinition, variables m
 					rv = rv.Elem()
 				}
 
-				if err := validator.validateVarType(v.Type, rv); err != nil {
+				rval, err := validator.validateVarType(v.Type, rv)
+				if err != nil {
 					return nil, err
 				}
-
-				coercedVars[v.Variable] = val
+				coercedVars[v.Variable] = rval.Interface()
 			}
 		}
 
 		validator.path = validator.path[0 : len(validator.path)-1]
 	}
-
 	return coercedVars, nil
 }
 
@@ -73,53 +72,52 @@ type varValidator struct {
 	schema *ast.Schema
 }
 
-func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerror.Error {
+func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) (reflect.Value, *gqlerror.Error) {
 	currentPath := v.path
 	resetPath := func() {
 		v.path = currentPath
 	}
 	defer resetPath()
-
 	if typ.Elem != nil {
 		if val.Kind() != reflect.Slice {
-			return gqlerror.ErrorPathf(v.path, "must be an array")
+			// GraphQL spec says that non-null values should be coerced to an array when possible.
+			// Hence if the value is not a slice, we create a slice and add val to it.
+			slc := reflect.MakeSlice(reflect.SliceOf(val.Type()), 0, 0)
+			slc = reflect.Append(slc, val)
+			val = slc
 		}
-
 		for i := 0; i < val.Len(); i++ {
 			resetPath()
 			v.path = append(v.path, ast.PathIndex(i))
 			field := val.Index(i)
-
 			if field.Kind() == reflect.Ptr || field.Kind() == reflect.Interface {
 				if typ.Elem.NonNull && field.IsNil() {
-					return gqlerror.ErrorPathf(v.path, "cannot be null")
+					return val, gqlerror.ErrorPathf(v.path, "cannot be null")
 				}
 				field = field.Elem()
 			}
-
-			if err := v.validateVarType(typ.Elem, field); err != nil {
-				return err
+			_, err := v.validateVarType(typ.Elem, field)
+			if err != nil {
+				return val, err
 			}
 		}
-
-		return nil
+		return val, nil
 	}
-
 	def := v.schema.Types[typ.NamedType]
 	if def == nil {
 		panic(fmt.Errorf("missing def for %s", typ.NamedType))
 	}
 
 	if !typ.NonNull && !val.IsValid() {
 		// If the type is not null and we got a invalid value namely null/nil, then it's valid
-		return nil
+		return val, nil
 	}
 
 	switch def.Kind {
 	case ast.Enum:
 		kind := val.Type().Kind()
 		if kind != reflect.Int && kind != reflect.Int32 && kind != reflect.Int64 && kind != reflect.String {
-			return gqlerror.ErrorPathf(v.path, "enums must be ints or strings")
+			return val, gqlerror.ErrorPathf(v.path, "enums must be ints or strings")
 		}
 		isValidEnum := false
 		for _, enumVal := range def.EnumValues {
@@ -128,42 +126,42 @@ func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerr
 			}
 		}
 		if !isValidEnum {
-			return gqlerror.ErrorPathf(v.path, "%s is not a valid %s", val.String(), def.Name)
+			return val, gqlerror.ErrorPathf(v.path, "%s is not a valid %s", val.String(), def.Name)
 		}
-		return nil
+		return val, nil
 	case ast.Scalar:
 		kind := val.Type().Kind()
 		switch typ.NamedType {
 		case "Int":
 			if kind == reflect.String || kind == reflect.Int || kind == reflect.Int32 || kind == reflect.Int64 {
-				return nil
+				return val, nil
 			}
 		case "Float":
 			if kind == reflect.String || kind == reflect.Float32 || kind == reflect.Float64 || kind == reflect.Int || kind == reflect.Int32 || kind == reflect.Int64 {
-				return nil
+				return val, nil
 			}
 		case "String":
 			if kind == reflect.String {
-				return nil
+				return val, nil
 			}
 
 		case "Boolean":
 			if kind == reflect.Bool {
-				return nil
+				return val, nil
 			}
 
 		case "ID":
 			if kind == reflect.Int || kind == reflect.Int32 || kind == reflect.Int64 || kind == reflect.String {
-				return nil
+				return val, nil
 			}
 		default:
 			// assume custom scalars are ok
-			return nil
+			return val, nil
 		}
-		return gqlerror.ErrorPathf(v.path, "cannot use %s as %s", kind.String(), typ.NamedType)
+		return val, gqlerror.ErrorPathf(v.path, "cannot use %s as %s", kind.String(), typ.NamedType)
 	case ast.InputObject:
 		if val.Kind() != reflect.Map {
-			return gqlerror.ErrorPathf(v.path, "must be a %s", def.Name)
+			return val, gqlerror.ErrorPathf(v.path, "must be a %s", def.Name)
 		}
 
 		// check for unknown fields
@@ -174,7 +172,7 @@ func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerr
 			v.path = append(v.path, ast.PathName(name.String()))
 
 			if fieldDef == nil {
-				return gqlerror.ErrorPathf(v.path, "unknown field")
+				return val, gqlerror.ErrorPathf(v.path, "unknown field")
 			}
 		}
 
@@ -192,30 +190,29 @@ func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerr
 							continue
 						}
 					}
-					return gqlerror.ErrorPathf(v.path, "must be defined")
+					return val, gqlerror.ErrorPathf(v.path, "must be defined")
 				}
 				continue
 			}
 
 			if field.Kind() == reflect.Ptr || field.Kind() == reflect.Interface {
 				if fieldDef.Type.NonNull && field.IsNil() {
-					return gqlerror.ErrorPathf(v.path, "cannot be null")
+					return val, gqlerror.ErrorPathf(v.path, "cannot be null")
 				}
 				//allow null object field and skip it
 				if !fieldDef.Type.NonNull && field.IsNil() {
 					continue
 				}
 				field = field.Elem()
 			}
-
-			err := v.validateVarType(fieldDef.Type, field)
+			cval, err := v.validateVarType(fieldDef.Type, field)
 			if err != nil {
-				return err
+				return val, err
 			}
+			val.SetMapIndex(reflect.ValueOf(fieldDef.Name), cval)
 		}
 	default:
 		panic(fmt.Errorf("unsupported type %s", def.Kind))
 	}
-
-	return nil
+	return val, nil
 }

validator/vars_test.go

@@ -151,12 +151,33 @@ func TestValidateVars(t *testing.T) {
 	})
 
 	t.Run("array", func(t *testing.T) {
-		t.Run("non array", func(t *testing.T) {
+		t.Run("non-null object value should be coerced to an array", func(t *testing.T) {
 			q := gqlparser.MustLoadQuery(schema, `query foo($var: [InputType!]) { arrayArg(i: $var) }`)
-			_, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{
-				"var": "hello",
+			vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{
+				"var": map[string]interface{}{"name": "hello"},
+			})
+			require.Nil(t, gerr)
+			require.EqualValues(t, []map[string]interface{}{{"name": "hello"}}, vars["var"])
+		})
+
+		t.Run("non-null int value should be coerced to an array", func(t *testing.T) {
+			q := gqlparser.MustLoadQuery(schema, `query foo($var: [Int!]) { intArrayArg(i: $var) }`)
+			vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{
+				"var": 5,
 			})
-			require.EqualError(t, gerr, "input: variable.var must be an array")
+			require.Nil(t, gerr)
+			expected := []int{5}
+			require.EqualValues(t, expected, vars["var"])
+		})
+
+		t.Run("non-null int deep value should be coerced to an array", func(t *testing.T) {
+			q := gqlparser.MustLoadQuery(schema, `query foo($var: [CustomType]) { typeArrayArg(i: $var) }`)
+			vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{
+				"var": []map[string]interface{}{{"and": 5}},
+			})
+			require.Nil(t, gerr)
+			expected := []map[string]interface{}{{"and": []int{5}}}
+			require.EqualValues(t, expected, vars["var"])
 		})
 
 		t.Run("defaults", func(t *testing.T) {