From 32f1c98c4de8ffd0735ec1bf637d62923e75023d Mon Sep 17 00:00:00 2001 From: Mike Date: Sun, 23 Sep 2018 20:28:22 +1000 Subject: [PATCH 1/6] Add support for null literals --- definition.go | 3 +++ language/ast/node.go | 1 + language/ast/values.go | 29 +++++++++++++++++++++++++++++ language/kinds/kinds.go | 1 + language/parser/parser.go | 10 +++++++++- language/parser/parser_test.go | 10 +--------- language/printer/printer.go | 9 +++++++++ rules.go | 6 ++++++ scalars.go | 14 ++++++++++++++ 9 files changed, 73 insertions(+), 10 deletions(-) diff --git a/definition.go b/definition.go index 4b132991..a6d011d6 100644 --- a/definition.go +++ b/definition.go @@ -158,6 +158,9 @@ 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 diff --git a/language/ast/node.go b/language/ast/node.go index cd63a0fc..d7cc3a80 100644 --- a/language/ast/node.go +++ b/language/ast/node.go @@ -22,6 +22,7 @@ var _ Node = (*IntValue)(nil) var _ Node = (*FloatValue)(nil) var _ Node = (*StringValue)(nil) var _ Node = (*BooleanValue)(nil) +var _ Node = (*NullValue)(nil) var _ Node = (*EnumValue)(nil) var _ Node = (*ListValue)(nil) var _ Node = (*ObjectValue)(nil) diff --git a/language/ast/values.go b/language/ast/values.go index 6c3c8864..54fa1861 100644 --- a/language/ast/values.go +++ b/language/ast/values.go @@ -16,6 +16,7 @@ var _ Value = (*IntValue)(nil) var _ Value = (*FloatValue)(nil) var _ Value = (*StringValue)(nil) var _ Value = (*BooleanValue)(nil) +var _ Value = (*NullValue)(nil) var _ Value = (*EnumValue)(nil) var _ Value = (*ListValue)(nil) var _ Value = (*ObjectValue)(nil) @@ -172,6 +173,34 @@ func (v *BooleanValue) GetValue() interface{} { return v.Value } +// NullValue implements Node, Value +type NullValue struct { + Kind string + Loc *Location + Value interface{} +} + +func NewNullValue(v *NullValue) *NullValue { + + return &NullValue{ + Kind: kinds.NullValue, + Loc: v.Loc, + Value: v.Value, + } +} + +func (v *NullValue) GetKind() string { + return v.Kind +} + +func (v *NullValue) GetLoc() *Location { + return v.Loc +} + +func (v *NullValue) GetValue() interface{} { + return nil +} + // EnumValue implements Node, Value type EnumValue struct { Kind string diff --git a/language/kinds/kinds.go b/language/kinds/kinds.go index 40bc994e..f70b1a3c 100644 --- a/language/kinds/kinds.go +++ b/language/kinds/kinds.go @@ -23,6 +23,7 @@ const ( FloatValue = "FloatValue" StringValue = "StringValue" BooleanValue = "BooleanValue" + NullValue = "NullValue" EnumValue = "EnumValue" ListValue = "ListValue" ObjectValue = "ObjectValue" diff --git a/language/parser/parser.go b/language/parser/parser.go index 4ee1577c..d2900b6a 100644 --- a/language/parser/parser.go +++ b/language/parser/parser.go @@ -610,7 +610,15 @@ func parseValueLiteral(parser *Parser, isConst bool) (ast.Value, error) { Value: value, Loc: loc(parser, token.Start), }), nil - } else if token.Value != "null" { + } else if token.Value == "null" { + if err := advance(parser); err != nil { + return nil, err + } + return ast.NewNullValue(&ast.NullValue{ + Value: nil, + Loc: loc(parser, token.Start), + }), nil + } else { if err := advance(parser); err != nil { return nil, err } diff --git a/language/parser/parser_test.go b/language/parser/parser_test.go index 3cc4253a..ec260ea2 100644 --- a/language/parser/parser_test.go +++ b/language/parser/parser_test.go @@ -183,15 +183,6 @@ func TestDoesNotAcceptFragmentsSpreadOfOn(t *testing.T) { testErrorMessage(t, test) } -func TestDoesNotAllowNullAsValue(t *testing.T) { - test := errorMessageTest{ - `{ fieldWithNullableStringInput(input: null) }'`, - `Syntax Error GraphQL (1:39) Unexpected Name "null"`, - false, - } - testErrorMessage(t, test) -} - func TestParsesMultiByteCharacters_Unicode(t *testing.T) { doc := ` @@ -367,6 +358,7 @@ func TestAllowsNonKeywordsAnywhereNameIsAllowed(t *testing.T) { "subscription", "true", "false", + "null", } for _, keyword := range nonKeywords { fragmentName := keyword diff --git a/language/printer/printer.go b/language/printer/printer.go index eb4d5ec1..aaf23833 100644 --- a/language/printer/printer.go +++ b/language/printer/printer.go @@ -367,6 +367,15 @@ var printDocASTReducer = map[string]visitor.VisitFunc{ } return visitor.ActionNoChange, nil }, + "NullValue": func(p visitor.VisitFuncParams) (string, interface{}) { + switch node := p.Node.(type) { + case *ast.NullValue: + return visitor.ActionUpdate, fmt.Sprintf("%v", node.Value) + case map[string]interface{}: + return visitor.ActionUpdate, getMapValueString(node, "Value") + } + return visitor.ActionNoChange, nil + }, "EnumValue": func(p visitor.VisitFuncParams) (string, interface{}) { switch node := p.Node.(type) { case *ast.EnumValue: diff --git a/rules.go b/rules.go index ae0c75b9..4d9ae78c 100644 --- a/rules.go +++ b/rules.go @@ -1730,6 +1730,12 @@ 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 + } + // This function only tests literals, and assumes variables will provide // values of the correct type. if valueAST.GetKind() == kinds.Variable { diff --git a/scalars.go b/scalars.go index 45479b54..0b40ff01 100644 --- a/scalars.go +++ b/scalars.go @@ -162,6 +162,8 @@ var Int = NewScalar(ScalarConfig{ if intValue, err := strconv.Atoi(valueAST.Value); err == nil { return intValue } + case *ast.NullValue: + return NullValue{} } return nil }, @@ -299,6 +301,8 @@ var Float = NewScalar(ScalarConfig{ if floatValue, err := strconv.ParseFloat(valueAST.Value, 64); err == nil { return floatValue } + case *ast.NullValue: + return NullValue{} } return nil }, @@ -326,7 +330,10 @@ var String = NewScalar(ScalarConfig{ switch valueAST := valueAST.(type) { case *ast.StringValue: return valueAST.Value + case *ast.NullValue: + return NullValue{} } + return nil }, }) @@ -485,6 +492,8 @@ var Boolean = NewScalar(ScalarConfig{ switch valueAST := valueAST.(type) { case *ast.BooleanValue: return valueAST.Value + case *ast.NullValue: + return NullValue{} } return nil }, @@ -506,6 +515,8 @@ var ID = NewScalar(ScalarConfig{ return valueAST.Value case *ast.StringValue: return valueAST.Value + case *ast.NullValue: + return NullValue{} } return nil }, @@ -564,6 +575,9 @@ var DateTime = NewScalar(ScalarConfig{ switch valueAST := valueAST.(type) { case *ast.StringValue: return unserializeDateTime(valueAST.Value) + return valueAST.Value + case *ast.NullValue: + return NullValue{} } return nil }, From 99cf6bd01a13ea3e3092758c814d746c2cc591d0 Mon Sep 17 00:00:00 2001 From: Michael Penick Date: Fri, 20 Mar 2020 15:59:41 -0400 Subject: [PATCH 2/6] Fix list element index in error messages --- rules.go | 4 ++-- rules_arguments_of_correct_type_test.go | 4 ++-- rules_default_values_of_correct_type_test.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/rules.go b/rules.go index 4d9ae78c..fcdad391 100644 --- a/rules.go +++ b/rules.go @@ -1761,9 +1761,9 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) { itemType, _ := ttype.OfType.(Input) if valueAST, ok := valueAST.(*ast.ListValue); ok { messagesReduce := []string{} - for _, value := range valueAST.Values { + for idx, value := range valueAST.Values { _, messages := isValidLiteralValue(itemType, value) - for idx, message := range messages { + for _, message := range messages { messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, idx+1, message)) } } diff --git a/rules_arguments_of_correct_type_test.go b/rules_arguments_of_correct_type_test.go index ecd4bea4..a8c51395 100644 --- a/rules_arguments_of_correct_type_test.go +++ b/rules_arguments_of_correct_type_test.go @@ -500,7 +500,7 @@ func TestValidate_ArgValuesOfCorrectType_InvalidListValue_IncorrectItemType(t *t `, []gqlerrors.FormattedError{ testutil.RuleError( - "Argument \"stringListArg\" has invalid value [\"one\", 2].\nIn element #1: Expected type \"String\", found 2.", + "Argument \"stringListArg\" has invalid value [\"one\", 2].\nIn element #2: Expected type \"String\", found 2.", 4, 47, ), }) @@ -742,7 +742,7 @@ func TestValidate_ArgValuesOfCorrectType_InvalidInputObjectValue_PartialObject_I `, []gqlerrors.FormattedError{ testutil.RuleError( - "Argument \"complexArg\" has invalid value {stringListField: [\"one\", 2], requiredField: true}.\nIn field \"stringListField\": In element #1: Expected type \"String\", found 2.", + "Argument \"complexArg\" has invalid value {stringListField: [\"one\", 2], requiredField: true}.\nIn field \"stringListField\": In element #2: Expected type \"String\", found 2.", 4, 41, ), }) diff --git a/rules_default_values_of_correct_type_test.go b/rules_default_values_of_correct_type_test.go index 8457b388..a496c1cd 100644 --- a/rules_default_values_of_correct_type_test.go +++ b/rules_default_values_of_correct_type_test.go @@ -97,7 +97,7 @@ func TestValidate_VariableDefaultValuesOfCorrectType_ListVariablesWithInvalidIte []gqlerrors.FormattedError{ testutil.RuleError( `Variable "$a" has invalid default value: ["one", 2].`+ - "\nIn element #1: Expected type \"String\", found 2.", + "\nIn element #2: Expected type \"String\", found 2.", 2, 40), }) } From c6692af772ea779d623d0ef841f496dd1eb89dd3 Mon Sep 17 00:00:00 2001 From: Michael Penick Date: Fri, 20 Mar 2020 16:02:05 -0400 Subject: [PATCH 3/6] Fix support for null literals --- definition.go | 3 - graphql_test.go | 297 ++++++++++++++++++++++++++++++++++++++++++++++ rules.go | 11 +- scalars.go | 12 -- values.go | 62 +++++++--- variables_test.go | 6 +- 6 files changed, 353 insertions(+), 38 deletions(-) diff --git a/definition.go b/definition.go index a6d011d6..4b132991 100644 --- a/definition.go +++ b/definition.go @@ -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 diff --git a/graphql_test.go b/graphql_test.go index 8b06a7b1..54926c4f 100644 --- a/graphql_test.go +++ b/graphql_test.go @@ -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 . +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 [, ]. +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) + } +} diff --git a/rules.go b/rules.go index fcdad391..4c0e239a 100644 --- a/rules.go +++ b/rules.go @@ -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 } @@ -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())} } @@ -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."} + } else { + _, messages = isValidLiteralValue(itemType, value) + } for _, message := range messages { messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, idx+1, message)) } diff --git a/scalars.go b/scalars.go index 0b40ff01..8b728aef 100644 --- a/scalars.go +++ b/scalars.go @@ -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 }, @@ -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 }, @@ -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 @@ -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 }, @@ -515,8 +507,6 @@ var ID = NewScalar(ScalarConfig{ return valueAST.Value case *ast.StringValue: return valueAST.Value - case *ast.NullValue: - return NullValue{} } return nil }, @@ -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 }, diff --git a/values.go b/values.go index 06c08af6..302d929f 100644 --- a/values.go +++ b/values.go @@ -14,6 +14,9 @@ import ( "github.com/graphql-go/graphql/language/printer" ) +// Used to detect the difference between a "null" literal and not present +type nullValue struct {} + // Prepares an object map of variableValues of the correct type based on the // provided variable definitions and arbitrary input. If the input cannot be // parsed to match the variable definitions, a GraphQLError will be returned. @@ -27,7 +30,7 @@ func getVariableValues( continue } varName := defAST.Variable.Name.Value - if varValue, err := getVariableValue(schema, defAST, inputs[varName]); err != nil { + if varValue, err := getVariableValue(schema, defAST, getValueOrNull(inputs, varName)); err != nil { return values, err } else { values[varName] = varValue @@ -36,6 +39,25 @@ func getVariableValues( return values, nil } +func getValueOrNull(values map[string]interface{}, name string) interface{} { + if tmp, ok := values[name]; ok { // Is present + if tmp == nil { + return nullValue{} // Null value + } else { + return tmp + } + } + return nil // Not present +} + +func addValueOrNull(values map[string]interface{}, name string, value interface{}) { + if _, ok := value.(nullValue); ok { // Null value + values[name] = nil + } else if !isNullish(value) { // Not present + values[name] = value + } +} + // Prepares an object map of argument values given a list of argument // definitions and list of argument AST nodes. func getArgumentValues( @@ -60,9 +82,10 @@ func getArgumentValues( if tmp = valueFromAST(value, argDef.Type, variableValues); isNullish(tmp) { tmp = argDef.DefaultValue } - if !isNullish(tmp) { - results[argDef.PrivateName] = tmp - } + addValueOrNull(results, argDef.PrivateName, tmp) + //if !isNullish(tmp) { + // results[argDef.PrivateName] = tmp + //} } return results } @@ -97,7 +120,7 @@ func getVariableValue(schema Schema, definitionAST *ast.VariableDefinition, inpu } return coerceValue(ttype, input), nil } - if isNullish(input) { + if _, ok := input.(nullValue); ok || isNullish(input) { return "", gqlerrors.NewError( fmt.Sprintf(`Variable "$%v" of required type `+ `"%v" was not provided.`, variable.Name.Value, printer.Print(definitionAST.Type)), @@ -134,6 +157,9 @@ func coerceValue(ttype Input, value interface{}) interface{} { if isNullish(value) { return nil } + if _, ok := value.(nullValue); ok { + return nullValue{} + } switch ttype := ttype.(type) { case *NonNull: return coerceValue(ttype.OfType, value) @@ -156,13 +182,11 @@ func coerceValue(ttype Input, value interface{}) interface{} { } for name, field := range ttype.Fields() { - fieldValue := coerceValue(field.Type, valueMap[name]) + fieldValue := coerceValue(field.Type, getValueOrNull(valueMap, name)) if isNullish(fieldValue) { fieldValue = field.DefaultValue } - if !isNullish(fieldValue) { - obj[name] = fieldValue - } + addValueOrNull(obj, name, fieldValue) } return obj case *Scalar: @@ -212,7 +236,7 @@ func typeFromAST(schema Schema, inputTypeAST ast.Type) (Type, error) { // accepted for that type. This is primarily useful for validating the // runtime values of query variables. func isValidInputValue(value interface{}, ttype Input) (bool, []string) { - if isNullish(value) { + if _, ok := value.(nullValue); ok || isNullish(value) { if ttype, ok := ttype.(*NonNull); ok { if ttype.OfType.Name() != "" { return false, []string{fmt.Sprintf(`Expected "%v!", found null.`, ttype.OfType.Name())} @@ -233,9 +257,14 @@ func isValidInputValue(value interface{}, ttype Input) (bool, []string) { messagesReduce := []string{} for i := 0; i < valType.Len(); i++ { val := valType.Index(i).Interface() - _, messages := isValidInputValue(val, ttype.OfType) - for idx, message := range messages { - messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, idx+1, message)) + var messages []string + if _, ok := val.(nullValue); ok { + messages = []string{"Unexpected null value."} + } else { + _, messages = isValidInputValue(val, ttype.OfType) + } + for _, message := range messages { + messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, i+1, message)) } } return (len(messagesReduce) == 0), messagesReduce @@ -352,6 +381,9 @@ func valueFromAST(valueAST ast.Value, ttype Input, variables map[string]interfac if valueAST == nil { return nil } + if valueAST.GetKind() == kinds.NullValue { + return nullValue{} + } // precedence: value > type if valueAST, ok := valueAST.(*ast.Variable); ok { if valueAST.Name == nil || variables == nil { @@ -398,9 +430,7 @@ func valueFromAST(valueAST ast.Value, ttype Input, variables map[string]interfac } else { value = field.DefaultValue } - if !isNullish(value) { - obj[name] = value - } + addValueOrNull(obj, name, value) } return obj case *Scalar: diff --git a/variables_test.go b/variables_test.go index 9dc430df..bcd49e60 100644 --- a/variables_test.go +++ b/variables_test.go @@ -67,7 +67,7 @@ var testNestedInputObject *graphql.InputObject = graphql.NewInputObject(graphql. func inputResolved(p graphql.ResolveParams) (interface{}, error) { input, ok := p.Args["input"] - if !ok { + if !ok || input == nil { return nil, nil } b, err := json.Marshal(input) @@ -1188,7 +1188,7 @@ func TestVariables_ListsAndNullability_DoesNotAllowListOfNonNullsToContainNull(t { Message: `Variable "$input" got invalid value ` + `["A",null,"B"].` + - "\nIn element #1: Expected \"String!\", found null.", + "\nIn element #2: Expected \"String!\", found null.", Locations: []location.SourceLocation{ { Line: 2, Column: 17, @@ -1290,7 +1290,7 @@ func TestVariables_ListsAndNullability_DoesNotAllowNonNullListOfNonNullsToContai { Message: `Variable "$input" got invalid value ` + `["A",null,"B"].` + - "\nIn element #1: Expected \"String!\", found null.", + "\nIn element #2: Expected \"String!\", found null.", Locations: []location.SourceLocation{ { Line: 2, Column: 17, From fdb30c69b33370a24c6122cfede5098ba3bfa307 Mon Sep 17 00:00:00 2001 From: Michael Penick Date: Wed, 8 Apr 2020 14:58:07 -0400 Subject: [PATCH 4/6] Remove leftover changes --- scalars.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/scalars.go b/scalars.go index 8b728aef..45479b54 100644 --- a/scalars.go +++ b/scalars.go @@ -327,7 +327,6 @@ var String = NewScalar(ScalarConfig{ case *ast.StringValue: return valueAST.Value } - return nil }, }) @@ -565,7 +564,6 @@ var DateTime = NewScalar(ScalarConfig{ switch valueAST := valueAST.(type) { case *ast.StringValue: return unserializeDateTime(valueAST.Value) - return valueAST.Value } return nil }, From e1fd52fc8a920dc404c854df48f4c8125fa2d1bc Mon Sep 17 00:00:00 2001 From: Michael Penick Date: Wed, 8 Apr 2020 14:59:30 -0400 Subject: [PATCH 5/6] Fix formatting --- language/kinds/kinds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/language/kinds/kinds.go b/language/kinds/kinds.go index f70b1a3c..370ba6fe 100644 --- a/language/kinds/kinds.go +++ b/language/kinds/kinds.go @@ -23,7 +23,7 @@ const ( FloatValue = "FloatValue" StringValue = "StringValue" BooleanValue = "BooleanValue" - NullValue = "NullValue" + NullValue = "NullValue" EnumValue = "EnumValue" ListValue = "ListValue" ObjectValue = "ObjectValue" From 50ac617737d69a9dd65f7ff5e7121859e3548d8f Mon Sep 17 00:00:00 2001 From: Michael Penick Date: Wed, 8 Apr 2020 15:01:35 -0400 Subject: [PATCH 6/6] Remove debug code --- values.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/values.go b/values.go index 302d929f..2d71db6b 100644 --- a/values.go +++ b/values.go @@ -83,9 +83,6 @@ func getArgumentValues( tmp = argDef.DefaultValue } addValueOrNull(results, argDef.PrivateName, tmp) - //if !isNullish(tmp) { - // results[argDef.PrivateName] = tmp - //} } return results }