Skip to content

Commit

Permalink
Merge pull request #7 from dgraph-io/jatin/GraphQl-789
Browse files Browse the repository at this point in the history 
Fixes GRAPHQL-789
This PR allows coercing of single values in variables to list.
  • Loading branch information
@JatinDev543
JatinDev543 authored Nov 3, 2020
2 parents 11dafed + 8dcf919 commit c23012d
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 41 deletions.
7 changes: 6 additions & 1 deletion validator/testdata/vars.graphql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Query {
intArrayArg(i: [Int]): Boolean!
stringArrayArg(i: [String]): Boolean!
boolArrayArg(i: [Boolean]): Boolean!
typeArrayArg(i: [CustomType]): Boolean!
}

input InputType {
Expand All @@ -26,8 +27,12 @@ input Embedded {
name: String!
}

input CustomType {
and: [Int!]
}

enum Enum {
A
}

scalar Custom
scalar Custom
69 changes: 33 additions & 36 deletions validator/vars.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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")
}
}

Expand All @@ -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
}
29 changes: 25 additions & 4 deletions validator/vars_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit c23012d

Please sign in to comment.