Skip to content

Commit

Permalink
Sergiy/eng 5001 fix variables validation (#767)
Browse files Browse the repository at this point in the history
  • Loading branch information
@devsergiy
devsergiy authored Apr 9, 2024
1 parent faf904e commit 7975610
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 68 deletions.
2 changes: 1 addition & 1 deletion execution/engine/federation_integration_static_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ subscription UpdatedPrice {
}()

assert.Eventuallyf(t, func() bool {
msg := `{"data":{"updatedPrice":{"name":"Trilby","price":%d,"reviews":[{"body":"A highly effective form of birth control.","author":{"id":"1234","username":"Me"}}]}}}`
msg := `{"data":{"updatedPrice":{"name":"Boater","price":%d,"reviews":[{"body":"This is the last straw. Hat you will wear. 11/10","author":{"id":"7777","username":"User 7777"}}]}}}`
price := 10
if secondRun {
price += 2
Expand Down
18 changes: 4 additions & 14 deletions execution/engine/federation_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestFederationIntegrationTest(t *testing.T) {

t.Run("query spans multiple federated servers", func(t *testing.T) {
resp := gqlClient.Query(ctx, setup.GatewayServer.URL, testQueryPath("queries/multiple_upstream.query"), nil, t)
assert.Equal(t, `{"data":{"topProducts":[{"name":"Trilby","reviews":[{"body":"A highly effective form of birth control.","author":{"username":"Me"}}]},{"name":"Fedora","reviews":[{"body":"Fedoras are one of the most fashionable hats around and can look great with a variety of outfits.","author":{"username":"Me"}}]},{"name":"Boater","reviews":[{"body":"This is the last straw. Hat you will wear. 11/10","author":{"username":"User 7777"}}]}]}}`, string(resp))
assert.Equal(t, `{"data":{"topProducts":[{"name":"Trilby","reviews":[{"body":"A highly effective form of birth control.","author":{"username":"Me"}}]},{"name":"Fedora","reviews":[{"body":"Fedoras are one of the most fashionable hats around and can look great with a variety of outfits.","author":{"username":"Me"}}]}]}}`, string(resp))
})

t.Run("mutation operation with variables", func(t *testing.T) {
Expand Down Expand Up @@ -130,18 +130,13 @@ func TestFederationIntegrationTest(t *testing.T) {
"topProducts": [
{
"__typename": "Product",
"price": 2,
"price": 11,
"upc": "top-1"
},
{
"__typename": "Product",
"price": 22,
"upc": "top-2"
},
{
"__typename": "Product",
"price": 33,
"upc": "top-3"
}
],
"me": {
Expand All @@ -153,7 +148,7 @@ func TestFederationIntegrationTest(t *testing.T) {
"__typename": "Review",
"product": {
"__typename": "Product",
"price": 2,
"price": 11,
"upc": "top-1"
}
},
Expand Down Expand Up @@ -182,18 +177,13 @@ func TestFederationIntegrationTest(t *testing.T) {
"topProducts": [
{
"__typename": "Product",
"price": 2,
"price": 11,
"upc": "top-1"
},
{
"__typename": "Product",
"price": 22,
"upc": "top-2"
},
{
"__typename": "Product",
"price": 33,
"upc": "top-3"
}
],
"me": {
Expand Down
4 changes: 2 additions & 2 deletions execution/federationtesting/products/graph/schema.resolvers.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

126 changes: 75 additions & 51 deletions v2/pkg/variablesvalidation/variablesvalidation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,39 +114,52 @@ func (v *variablesVisitor) popPath() {
func (v *variablesVisitor) EnterVariableDefinition(ref int) {
varTypeRef := v.operation.VariableDefinitions[ref].Type
varName := v.operation.VariableValueNameBytes(v.operation.VariableDefinitions[ref].VariableValue.Ref)
varTypeName := v.operation.ResolveTypeNameBytes(varTypeRef)
jsonField := v.variables.GetObjectFieldBytes(v.variables.RootNode, varName)
if v.operation.TypeIsNonNull(varTypeRef) {
if jsonField == -1 {
v.renderVariableRequiredError(varName, varTypeRef)
jsonFieldRef := v.variables.GetObjectFieldBytes(v.variables.RootNode, varName)

v.path = v.path[:0]
v.pushObjectPath(varName)
v.currentVariableName = varName
v.currentVariableJsonNodeRef = jsonFieldRef

v.traverseOperationType(jsonFieldRef, varTypeRef)
}

func (v *variablesVisitor) traverseOperationType(jsonFieldRef int, operationTypeRef int) {
if v.operation.TypeIsNonNull(operationTypeRef) {
if jsonFieldRef == -1 {
v.renderVariableRequiredError(v.currentVariableName, operationTypeRef)
return
}
if v.variables.Nodes[jsonField].Kind == astjson.NodeKindNull {
v.renderVariableInvalidNullError(varName, varTypeRef)
if v.variables.Nodes[jsonFieldRef].Kind == astjson.NodeKindNull {
v.renderVariableInvalidNullError(v.currentVariableName, operationTypeRef)
return
}

v.traverseOperationType(jsonFieldRef, v.operation.Types[operationTypeRef].OfType)
return
}
if !v.variables.NodeIsDefined(jsonField) {

if !v.variables.NodeIsDefined(jsonFieldRef) {
return
}
v.path = v.path[:0]
v.pushObjectPath(varName)
v.currentVariableName = varName
v.currentVariableJsonNodeRef = jsonField
if v.operation.TypeIsList(varTypeRef) {
if v.variables.Nodes[jsonField].Kind != astjson.NodeKindArray {
v.renderVariableInvalidTypeError(varTypeName, v.variables.Nodes[jsonField])

varTypeName := v.operation.ResolveTypeNameBytes(operationTypeRef)

if v.operation.TypeIsList(operationTypeRef) {
if v.variables.Nodes[jsonFieldRef].Kind != astjson.NodeKindArray {
v.renderVariableInvalidObjectTypeError(varTypeName, v.variables.Nodes[jsonFieldRef])
return
}
for i, arrayValue := range v.variables.Nodes[jsonField].ArrayValues {
for i, arrayValue := range v.variables.Nodes[jsonFieldRef].ArrayValues {
v.pushArrayPath(i)
v.traverseNode(arrayValue, varTypeName)
v.traverseOperationType(arrayValue, v.operation.Types[operationTypeRef].OfType)
v.popPath()
continue
}
return
}
v.traverseNode(jsonField, varTypeName)

v.traverseNamedTypeNode(jsonFieldRef, varTypeName)
}

func (v *variablesVisitor) renderVariableRequiredError(variableName []byte, typeRef int) {
Expand All @@ -161,7 +174,7 @@ func (v *variablesVisitor) renderVariableRequiredError(variableName []byte, type
}
}

func (v *variablesVisitor) renderVariableInvalidTypeError(typeName []byte, variablesNode astjson.Node) {
func (v *variablesVisitor) renderVariableInvalidObjectTypeError(typeName []byte, variablesNode astjson.Node) {
out := &bytes.Buffer{}
err := v.variables.PrintNode(variablesNode, out)
if err != nil {
Expand Down Expand Up @@ -292,7 +305,45 @@ func (v *variablesVisitor) renderVariableInvalidNullError(variableName []byte, t
}
}

func (v *variablesVisitor) traverseNode(jsonNodeRef int, typeName []byte) {
func (v *variablesVisitor) traverseFieldDefitionType(fieldTypeDefinitionNodeKind ast.NodeKind, fieldName ast.ByteSlice, typeRef int, fieldVariablesJsonNodeRef int) {
if v.definition.TypeIsNonNull(typeRef) {
if !v.variables.NodeIsDefined(fieldVariablesJsonNodeRef) {
v.renderVariableRequiredNotProvidedError(fieldName, typeRef)
}

v.traverseFieldDefitionType(fieldTypeDefinitionNodeKind, fieldName, v.definition.Types[typeRef].OfType, fieldVariablesJsonNodeRef)

return
}

if !v.variables.NodeIsDefined(fieldVariablesJsonNodeRef) {
return
}

typeName := v.definition.ResolveTypeNameBytes(typeRef)

if v.definition.TypeIsList(typeRef) {
if v.variables.Nodes[fieldVariablesJsonNodeRef].Kind != astjson.NodeKindArray {
v.renderVariableInvalidNestedTypeError(fieldVariablesJsonNodeRef, fieldTypeDefinitionNodeKind, typeName)
return
}
if len(v.variables.Nodes[fieldVariablesJsonNodeRef].ArrayValues) == 0 {
return
}

for i, arrayValue := range v.variables.Nodes[fieldVariablesJsonNodeRef].ArrayValues {
v.pushArrayPath(i)
v.traverseFieldDefitionType(fieldTypeDefinitionNodeKind, fieldName, v.definition.Types[typeRef].OfType, arrayValue)
v.popPath()
continue
}
return
}

v.traverseNamedTypeNode(fieldVariablesJsonNodeRef, v.definition.ResolveTypeNameBytes(typeRef))
}

func (v *variablesVisitor) traverseNamedTypeNode(jsonNodeRef int, typeName []byte) {
if v.err != nil {
return
}
Expand All @@ -303,7 +354,7 @@ func (v *variablesVisitor) traverseNode(jsonNodeRef int, typeName []byte) {
switch fieldTypeDefinitionNode.Kind {
case ast.NodeKindInputObjectTypeDefinition:
if v.variables.Nodes[jsonNodeRef].Kind != astjson.NodeKindObject {
v.renderVariableInvalidTypeError(typeName, v.variables.Nodes[jsonNodeRef])
v.renderVariableInvalidObjectTypeError(typeName, v.variables.Nodes[jsonNodeRef])
return
}
fields := v.definition.NodeInputFieldDefinitions(fieldTypeDefinitionNode)
Expand All @@ -312,41 +363,14 @@ func (v *variablesVisitor) traverseNode(jsonNodeRef int, typeName []byte) {
return
}
fieldName := v.definition.InputValueDefinitionNameBytes(field)
fieldType := v.definition.InputValueDefinitionType(field)
fieldTypeRef := v.definition.InputValueDefinitionType(field)
fieldVariablesJsonNodeRef := v.variables.GetObjectFieldBytes(jsonNodeRef, fieldName)
if v.definition.TypeIsNonNull(fieldType) && !v.variables.NodeIsDefined(fieldVariablesJsonNodeRef) {
v.renderVariableRequiredNotProvidedError(fieldName, fieldType)
return
}

if !v.variables.NodeIsDefined(fieldVariablesJsonNodeRef) {
continue
}

if v.definition.TypeIsList(fieldType) {
if v.variables.Nodes[fieldVariablesJsonNodeRef].Kind != astjson.NodeKindArray {
v.renderVariableInvalidNestedTypeError(fieldVariablesJsonNodeRef, fieldTypeDefinitionNode.Kind, typeName)
return
}
if len(v.variables.Nodes[fieldVariablesJsonNodeRef].ArrayValues) == 0 {
continue
}
fieldTypeName := v.definition.ResolveTypeNameBytes(fieldType)
v.pushObjectPath(fieldName)
for i, arrayValue := range v.variables.Nodes[fieldVariablesJsonNodeRef].ArrayValues {
v.pushArrayPath(i)
v.traverseNode(arrayValue, fieldTypeName)
v.popPath()
continue
}
v.popPath()
continue
}

v.pushObjectPath(fieldName)
v.traverseNode(fieldVariablesJsonNodeRef, v.definition.ResolveTypeNameBytes(fieldType))
v.traverseFieldDefitionType(fieldTypeDefinitionNode.Kind, fieldName, fieldTypeRef, fieldVariablesJsonNodeRef)
v.popPath()
}
// validate that all fields present in object are defined in the input object definition
for _, field := range v.variables.Nodes[jsonNodeRef].ObjectFields {
inputFieldName := v.variables.ObjectFieldKey(field)
inputValueDefinitionRef := v.definition.InputObjectTypeDefinitionInputValueDefinitionByName(fieldTypeDefinitionNode.Ref, inputFieldName)
Expand Down
61 changes: 61 additions & 0 deletions v2/pkg/variablesvalidation/variablesvalidation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,67 @@ func TestVariablesValidation(t *testing.T) {
require.Error(t, err)
assert.Equal(t, `Variable "$input" got invalid value "hello" at "input.bat"; Int cannot represent non-integer value: "hello"`, err.Error())
})

t.Run("input field is a double nested list", func(t *testing.T) {
tc := testCase{
schema: `input Filter { option: String! } input FilterWrapper { filters: [[Filter!]!] } type Query { hello(filter: FilterWrapper): String }`,
operation: `query Foo($input: FilterWrapper) { hello(filter: $input) }`,
variables: `{"input":{"filters":[[{"option": "a"}]]}}`,
}
err := runTest(t, tc)
require.NoError(t, err)
})

t.Run("variable of double nested list type", func(t *testing.T) {
tc := testCase{
schema: `type Query { hello(filter: [[String]]): String }`,
operation: `query Foo($input: [[String]]) { hello(filter: $input) }`,
variables: `{"input":[["value"]]}`,
}
err := runTest(t, tc)
require.NoError(t, err)
})

t.Run("triple nested value into variable of double nested list type", func(t *testing.T) {
tc := testCase{
schema: `type Query { hello(filter: [[String]]): String }`,
operation: `query Foo($input: [[String]]) { hello(filter: $input) }`,
variables: `{"input":[[["value"]]]}`,
}
err := runTest(t, tc)
require.Error(t, err)
assert.Equal(t, `Variable "$input" got invalid value ["value"] at "input.[0].[0]"; String cannot represent a non string value: ["value"]`, err.Error())
})

t.Run("null into non required list value", func(t *testing.T) {
tc := testCase{
schema: `type Query { hello(filter: [String]): String }`,
operation: `query Foo($input: [String]) { hello(filter: $input) }`,
variables: `{"input":[null]}`,
}
err := runTest(t, tc)
require.NoError(t, err)
})

t.Run("value and null into non required list value", func(t *testing.T) {
tc := testCase{
schema: `type Query { hello(filter: [String]): String }`,
operation: `query Foo($input: [String]) { hello(filter: $input) }`,
variables: `{"input":["ok", null]}`,
}
err := runTest(t, tc)
require.NoError(t, err)
})

t.Run("null into non required value", func(t *testing.T) {
tc := testCase{
schema: `type Query { hello(filter: String): String }`,
operation: `query Foo($input: String) { hello(filter: $input) }`,
variables: `{"input":null}`,
}
err := runTest(t, tc)
require.NoError(t, err)
})
}

type testCase struct {
Expand Down

0 comments on commit 7975610

Please sign in to comment.