Fix GraphQL values parsing (#1199)

Motivation

We want to create our own scalar type with JSON parsing. Due to the fact
that the `parseValue` function isn't called, it's impossible to make the
correct use of the variables with JSON type.

In particular this test fails:

```lua
local resp = server:graphql({
    query = 'query($field: Json) { test_json_type(field: $field) }',
    variables = {field = '{"test": 123}'}}
)

t.assert_equals(resp.data.test_json_type, '{"test":123}')
```

Changes

- The `parseValue` function is called during the variables parsing step.
- In `parseValue` and `parseLiteral` one can use `box.NULL` as a return
  value, returning `nil` is considered as a parsing error.
- The `serialize` function is called for `nil` scalar values too.
- Parsing of primitive scalar variables is fixed, `parseValue` is
  eliminated there.
- The `parseLiteral` function is mandatory for scalar types

Links

https://medium.com/@alizhdanov/lets-understand-graphql-scalars-3b2b016feb4a
https://atheros.ai/blog/how-to-design-graphql-custom-scalars

Close #1198

Author: hackallcode

cartridge/graphql.lua

@@ -37,7 +37,7 @@ local function funcall_wrap(fun_name, operation, field_name)
 
         local res, err = funcall.call(fun_name, ...)
 
-        if res == nil then
+        if err ~= nil then
             error(err, 0)
         end
 

cartridge/graphql/execute.lua

@@ -189,6 +189,10 @@ local function completeValue(fieldType, result, subSelections, context, opts)
     return completedResult
   end
 
+  if fieldTypeName == 'Scalar' or fieldTypeName == 'Enum' then
+    return fieldType.serialize(result)
+  end
+
   if result == nil then
     return nil
   end
@@ -212,10 +216,6 @@ local function completeValue(fieldType, result, subSelections, context, opts)
     return values
   end
 
-  if fieldTypeName == 'Scalar' or fieldTypeName == 'Enum' then
-    return fieldType.serialize(result)
-  end
-
   if fieldTypeName == 'Object' then
     if type(result) ~= 'table' then
       local message = ('Expected %q to be a "map", got %q'):format(fieldName, type(result))
@@ -260,14 +260,10 @@ local function getFieldEntry(objectType, object, fields, context)
 
   local arguments = util.map(fieldType.arguments or {}, function(argument, name)
     local supplied = argumentMap[name] and argumentMap[name].value
-
-    supplied = util.coerceValue(supplied, argument, context.variables,
-      {strict_non_null = true})
-    if type(supplied) ~= 'nil' then
-      return supplied
-    end
-
-    return defaultValues[name]
+    return util.coerceValue(supplied, argument, context.variables, {
+      strict_non_null = true,
+      defaultValues = defaultValues,
+    })
   end)
 
   --[[

cartridge/graphql/types.lua

@@ -87,11 +87,9 @@ function types.scalar(config)
   assert(type(config.name) == 'string', 'type name must be provided as a string')
   assert(type(config.serialize) == 'function', 'serialize must be a function')
   assert(type(config.isValueOfTheType) == 'function', 'isValueOfTheType must be a function')
-  if config.parseValue or config.parseLiteral then
-    assert(
-      type(config.parseValue) == 'function' and type(config.parseLiteral) == 'function',
-      'must provide both parseValue and parseLiteral to scalar type'
-    )
+  assert(type(config.parseLiteral) == 'function', 'parseLiteral must be a function')
+  if config.parseValue then
+    assert(type(config.parseValue) == 'function', 'parseValue must be a function')
   end
 
   local instance = {
@@ -260,6 +258,26 @@ local function isInt(value)
   return false
 end
 
+local function coerceInt(value)
+  if value ~= nil then
+    value = tonumber(value)
+    if not isInt(value) then return end
+  end
+
+  return value
+end
+
+types.int = types.scalar({
+  name = 'Int',
+  description = "The `Int` scalar type represents non-fractional signed whole numeric values. " ..
+                "Int can represent values from -(2^31) to 2^31 - 1, inclusive.",
+  serialize = coerceInt,
+  parseLiteral = function(node)
+    return coerceInt(node.value)
+  end,
+  isValueOfTheType = isInt,
+})
+
 -- The code from tarantool/checks.
 local function isLong(value)
   if type(value) == 'number' then
@@ -279,101 +297,97 @@ local function isLong(value)
   return false
 end
 
-local function coerceInt(value)
-  local value = tonumber(value)
-
-  if value == nil then return end
-  if not isInt(value) then return end
-
-  return value
-end
-
 local function coerceLong(value)
-  local value = tonumber64(value)
-
-  if value == nil then return end
-  if not isLong(value) then return end
+  if value ~= nil then
+    value = tonumber64(value)
+    if not isLong(value) then return end
+  end
 
   return value
 end
 
-types.int = types.scalar({
-  name = 'Int',
-  description = "The `Int` scalar type represents non-fractional signed whole numeric values. " ..
-                "Int can represent values from -(2^31) to 2^31 - 1, inclusive.",
-  serialize = coerceInt,
-  parseValue = coerceInt,
-  parseLiteral = function(node)
-    if node.kind == 'int' then
-      return coerceInt(node.value)
-    end
-  end,
-  isValueOfTheType = isInt,
-})
-
 types.long = types.scalar({
   name = 'Long',
   description = "The `Long` scalar type represents non-fractional signed whole numeric values. " ..
                 "Long can represent values from -(2^52) to 2^52 - 1, inclusive.",
   serialize = coerceLong,
-  parseValue = coerceLong,
   parseLiteral = function(node)
-    if node.kind == 'long' or node.kind == 'int' then
-      return coerceLong(node.value)
-    end
+    return coerceLong(node.value)
   end,
   isValueOfTheType = isLong,
 })
 
+local function isFloat(value)
+  return type(value) == 'number'
+end
+
+local function coerceFloat(value)
+  if value ~= nil then
+    value = tonumber(value)
+    if not isFloat(value) then return end
+  end
+
+  return value
+end
+
 types.float = types.scalar({
   name = 'Float',
-  serialize = tonumber,
-  parseValue = tonumber,
+  serialize = coerceFloat,
   parseLiteral = function(node)
-    if node.kind == 'float' or node.kind == 'int' then
-      return tonumber(node.value)
-    end
-  end,
-  isValueOfTheType = function(value)
-    return type(value) == 'number'
+    return coerceFloat(node.value)
   end,
+  isValueOfTheType = isFloat,
 })
 
+local function isString(value)
+  return type(value) == 'string'
+end
+
+local function coerceString(value)
+  if value ~= nil then
+    value = tostring(value)
+    if not isString(value) then return end
+  end
+
+  return value
+end
+
 types.string = types.scalar({
   name = 'String',
   description = "The `String` scalar type represents textual data, represented as UTF-8 character sequences. " ..
                 "The String type is most often used by GraphQL to represent free-form human-readable text.",
-  serialize = tostring,
-  parseValue = tostring,
+  serialize = coerceString,
   parseLiteral = function(node)
-    if node.kind == 'string' then
-      return node.value
-    end
-  end,
-  isValueOfTheType = function(value)
-    return type(value) == 'string'
+    return coerceString(node.value)
   end,
+  isValueOfTheType = isString,
 })
 
 local function toboolean(x)
   return (x and x ~= 'false') and true or false
 end
 
+local function isBoolean(value)
+  return type(value) == 'boolean'
+end
+
+local function coerceBoolean(value)
+  if value ~= nil then
+    value = toboolean(value)
+    if not isBoolean(value) then return end
+  end
+
+  return value
+end
+
 types.boolean = types.scalar({
   name = 'Boolean',
   description = "The `Boolean` scalar type represents `true` or `false`.",
-  serialize = toboolean,
-  parseValue = toboolean,
+  serialize = coerceBoolean,
   parseLiteral = function(node)
-    if node.kind == 'boolean' then
-      return toboolean(node.value)
-    else
-      return nil
-    end
-  end,
-  isValueOfTheType = function(value)
-    return type(value) == 'boolean'
+    return coerceBoolean(node.value)
   end,
+  isValueOfTheType = isBoolean,
 })
 
 --[[
@@ -384,14 +398,11 @@ however, defining it as an ID signifies that it is not intended to be human‐re
 --]]
 types.id = types.scalar({
   name = 'ID',
-  serialize = tostring,
-  parseValue = tostring,
+  serialize = coerceString,
   parseLiteral = function(node)
-    return node.kind == 'string' or node.kind == 'int' and node.value or nil
-  end,
-  isValueOfTheType = function(value)
-    return type(value) == 'string'
+    return coerceString(node.value)
   end,
+  isValueOfTheType = isString,
 })
 
 function types.directive(config)

cartridge/graphql/util.lua

@@ -66,6 +66,7 @@ local function coerceValue(node, schemaType, variables, opts)
   variables = variables or {}
   opts = opts or {}
   local strict_non_null = opts.strict_non_null or false
+  local defaultValues = opts.defaultValues or {}
 
   if schemaType.__type == 'NonNull' then
     local res = coerceValue(node, schemaType.ofType, variables, opts)
@@ -86,7 +87,20 @@ local function coerceValue(node, schemaType, variables, opts)
   end
 
   if node.kind == 'variable' then
-    return variables[node.name.value]
+    local value = variables[node.name.value]
+    local defaultValue = defaultValues[node.name.value]
+    if type(value) == 'nil' and type(defaultValue) ~= 'nil' then
+      -- default value was parsed by parseLiteral
+      value = defaultValue
+    elseif schemaType.parseValue ~= nil then
+      value = schemaType.parseValue(value)
+      if strict_non_null and type(value) == 'nil' then
+        error(('Could not coerce variable "%s" with value "%s" to type "%s"'):format(
+            node.name.value, variables[node.name.value], schemaType.name
+        ))
+      end
+    end
+    return value
   end
 
   if schemaType.__type == 'List' then
@@ -144,12 +158,11 @@ local function coerceValue(node, schemaType, variables, opts)
   end
 
   if schemaType.__type == 'Scalar' then
-    if schemaType.parseLiteral(node) == nil then
-      error(('Could not coerce "%s" to "%s"'):format(
-        node.value or node.kind, schemaType.name))
+    local value = schemaType.parseLiteral(node)
+    if strict_non_null and type(value) == 'nil' then
+      error(('Could not coerce value "%s" to type "%s"'):format(node.value or node.kind, schemaType.name))
     end
-
-    return schemaType.parseLiteral(node)
+    return value
   end
 end