diff --git a/argument_coercion_test.go b/argument_coercion_test.go new file mode 100644 index 0000000..ca26308 --- /dev/null +++ b/argument_coercion_test.go @@ -0,0 +1,1072 @@ +package graphql_test + +import ( + "encoding/json" + "sort" + "testing" + + "github.com/tailor-platform/graphql" + "github.com/tailor-platform/graphql/gqlerrors" + "github.com/tailor-platform/graphql/testutil" +) + +// Serialises p.Args so tests can tell "absent", "null", and "value" apart. +func probeArgs(p graphql.ResolveParams) (interface{}, error) { + keys := make([]string, 0, len(p.Args)) + for k := range p.Args { + keys = append(keys, k) + } + sort.Strings(keys) + out := map[string]interface{}{"keys": keys} + for _, k := range keys { + v := p.Args[k] + if v == nil { + out[k] = "null" + } else { + out[k] = v + } + } + b, _ := json.Marshal(out) + return string(b), nil +} + +// Serialises the "input" argument so tests can tell an absent input-object +// field apart from one present as null. +func probeObjectArgs(p graphql.ResolveParams) (interface{}, error) { + obj, _ := p.Args["input"].(map[string]interface{}) + keys := make([]string, 0, len(obj)) + for k := range obj { + keys = append(keys, k) + } + sort.Strings(keys) + b, _ := json.Marshal(map[string]interface{}{ + "keys": keys, + "obj": obj, + }) + return string(b), nil +} + +var coercionProbeInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeInput", + Fields: graphql.InputObjectConfigFieldMap{ + "a": &graphql.InputObjectFieldConfig{Type: graphql.String}, + "b": &graphql.InputObjectFieldConfig{Type: graphql.String}, + }, +}) + +// Same shape as coercionProbeInputObject, but field "a" declares a default so +// tests can pin down how a default interacts with absent / explicit null. +var coercionProbeDefaultInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeDefaultInput", + Fields: graphql.InputObjectConfigFieldMap{ + "a": &graphql.InputObjectFieldConfig{Type: graphql.String, DefaultValue: "FIELDDEF"}, + "b": &graphql.InputObjectFieldConfig{Type: graphql.String}, + }, +}) + +var coercionProbeNestedInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeNestedInput", + Fields: graphql.InputObjectConfigFieldMap{ + "inner": &graphql.InputObjectFieldConfig{Type: coercionProbeInputObject}, + }, +}) + +// Two levels deep, with the default declared on the innermost field, so the +// recursive coercion paths are exercised rather than just the top level. +var coercionProbeNestedDefaultInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeNestedDefaultInput", + Fields: graphql.InputObjectConfigFieldMap{ + "inner": &graphql.InputObjectFieldConfig{Type: coercionProbeDefaultInputObject}, + }, +}) + +// Three levels deep: proves the rule keeps holding as recursion gets deeper. +var coercionProbeDeepInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeDeepInput", + Fields: graphql.InputObjectConfigFieldMap{ + "level2": &graphql.InputObjectFieldConfig{Type: coercionProbeNestedDefaultInputObject}, + }, +}) + +// A non-null field that declares a default. Spec §3.10 and §5.6.4 make such a +// field optional, so a document may omit it and take the default. +var coercionProbeNonNullFieldDefaultInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeNonNullFieldDefaultInput", + Fields: graphql.InputObjectConfigFieldMap{ + "a": &graphql.InputObjectFieldConfig{ + Type: graphql.NewNonNull(graphql.String), + DefaultValue: "FIELDDEF", + }, + }, +}) + +// The same field one level down, so the recursive paths are exercised too. +var coercionProbeNonNullFieldNestedInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeNonNullFieldNestedInput", + Fields: graphql.InputObjectConfigFieldMap{ + "inner": &graphql.InputObjectFieldConfig{Type: coercionProbeNonNullFieldDefaultInputObject}, + }, +}) + +// A non-null field whose default is a typed nil pointer. isNullish treats that as +// no value, so coercion will not substitute it; validation has to reach the same +// conclusion or the field goes missing from the coerced map. +var coercionProbeNilString *string + +var coercionProbeNullishDefaultInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeNullishDefaultInput", + Fields: graphql.InputObjectConfigFieldMap{ + "a": &graphql.InputObjectFieldConfig{ + Type: graphql.NewNonNull(graphql.String), + DefaultValue: coercionProbeNilString, + }, + }, +}) + +// A non-null field with no default: genuinely required, so it stays required in +// both modes. +var coercionProbeRequiredInputObject = graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "CoercionProbeRequiredInput", + Fields: graphql.InputObjectConfigFieldMap{ + "a": &graphql.InputObjectFieldConfig{Type: graphql.NewNonNull(graphql.String)}, + }, +}) + +var coercionProbeType = graphql.NewObject(graphql.ObjectConfig{ + Name: "CoercionProbeQuery", + Fields: graphql.Fields{ + "probe": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{Type: graphql.String}, + "b": &graphql.ArgumentConfig{Type: graphql.String}, + }, + Resolve: probeArgs, + }, + "probeArgDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{Type: graphql.String, DefaultValue: "ARGDEF"}, + }, + Resolve: probeArgs, + }, + // Non-null argument carrying a default: spec §5.4.2.1 says it is + // optional, so omitting it must validate and resolve to the default. + "probeNonNullDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.String), + DefaultValue: "NNDEF", + }, + }, + Resolve: probeArgs, + }, + "probeList": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{Type: graphql.NewList(graphql.String)}, + }, + Resolve: probeArgs, + }, + "probeObject": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeObjectDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeDefaultInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeNested": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeNestedInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeNestedDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeNestedDefaultInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeDeep": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeDeepInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeObjectList": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: graphql.NewList(coercionProbeDefaultInputObject)}, + }, + Resolve: probeArgs, + }, + // Same argument as probeObject, but serialised with probeArgs so tests + // can tell an absent input-object argument from one that is null. + "probeObjectRaw": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeInputObject}, + }, + Resolve: probeArgs, + }, + "probeNonNullFieldDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeNonNullFieldDefaultInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeNonNullFieldNested": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeNonNullFieldNestedInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeNonNullFieldList": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: graphql.NewList(coercionProbeNonNullFieldDefaultInputObject)}, + }, + Resolve: probeArgs, + }, + // A list whose item type is non-null, with a default on the argument. Spec + // §5.8.5 draws hasLocationDefaultValue from the argument or input object + // field a usage sits in, and a list position carries none. + "probeNonNullItemList": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{ + Type: graphql.NewList(graphql.NewNonNull(graphql.String)), + DefaultValue: []interface{}{"LISTDEF"}, + }, + }, + Resolve: probeArgs, + }, + // A non-null argument whose default is a typed nil pointer: nullish, so + // coercion will not substitute it and the argument stays required. + "probeNullishArgDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.String), + DefaultValue: coercionProbeNilString, + }, + }, + Resolve: probeArgs, + }, + "probeNullishFieldDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeNullishDefaultInputObject}, + }, + Resolve: probeObjectArgs, + }, + "probeObjectRequired": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: coercionProbeRequiredInputObject}, + }, + Resolve: probeObjectArgs, + }, + }, +}) + +// The same probe types under both modes. Spec-compliant handling is the +// default, so the zero-valued config exercises the fix; +// NonSpecArgumentHandling opts back out and must reproduce the behaviour +// shipped before this change byte-for-byte. +var coercionProbeSpecSchema, _ = graphql.NewSchema(graphql.SchemaConfig{ + Query: coercionProbeType, +}) + +var coercionProbeNonSpecSchema, _ = graphql.NewSchema(graphql.SchemaConfig{ + Query: coercionProbeType, + NonSpecArgumentHandling: true, +}) + +func execProbe(t *testing.T, schema graphql.Schema, field, doc string, vars map[string]interface{}) string { + t.Helper() + parsed := testutil.TestParse(t, doc) + result := testutil.TestExecute(t, graphql.ExecuteParams{ + Schema: schema, + AST: parsed, + Args: vars, + }) + if len(result.Errors) > 0 { + t.Fatalf("unexpected errors: %v", result.Errors) + } + data, _ := result.Data.(map[string]interface{}) + got, _ := data[field].(string) + return got +} + +// runProbe asserts both coercion modes agree — the cases the flag does not +// change. +func runProbe(t *testing.T, field, doc string, vars map[string]interface{}, want string) { + t.Helper() + runProbeModes(t, field, doc, vars, want, want) +} + +// runProbeModes pins down a case where the flag changes the outcome: the +// non-spec column is the regression guard, the spec column is the fix. +func runProbeModes(t *testing.T, field, doc string, vars map[string]interface{}, wantNonSpec, wantSpec string) { + t.Helper() + if got := execProbe(t, coercionProbeNonSpecSchema, field, doc, vars); got != wantNonSpec { + t.Errorf("non-spec mode mismatch\n got: %s\n want: %s", got, wantNonSpec) + } + if got := execProbe(t, coercionProbeSpecSchema, field, doc, vars); got != wantSpec { + t.Errorf("spec mode mismatch\n got: %s\n want: %s", got, wantSpec) + } +} + +// execDo runs the document through graphql.Do, so document validation runs as +// well as coercion. An error comes back as a string so a test can pin an +// expected rejection instead of failing the run. +func execDo(t *testing.T, schema graphql.Schema, field, doc string, vars map[string]interface{}) string { + t.Helper() + result := graphql.Do(graphql.Params{ + Schema: schema, + RequestString: doc, + VariableValues: vars, + }) + if len(result.Errors) > 0 { + return "ERROR: " + result.Errors[0].Message + } + data, _ := result.Data.(map[string]interface{}) + got, _ := data[field].(string) + return got +} + +// runDoModes pins a case where the flag changes the outcome, end to end through +// graphql.Do: the non-spec column is the regression guard, the spec column is +// the fix. +func runDoModes(t *testing.T, field, doc string, vars map[string]interface{}, wantNonSpec, wantSpec string) { + t.Helper() + if got := execDo(t, coercionProbeNonSpecSchema, field, doc, vars); got != wantNonSpec { + t.Errorf("non-spec mode mismatch\n got: %s\n want: %s", got, wantNonSpec) + } + if got := execDo(t, coercionProbeSpecSchema, field, doc, vars); got != wantSpec { + t.Errorf("spec mode mismatch\n got: %s\n want: %s", got, wantSpec) + } +} + +func TestArgumentCoercion_ScalarVariable_PreservesThreeStates(t *testing.T) { + doc := `query Probe($a: String, $b: String) { probe(a: $a, b: $b) }` + + t.Run("variable omitted -> argument absent", func(t *testing.T) { + runProbeModes(t, "probe", doc, + map[string]interface{}{"a": "x"}, + `{"a":"x","b":"null","keys":["a","b"]}`, + `{"a":"x","keys":["a"]}`) + }) + t.Run("variable explicitly null -> argument present as null", func(t *testing.T) { + runProbe(t, "probe", doc, + map[string]interface{}{"a": "x", "b": nil}, + `{"a":"x","b":"null","keys":["a","b"]}`) + }) + t.Run("variable with value -> argument present with value", func(t *testing.T) { + runProbe(t, "probe", doc, + map[string]interface{}{"a": "x", "b": "y"}, + `{"a":"x","b":"y","keys":["a","b"]}`) + }) +} + +func TestArgumentCoercion_InputObjectVariable_PreservesThreeStates(t *testing.T) { + doc := `query Probe($a: String, $b: String) { probeObject(input: {a: $a, b: $b}) }` + + t.Run("nested variable omitted -> field absent in object", func(t *testing.T) { + runProbeModes(t, "probeObject", doc, + map[string]interface{}{"a": "x"}, + `{"keys":["a","b"],"obj":{"a":"x","b":null}}`, + `{"keys":["a"],"obj":{"a":"x"}}`) + }) + t.Run("nested variable explicitly null -> field present as null", func(t *testing.T) { + runProbe(t, "probeObject", doc, + map[string]interface{}{"a": "x", "b": nil}, + `{"keys":["a","b"],"obj":{"a":"x","b":null}}`) + }) + t.Run("nested variable with value -> field present with value", func(t *testing.T) { + runProbe(t, "probeObject", doc, + map[string]interface{}{"a": "x", "b": "y"}, + `{"keys":["a","b"],"obj":{"a":"x","b":"y"}}`) + }) +} + +func TestArgumentCoercion_InputObjectLiteral_OmittedFieldStaysAbsent(t *testing.T) { + doc := `{ probeObject(input: {a: "x"}) }` + runProbe(t, "probeObject", doc, nil, + `{"keys":["a"],"obj":{"a":"x"}}`) +} + +func TestArgumentCoercion_ScalarArgument_OmittedFromQueryStaysAbsent(t *testing.T) { + // The argument is not written in the query at all: CoerceArgumentValues + // leaves hasValue false and there is no default, so nothing is added. + runProbe(t, "probe", `{ probe(a: "x") }`, nil, + `{"a":"x","keys":["a"]}`) +} + +// Spec: CoerceArgumentValues (§6.4.1). The argument default applies only when +// the caller supplied no value at all. An explicit null is a supplied value and +// must survive as null rather than fall back to the default. +func TestArgumentCoercion_ArgumentDefault_AppliesOnlyWhenValueAbsent(t *testing.T) { + doc := `query Probe($a: String) { probeArgDefault(a: $a) }` + + t.Run("argument omitted from query -> default", func(t *testing.T) { + runProbe(t, "probeArgDefault", `{ probeArgDefault }`, nil, + `{"a":"ARGDEF","keys":["a"]}`) + }) + t.Run("variable omitted -> default", func(t *testing.T) { + runProbe(t, "probeArgDefault", doc, + map[string]interface{}{}, + `{"a":"ARGDEF","keys":["a"]}`) + }) + t.Run("variable explicitly null -> null, not the default", func(t *testing.T) { + runProbeModes(t, "probeArgDefault", doc, + map[string]interface{}{"a": nil}, + `{"a":"ARGDEF","keys":["a"]}`, + `{"a":"null","keys":["a"]}`) + }) + t.Run("variable with value -> value", func(t *testing.T) { + runProbe(t, "probeArgDefault", doc, + map[string]interface{}{"a": "v"}, + `{"a":"v","keys":["a"]}`) + }) + // A literal the argument's type cannot parse is neither absent nor null. The + // specification calls for a field error (§6.4.1); this implementation has + // always fallen back to the default instead, and such a document fails + // validation anyway, so both modes keep that behaviour. + t.Run("literal cannot be parsed -> default in both modes", func(t *testing.T) { + runProbe(t, "probeArgDefault", `{ probeArgDefault(a: WRONG_TYPE) }`, nil, + `{"a":"ARGDEF","keys":["a"]}`) + }) +} + +// Spec: input object field defaults (§3.10 Input Coercion), reached through an +// object literal written in the query document. +func TestArgumentCoercion_InputObjectLiteralFieldDefault_AppliesOnlyWhenValueAbsent(t *testing.T) { + doc := `query Probe($a: String) { probeObjectDefault(input: {a: $a}) }` + + t.Run("field omitted from literal -> default", func(t *testing.T) { + runProbe(t, "probeObjectDefault", `{ probeObjectDefault(input: {}) }`, nil, + `{"keys":["a"],"obj":{"a":"FIELDDEF"}}`) + }) + t.Run("nested variable omitted -> default", func(t *testing.T) { + runProbeModes(t, "probeObjectDefault", doc, + map[string]interface{}{}, + `{"keys":["a"],"obj":{"a":null}}`, + `{"keys":["a"],"obj":{"a":"FIELDDEF"}}`) + }) + t.Run("nested variable explicitly null -> null, not the default", func(t *testing.T) { + runProbe(t, "probeObjectDefault", doc, + map[string]interface{}{"a": nil}, + `{"keys":["a"],"obj":{"a":null}}`) + }) + t.Run("nested variable with value -> value", func(t *testing.T) { + runProbe(t, "probeObjectDefault", doc, + map[string]interface{}{"a": "v"}, + `{"keys":["a"],"obj":{"a":"v"}}`) + }) +} + +// The whole input object arrives as one variable, so presence is decided by +// whether the key exists in the supplied JSON object (coerceValue path). +func TestArgumentCoercion_WholeObjectVariable_PreservesThreeStates(t *testing.T) { + doc := `query Probe($in: CoercionProbeInput) { probeObject(input: $in) }` + + t.Run("key absent in object -> field absent", func(t *testing.T) { + runProbe(t, "probeObject", doc, + map[string]interface{}{"in": map[string]interface{}{"a": "x"}}, + `{"keys":["a"],"obj":{"a":"x"}}`) + }) + t.Run("key explicitly null -> field present as null", func(t *testing.T) { + runProbe(t, "probeObject", doc, + map[string]interface{}{"in": map[string]interface{}{"a": "x", "b": nil}}, + `{"keys":["a","b"],"obj":{"a":"x","b":null}}`) + }) + t.Run("key with value -> field present with value", func(t *testing.T) { + runProbe(t, "probeObject", doc, + map[string]interface{}{"in": map[string]interface{}{"a": "x", "b": "y"}}, + `{"keys":["a","b"],"obj":{"a":"x","b":"y"}}`) + }) +} + +func TestArgumentCoercion_WholeObjectVariableFieldDefault_AppliesOnlyWhenValueAbsent(t *testing.T) { + doc := `query Probe($in: CoercionProbeDefaultInput) { probeObjectDefault(input: $in) }` + + t.Run("key absent in object -> default", func(t *testing.T) { + runProbe(t, "probeObjectDefault", doc, + map[string]interface{}{"in": map[string]interface{}{}}, + `{"keys":["a"],"obj":{"a":"FIELDDEF"}}`) + }) + t.Run("key explicitly null -> null, not the default", func(t *testing.T) { + runProbeModes(t, "probeObjectDefault", doc, + map[string]interface{}{"in": map[string]interface{}{"a": nil}}, + `{"keys":["a"],"obj":{"a":"FIELDDEF"}}`, + `{"keys":["a"],"obj":{"a":null}}`) + }) + t.Run("key with value -> value", func(t *testing.T) { + runProbe(t, "probeObjectDefault", doc, + map[string]interface{}{"in": map[string]interface{}{"a": "v"}}, + `{"keys":["a"],"obj":{"a":"v"}}`) + }) +} + +// Spec: CoerceVariableValues (§6.1.2). Same rule one level up — the variable +// default applies only when the caller supplied no value for that variable. +func TestArgumentCoercion_VariableDefault_AppliesOnlyWhenValueAbsent(t *testing.T) { + doc := `query Probe($a: String = "VARDEF") { probe(a: $a) }` + + t.Run("variable omitted -> variable default", func(t *testing.T) { + runProbe(t, "probe", doc, + map[string]interface{}{}, + `{"a":"VARDEF","keys":["a"]}`) + }) + t.Run("variable explicitly null -> null, not the default", func(t *testing.T) { + runProbeModes(t, "probe", doc, + map[string]interface{}{"a": nil}, + `{"a":"VARDEF","keys":["a"]}`, + `{"a":"null","keys":["a"]}`) + }) + t.Run("variable with value -> value", func(t *testing.T) { + runProbe(t, "probe", doc, + map[string]interface{}{"a": "v"}, + `{"a":"v","keys":["a"]}`) + }) +} + +// A variable default and an argument default in the same position: the variable +// default wins because it makes the argument "supplied". +func TestArgumentCoercion_VariableDefaultTakesPrecedenceOverArgumentDefault(t *testing.T) { + doc := `query Probe($a: String = "VARDEF") { probeArgDefault(a: $a) }` + + t.Run("variable omitted -> variable default, not argument default", func(t *testing.T) { + runProbe(t, "probeArgDefault", doc, + map[string]interface{}{}, + `{"a":"VARDEF","keys":["a"]}`) + }) + t.Run("variable explicitly null -> null, neither default", func(t *testing.T) { + runProbeModes(t, "probeArgDefault", doc, + map[string]interface{}{"a": nil}, + `{"a":"VARDEF","keys":["a"]}`, + `{"a":"null","keys":["a"]}`) + }) +} + +func TestArgumentCoercion_ListArgument_PreservesThreeStates(t *testing.T) { + doc := `query Probe($a: [String]) { probeList(a: $a) }` + + t.Run("variable omitted -> argument absent", func(t *testing.T) { + runProbeModes(t, "probeList", doc, + map[string]interface{}{}, + `{"a":"null","keys":["a"]}`, + `{"keys":[]}`) + }) + t.Run("variable explicitly null -> argument present as null", func(t *testing.T) { + runProbe(t, "probeList", doc, + map[string]interface{}{"a": nil}, + `{"a":"null","keys":["a"]}`) + }) + t.Run("variable with value -> argument present with value", func(t *testing.T) { + runProbe(t, "probeList", doc, + map[string]interface{}{"a": []interface{}{"x"}}, + `{"a":["x"],"keys":["a"]}`) + }) + t.Run("unprovided variable inside a list literal -> null item", func(t *testing.T) { + runProbe(t, "probeList", `query Probe($v: String) { probeList(a: ["x", $v]) }`, + map[string]interface{}{}, + `{"a":["x",null],"keys":["a"]}`) + }) +} + +func TestArgumentCoercion_NestedInputObject_PreservesAbsentAndNull(t *testing.T) { + t.Run("literal: unprovided variable in nested object stays absent", func(t *testing.T) { + runProbeModes(t, "probeNested", + `query Probe($a: String) { probeNested(input: {inner: {a: $a}}) }`, + map[string]interface{}{}, + `{"keys":["inner"],"obj":{"inner":{"a":null}}}`, + `{"keys":["inner"],"obj":{"inner":{}}}`) + }) + t.Run("literal: explicit null in nested object stays null", func(t *testing.T) { + runProbe(t, "probeNested", + `query Probe($a: String) { probeNested(input: {inner: {a: $a}}) }`, + map[string]interface{}{"a": nil}, + `{"keys":["inner"],"obj":{"inner":{"a":null}}}`) + }) + t.Run("whole variable: absent nested key stays absent", func(t *testing.T) { + runProbe(t, "probeNested", + `query Probe($in: CoercionProbeNestedInput) { probeNested(input: $in) }`, + map[string]interface{}{"in": map[string]interface{}{"inner": map[string]interface{}{}}}, + `{"keys":["inner"],"obj":{"inner":{}}}`) + }) + t.Run("whole variable: explicit null nested key stays null", func(t *testing.T) { + runProbe(t, "probeNested", + `query Probe($in: CoercionProbeNestedInput) { probeNested(input: $in) }`, + map[string]interface{}{"in": map[string]interface{}{"inner": map[string]interface{}{"a": nil}}}, + `{"keys":["inner"],"obj":{"inner":{"a":null}}}`) + }) +} + +// The nested object itself — not one of its fields — is the thing that is +// absent or null. +func TestArgumentCoercion_NestedInputObject_ObjectValuedFieldAbsentVsNull(t *testing.T) { + litDoc := `query Probe($innerVar: CoercionProbeInput) { probeNested(input: {inner: $innerVar}) }` + varDoc := `query Probe($in: CoercionProbeNestedInput) { probeNested(input: $in) }` + + t.Run("literal: unprovided object variable -> field absent", func(t *testing.T) { + runProbeModes(t, "probeNested", litDoc, + map[string]interface{}{}, + `{"keys":["inner"],"obj":{"inner":null}}`, + `{"keys":[],"obj":{}}`) + }) + t.Run("literal: explicitly null object variable -> field present as null", func(t *testing.T) { + runProbe(t, "probeNested", litDoc, + map[string]interface{}{"innerVar": nil}, + `{"keys":["inner"],"obj":{"inner":null}}`) + }) + t.Run("whole variable: object key absent -> field absent", func(t *testing.T) { + runProbe(t, "probeNested", varDoc, + map[string]interface{}{"in": map[string]interface{}{}}, + `{"keys":[],"obj":{}}`) + }) + t.Run("whole variable: object key explicitly null -> field present as null", func(t *testing.T) { + runProbe(t, "probeNested", varDoc, + map[string]interface{}{"in": map[string]interface{}{"inner": nil}}, + `{"keys":["inner"],"obj":{"inner":null}}`) + }) +} + +// The input-object argument itself is absent or null, one level above the +// object's fields. +func TestArgumentCoercion_InputObjectArgument_AbsentVsNull(t *testing.T) { + doc := `query Probe($in: CoercionProbeInput) { probeObjectRaw(input: $in) }` + + t.Run("variable omitted -> argument absent", func(t *testing.T) { + runProbeModes(t, "probeObjectRaw", doc, + map[string]interface{}{}, + `{"input":"null","keys":["input"]}`, + `{"keys":[]}`) + }) + t.Run("variable explicitly null -> argument present as null", func(t *testing.T) { + runProbe(t, "probeObjectRaw", doc, + map[string]interface{}{"in": nil}, + `{"input":"null","keys":["input"]}`) + }) + t.Run("variable with value -> argument present with value", func(t *testing.T) { + runProbe(t, "probeObjectRaw", doc, + map[string]interface{}{"in": map[string]interface{}{"a": "x"}}, + `{"input":{"a":"x"},"keys":["input"]}`) + }) +} + +// The default lives on a field one level down, so this only passes if the +// absent/null rule is applied by the recursive step and not just at the top. +func TestArgumentCoercion_NestedFieldDefault_AppliesOnlyWhenValueAbsent(t *testing.T) { + litDoc := `query Probe($a: String) { probeNestedDefault(input: {inner: {a: $a}}) }` + varDoc := `query Probe($in: CoercionProbeNestedDefaultInput) { probeNestedDefault(input: $in) }` + + t.Run("literal: nested field omitted -> default", func(t *testing.T) { + runProbe(t, "probeNestedDefault", + `{ probeNestedDefault(input: {inner: {}}) }`, nil, + `{"keys":["inner"],"obj":{"inner":{"a":"FIELDDEF"}}}`) + }) + t.Run("literal: nested variable omitted -> default", func(t *testing.T) { + runProbeModes(t, "probeNestedDefault", litDoc, + map[string]interface{}{}, + `{"keys":["inner"],"obj":{"inner":{"a":null}}}`, + `{"keys":["inner"],"obj":{"inner":{"a":"FIELDDEF"}}}`) + }) + t.Run("literal: nested variable explicitly null -> null, not the default", func(t *testing.T) { + runProbe(t, "probeNestedDefault", litDoc, + map[string]interface{}{"a": nil}, + `{"keys":["inner"],"obj":{"inner":{"a":null}}}`) + }) + t.Run("whole variable: nested key absent -> default", func(t *testing.T) { + runProbe(t, "probeNestedDefault", varDoc, + map[string]interface{}{"in": map[string]interface{}{"inner": map[string]interface{}{}}}, + `{"keys":["inner"],"obj":{"inner":{"a":"FIELDDEF"}}}`) + }) + t.Run("whole variable: nested key explicitly null -> null, not the default", func(t *testing.T) { + runProbeModes(t, "probeNestedDefault", varDoc, + map[string]interface{}{"in": map[string]interface{}{"inner": map[string]interface{}{"a": nil}}}, + `{"keys":["inner"],"obj":{"inner":{"a":"FIELDDEF"}}}`, + `{"keys":["inner"],"obj":{"inner":{"a":null}}}`) + }) +} + +func TestArgumentCoercion_DeeplyNestedFieldDefault_AppliesOnlyWhenValueAbsent(t *testing.T) { + litDoc := `query Probe($a: String) { probeDeep(input: {level2: {inner: {a: $a}}}) }` + varDoc := `query Probe($in: CoercionProbeDeepInput) { probeDeep(input: $in) }` + + t.Run("literal: three levels down, variable omitted -> default", func(t *testing.T) { + runProbeModes(t, "probeDeep", litDoc, + map[string]interface{}{}, + `{"keys":["level2"],"obj":{"level2":{"inner":{"a":null}}}}`, + `{"keys":["level2"],"obj":{"level2":{"inner":{"a":"FIELDDEF"}}}}`) + }) + t.Run("literal: three levels down, explicitly null -> null, not the default", func(t *testing.T) { + runProbe(t, "probeDeep", litDoc, + map[string]interface{}{"a": nil}, + `{"keys":["level2"],"obj":{"level2":{"inner":{"a":null}}}}`) + }) + t.Run("whole variable: three levels down, key absent -> default", func(t *testing.T) { + runProbe(t, "probeDeep", varDoc, + map[string]interface{}{"in": map[string]interface{}{ + "level2": map[string]interface{}{"inner": map[string]interface{}{}}, + }}, + `{"keys":["level2"],"obj":{"level2":{"inner":{"a":"FIELDDEF"}}}}`) + }) + t.Run("whole variable: three levels down, explicitly null -> null, not the default", func(t *testing.T) { + runProbeModes(t, "probeDeep", varDoc, + map[string]interface{}{"in": map[string]interface{}{ + "level2": map[string]interface{}{"inner": map[string]interface{}{"a": nil}}, + }}, + `{"keys":["level2"],"obj":{"level2":{"inner":{"a":"FIELDDEF"}}}}`, + `{"keys":["level2"],"obj":{"level2":{"inner":{"a":null}}}}`) + }) +} + +// Input objects inside a list: the recursive step runs per element, so each +// element must keep its own absent / null / value state. +func TestArgumentCoercion_ListOfInputObjects_PreservesPerElementState(t *testing.T) { + t.Run("literal: element field omitted -> default", func(t *testing.T) { + runProbeModes(t, "probeObjectList", + `query Probe($a: String) { probeObjectList(input: [{a: $a}]) }`, + map[string]interface{}{}, + `{"input":[{"a":null}],"keys":["input"]}`, + `{"input":[{"a":"FIELDDEF"}],"keys":["input"]}`) + }) + t.Run("literal: element field explicitly null -> null, not the default", func(t *testing.T) { + runProbe(t, "probeObjectList", + `query Probe($a: String) { probeObjectList(input: [{a: $a}]) }`, + map[string]interface{}{"a": nil}, + `{"input":[{"a":null}],"keys":["input"]}`) + }) + t.Run("whole variable: per-element null and absent are independent", func(t *testing.T) { + runProbeModes(t, "probeObjectList", + `query Probe($in: [CoercionProbeDefaultInput]) { probeObjectList(input: $in) }`, + map[string]interface{}{"in": []interface{}{ + map[string]interface{}{"a": nil}, + map[string]interface{}{}, + }}, + `{"input":[{"a":"FIELDDEF"},{"a":"FIELDDEF"}],"keys":["input"]}`, + `{"input":[{"a":null},{"a":"FIELDDEF"}],"keys":["input"]}`) + }) +} + +// Spec §5.4.2.1, end to end through graphql.Do so document validation runs too. +// A non-null argument carrying a default is optional, so omitting it validates +// and resolves to the default. +func TestArgumentCoercion_NonNullArgumentWithDefault_IsOptionalInSpecMode(t *testing.T) { + result := graphql.Do(graphql.Params{ + Schema: coercionProbeSpecSchema, + RequestString: `{ probeNonNullDefault }`, + }) + if len(result.Errors) > 0 { + t.Fatalf("unexpected errors: %v", result.Errors) + } + data, _ := result.Data.(map[string]interface{}) + got, _ := data["probeNonNullDefault"].(string) + want := `{"a":"NNDEF","keys":["a"]}` + if got != want { + t.Fatalf("probe mismatch\n got: %s\n want: %s", got, want) + } +} + +// The stricter reading of §5.4.2.1 is part of what NonSpecArgumentHandling +// restores: a schema that opts out treats every non-null argument as required, +// so the same document fails validation instead of reaching a resolver. +func TestArgumentCoercion_NonNullArgumentWithDefault_IsRequiredInNonSpecMode(t *testing.T) { + result := graphql.Do(graphql.Params{ + Schema: coercionProbeNonSpecSchema, + RequestString: `{ probeNonNullDefault }`, + }) + if len(result.Errors) != 1 { + t.Fatalf("expected exactly one validation error, got: %v", result.Errors) + } + want := `Field "probeNonNullDefault" argument "a" of type "String!" is required but not provided.` + if got := result.Errors[0].Message; got != want { + t.Fatalf("error mismatch\n got: %s\n want: %s", got, want) + } + if result.Data != nil { + t.Fatalf("expected no data on a validation failure, got: %v", result.Data) + } +} + +// Spec §3.10 Input Coercion and §5.6.4 Input Object Required Fields: an input +// field is required only when its type is non-null AND it declares no default +// value. A field that declares a default may be omitted, and the default applies. +func TestArgumentCoercion_NonNullInputFieldWithDefault_IsOptional(t *testing.T) { + t.Run("literal omits the field -> default", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldDefault", + `{ probeNonNullFieldDefault(input: {}) }`, nil, + "ERROR: Argument \"input\" has invalid value {}.\nIn field \"a\": Expected \"String!\", found null.", + `{"keys":["a"],"obj":{"a":"FIELDDEF"}}`) + }) + t.Run("variable object omits the key -> default", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldDefault", + `query Probe($in: CoercionProbeNonNullFieldDefaultInput) { probeNonNullFieldDefault(input: $in) }`, + map[string]interface{}{"in": map[string]interface{}{}}, + "ERROR: Variable \"$in\" got invalid value {}.\nIn field \"a\": Expected \"String!\", found null.", + `{"keys":["a"],"obj":{"a":"FIELDDEF"}}`) + }) + t.Run("nested literal omits the field -> default", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldNested", + `{ probeNonNullFieldNested(input: {inner: {}}) }`, nil, + "ERROR: Argument \"input\" has invalid value {inner: {}}.\nIn field \"inner\": In field \"a\": Expected \"String!\", found null.", + `{"keys":["inner"],"obj":{"inner":{"a":"FIELDDEF"}}}`) + }) + t.Run("list element omits the field -> default", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldList", + `query Probe($in: [CoercionProbeNonNullFieldDefaultInput]) { probeNonNullFieldList(input: $in) }`, + map[string]interface{}{"in": []interface{}{map[string]interface{}{}}}, + "ERROR: Variable \"$in\" got invalid value [{}].\nIn element #1: In field \"a\": Expected \"String!\", found null.", + `{"input":[{"a":"FIELDDEF"}],"keys":["input"]}`) + }) +} + +// An explicit null is a supplied value, so no default stands in for it and a +// non-null field must still reject it — in both modes. +func TestArgumentCoercion_NonNullInputFieldWithDefault_RejectsExplicitNull(t *testing.T) { + want := "ERROR: Variable \"$in\" got invalid value {\"a\":null}.\nIn field \"a\": Expected \"String!\", found null." + runDoModes(t, "probeNonNullFieldDefault", + `query Probe($in: CoercionProbeNonNullFieldDefaultInput) { probeNonNullFieldDefault(input: $in) }`, + map[string]interface{}{"in": map[string]interface{}{"a": nil}}, want, want) +} + +// A non-null field that declares no default is genuinely required, in both modes. +func TestArgumentCoercion_NonNullInputFieldWithoutDefault_StaysRequired(t *testing.T) { + want := "ERROR: Argument \"input\" has invalid value {}.\nIn field \"a\": Expected \"String!\", found null." + runDoModes(t, "probeObjectRequired", `{ probeObjectRequired(input: {}) }`, nil, want, want) +} + +// Spec §6.1.2 CoerceVariableValues: the default is applied when the caller +// supplied no value, and that check comes before the non-null requirement. A +// non-null variable may therefore declare a default and be omitted. +func TestArgumentCoercion_NonNullVariableWithDefault_UsesDefaultWhenOmitted(t *testing.T) { + doc := `query Probe($a: String! = "VARDEF") { probe(a: $a) }` + nonSpecWant := `ERROR: Variable "$a" of type "String!" is required and will not use the default value. Perhaps you meant to use type "String".` + + t.Run("variable omitted -> default", func(t *testing.T) { + runDoModes(t, "probe", doc, map[string]interface{}{}, + nonSpecWant, `{"a":"VARDEF","keys":["a"]}`) + }) + t.Run("variable with value -> value", func(t *testing.T) { + runDoModes(t, "probe", doc, map[string]interface{}{"a": "v"}, + nonSpecWant, `{"a":"v","keys":["a"]}`) + }) + t.Run("variable explicitly null -> error in both modes", func(t *testing.T) { + runDoModes(t, "probe", doc, map[string]interface{}{"a": nil}, + nonSpecWant, `ERROR: Variable "$a" of required type "String!" was not provided.`) + }) +} + +// Only the blanket "a non-null variable must not declare a default" rejection +// goes away. A default whose type does not match is still rejected. +func TestArgumentCoercion_NonNullVariableWithWronglyTypedDefault_StaysAnError(t *testing.T) { + runDoModes(t, "probe", `query Probe($a: String! = 123) { probe(a: $a) }`, map[string]interface{}{}, + `ERROR: Variable "$a" of type "String!" is required and will not use the default value. Perhaps you meant to use type "String".`, + "ERROR: Variable \"$a\" has invalid default value: 123.\nExpected type \"String\", found 123.") +} + +// execProbeErrors runs the document through graphql.Execute, which skips +// document validation, so a coercion-level error can be observed on its own. +func execProbeErrors(t *testing.T, schema graphql.Schema, doc string, vars map[string]interface{}) []gqlerrors.FormattedError { + t.Helper() + parsed := testutil.TestParse(t, doc) + result := graphql.Execute(graphql.ExecuteParams{ + Schema: schema, + AST: parsed, + Args: vars, + }) + return result.Errors +} + +// Spec §6.4.1 ②: a non-null argument whose value resolves to null is a field +// error. The default only stands in for a value the caller did not supply, so it +// cannot rescue a supplied null. Observed through graphql.Execute because +// document validation rejects this document earlier until §5.8.5 is implemented. +func TestArgumentCoercion_NonNullArgumentGivenExplicitNull_IsAFieldError(t *testing.T) { + doc := `query Probe($x: String) { probeNonNullDefault(a: $x) }` + vars := map[string]interface{}{"x": nil} + + if errs := execProbeErrors(t, coercionProbeNonSpecSchema, doc, vars); len(errs) != 0 { + t.Errorf("non-spec mode: expected no error, got: %v", errs) + } + + errs := execProbeErrors(t, coercionProbeSpecSchema, doc, vars) + if len(errs) != 1 { + t.Fatalf("spec mode: expected exactly one field error, got: %v", errs) + } + want := `Argument "a" of non-null type "String!" must not be null.` + if errs[0].Message != want { + t.Errorf("message mismatch\n got: %s\n want: %s", errs[0].Message, want) + } +} + +// The same argument still takes its default when the caller supplied nothing. +func TestArgumentCoercion_NonNullArgumentWithDefault_StillTakesDefault(t *testing.T) { + doc := `query Probe($x: String) { probeNonNullDefault(a: $x) }` + for _, tc := range []struct { + mode string + schema graphql.Schema + }{ + {"non-spec", coercionProbeNonSpecSchema}, + {"spec", coercionProbeSpecSchema}, + } { + t.Run(tc.mode, func(t *testing.T) { + if got := execProbe(t, tc.schema, "probeNonNullDefault", doc, map[string]interface{}{}); got != `{"a":"NNDEF","keys":["a"]}` { + t.Errorf("probe mismatch, got: %s", got) + } + }) + } +} + +// Spec §5.8.5 IsVariableUsageAllowed: a nullable variable may be used at a +// non-null location when that location declares a default value. The default +// covers the case where the caller supplies nothing. +func TestArgumentCoercion_NullableVariableAtNonNullArgumentWithDefault_IsAllowed(t *testing.T) { + doc := `query Probe($x: String) { probeNonNullDefault(a: $x) }` + nonSpecWant := `ERROR: Variable "$x" of type "String" used in position expecting type "String!".` + + t.Run("variable omitted -> argument default", func(t *testing.T) { + runDoModes(t, "probeNonNullDefault", doc, map[string]interface{}{}, + nonSpecWant, `{"a":"NNDEF","keys":["a"]}`) + }) + t.Run("variable explicitly null -> field error", func(t *testing.T) { + runDoModes(t, "probeNonNullDefault", doc, map[string]interface{}{"x": nil}, + nonSpecWant, `ERROR: Argument "a" of non-null type "String!" must not be null.`) + }) +} + +// The spec names ObjectField alongside Argument, so an input object field that +// declares a default permits the same usage. +func TestArgumentCoercion_NullableVariableAtNonNullInputFieldWithDefault_IsAllowed(t *testing.T) { + runDoModes(t, "probeNonNullFieldDefault", + `query Probe($x: String) { probeNonNullFieldDefault(input: {a: $x}) }`, + map[string]interface{}{}, + `ERROR: Variable "$x" of type "String" used in position expecting type "String!".`, + `{"keys":["a"],"obj":{"a":"FIELDDEF"}}`) +} + +// A non-null location that declares no default still rejects a nullable variable. +func TestArgumentCoercion_NullableVariableAtNonNullWithoutDefault_StaysRejected(t *testing.T) { + want := `ERROR: Variable "$x" of type "String" used in position expecting type "String!".` + runDoModes(t, "probeObjectRequired", + `query Probe($x: String) { probeObjectRequired(input: {a: $x}) }`, + map[string]interface{}{}, want, want) +} + +// Spec §5.8.5 takes hasLocationDefaultValue from the argument or input object +// field the usage sits in; a list position never carries one. The argument's own +// default must therefore not rescue a nullable variable used as a non-null list +// item. +func TestArgumentCoercion_NullableVariableAtNonNullListItem_StaysRejected(t *testing.T) { + want := `ERROR: Variable "$x" of type "String" used in position expecting type "String!".` + runDoModes(t, "probeNonNullItemList", + `query Probe($x: String) { probeNonNullItemList(a: [$x]) }`, + map[string]interface{}{}, want, want) +} + +// Spec §3.10 Input Coercion, fourth rule: "If a variable is provided for an +// input object field, the runtime value of that variable must be used. If the +// runtime value is null and the field type is non-null, a field error must be +// raised. If no runtime value is provided, the variable definition's default +// value should be used. If the variable definition does not provide a default +// value, the input object field definition's default value should be used." +// +// The null case only becomes reachable once §5.8.5 lets a nullable variable sit +// at a non-null field that declares a default; before that the document was +// rejected during validation. +func TestArgumentCoercion_NullVariableAtNonNullInputField_IsAFieldError(t *testing.T) { + nonSpecWant := `ERROR: Variable "$x" of type "String" used in position expecting type "String!".` + + t.Run("field with a default, variable is null -> field error", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldDefault", + `query Probe($x: String) { probeNonNullFieldDefault(input: {a: $x}) }`, + map[string]interface{}{"x": nil}, + nonSpecWant, + "ERROR: Argument \"input\" has invalid value.\nIn field \"a\": Expected \"String!\", found null.") + }) + t.Run("nested field with a default, variable is null -> field error", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldNested", + `query Probe($x: String) { probeNonNullFieldNested(input: {inner: {a: $x}}) }`, + map[string]interface{}{"x": nil}, + nonSpecWant, + "ERROR: Argument \"input\" has invalid value.\nIn field \"inner\": In field \"a\": Expected \"String!\", found null.") + }) + t.Run("list element field with a default, variable is null -> field error", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldList", + `query Probe($x: String) { probeNonNullFieldList(input: [{a: $x}]) }`, + map[string]interface{}{"x": nil}, + nonSpecWant, + "ERROR: Argument \"input\" has invalid value.\nIn element #1: In field \"a\": Expected \"String!\", found null.") + }) + t.Run("field with a default, variable has a value -> value", func(t *testing.T) { + runDoModes(t, "probeNonNullFieldDefault", + `query Probe($x: String) { probeNonNullFieldDefault(input: {a: $x}) }`, + map[string]interface{}{"x": "v"}, + nonSpecWant, `{"keys":["a"],"obj":{"a":"v"}}`) + }) + // A field with no default keeps being rejected during validation, because + // §5.8.5 finds neither a variable nor a location default. + t.Run("field without a default stays a validation error", func(t *testing.T) { + runDoModes(t, "probeObjectRequired", + `query Probe($x: String) { probeObjectRequired(input: {a: $x}) }`, + map[string]interface{}{"x": nil}, nonSpecWant, nonSpecWant) + }) +} + +// Spec §3.10: a field is optional when its definition "provides a default +// value". A default that coercion cannot substitute — isNullish treats a typed +// nil pointer as no value — does not make the field optional, so a non-null +// field carrying one stays required. Without this the field passed validation +// and was then dropped by coercion, handing the resolver an input object that +// is missing a field its own schema declares non-null. +func TestArgumentCoercion_NullishFieldDefault_DoesNotMakeFieldOptional(t *testing.T) { + t.Run("literal omits the field", func(t *testing.T) { + want := "ERROR: Argument \"input\" has invalid value {}.\nIn field \"a\": Expected \"String!\", found null." + runDoModes(t, "probeNullishFieldDefault", + `{ probeNullishFieldDefault(input: {}) }`, nil, want, want) + }) + t.Run("variable object omits the key", func(t *testing.T) { + want := "ERROR: Variable \"$in\" got invalid value {}.\nIn field \"a\": Expected \"String!\", found null." + runDoModes(t, "probeNullishFieldDefault", + `query Probe($in: CoercionProbeNullishDefaultInput) { probeNullishFieldDefault(input: $in) }`, + map[string]interface{}{"in": map[string]interface{}{}}, want, want) + }) +} + +// The same reasoning as TestArgumentCoercion_NullishFieldDefault, one level up: +// a default coercion will not substitute does not make an argument optional +// either. Spec §5.4.2.1 (§5.4.3 in draft) for the first case, §5.8.5 for the +// rest. +func TestArgumentCoercion_NullishArgumentDefault_KeepsArgumentRequired(t *testing.T) { + t.Run("argument omitted from the document", func(t *testing.T) { + want := `ERROR: Field "probeNullishArgDefault" argument "a" of type "String!" is required but not provided.` + runDoModes(t, "probeNullishArgDefault", `{ probeNullishArgDefault }`, nil, want, want) + }) + + doc := `query Probe($x: String) { probeNullishArgDefault(a: $x) }` + want := `ERROR: Variable "$x" of type "String" used in position expecting type "String!".` + + t.Run("nullable variable, not supplied", func(t *testing.T) { + runDoModes(t, "probeNullishArgDefault", doc, map[string]interface{}{}, want, want) + }) + // Validation is static: the document must be rejected whether or not this + // particular request happens to supply a value for the variable. + t.Run("nullable variable with a value, still rejected", func(t *testing.T) { + runDoModes(t, "probeNullishArgDefault", doc, map[string]interface{}{"x": "v"}, want, want) + }) +} diff --git a/directives_test.go b/directives_test.go index 991a8c8..0e6f36a 100644 --- a/directives_test.go +++ b/directives_test.go @@ -1,6 +1,7 @@ package graphql_test import ( + "encoding/json" "errors" "testing" @@ -514,3 +515,71 @@ func TestDirectivesWorksWithSkipAndIncludeDirectives_NoIncludeOrSkip(t *testing. t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } } + +// The same two fields under both modes, so the @skip / @include tests below can +// pin the opt-out column as a regression guard. +var directivesNonSpecTestSchema, _ = graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "TestType", + Fields: graphql.Fields{ + "a": &graphql.Field{Type: graphql.String}, + "b": &graphql.Field{Type: graphql.String}, + }, + }), + NonSpecArgumentHandling: true, +}) + +// runDirectiveModes executes doc through graphql.Do under both modes. Results are +// compared as the serialised data map plus the first error, so a test can pin an +// expected rejection as well as an expected shape. +func runDirectiveModes(t *testing.T, doc string, vars map[string]interface{}, wantNonSpec, wantSpec string) { + t.Helper() + run := func(schema graphql.Schema) string { + result := graphql.Do(graphql.Params{ + Schema: schema, + RequestString: doc, + VariableValues: vars, + RootObject: directivesTestData, + }) + data, _ := json.Marshal(result.Data) + if len(result.Errors) > 0 { + return string(data) + " ERROR: " + result.Errors[0].Message + } + return string(data) + } + if got := run(directivesNonSpecTestSchema); got != wantNonSpec { + t.Errorf("non-spec mode mismatch\n got: %s\n want: %s", got, wantNonSpec) + } + if got := run(directivesTestSchema); got != wantSpec { + t.Errorf("spec mode mismatch\n got: %s\n want: %s", got, wantSpec) + } +} + +// Spec §6.3.2 CollectFields tests @skip and @include structurally rather than +// coercing their arguments: @skip drops the selection when if is true, and +// @include keeps it only when if is true. An if that is neither — a variable +// carrying null, say — is simply "not true", and no error is raised. +// +// The one document that reaches this at runtime declares a variable with a +// non-null default, which §5.8.5 allows at the non-null if argument, and then +// supplies null for it. +func TestDirectives_NullIfArgument_IsNotTrue(t *testing.T) { + vars := map[string]interface{}{"x": nil} + + t.Run("@skip with a null if keeps the selection", func(t *testing.T) { + // Non-spec mode never sees the null: the variable's default replaces it. + runDirectiveModes(t, `query Q($x: Boolean = true){ a @skip(if: $x) b }`, vars, + `{"b":"b"}`, `{"a":"a","b":"b"}`) + }) + t.Run("@include with a null if drops the selection", func(t *testing.T) { + runDirectiveModes(t, `query Q($x: Boolean = true){ a @include(if: $x) b }`, vars, + `{"a":"a","b":"b"}`, `{"b":"b"}`) + }) + t.Run("a true value still behaves normally", func(t *testing.T) { + v := map[string]interface{}{"x": true} + runDirectiveModes(t, `query Q($x: Boolean = true){ a @skip(if: $x) b }`, v, + `{"b":"b"}`, `{"b":"b"}`) + runDirectiveModes(t, `query Q($x: Boolean = true){ a @include(if: $x) b }`, v, + `{"a":"a","b":"b"}`, `{"a":"a","b":"b"}`) + }) +} diff --git a/executor.go b/executor.go index 0b35b07..ddaec40 100644 --- a/executor.go +++ b/executor.go @@ -481,10 +481,7 @@ func collectFields(p collectFieldsParams) (fields map[string][]*ast.Field) { // Determines if a field should be included based on the @include and @skip // directives, where @skip has higher precedence than @include. func shouldIncludeNode(eCtx *executionContext, directives []*ast.Directive) bool { - var ( - skipAST, includeAST *ast.Directive - argValues map[string]interface{} - ) + var skipAST, includeAST *ast.Directive for _, directive := range directives { if directive == nil || directive.Name == nil { continue @@ -496,22 +493,40 @@ func shouldIncludeNode(eCtx *executionContext, directives []*ast.Directive) bool includeAST = directive } } + nonSpec := eCtx.Schema.nonSpecArgumentHandling // precedence: skipAST > includeAST if skipAST != nil { - argValues = getArgumentValues(SkipDirective.Args, skipAST.Arguments, eCtx.VariableValues) - if skipIf, ok := argValues["if"].(bool); ok && skipIf { + if directiveIfIsTrue(SkipDirective.Args, skipAST.Arguments, eCtx.VariableValues, nonSpec) { return false // excluded selectionSet's fields } } if includeAST != nil { - argValues = getArgumentValues(IncludeDirective.Args, includeAST.Arguments, eCtx.VariableValues) - if includeIf, ok := argValues["if"].(bool); ok && !includeIf { + if nonSpec { + argValues, _ := getArgumentValues(IncludeDirective.Args, includeAST.Arguments, eCtx.VariableValues, nonSpec) + if includeIf, ok := argValues["if"].(bool); ok && !includeIf { + return false // excluded selectionSet's fields + } + } else if !directiveIfIsTrue(IncludeDirective.Args, includeAST.Arguments, eCtx.VariableValues, nonSpec) { + // Spec §6.3.2: the selection is kept only when if is true. return false // excluded selectionSet's fields } } return true } +// Reports whether the directive's "if" argument resolves to true. Spec §6.3.2 +// CollectFields asks only that question of @skip and @include — it does not +// coerce their arguments — so a value that is not true, including one a coercion +// failure left unusable, simply answers false rather than raising an error. +func directiveIfIsTrue(argDefs []*Argument, argASTs []*ast.Argument, variableValues map[string]interface{}, nonSpec bool) bool { + argValues, err := getArgumentValues(argDefs, argASTs, variableValues, nonSpec) + if err != nil { + return false + } + ifValue, ok := argValues["if"].(bool) + return ok && ifValue +} + // Determines if a fragment is applicable to the given type. func doesFragmentConditionMatch(eCtx *executionContext, fragment ast.Node, ttype *Object) bool { @@ -624,7 +639,12 @@ func resolveField(eCtx *executionContext, parentType *Object, source interface{} // Build a map of arguments from the field.arguments AST, using the // variables scope to fulfill any variable references. // TODO: find a way to memoize, in case this field is within a List type. - args := getArgumentValues(fieldDef.Args, fieldAST.Arguments, eCtx.VariableValues) + args, argErr := getArgumentValues(fieldDef.Args, fieldAST.Arguments, eCtx.VariableValues, eCtx.Schema.nonSpecArgumentHandling) + if argErr != nil { + // Same idiom as a failing resolver below: the deferred recover turns this + // into a field error via handleFieldError. + panic(argErr) + } info := ResolveInfo{ FieldName: fieldName, diff --git a/rules.go b/rules.go index 4fc35f3..b3240e9 100644 --- a/rules.go +++ b/rules.go @@ -68,13 +68,15 @@ func reportError(context *ValidationContext, message string, nodes []ast.Node) ( // A GraphQL document is only valid if all field argument literal values are // of the type expected by their position. func ArgumentsOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance { + nonSpec := context.Schema().nonSpecArgumentHandling + visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ kinds.Argument: { Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if argAST, ok := p.Node.(*ast.Argument); ok { if argDef := context.Argument(); argDef != nil { - if isValid, messages := isValidLiteralValue(argDef.Type, argAST.Value); !isValid { + if isValid, messages := isValidLiteralValue(argDef.Type, argAST.Value, nonSpec); !isValid { var messagesStr, argNameValue string if argAST.Name != nil { argNameValue = argAST.Name.Value @@ -108,6 +110,8 @@ func ArgumentsOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInsta // A GraphQL document is only valid if all variable default values are of the // type expected by their definition. func DefaultValuesOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance { + nonSpec := context.Schema().nonSpecArgumentHandling + visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ kinds.VariableDefinition: { @@ -123,16 +127,21 @@ func DefaultValuesOfCorrectTypeRule(context *ValidationContext) *ValidationRuleI } ttype := context.InputType() - // when input variable value must be nonNull, and set default value is unnecessary - if ttype, ok := ttype.(*NonNull); ok && defaultValue != nil { - reportError( - context, - fmt.Sprintf(`Variable "$%v" of type "%v" is required and will not use the default value. Perhaps you meant to use type "%v".`, - name, ttype, ttype.OfType), - []ast.Node{defaultValue}, - ) + // Spec §6.1.2 applies a variable's default before checking the + // non-null requirement, so a non-null variable may declare one and + // nothing in the Validation section forbids it. Only a schema that + // opted into NonSpecArgumentHandling keeps rejecting it. + if nonSpec { + if ttype, ok := ttype.(*NonNull); ok && defaultValue != nil { + reportError( + context, + fmt.Sprintf(`Variable "$%v" of type "%v" is required and will not use the default value. Perhaps you meant to use type "%v".`, + name, ttype, ttype.OfType), + []ast.Node{defaultValue}, + ) + } } - if isValid, messages := isValidLiteralValue(ttype, defaultValue); !isValid && defaultValue != nil { + if isValid, messages := isValidLiteralValue(ttype, defaultValue, nonSpec); !isValid && defaultValue != nil { if len(messages) > 0 { messagesStr = "\n" + strings.Join(messages, "\n") } @@ -1247,6 +1256,14 @@ func PossibleFragmentSpreadsRule(context *ValidationContext) *ValidationRuleInst // have been provided. func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleInstance { + // Spec §5.4.2.1: an argument is required only when its type is non-null and + // it declares no default value. Nullish is the test that coercion applies, so + // a default that coercion will not substitute leaves the argument required. A + // schema that opted into NonSpecArgumentHandling keeps the older, stricter + // reading, under which every non-null argument is required whether or not it + // has a default. + nonSpec := context.Schema().nonSpecArgumentHandling + visitorOpts := &visitor.VisitorOptions{ KindFuncMap: map[string]visitor.NamedVisitFuncs{ kinds.Field: { @@ -1271,7 +1288,7 @@ func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleIns for _, argDef := range fieldDef.Args { argAST, _ := argASTMap[argDef.Name()] if argAST == nil { - if argDefType, ok := argDef.Type.(*NonNull); ok { + if argDefType, ok := argDef.Type.(*NonNull); ok && (nonSpec || isNullish(argDef.DefaultValue)) { fieldName := "" if fieldAST.Name != nil { fieldName = fieldAST.Name.Value @@ -1312,7 +1329,7 @@ func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleIns for _, argDef := range directiveDef.Args { argAST, _ := argASTMap[argDef.Name()] if argAST == nil { - if argDefType, ok := argDef.Type.(*NonNull); ok { + if argDefType, ok := argDef.Type.(*NonNull); ok && (nonSpec || isNullish(argDef.DefaultValue)) { directiveName := "" if directiveAST.Name != nil { directiveName = directiveAST.Name.Value @@ -1656,8 +1673,33 @@ func effectiveType(varType Type, varDef *ast.VariableDefinition) Type { return NewNonNull(varType) } +// allowedVariableUsage implements spec §5.8.5 IsVariableUsageAllowed. A nullable +// variable is allowed at a non-null location when either the variable or the +// location declares a default value: whichever one exists covers the case where +// the caller supplies nothing. +func allowedVariableUsage(schema *Schema, varType Type, varDefaultValue ast.Value, + locationType Input, locationDefaultValue interface{}) bool { + if locType, ok := locationType.(*NonNull); ok { + if _, isNonNull := varType.(*NonNull); !isNonNull { + // The parser does not accept the null literal, so a default value that + // exists is necessarily not null. Revisit if that ever changes. + hasNonNullVariableDefaultValue := varDefaultValue != nil + // Nullish is the test that coercion applies to a declared default, so a + // default it will not substitute does not count as one here either. + hasLocationDefaultValue := !isNullish(locationDefaultValue) + if !hasNonNullVariableDefaultValue && !hasLocationDefaultValue { + return false + } + ofType, _ := locType.OfType.(Input) + return isTypeSubTypeOf(schema, varType, ofType) + } + } + return isTypeSubTypeOf(schema, varType, locationType) +} + // VariablesInAllowedPositionRule Variables passed to field arguments conform to type func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleInstance { + nonSpec := context.Schema().nonSpecArgumentHandling varDefMap := map[string]*ast.VariableDefinition{} @@ -1683,7 +1725,15 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI if err != nil { varType = nil } - if varType != nil && !isTypeSubTypeOf(context.Schema(), effectiveType(varType, varDef), usage.Type) { + allowed := true + if varType != nil { + if nonSpec { + allowed = isTypeSubTypeOf(context.Schema(), effectiveType(varType, varDef), usage.Type) + } else { + allowed = allowedVariableUsage(context.Schema(), varType, varDef.DefaultValue, usage.Type, usage.LocationDefaultValue) + } + } + if varType != nil && !allowed { reportError( context, fmt.Sprintf(`Variable "$%v" of type "%v" used in position `+ @@ -1724,7 +1774,7 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI // // Note that this only validates literal values, variables are assumed to // provide values of the correct type. -func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) { +func isValidLiteralValue(ttype Input, valueAST ast.Value, nonSpec bool) (bool, []string) { if _, ok := ttype.(*NonNull); !ok { if valueAST == nil { return true, nil @@ -1749,21 +1799,21 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) { return false, []string{"Expected non-null value, found null."} } ofType, _ := ttype.OfType.(Input) - return isValidLiteralValue(ofType, valueAST) + return isValidLiteralValue(ofType, valueAST, nonSpec) case *List: // Lists accept a non-list value as a list of one. itemType, _ := ttype.OfType.(Input) if valueAST, ok := valueAST.(*ast.ListValue); ok { messagesReduce := []string{} for _, value := range valueAST.Values { - _, messages := isValidLiteralValue(itemType, value) + _, messages := isValidLiteralValue(itemType, value, nonSpec) for idx, message := range messages { messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, idx+1, message)) } } return (len(messagesReduce) == 0), messagesReduce } - return isValidLiteralValue(itemType, valueAST) + return isValidLiteralValue(itemType, valueAST, nonSpec) case *InputObject: // Input objects check each defined field and look for undefined fields. valueAST, ok := valueAST.(*ast.ObjectValue) @@ -1786,10 +1836,21 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) { // Ensure every defined field is valid. for fieldName, field := range fields { var fieldASTValue ast.Value - if fieldAST := fieldASTMap[fieldName]; fieldAST != nil { + fieldAST, ok := fieldASTMap[fieldName] + if ok && fieldAST != nil { fieldASTValue = fieldAST.Value } - if isValid, messages := isValidLiteralValue(field.Type, fieldASTValue); !isValid { + // Spec §3.10 Input Coercion and §5.6.4 Input Object Required Fields: a + // field that declares a default value is optional even when its type is + // non-null. A field written in the literal is still validated. + // + // Nullish is the same test that coercion applies, so a default that + // coercion will not substitute does not make the field optional here + // either. + if !nonSpec && !ok && !isNullish(field.DefaultValue) { + continue + } + if isValid, messages := isValidLiteralValue(field.Type, fieldASTValue, nonSpec); !isValid { for _, message := range messages { messagesReduce = append(messagesReduce, fmt.Sprintf("In field \"%v\": %v", fieldName, message)) } diff --git a/rules_arguments_of_correct_type_test.go b/rules_arguments_of_correct_type_test.go index 638951c..406fa04 100644 --- a/rules_arguments_of_correct_type_test.go +++ b/rules_arguments_of_correct_type_test.go @@ -799,3 +799,64 @@ func TestValidate_ArgValuesOfCorrectType_DirectiveArguments_WithDirectivesWithIn ), }) } + +// One field whose input object declares a non-null field with a default value, +// under whichever mode the caller asks for. +func nonNullInputFieldWithDefaultSchema(t *testing.T, nonSpec bool) graphql.Schema { + t.Helper() + input := graphql.NewInputObject(graphql.InputObjectConfig{ + Name: "FieldDefaultInput", + Fields: graphql.InputObjectConfigFieldMap{ + "a": &graphql.InputObjectFieldConfig{ + Type: graphql.NewNonNull(graphql.String), + DefaultValue: "FIELDDEF", + }, + }, + }) + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "f": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "input": &graphql.ArgumentConfig{Type: input}, + }, + }, + }, + }), + NonSpecArgumentHandling: nonSpec, + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + return schema +} + +// Spec §5.6.4: an input field is required only when its type is non-null AND it +// declares no default value. +func TestValidate_ArgumentsOfCorrectType_NonNullInputFieldWithDefaultMayBeOmitted(t *testing.T) { + schema := nonNullInputFieldWithDefaultSchema(t, false) + testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.ArgumentsOfCorrectTypeRule, ` + { + f(input: {}) + } + `) +} + +// NonSpecArgumentHandling keeps the older reading, under which every non-null +// input field is required whether or not it declares a default. +func TestValidate_ArgumentsOfCorrectType_NonSpecKeepsNonNullInputFieldRequired(t *testing.T) { + schema := nonNullInputFieldWithDefaultSchema(t, true) + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.ArgumentsOfCorrectTypeRule, ` + { + f(input: {}) + } + `, []gqlerrors.FormattedError{ + testutil.RuleError( + `Argument "input" has invalid value {}.`+ + "\nIn field \"a\": Expected \"String!\", found null.", + 3, 20, + ), + }) +} diff --git a/rules_default_values_of_correct_type_test.go b/rules_default_values_of_correct_type_test.go index e7eede0..ca18730 100644 --- a/rules_default_values_of_correct_type_test.go +++ b/rules_default_values_of_correct_type_test.go @@ -33,24 +33,18 @@ func TestValidate_VariableDefaultValuesOfCorrectType_VariablesWithValidDefaultVa } `) } -func TestValidate_VariableDefaultValuesOfCorrectType_NoRequiredVariablesWithDefaultValues(t *testing.T) { - testutil.ExpectFailsRule(t, graphql.DefaultValuesOfCorrectTypeRule, ` - query UnreachableDefaultValues($a: Int! = 3, $b: String! = "default") { + +// Spec §6.1.2 applies a variable's default before checking the non-null +// requirement, so a non-null variable may declare one and the default is +// reached. This test asserted the opposite before the coercion fix; the +// rejection now lives behind NonSpecArgumentHandling and is covered by +// TestValidate_VariableDefaultValuesOfCorrectType_NonSpecRejectsNonNullVariableDefault. +func TestValidate_VariableDefaultValuesOfCorrectType_NonNullVariablesWithDefaultValues(t *testing.T) { + testutil.ExpectPassesRule(t, graphql.DefaultValuesOfCorrectTypeRule, ` + query NonNullDefaultValues($a: Int! = 3, $b: String! = "default") { dog { name } } - `, - []gqlerrors.FormattedError{ - testutil.RuleError( - `Variable "$a" of type "Int!" is required and will not `+ - `use the default value. Perhaps you meant to use type "Int".`, - 2, 49, - ), - testutil.RuleError( - `Variable "$b" of type "String!" is required and will not `+ - `use the default value. Perhaps you meant to use type "String".`, - 2, 66, - ), - }) + `) } func TestValidate_VariableDefaultValuesOfCorrectType_VariablesWithInvalidDefaultValues(t *testing.T) { testutil.ExpectFailsRule(t, graphql.DefaultValuesOfCorrectTypeRule, ` @@ -105,3 +99,51 @@ func TestValidate_VariableDefaultValuesOfCorrectType_ListVariablesWithInvalidIte func TestValidate_VariableDefaultValuesOfCorrectType_InvalidNonNull(t *testing.T) { testutil.ExpectPassesRule(t, graphql.DefaultValuesOfCorrectTypeRule, `query($g:e!){a}`) } + +// One nullable argument, so a non-null variable can be used at it, under +// whichever mode the caller asks for. +func nonNullVariableDefaultSchema(t *testing.T, nonSpec bool) graphql.Schema { + t.Helper() + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "f": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{Type: graphql.String}, + }, + }, + }, + }), + NonSpecArgumentHandling: nonSpec, + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + return schema +} + +// Spec §6.1.2 applies a variable's default before checking the non-null +// requirement, and nothing in the Validation section forbids a non-null variable +// from declaring one. +func TestValidate_VariableDefaultValuesOfCorrectType_NonNullVariableMayHaveDefault(t *testing.T) { + schema := nonNullVariableDefaultSchema(t, false) + testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.DefaultValuesOfCorrectTypeRule, ` + query Probe($a: String! = "VARDEF") { + f(a: $a) + } + `) +} + +// NonSpecArgumentHandling keeps rejecting it. +func TestValidate_VariableDefaultValuesOfCorrectType_NonSpecRejectsNonNullVariableDefault(t *testing.T) { + schema := nonNullVariableDefaultSchema(t, true) + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.DefaultValuesOfCorrectTypeRule, ` + query Probe($a: String! = "VARDEF") { + f(a: $a) + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Variable "$a" of type "String!" is required and will not use the default value. Perhaps you meant to use type "String".`, 2, 33), + }) +} diff --git a/rules_provided_non_null_arguments_test.go b/rules_provided_non_null_arguments_test.go index dc3c055..9a0483c 100644 --- a/rules_provided_non_null_arguments_test.go +++ b/rules_provided_non_null_arguments_test.go @@ -175,3 +175,172 @@ func TestValidate_ProvidedNonNullArguments_DirectiveArguments_WithDirectiveWithM testutil.RuleError(`Directive "@skip" argument "if" of type "Boolean!" is required but not provided.`, 4, 18), }) } + +// One field whose non-null argument declares a default value, under whichever +// mode the caller asks for. +func nonNullArgWithDefaultSchema(t *testing.T, nonSpec bool) graphql.Schema { + t.Helper() + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "fieldWithDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "arg": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.Boolean), + DefaultValue: true, + }, + }, + }, + }, + }), + NonSpecArgumentHandling: nonSpec, + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + return schema +} + +// Spec §5.4.2.1: "An argument is required if the argument type is non-null and +// does not have a default value. Otherwise, the argument is optional." +// See graphql-go/graphql#739. +func TestValidate_ProvidedNonNullArguments_FieldArguments_NoErrorOnNonNullArgumentWithDefaultValue(t *testing.T) { + schema := nonNullArgWithDefaultSchema(t, false) + testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.ProvidedNonNullArgumentsRule, ` + { + fieldWithDefault + } + `) +} + +// NonSpecArgumentHandling restores the older, stricter reading, under which a +// non-null argument is required whether or not it declares a default. +func TestValidate_ProvidedNonNullArguments_FieldArguments_NonSpecErrorsOnNonNullArgumentWithDefaultValue(t *testing.T) { + schema := nonNullArgWithDefaultSchema(t, true) + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.ProvidedNonNullArgumentsRule, ` + { + fieldWithDefault + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Field "fieldWithDefault" argument "arg" of type "Boolean!" is required but not provided.`, 3, 11), + }) +} + +func TestValidate_ProvidedNonNullArguments_FieldArguments_StillErrorsOnNonNullArgumentWithoutDefaultValue(t *testing.T) { + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "fieldWithoutDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "arg": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.Boolean), + }, + }, + }, + }, + }), + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.ProvidedNonNullArgumentsRule, ` + { + fieldWithoutDefault + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Field "fieldWithoutDefault" argument "arg" of type "Boolean!" is required but not provided.`, 3, 11), + }) +} + +// One directive whose non-null argument declares a default value, under +// whichever mode the caller asks for. +func nonNullDirectiveArgWithDefaultSchema(t *testing.T, nonSpec bool) graphql.Schema { + t.Helper() + deferDirective := graphql.NewDirective(graphql.DirectiveConfig{ + Name: "defer", + Locations: []string{ + graphql.DirectiveLocationFragmentSpread, + graphql.DirectiveLocationInlineFragment, + }, + Args: graphql.FieldConfigArgument{ + "if": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.Boolean), + DefaultValue: true, + }, + }, + }) + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "a": &graphql.Field{Type: graphql.String}, + }, + }), + Directives: []*graphql.Directive{deferDirective}, + NonSpecArgumentHandling: nonSpec, + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + return schema +} + +func TestValidate_ProvidedNonNullArguments_DirectiveArguments_NoErrorOnNonNullArgumentWithDefaultValue(t *testing.T) { + schema := nonNullDirectiveArgWithDefaultSchema(t, false) + testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.ProvidedNonNullArgumentsRule, ` + { + ... on Query @defer { + a + } + } + `) +} + +func TestValidate_ProvidedNonNullArguments_DirectiveArguments_NonSpecErrorsOnNonNullArgumentWithDefaultValue(t *testing.T) { + schema := nonNullDirectiveArgWithDefaultSchema(t, true) + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.ProvidedNonNullArgumentsRule, ` + { + ... on Query @defer { + a + } + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Directive "@defer" argument "if" of type "Boolean!" is required but not provided.`, 3, 24), + }) +} + +// A default coercion will not substitute — a typed nil pointer is nullish — +// does not make a non-null argument optional. +func TestValidate_ProvidedNonNullArguments_FieldArguments_NullishDefaultKeepsArgumentRequired(t *testing.T) { + var nilString *string + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "fieldWithNullishDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "arg": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.Boolean), + DefaultValue: nilString, + }, + }, + }, + }, + }), + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.ProvidedNonNullArgumentsRule, ` + { + fieldWithNullishDefault + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Field "fieldWithNullishDefault" argument "arg" of type "Boolean!" is required but not provided.`, 3, 11), + }) +} diff --git a/rules_variables_in_allowed_position_test.go b/rules_variables_in_allowed_position_test.go index 8739d3b..76004d0 100644 --- a/rules_variables_in_allowed_position_test.go +++ b/rules_variables_in_allowed_position_test.go @@ -244,3 +244,108 @@ func TestValidate_VariablesInAllowedPosition_StringToNonNullableBooleanInDirecti `expecting type "Boolean!".`, 2, 19, 3, 26), }) } + +// Two non-null arguments, one declaring a default and one not, under whichever +// mode the caller asks for. +func nonNullArgDefaultPositionSchema(t *testing.T, nonSpec bool) graphql.Schema { + t.Helper() + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "withDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.String), + DefaultValue: "NNDEF", + }, + }, + }, + "withoutDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{Type: graphql.NewNonNull(graphql.String)}, + }, + }, + }, + }), + NonSpecArgumentHandling: nonSpec, + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + return schema +} + +// Spec §5.8.5: hasLocationDefaultValue permits a nullable variable at a non-null +// location that declares a default. +func TestValidate_VariablesInAllowedPosition_StringToNonNullStringWithArgumentDefault(t *testing.T) { + schema := nonNullArgDefaultPositionSchema(t, false) + testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.VariablesInAllowedPositionRule, ` + query Probe($x: String) { + withDefault(a: $x) + } + `) +} + +// Without a default on either side the usage stays rejected. +func TestValidate_VariablesInAllowedPosition_StringToNonNullStringWithoutArgumentDefault(t *testing.T) { + schema := nonNullArgDefaultPositionSchema(t, false) + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.VariablesInAllowedPositionRule, ` + query Probe($x: String) { + withoutDefault(a: $x) + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Variable "$x" of type "String" used in position `+ + `expecting type "String!".`, 2, 19, 3, 27), + }) +} + +// NonSpecArgumentHandling only ever considered the variable's own default, so it +// keeps rejecting the usage. +func TestValidate_VariablesInAllowedPosition_NonSpecIgnoresArgumentDefault(t *testing.T) { + schema := nonNullArgDefaultPositionSchema(t, true) + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.VariablesInAllowedPositionRule, ` + query Probe($x: String) { + withDefault(a: $x) + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Variable "$x" of type "String" used in position `+ + `expecting type "String!".`, 2, 19, 3, 24), + }) +} + +// Spec §5.8.5 hasLocationDefaultValue: a default coercion will not substitute +// does not permit a nullable variable at a non-null location. Validation is +// static, so this holds whatever values the request later supplies. +func TestValidate_VariablesInAllowedPosition_NullishArgumentDefaultIsNotADefault(t *testing.T) { + var nilString *string + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "Query", + Fields: graphql.Fields{ + "withNullishDefault": &graphql.Field{ + Type: graphql.String, + Args: graphql.FieldConfigArgument{ + "a": &graphql.ArgumentConfig{ + Type: graphql.NewNonNull(graphql.String), + DefaultValue: nilString, + }, + }, + }, + }, + }), + }) + if err != nil { + t.Fatalf("Unexpected error, got: %v", err) + } + testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.VariablesInAllowedPositionRule, ` + query Probe($x: String) { + withNullishDefault(a: $x) + } + `, []gqlerrors.FormattedError{ + testutil.RuleError(`Variable "$x" of type "String" used in position `+ + `expecting type "String!".`, 2, 19, 3, 31), + }) +} diff --git a/schema.go b/schema.go index f4d7484..67365e4 100644 --- a/schema.go +++ b/schema.go @@ -7,6 +7,32 @@ type SchemaConfig struct { Types []Type Directives []*Directive Extensions []Extension + + // NonSpecArgumentHandling restores argument handling that contradicts the + // GraphQL specification. Leave it false: the zero value follows the + // specification (CoerceArgumentValues §6.4.1, CoerceVariableValues §6.1.2, + // input object coercion §3.10 and required arguments §5.4.2.1): + // + // - A variable the caller did not supply leaves its argument absent from + // ResolveParams.Args instead of materialising it as nil, so a resolver + // can tell "not provided" from "explicitly null". + // - A default value applies only when no value was supplied. An explicit + // null stays null instead of falling back to the default. + // - A non-null argument that declares a default value is optional, so a + // document may omit it and still validate. + // + // Releases before that fix collapsed the first two distinctions — every + // declared argument arrived present, and an explicit null was replaced by + // the default — and rejected any document that omitted a non-null argument, + // default or not. None of that was a deliberate design: it was a defect, + // and a schema that sets this flag keeps diverging from the specification + // and from every other GraphQL implementation. + // + // Deprecated: this exists only so an application built against the defect + // keeps working while it migrates, and it will be removed once no schema + // needs it. Set it to true to reproduce the old behaviour byte-for-byte, + // then migrate off it. + NonSpecArgumentHandling bool } type TypeMap map[string]Type @@ -43,6 +69,8 @@ type Schema struct { implementations map[string][]*Object possibleTypeMap map[string]map[string]bool extensions []Extension + + nonSpecArgumentHandling bool } func NewSchema(config SchemaConfig) (Schema, error) { @@ -65,6 +93,7 @@ func NewSchema(config SchemaConfig) (Schema, error) { schema.queryType = config.Query schema.mutationType = config.Mutation schema.subscriptionType = config.Subscription + schema.nonSpecArgumentHandling = config.NonSpecArgumentHandling // Provide specified directives (e.g. @include and @skip) by default. schema.directives = config.Directives diff --git a/subscription.go b/subscription.go index bdfd282..4c30ef0 100644 --- a/subscription.go +++ b/subscription.go @@ -166,7 +166,14 @@ func ExecuteSubscription(p ExecuteParams) chan *Result { Key: responseName, } - args := getArgumentValues(fieldDef.Args, fieldNode.Arguments, exeContext.VariableValues) + args, argErr := getArgumentValues(fieldDef.Args, fieldNode.Arguments, exeContext.VariableValues, exeContext.Schema.nonSpecArgumentHandling) + if argErr != nil { + resultChannel <- &Result{ + Errors: gqlerrors.FormatErrors(argErr), + } + + return + } info := ResolveInfo{ FieldName: fieldName, FieldASTs: fieldNodes, diff --git a/type_info.go b/type_info.go index f296444..dd2dce1 100644 --- a/type_info.go +++ b/type_info.go @@ -14,14 +14,15 @@ import ( type fieldDefFn func(schema *Schema, parentType Type, fieldAST *ast.Field) *FieldDefinition type TypeInfo struct { - schema *Schema - typeStack []Output - parentTypeStack []Composite - inputTypeStack []Input - fieldDefStack []*FieldDefinition - directive *Directive - argument *Argument - getFieldDef fieldDefFn + schema *Schema + typeStack []Output + parentTypeStack []Composite + inputTypeStack []Input + defaultValueStack []interface{} + fieldDefStack []*FieldDefinition + directive *Directive + argument *Argument + getFieldDef fieldDefFn } type TypeInfoConfig struct { @@ -64,6 +65,16 @@ func (ti *TypeInfo) InputType() Input { } return nil } + +// DefaultValue returns the default value declared by the argument or input +// object field currently being visited, or nil when the position declares none. +// Spec §5.8.5 calls this hasLocationDefaultValue. +func (ti *TypeInfo) DefaultValue() interface{} { + if len(ti.defaultValueStack) > 0 { + return ti.defaultValueStack[len(ti.defaultValueStack)-1] + } + return nil +} func (ti *TypeInfo) FieldDef() *FieldDefinition { if len(ti.fieldDefStack) > 0 { return ti.fieldDefStack[len(ti.fieldDefStack)-1] @@ -163,9 +174,16 @@ func (ti *TypeInfo) Enter(node ast.Node) { argType = argDef.Type } ti.argument = argDef + var argDefault interface{} + if argDef != nil { + argDefault = argDef.DefaultValue + } + ti.defaultValueStack = append(ti.defaultValueStack, argDefault) ti.inputTypeStack = append(ti.inputTypeStack, argType) case *ast.ListValue: listType := GetNullable(ti.InputType()) + // List positions never have a default value. + ti.defaultValueStack = append(ti.defaultValueStack, nil) if list, ok := listType.(*List); ok { ti.inputTypeStack = append(ti.inputTypeStack, list.OfType) } else { @@ -173,6 +191,7 @@ func (ti *TypeInfo) Enter(node ast.Node) { } case *ast.ObjectField: var fieldType Input + var fieldDefault interface{} objectType := GetNamed(ti.InputType()) if objectType, ok := objectType.(*InputObject); ok { @@ -182,8 +201,10 @@ func (ti *TypeInfo) Enter(node ast.Node) { } if inputField, ok := objectType.Fields()[nameVal]; ok { fieldType = inputField.Type + fieldDefault = inputField.DefaultValue } } + ti.defaultValueStack = append(ti.defaultValueStack, fieldDefault) ti.inputTypeStack = append(ti.inputTypeStack, fieldType) } } @@ -218,11 +239,19 @@ func (ti *TypeInfo) Leave(node ast.Node) { } case kinds.Argument: ti.argument = nil + // pop ti.defaultValueStack + if len(ti.defaultValueStack) > 0 { + _, ti.defaultValueStack = ti.defaultValueStack[len(ti.defaultValueStack)-1], ti.defaultValueStack[:len(ti.defaultValueStack)-1] + } // pop ti.inputTypeStack if len(ti.inputTypeStack) > 0 { _, ti.inputTypeStack = ti.inputTypeStack[len(ti.inputTypeStack)-1], ti.inputTypeStack[:len(ti.inputTypeStack)-1] } case kinds.ListValue, kinds.ObjectField: + // pop ti.defaultValueStack + if len(ti.defaultValueStack) > 0 { + _, ti.defaultValueStack = ti.defaultValueStack[len(ti.defaultValueStack)-1], ti.defaultValueStack[:len(ti.defaultValueStack)-1] + } // pop ti.inputTypeStack if len(ti.inputTypeStack) > 0 { _, ti.inputTypeStack = ti.inputTypeStack[len(ti.inputTypeStack)-1], ti.inputTypeStack[:len(ti.inputTypeStack)-1] diff --git a/validator.go b/validator.go index b003aa0..af76ab6 100644 --- a/validator.go +++ b/validator.go @@ -83,6 +83,9 @@ var _ HasSelectionSet = (*ast.FragmentDefinition)(nil) type VariableUsage struct { Node *ast.Variable Type Input + // LocationDefaultValue is the default value declared by the argument or input + // object field where this usage sits. Spec §5.8.5 hasLocationDefaultValue. + LocationDefaultValue interface{} } type ValidationContext struct { @@ -239,8 +242,9 @@ func (ctx *ValidationContext) VariableUsages(node HasSelectionSet) []*VariableUs Kind: func(p visitor.VisitFuncParams) (string, interface{}) { if node, ok := p.Node.(*ast.Variable); ok && node != nil { usages = append(usages, &VariableUsage{ - Node: node, - Type: typeInfo.InputType(), + Node: node, + Type: typeInfo.InputType(), + LocationDefaultValue: typeInfo.DefaultValue(), }) } return visitor.ActionNoChange, nil diff --git a/values.go b/values.go index 4ed4b46..e7cb245 100644 --- a/values.go +++ b/values.go @@ -27,9 +27,12 @@ func getVariableValues( continue } varName := defAST.Variable.Name.Value - if varValue, err := getVariableValue(schema, defAST, inputs[varName]); err != nil { + input, provided := inputs[varName] + varValue, err := getVariableValue(schema, defAST, input, provided) + if err != nil { return values, err - } else { + } + if schema.nonSpecArgumentHandling || provided || defAST.DefaultValue != nil { values[varName] = varValue } } @@ -40,7 +43,7 @@ func getVariableValues( // definitions and list of argument AST nodes. func getArgumentValues( argDefs []*Argument, argASTs []*ast.Argument, - variableValues map[string]interface{}) map[string]interface{} { + variableValues map[string]interface{}, nonSpec bool) (map[string]interface{}, error) { argASTMap := map[string]*ast.Argument{} for _, argAST := range argASTs { @@ -50,31 +53,144 @@ func getArgumentValues( } results := map[string]interface{}{} for _, argDef := range argDefs { - var ( - tmp interface{} - value ast.Value - isUndefined bool - ) - if tmpValue, ok := argASTMap[argDef.PrivateName]; ok { - value = tmpValue.Value - } else { - isUndefined = true + var value ast.Value + argAST, ok := argASTMap[argDef.PrivateName] + if ok { + value = argAST.Value + } + if nonSpec { + isUndefined := !ok + tmp := valueFromAST(value, argDef.Type, variableValues, nonSpec) + if isNullish(tmp) { + tmp = argDef.DefaultValue + } + if !isUndefined && tmp == nil { + results[argDef.PrivateName] = nil + } else if !isNullish(tmp) { + results[argDef.PrivateName] = tmp + } + continue } - if tmp = valueFromAST(value, argDef.Type, variableValues); isNullish(tmp) { + // hasValue is false when the argument is not written in the query, or + // when it references a variable the caller did not supply. Only then + // does the default apply — an explicit null is a supplied value. + isUndefined := !ok || isUnprovidedVariable(value, variableValues) + tmp := valueFromAST(value, argDef.Type, variableValues, nonSpec) + // A literal the argument's type cannot parse also leaves tmp nullish. The + // specification calls for a field error there (CoerceArgumentValues + // §6.4.1); this implementation has always fallen back to the default + // instead, and such a document fails validation anyway, so that case is + // left as it is. Only a supplied null keeps its null. + if isNullish(tmp) && !isProvidedNullVariable(value, variableValues) { tmp = argDef.DefaultValue } - if !isUndefined && tmp == nil { - results[argDef.PrivateName] = nil - } else if !isNullish(tmp) { + // Spec §6.4.1 ② for the argument itself, and the fourth rule of input + // object coercion (§3.10) for a field whose value came from a variable: a + // null at a non-null position is a field error. The default was already + // applied above, so a null still standing here is one the caller supplied + // — or, for the argument itself, a value that was never supplied and has + // no default to stand in. + if message, found := firstNonNullViolation(tmp, argDef.Type); found { + var nodes []ast.Node + if argAST != nil { + nodes = []ast.Node{argAST} + } + text := fmt.Sprintf("Argument %q has invalid value.\n%v", argDef.PrivateName, message) + if _, isNonNull := argDef.Type.(*NonNull); isNonNull && isNullish(tmp) { + // The argument itself is the null, so there is no path to report. + text = fmt.Sprintf(`Argument "%v" of non-null type "%v" must not be null.`, + argDef.PrivateName, argDef.Type) + } + return nil, gqlerrors.NewError(text, nodes, "", nil, []int{}, nil) + } + if !isUndefined || !isNullish(tmp) { results[argDef.PrivateName] = tmp } } - return results + return results, nil +} + +// Walks a coerced argument value and reports the first null sitting at a +// non-null position, together with the path leading to it. Spec §6.4.1 ② covers +// the argument itself and the fourth rule of input object coercion (§3.10) +// covers a field whose value came from a variable: in both cases a null that the +// caller supplied where the type is non-null is a field error. +// +// Only entries the coerced value actually carries are visited. A field the +// caller never supplied is absent from the map, and whether that is allowed is +// the validation rules' business, not this walk's. +func firstNonNullViolation(value interface{}, ttype Input) (string, bool) { + switch ttype := ttype.(type) { + case *NonNull: + if isNullish(value) { + if name := ttype.OfType.Name(); name != "" { + return fmt.Sprintf(`Expected "%v!", found null.`, name), true + } + return "Expected non-null value, found null.", true + } + ofType, _ := ttype.OfType.(Input) + return firstNonNullViolation(value, ofType) + case *List: + itemType, _ := ttype.OfType.(Input) + valType := reflect.ValueOf(value) + if valType.Kind() != reflect.Slice { + return "", false + } + for i := 0; i < valType.Len(); i++ { + if message, found := firstNonNullViolation(valType.Index(i).Interface(), itemType); found { + return fmt.Sprintf("In element #%v: %v", i+1, message), true + } + } + case *InputObject: + valueMap, ok := value.(map[string]interface{}) + if !ok { + return "", false + } + // Sorted so the reported field is stable across runs. + fieldNames := make([]string, 0, len(valueMap)) + for fieldName := range valueMap { + fieldNames = append(fieldNames, fieldName) + } + sort.Strings(fieldNames) + fields := ttype.Fields() + for _, fieldName := range fieldNames { + field, ok := fields[fieldName] + if !ok { + continue + } + if message, found := firstNonNullViolation(valueMap[fieldName], field.Type); found { + return fmt.Sprintf("In field %q: %v", fieldName, message), true + } + } + } + return "", false +} + +// Returns true if value is a reference to a variable the caller did not supply. +func isUnprovidedVariable(value ast.Value, variables map[string]interface{}) bool { + v, ok := value.(*ast.Variable) + if !ok || v.Name == nil { + return false + } + _, provided := variables[v.Name.Value] + return !provided +} + +// Returns true if value is a reference to a variable the caller supplied as +// null. Such a null is a value of its own, so no default may stand in for it. +func isProvidedNullVariable(value ast.Value, variables map[string]interface{}) bool { + v, ok := value.(*ast.Variable) + if !ok || v.Name == nil { + return false + } + supplied, provided := variables[v.Name.Value] + return provided && isNullish(supplied) } // Given a variable definition, and any value of input, return a value which // adheres to the variable definition, or throw an error. -func getVariableValue(schema Schema, definitionAST *ast.VariableDefinition, input interface{}) (interface{}, error) { +func getVariableValue(schema Schema, definitionAST *ast.VariableDefinition, input interface{}, provided bool) (interface{}, error) { + nonSpec := schema.nonSpecArgumentHandling ttype, err := typeFromAST(schema, definitionAST.Type) if err != nil { return nil, err @@ -93,14 +209,25 @@ func getVariableValue(schema Schema, definitionAST *ast.VariableDefinition, inpu ) } - isValid, messages := isValidInputValue(input, ttype) + // Spec §6.1.2 ①: the default stands in for a value the caller did not supply, + // and that step precedes the non-null requirement in ②. Applying it here is + // what lets a non-null variable declare a default — the validation below + // would otherwise reject the very case the default exists for. An explicitly + // supplied null is a value, so it does not reach this branch. + if !nonSpec && !provided && definitionAST.DefaultValue != nil { + return valueFromAST(definitionAST.DefaultValue, ttype, nil, nonSpec), nil + } + + isValid, messages := isValidInputValue(input, ttype, nonSpec) if isValid { if isNullish(input) { - if definitionAST.DefaultValue != nil { - return valueFromAST(definitionAST.DefaultValue, ttype, nil), nil + // Only a schema that opted out still lets the default replace a supplied + // null; the spec-compliant path applied the default above. + if nonSpec && definitionAST.DefaultValue != nil { + return valueFromAST(definitionAST.DefaultValue, ttype, nil, nonSpec), nil } } - return coerceValue(ttype, input), nil + return coerceValue(ttype, input, nonSpec), nil } if isNullish(input) { return "", gqlerrors.NewError( @@ -135,24 +262,24 @@ func getVariableValue(schema Schema, definitionAST *ast.VariableDefinition, inpu } // Given a type and any value, return a runtime value coerced to match the type. -func coerceValue(ttype Input, value interface{}) interface{} { +func coerceValue(ttype Input, value interface{}, nonSpec bool) interface{} { if isNullish(value) { return nil } switch ttype := ttype.(type) { case *NonNull: - return coerceValue(ttype.OfType, value) + return coerceValue(ttype.OfType, value, nonSpec) case *List: var values = []interface{}{} valType := reflect.ValueOf(value) if valType.Kind() == reflect.Slice { for i := 0; i < valType.Len(); i++ { val := valType.Index(i).Interface() - values = append(values, coerceValue(ttype.OfType, val)) + values = append(values, coerceValue(ttype.OfType, val, nonSpec)) } return values } - return append(values, coerceValue(ttype.OfType, value)) + return append(values, coerceValue(ttype.OfType, value, nonSpec)) case *InputObject: var obj = map[string]interface{}{} valueMap, _ := value.(map[string]interface{}) @@ -165,7 +292,13 @@ func coerceValue(ttype Input, value interface{}) interface{} { if !ok && isNullish(field.DefaultValue) { continue } - fieldValue := coerceValue(field.Type, v) + // The key is present and holds null: the caller supplied a value, so + // the field's default must not stand in for it. + if !nonSpec && ok && isNullish(v) { + obj[name] = nil + continue + } + fieldValue := coerceValue(field.Type, v, nonSpec) if isNullish(fieldValue) { fieldValue = field.DefaultValue } @@ -217,7 +350,7 @@ func typeFromAST(schema Schema, inputTypeAST ast.Type) (Type, error) { // Given a value and a GraphQL type, determine if the value will be // accepted for that type. This is primarily useful for validating the // runtime values of query variables. -func isValidInputValue(value interface{}, ttype Input) (bool, []string) { +func isValidInputValue(value interface{}, ttype Input, nonSpec bool) (bool, []string) { if isNullish(value) { if ttype, ok := ttype.(*NonNull); ok { if ttype.OfType.Name() != "" { @@ -229,7 +362,7 @@ func isValidInputValue(value interface{}, ttype Input) (bool, []string) { } switch ttype := ttype.(type) { case *NonNull: - return isValidInputValue(value, ttype.OfType) + return isValidInputValue(value, ttype.OfType, nonSpec) case *List: valType := reflect.ValueOf(value) if valType.Kind() == reflect.Ptr { @@ -239,14 +372,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) + _, messages := isValidInputValue(val, ttype.OfType, nonSpec) for idx, message := range messages { messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, idx+1, message)) } } return (len(messagesReduce) == 0), messagesReduce } - return isValidInputValue(value, ttype.OfType) + return isValidInputValue(value, ttype.OfType, nonSpec) case *InputObject: messagesReduce := []string{} @@ -280,7 +413,22 @@ func isValidInputValue(value interface{}, ttype Input) (bool, []string) { // Ensure every defined field is valid. for _, fieldName := range fieldNames { - _, messages := isValidInputValue(valueMap[fieldName], fields[fieldName].Type) + field := fields[fieldName] + v, ok := valueMap[fieldName] + // Spec §3.10 Input Coercion and §5.6.4 Input Object Required Fields: a + // field that declares a default value is optional even when its type is + // non-null, because the default stands in for the value the caller did + // not supply. A key that is present and holds null is a supplied value, + // so it is still validated. + // + // The test matches the one coercion applies in coerceValue and + // valueFromAST: a nullish default is not a value either of them will + // substitute, so treating it as one here would pass a field that then + // goes missing from the coerced map. + if !nonSpec && !ok && !isNullish(field.DefaultValue) { + continue + } + _, messages := isValidInputValue(v, field.Type, nonSpec) if messages != nil { for _, message := range messages { messagesReduce = append(messagesReduce, fmt.Sprintf(`In field "%v": %v`, fieldName, message)) @@ -354,7 +502,7 @@ func isIterable(src interface{}) bool { * | Int / Float | Number | * */ -func valueFromAST(valueAST ast.Value, ttype Input, variables map[string]interface{}) interface{} { +func valueFromAST(valueAST ast.Value, ttype Input, variables map[string]interface{}, nonSpec bool) interface{} { if valueAST == nil { return nil } @@ -370,27 +518,23 @@ func valueFromAST(valueAST ast.Value, ttype Input, variables map[string]interfac } switch ttype := ttype.(type) { case *NonNull: - return valueFromAST(valueAST, ttype.OfType, variables) + return valueFromAST(valueAST, ttype.OfType, variables, nonSpec) case *List: values := []interface{}{} if valueAST, ok := valueAST.(*ast.ListValue); ok { for _, itemAST := range valueAST.Values { - values = append(values, valueFromAST(itemAST, ttype.OfType, variables)) + values = append(values, valueFromAST(itemAST, ttype.OfType, variables, nonSpec)) } return values } - return append(values, valueFromAST(valueAST, ttype.OfType, variables)) + return append(values, valueFromAST(valueAST, ttype.OfType, variables, nonSpec)) case *InputObject: - var ( - ok bool - ov *ast.ObjectValue - of *ast.ObjectField - ) - if ov, ok = valueAST.(*ast.ObjectValue); !ok { + ov, ok := valueAST.(*ast.ObjectValue) + if !ok { return nil } fieldASTs := map[string]*ast.ObjectField{} - for _, of = range ov.Fields { + for _, of := range ov.Fields { if of == nil || of.Name == nil { continue } @@ -398,20 +542,29 @@ func valueFromAST(valueAST ast.Value, ttype Input, variables map[string]interfac } obj := map[string]interface{}{} for name, field := range ttype.Fields() { - var ( - value interface{} - isUndefined bool - ) - if of, ok = fieldASTs[name]; ok { - value = valueFromAST(of.Value, field.Type, variables) - } else { - isUndefined = true - value = field.DefaultValue + of, ok := fieldASTs[name] + if nonSpec { + var value interface{} + if ok { + value = valueFromAST(of.Value, field.Type, variables, nonSpec) + } else { + value = field.DefaultValue + } + if ok && value == nil { + obj[name] = nil + } else if !isNullish(value) { + obj[name] = value + } + continue } - if !isUndefined && value == nil { - obj[name] = nil - } else if !isNullish(value) { - obj[name] = value + // The field is written in the literal and does not reference an + // unsupplied variable: the caller supplied a value, so the field's + // default must not stand in for it. + supplied := ok && !isUnprovidedVariable(of.Value, variables) + if supplied { + obj[name] = valueFromAST(of.Value, field.Type, variables, nonSpec) + } else if !isNullish(field.DefaultValue) { + obj[name] = field.DefaultValue } } return obj diff --git a/values_test.go b/values_test.go index 12bb5b9..dc03ebf 100644 --- a/values_test.go +++ b/values_test.go @@ -1,6 +1,7 @@ package graphql import ( + "math" "reflect" "testing" ) @@ -56,14 +57,87 @@ func Test_coerceValue(t *testing.T) { }, } + // None of these cases involve a default value, so both coercion modes must + // agree on all of them. for name, tc := range testCases { name, tc := name, tc - t.Run(name, func(t *testing.T) { - t.Parallel() + for _, nonSpec := range []bool{false, true} { + nonSpec := nonSpec + mode := "spec" + if nonSpec { + mode = "nonSpec" + } + t.Run(name+"/"+mode, func(t *testing.T) { + t.Parallel() + + got := coerceValue(tc.input.ttype, tc.input.value, nonSpec) + if !reflect.DeepEqual(tc.expected, got) { + t.Errorf("unexpected result, expected: %v, got: %v", tc.expected, got) + } + }) + } + } +} + +// Spec §3.10 / §5.6.4: an input field is required only when its type is non-null +// AND it declares no default value. Pinned at the function level so the three +// states stay distinct: absent-with-default, absent-without-default, explicit null. +func Test_isValidInputValue_NonNullFieldWithDefault(t *testing.T) { + withDefault := NewInputObject(InputObjectConfig{ + Name: "WithDefault", + Fields: InputObjectConfigFieldMap{ + "a": &InputObjectFieldConfig{Type: NewNonNull(String), DefaultValue: "FIELDDEF"}, + }, + }) + withoutDefault := NewInputObject(InputObjectConfig{ + Name: "WithoutDefault", + Fields: InputObjectConfigFieldMap{ + "a": &InputObjectFieldConfig{Type: NewNonNull(String)}, + }, + }) - got := coerceValue(tc.input.ttype, tc.input.value) - if !reflect.DeepEqual(tc.expected, got) { - t.Errorf("unexpected result, expected: %v, got: %v", tc.expected, got) + if isValid, messages := isValidInputValue(map[string]interface{}{}, withDefault, false); !isValid { + t.Errorf("spec mode: expected an omitted field with a default to be valid, got: %v", messages) + } + if isValid, _ := isValidInputValue(map[string]interface{}{}, withDefault, true); isValid { + t.Error("non-spec mode: expected an omitted non-null field to be invalid") + } + for _, nonSpec := range []bool{false, true} { + if isValid, _ := isValidInputValue(map[string]interface{}{"a": nil}, withDefault, nonSpec); isValid { + t.Errorf("expected an explicit null to be invalid (nonSpec=%v)", nonSpec) + } + if isValid, _ := isValidInputValue(map[string]interface{}{}, withoutDefault, nonSpec); isValid { + t.Errorf("expected an omitted field without a default to be invalid (nonSpec=%v)", nonSpec) + } + } +} + +// A default that is nullish but not nil — a typed nil pointer, say — is not a +// default coercion can substitute: coerceValue and valueFromAST both skip it +// under isNullish. Validation has to agree, otherwise a non-null field passes +// validation and then goes missing from the coerced map. +func Test_isValidInputValue_NullishDefaultIsNotADefault(t *testing.T) { + var nilString *string + for _, tc := range []struct { + name string + defaultVal interface{} + wantValid bool + }{ + {"usable default", "FIELDDEF", true}, + {"no default", nil, false}, + {"typed nil pointer", nilString, false}, + {"NaN", math.NaN(), false}, + } { + t.Run(tc.name, func(t *testing.T) { + in := NewInputObject(InputObjectConfig{ + Name: "NullishDefault" + tc.name, + Fields: InputObjectConfigFieldMap{ + "a": &InputObjectFieldConfig{Type: NewNonNull(String), DefaultValue: tc.defaultVal}, + }, + }) + isValid, messages := isValidInputValue(map[string]interface{}{}, in, false) + if isValid != tc.wantValid { + t.Errorf("isValid = %v, want %v (messages: %v)", isValid, tc.wantValid, messages) } }) } diff --git a/variables_test.go b/variables_test.go index a6813b9..8f89e62 100644 --- a/variables_test.go +++ b/variables_test.go @@ -898,21 +898,29 @@ func TestVariables_NonNullableScalars_AllowsNonNullableInputsToBeSetToAValueDire t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } } -func TestVariables_NonNullableScalars_PassesAlongNullForNonNullableInputsIfExplicitlySetInTheQuery(t *testing.T) { + +// Spec §6.4.1 ②: a non-null argument is a field error when no value was supplied +// or the supplied value is null. Reached here only through Execute, because +// ProvidedNonNullArgumentsRule rejects such a document during validation. Before +// the coercion fix this returned null with no error, leaving the resolver unable +// to tell that a required argument never arrived. +func TestVariables_NonNullableScalars_DoesNotAllowNonNullableInputsToBeOmittedDirectly(t *testing.T) { doc := ` { fieldWithNonNullableStringInput } ` - params := map[string]interface{}{ - "value": "a", - } - expected := &graphql.Result{ Data: map[string]interface{}{ "fieldWithNonNullableStringInput": nil, }, + Errors: []gqlerrors.FormattedError{ + { + Message: `Argument "input" of non-null type "String!" must not be null.`, + Locations: []location.SourceLocation{}, + }, + }, } ast := testutil.TestParse(t, doc) @@ -921,13 +929,9 @@ func TestVariables_NonNullableScalars_PassesAlongNullForNonNullableInputsIfExpli ep := graphql.ExecuteParams{ Schema: variablesTestSchema, AST: ast, - Args: params, } result := testutil.TestExecute(t, ep) - if len(result.Errors) > 0 { - t.Fatalf("wrong result, unexpected errors: %v", result.Errors) - } - if !reflect.DeepEqual(expected, result) { + if !testutil.EqualResults(expected, result) { t.Fatalf("Unexpected result, Diff: %v", testutil.Diff(expected, result)) } }