Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix support for null literals #536

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix support for null literals #536

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
32f1c98
Add support for null literals
Sep 23, 2018
99cf6bd
Fix list element index in error messages
mpenick Mar 20, 2020
c6692af
Fix support for null literals
mpenick Mar 20, 2020
fdb30c6
Remove leftover changes
mpenick Apr 8, 2020
e1fd52f
Fix formatting
mpenick Apr 8, 2020
50ac617
Remove debug code
mpenick Apr 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions definition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,6 @@ func GetNullable(ttype Type) Nullable {
return ttype
}

// NullValue to be able to detect if a value is set to null or if it is omitted
type NullValue struct {}

// Named interface for types that do not include modifiers like List or NonNull.
type Named interface {
String() string
Expand Down
297 changes: 297 additions & 0 deletions graphql_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,300 @@ func TestEmptyStringIsNotNull(t *testing.T) {
t.Errorf("wrong result, query: %v, graphql result diff: %v", query, testutil.Diff(expected, result))
}
}

func TestNullLiteralArguments(t *testing.T) {
checkForNull := func(p graphql.ResolveParams) (interface{}, error) {
arg, ok := p.Args["arg"]
if !ok || arg != nil {
t.Errorf("expected null for input arg, got %#v", arg)
}
return "yay", nil
}
schema, err := graphql.NewSchema(graphql.SchemaConfig{
Query: graphql.NewObject(graphql.ObjectConfig{
Name: "Query",
Fields: graphql.Fields{
"checkNullStringArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.String},
},
Resolve: checkForNull,
},
"checkNullIntArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.Int},
},
Resolve: checkForNull,
},
"checkNullBooleanArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.Boolean},
},
Resolve: checkForNull,
},
"checkNullListArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.NewList(graphql.String)},
},
Resolve: checkForNull,
},
"checkNullInputObjectArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.NewInputObject(
graphql.InputObjectConfig{
Name: "InputType",
Fields: graphql.InputObjectConfigFieldMap{
"field1": {Type: graphql.String},
"field2": {Type: graphql.Int},
},
})},
},
Resolve: checkForNull,
},
},
}),
})
if err != nil {
t.Fatalf("wrong result, unexpected errors: %v", err.Error())
}
query := `{ checkNullStringArg(arg:null) checkNullIntArg(arg:null) checkNullBooleanArg(arg:null) checkNullListArg(arg:null) checkNullInputObjectArg(arg:null) }`

result := graphql.Do(graphql.Params{
Schema: schema,
RequestString: query,
})
if len(result.Errors) > 0 {
t.Fatalf("wrong result, unexpected errors: %v", result.Errors)
}
expected := map[string]interface{}{
"checkNullStringArg": "yay", "checkNullIntArg": "yay",
"checkNullBooleanArg": "yay", "checkNullListArg": "yay",
"checkNullInputObjectArg": "yay"}
if !reflect.DeepEqual(result.Data, expected) {
t.Errorf("wrong result, query: %v, graphql result diff: %v", query, testutil.Diff(expected, result))
}
}

func TestNullLiteralDefaultVariableValue(t *testing.T) {
checkForNull := func(p graphql.ResolveParams) (interface{}, error) {
arg, ok := p.Args["arg"]
if !ok || arg != nil {
t.Errorf("expected null for input arg, got %#v", arg)
}
return "yay", nil
}
schema, err := graphql.NewSchema(graphql.SchemaConfig{
Query: graphql.NewObject(graphql.ObjectConfig{
Name: "Query",
Fields: graphql.Fields{
"checkNullStringArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.String},
},
Resolve: checkForNull,
},
},
}),
})
if err != nil {
t.Fatalf("wrong result, unexpected errors: %v", err.Error())
}
query := `query Test($value: String = null) { checkNullStringArg(arg: $value) }`

result := graphql.Do(graphql.Params{
Schema: schema,
RequestString: query,
VariableValues: map[string]interface{}{"value2": nil},
})
if len(result.Errors) > 0 {
t.Fatalf("wrong result, unexpected errors: %v", result.Errors)
}
expected := map[string]interface{}{ "checkNullStringArg": "yay", }
if !reflect.DeepEqual(result.Data, expected) {
t.Errorf("wrong result, query: %v, graphql result diff: %v", query, testutil.Diff(expected, result))
}
}

