Properly handle nested input objects in GraphQL (#703)

Also make GraphQL errors neater: eliminate source path from messages.

Co-authored-by: Yaroslav Dynnikov <[email protected]>

Author: olegrok

CHANGELOG.md

@@ -16,20 +16,23 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 - Implement stateful failover mode.
 
-- Add new field Graphql field `cluster{ issues {topic} }`
+- Add new field GraphQL field `cluster{ issues {topic} }`
 
 - Extend issues API for stateful failover
 
-- New option in `cartridge.cfg({upgrade_schema=...}, ...)`
-  to perform auto upgrade schema to actual tarantool version
-  (only for leader). It also has bean added for `argparse`.
-
-- GraphQL validation significantly improved: scalar values can't have
-  subselections; composite types must have subselections; omitting
-  non-nullable arguments in variable list is forbidden.
+- New option in `cartridge.cfg({upgrade_schema=...})`
+  to automatically upgrade schema to modern tarantool version
+  (only for leader). It also has been added for `argparse`.
 
 - Indicate replication and failover issues in WebUI.
 
+### Changed
+
+- Make GraphQL validation stricter: scalar values can't have
+  sub-selections; composite types must have sub-selections; omitting
+  non-nullable arguments in variable list is forbidden. Your code **may
+  be affected** if it doesn't conform GraphQL specification.
+
 ### Deprecated
 
 Lua API:
@@ -44,7 +47,15 @@ GraphQL API:
 
 ### Fixed
 
-- Fix quering failure `cluster {issues {...} }` on uninitialized instance
+- Properly handle nested input object in GraphQL:
+```graphql
+mutation($uuid: String!) {
+  cluster { edit_topology(servers: [{uuid: $uuid ...}]) {} }
+}
+```
+
+- Fix bug in GraphQL query `cluster {issues {...} }` on uninitialized
+  instance.
 
 ## [2.0.2] - 2020-03-17
 

cartridge/graphql.lua

@@ -40,7 +40,7 @@ local function funcall_wrap(fun_name, operation, field_name)
         local res, err = funcall.call(fun_name, ...)
 
         if res == nil then
-            error(err)
+            error(err, 0)
         end
 
         return res
@@ -104,7 +104,7 @@ local function add_callback(opts)
     if opts.prefix then
         local obj = vars.callbacks[opts.prefix]
         if obj == nil then
-            error('No such callback prefix ' .. opts.prefix)
+            error('No such callback prefix ' .. opts.prefix, 0)
         end
 
         local oldkind = obj.kind
@@ -148,7 +148,7 @@ local function add_mutation(opts)
     if opts.prefix then
         local obj = vars.mutations[opts.prefix]
         if obj == nil then
-            error('No such mutation prefix ' .. opts.prefix)
+            error('No such mutation prefix ' .. opts.prefix, 0)
         end
 
         local oldkind = obj.kind

cartridge/graphql/execute.lua

@@ -5,6 +5,10 @@ local introspection = require(path .. '.introspection')
 local query_util = require(path .. '.query_util')
 local validate_variables = require(path .. '.validate_variables')
 
+local function error(...)
+  return _G.error(..., 0)
+end
+
 local function getFieldResponseKey(field)
   return field.alias and field.alias.name.value or field.name.value
 end

cartridge/graphql/introspection.lua

@@ -204,7 +204,7 @@ __Type = types.object({
             return 'NON_NULL'
           end
 
-          error('Unknown type ' .. kind)
+          error('Unknown type ' .. kind, 0)
         end
       },
 

cartridge/graphql/rules.lua

@@ -4,6 +4,10 @@ local util = require(path .. '.util')
 local introspection = require(path .. '.introspection')
 local query_util = require(path .. '.query_util')
 
+local function error(...)
+  return _G.error(..., 0)
+end
+
 local function getParentField(context, name, count)
   if introspection.fieldMap[name] then return introspection.fieldMap[name] end
 
@@ -413,11 +417,6 @@ function rules.variableDefaultValuesHaveCorrectType(node, context)
 end
 
 function rules.variablesAreUsed(node, context)
-  local operationName = node.name and node.name.value or ''
-  if context.skipVariableUseCheck[operationName] then
-    return
-  end
-
   if node.variableDefinitions then
     for _, definition in ipairs(node.variableDefinitions) do
       local variableName = definition.variable.name.value

cartridge/graphql/schema.lua

@@ -2,6 +2,10 @@ local path = (...):gsub('%.[^%.]+$', '')
 local types = require(path .. '.types')
 local introspection = require(path .. '.introspection')
 
+local function error(...)
+  return _G.error(..., 0)
+end
+
 local schema = {}
 schema.__index = schema
 

cartridge/graphql/types.lua

@@ -11,6 +11,10 @@ local types_load_error = errors.new_class("types_load_error")
 
 vars:new('registered_types', {})
 
+local function error(...)
+  return _G.error(..., 0)
+end
+
 local types = {}
 
 local function get_env()

cartridge/graphql/util.lua

@@ -1,6 +1,10 @@
 local ffi = require('ffi')
 local yaml = require('yaml').new({encode_use_tostring = true})
 
+local function error(...)
+  return _G.error(..., 0)
+end
+
 local function map(t, fn)
   local res = {}
   for k, v in pairs(t) do res[k] = fn(v, k) end

cartridge/graphql/validate.lua

@@ -258,9 +258,40 @@ local visitors = {
       end
     end,
 
+    children = function(node)
+      return util.map(node.value.values or {}, function(value)
+        if value.value ~= nil then
+          return value.value
+        end
+        return value
+      end)
+    end,
+
+    rules = { rules.uniqueInputObjectFields }
+  },
+
+  inputObject = {
+    children = function(node)
+      return util.map(node.values or {}, function(value)
+        return value.value
+      end)
+    end,
+
     rules = { rules.uniqueInputObjectFields }
   },
 
+  list = {
+    children = function(node)
+      return node.values
+    end,
+  },
+
+  variable = {
+    enter = function(node, context)
+      context.variableReferences[node.name.value] = true
+    end
+  },
+
   directive = {
     children = function(node, context)
       return node.arguments
@@ -278,7 +309,6 @@ local function validate(schema, tree)
     objects = {},
     currentOperation = nil,
     variableReferences = nil,
-    skipVariableUseCheck = {}, -- operation name -> boolean
   }
 
   local function visit(node)

cartridge/graphql/validate_variables.lua

@@ -3,6 +3,10 @@ local types = require(path .. '.types')
 local util = require(path .. '.util')
 local check = util.check
 
+local function error(...)
+  return _G.error(..., 0)
+end
+
 -- Traverse type more or less likewise util.coerceValue do.
 local function checkVariableValue(variableName, value, variableType)
   check(variableName, 'variableName', 'string')