Skip to content

Argument and variables nullability #64

New issue

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

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

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ coverage_result.txt
.DS_Store
.vscode
luacov.*.out*
/node_modules
**/node_modules
/package-lock.json
*.mo
.history
Expand Down
1 change: 1 addition & 0 deletions .luacheckrc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ include_files = {
}
exclude_files = {
'.rocks',
'test/integration/fuzzing_nullability_test.lua',
}
new_read_globals = {
box = { fields = {
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ SHELL := /bin/bash
.PHONY: .rocks
.rocks: graphql-scm-1.rockspec Makefile
tarantoolctl rocks make
tarantoolctl rocks install luatest 0.5.5
tarantoolctl rocks install luatest 0.5.7
tarantoolctl rocks install luacov 0.13.0
tarantoolctl rocks install luacheck 0.26.0

Expand Down
4 changes: 2 additions & 2 deletions graphql/execute.lua
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ local function completeValue(fieldType, result, subSelections, context, opts)
local innerType = fieldType.ofType
local completedResult = completeValue(innerType, result, subSelections, context, opts)

if completedResult == nil then
if type(completedResult) == 'nil' then
local err = string.format(
'No value provided for non-null %s %q',
(innerType.name or innerType.__type),
Expand All @@ -193,7 +193,7 @@ local function completeValue(fieldType, result, subSelections, context, opts)
end

if result == nil then
return nil
return result
end

if fieldTypeName == 'List' then
Expand Down
18 changes: 10 additions & 8 deletions graphql/rules.lua
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,15 @@ function rules.uniqueArgumentNames(node, _)
end
end

local coerce_opts = { strict_non_null = true, skip_variables = true }

function rules.argumentsOfCorrectType(node, context)
if node.arguments then
local parentField = getParentField(context, node.name.value)
for _, argument in pairs(node.arguments) do
local name = argument.name.value
local argumentType = parentField.arguments[name]
util.coerceValue(argument.value, argumentType.kind or argumentType)
util.coerceValue(argument.value, argumentType.kind or argumentType, nil, coerce_opts)
end
end
end
Expand Down Expand Up @@ -406,11 +408,9 @@ end
function rules.variableDefaultValuesHaveCorrectType(node, context)
if node.variableDefinitions then
for _, definition in ipairs(node.variableDefinitions) do
if definition.type.kind == 'nonNullType' and definition.defaultValue then
error('Non-null variables can not have default values')
elseif definition.defaultValue then
if definition.defaultValue then
local variableType = query_util.typeFromAST(definition.type, context.schema)
util.coerceValue(definition.defaultValue, variableType)
util.coerceValue(definition.defaultValue, variableType, nil, coerce_opts)
end
end
end
Expand Down Expand Up @@ -507,12 +507,14 @@ local function isVariableTypesValid(argument, argumentType, context,
error('Unknown variable "' .. variableName .. '"')
end

local hasDefault = variableDefinition.defaultValue ~= nil
local hasNonNullDefault = (variableDefinition.defaultValue ~= nil) and
(variableDefinition.defaultValue.kind ~= 'null')

local variableType = query_util.typeFromAST(variableDefinition.type,
context.schema)

if hasDefault and variableType.__type ~= 'NonNull' then
local realVariableTypeName = util.getTypeName(variableType)
if hasNonNullDefault and variableType.__type ~= 'NonNull' then
variableType = types.nonNull(variableType)
end

Expand All @@ -526,7 +528,7 @@ local function isVariableTypesValid(argument, argumentType, context,
if not isTypeSubTypeOf(variableType, argumentType, context) then
return false, ('Variable "%s" type mismatch: the variable type "%s" ' ..
'is not compatible with the argument type "%s"'):format(variableName,
util.getTypeName(variableType), util.getTypeName(argumentType))
realVariableTypeName, util.getTypeName(argumentType))
end
elseif argument.value.kind == 'list' then
-- find variables deeper
Expand Down
5 changes: 5 additions & 0 deletions graphql/util.lua
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,13 @@ local function coerceValue(node, schemaType, variables, opts)
variables = variables or {}
opts = opts or {}
local strict_non_null = opts.strict_non_null or false
local skip_variables = opts.skip_variables or false
local defaultValues = opts.defaultValues or {}

if node and node.kind == 'variable' and skip_variables then
return nil
end

if schemaType.__type == 'NonNull' then
local res = coerceValue(node, schemaType.ofType, variables, opts)
if strict_non_null and res == nil then
Expand Down
19 changes: 16 additions & 3 deletions graphql/validate_variables.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@ local function error(...)
end

-- Traverse type more or less likewise util.coerceValue do.
local function checkVariableValue(variableName, value, variableType)
local function checkVariableValue(variableName, value, variableType, isNonNullDefaultDefined)
check(variableName, 'variableName', 'string')
check(variableType, 'variableType', 'table')

local isNonNull = variableType.__type == 'NonNull'
isNonNullDefaultDefined = isNonNullDefaultDefined or false

if isNonNull then
variableType = types.nullable(variableType)
if value == nil then
if (type(value) == 'cdata' and value == nil) or
(type(value) == 'nil' and not isNonNullDefaultDefined) then
error(('Variable %q expected to be non-null'):format(variableName))
end
end
Expand Down Expand Up @@ -116,8 +118,19 @@ local function validate_variables(context)

-- check that variable values have correct type
for variableName, variableType in pairs(context.variableTypes) do
-- Check if default value presents.
local isNonNullDefaultDefined = false
for _, variableDefinition in ipairs(context.operation.variableDefinitions) do
if variableDefinition.variable.name.value == variableName and
variableDefinition.defaultValue ~= nil then
if (variableDefinition.defaultValue.value ~= nil) or (variableDefinition.defaultValue.values ~= nil) then
isNonNullDefaultDefined = true
end
end
end

local value = (context.variables or {})[variableName]
checkVariableValue(variableName, value, variableType)
checkVariableValue(variableName, value, variableType, isNonNullDefaultDefined)
end
end

Expand Down
36 changes: 36 additions & 0 deletions test/helpers.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
local types = require('graphql.types')
local schema = require('graphql.schema')
local parse = require('graphql.parse')
local validate = require('graphql.validate')
local execute = require('graphql.execute')

local helpers = {}

helpers.test_schema_name = 'default'

function helpers.check_request(query, query_schema, mutation_schema, directives, opts)
opts = opts or {}
local root = {
query = types.object({
name = 'Query',
fields = query_schema or {},
}),
mutation = types.object({
name = 'Mutation',
fields = mutation_schema or {},
}),
directives = directives,
}

local compiled_schema = schema.create(root, helpers.test_schema_name, opts)

local parsed = parse.parse(query)

validate.validate(compiled_schema, parsed)

local rootValue = {}
local variables = opts.variables or {}
return execute.execute(compiled_schema, parsed, rootValue, variables)
end

return helpers
10 changes: 10 additions & 0 deletions test/integration/codegen/fuzzing_nullability/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
To install dependencies, run
```bash
npm install
```

To generate test file, run
```bash
node generate.js > ../../fuzzing_nullability_test.lua
```
in this directory.
Loading