func TestNullLiteralVariables(t *testing.T) {
checkForNull := func(p graphql.ResolveParams) (interface{}, error) {
arg, ok := p.Args["arg"]
if !ok || arg != nil {
t.Errorf("expected null for input arg, got %#v", arg)
}
return "yay", nil
}
schema, err := graphql.NewSchema(graphql.SchemaConfig{
Query: graphql.NewObject(graphql.ObjectConfig{
Name: "Query",
Fields: graphql.Fields{
"checkNullStringArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.String},
},
Resolve: checkForNull,
},
},
}),
})
if err != nil {
t.Fatalf("wrong result, unexpected errors: %v", err.Error())
}
query := `query Test($value: String) { checkNullStringArg(arg: $value) }`

result := graphql.Do(graphql.Params{
Schema: schema,
RequestString: query,
VariableValues: map[string]interface{}{"value": nil},
})
if len(result.Errors) > 0 {
t.Fatalf("wrong result, unexpected errors: %v", result.Errors)
}
expected := map[string]interface{}{ "checkNullStringArg": "yay", }
if !reflect.DeepEqual(result.Data, expected) {
t.Errorf("wrong result, query: %v, graphql result diff: %v", query, testutil.Diff(expected, result))
}
}

func TestErrorNullLiteralForNotNullArgument(t *testing.T) {
checkNotCalled := func(p graphql.ResolveParams) (interface{}, error) {
t.Error("shouldn't have been called")
return nil, nil
}
schema, err := graphql.NewSchema(graphql.SchemaConfig{
Query: graphql.NewObject(graphql.ObjectConfig{
Name: "Query",
Fields: graphql.Fields{
"checkNotNullArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.NewNonNull(graphql.String) },
},
Resolve: checkNotCalled,
},
},
}),
})
if err != nil {
t.Fatalf("wrong result, unexpected errors: %v", err.Error())
}
query := `{ checkNotNullArg(arg:null) }`

result := graphql.Do(graphql.Params{
Schema: schema,
RequestString: query,
})

if len(result.Errors) == 0 {
t.Fatalf("expected errors, got: %v", result)
}

expectedMessage := `Argument "arg" has invalid value <nil>.
Expected "String!", found null.`;

if result.Errors[0].Message != expectedMessage {
t.Fatalf("unexpected error.\nexpected:\n%s\ngot:\n%s\n", expectedMessage, result.Errors[0].Message)
}
}

func TestNullInputObjectFields(t *testing.T) {
checkForNull := func(p graphql.ResolveParams) (interface{}, error) {
arg := p.Args["arg"]
expectedValue := map[string]interface{}{ "field1": nil, "field2": nil, "field3": nil, "field4" : "abc", "field5": 42, "field6": true}
if value, ok := arg.(map[string]interface{}); !ok {
t.Errorf("expected map[string]interface{} for input arg, got %#v", arg)
} else if !reflect.DeepEqual(expectedValue, value) {
t.Errorf("unexpected input object, diff: %v", testutil.Diff(expectedValue, value))
}
return "yay", nil
}
schema, err := graphql.NewSchema(graphql.SchemaConfig{
Query: graphql.NewObject(graphql.ObjectConfig{
Name: "Query",
Fields: graphql.Fields{
"checkNullInputObjectFields": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.NewInputObject(
graphql.InputObjectConfig{
Name: "InputType",
Fields: graphql.InputObjectConfigFieldMap{
"field1": {Type: graphql.String},
"field2": {Type: graphql.Int},
"field3": {Type: graphql.Boolean},
"field4": {Type: graphql.String},
"field5": {Type: graphql.Int},
"field6": {Type: graphql.Boolean},
},
})},
},
Resolve: checkForNull,
},
},
}),
})
if err != nil {
t.Fatalf("wrong result, unexpected errors: %v", err.Error())
}
query := `{ checkNullInputObjectFields(arg: {field1: null, field2: null, field3: null, field4: "abc", field5: 42, field6: true }) }`

result := graphql.Do(graphql.Params{
Schema: schema,
RequestString: query,
})
if len(result.Errors) > 0 {
t.Fatalf("wrong result, unexpected errors: %v", result.Errors)
}
expected := map[string]interface{}{ "checkNullInputObjectFields": "yay" }
if !reflect.DeepEqual(result.Data, expected) {
t.Errorf("wrong result, query: %v, graphql result diff: %v", query, testutil.Diff(expected, result))
}
}

func TestErrorNullInList(t *testing.T) {
checkNotCalled := func(p graphql.ResolveParams) (interface{}, error) {
t.Error("shouldn't have been called")
return nil, nil
}
schema, err := graphql.NewSchema(graphql.SchemaConfig{
Query: graphql.NewObject(graphql.ObjectConfig{
Name: "Query",
Fields: graphql.Fields{
"checkNotNullInListArg": &graphql.Field{
Type: graphql.String,
Args: graphql.FieldConfigArgument{
"arg": &graphql.ArgumentConfig{Type: graphql.NewList(graphql.String) },
},
Resolve: checkNotCalled,
},
},
}),
})
if err != nil {
t.Fatalf("wrong result, unexpected errors: %v", err.Error())
}
query := `{ checkNotNullInListArg(arg: [null, null]) }`

result := graphql.Do(graphql.Params{
Schema: schema,
RequestString: query,
})

if len(result.Errors) == 0 {
t.Fatalf("expected errors, got: %v", result)
}

expectedMessage := `Argument "arg" has invalid value [<nil>, <nil>].
In element #1: Unexpected null literal.
In element #2: Unexpected null literal.`

if result.Errors[0].Message != expectedMessage {
t.Fatalf("unexpected error.\nexpected:\n%s\ngot:\n%s\n", expectedMessage, result.Errors[0].Message)
}
}
11 changes: 7 additions & 4 deletions rules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1730,8 +1730,6 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) {
return true, nil
}

// This function only tests literals, and assumes variables will provide
// values of the correct type.
if valueAST.GetKind() == kinds.NullValue {
return true, nil
}
Expand All @@ -1748,7 +1746,7 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) {
if e := ttype.Error(); e != nil {
return false, []string{e.Error()}
}
if valueAST == nil {
if valueAST == nil || valueAST.GetKind() == kinds.NullValue {
if ttype.OfType.Name() != "" {
return false, []string{fmt.Sprintf(`Expected "%v!", found null.`, ttype.OfType.Name())}
}
Expand All @@ -1762,7 +1760,12 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) {
if valueAST, ok := valueAST.(*ast.ListValue); ok {
messagesReduce := []string{}
for idx, value := range valueAST.Values {
_, messages := isValidLiteralValue(itemType, value)
var messages []string
if value.GetKind() == kinds.NullValue {
messages = []string{"Unexpected null literal."}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpenick Is there a reason for this disallowing null in a list? There's even a test for it so it doesn't seem to be an accident.

Section 2.9.7 seems to allow nulls in a list.
https://spec.graphql.org/June2018/#ListValue

} else {
_, messages = isValidLiteralValue(itemType, value)
}
for _, message := range messages {
messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, idx+1, message))
}
Expand Down
12 changes: 0 additions & 12 deletions scalars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ var Int = NewScalar(ScalarConfig{
if intValue, err := strconv.Atoi(valueAST.Value); err == nil {
return intValue
}
case *ast.NullValue:
return NullValue{}
}
return nil
},
Expand Down Expand Up @@ -301,8 +299,6 @@ var Float = NewScalar(ScalarConfig{
if floatValue, err := strconv.ParseFloat(valueAST.Value, 64); err == nil {
return floatValue
}
case *ast.NullValue:
return NullValue{}
}
return nil
},
Expand Down Expand Up @@ -330,8 +326,6 @@ var String = NewScalar(ScalarConfig{
switch valueAST := valueAST.(type) {
case *ast.StringValue:
return valueAST.Value
case *ast.NullValue:
return NullValue{}
}

return nil
Expand Down Expand Up @@ -492,8 +486,6 @@ var Boolean = NewScalar(ScalarConfig{
switch valueAST := valueAST.(type) {
case *ast.BooleanValue:
return valueAST.Value
case *ast.NullValue:
return NullValue{}
}
return nil
},
Expand All @@ -515,8 +507,6 @@ var ID = NewScalar(ScalarConfig{
return valueAST.Value
case *ast.StringValue:
return valueAST.Value
case *ast.NullValue:
return NullValue{}
}
return nil
},
Expand Down Expand Up @@ -576,8 +566,6 @@ var DateTime = NewScalar(ScalarConfig{
case *ast.StringValue:
return unserializeDateTime(valueAST.Value)
return valueAST.Value
case *ast.NullValue:
return NullValue{}
}
return nil
},
Expand Down