refactor(enrich): replace manual AST traversal with ASTVisitor from…

… `graphql-js` (#139)

* WIP: Replace AST traversal with `ASTVisitor`

* WIP: literal parsing

* Add WIP comments

* WIP visitWithTypeInfo

* WIP

* WIP get types for scalar input literals

* Cleanup

* Fix parsing of top level argument literal values

* Remove unneeded top level if-statement

* Rename variable and argument

* Rename `_directPath` -> `_simplifiedPath`

* Add comments for function that finds type

* Add comments explaining enriching of AST

* Remove `skipParsing` flag from substituted variable values

* Improve comment and reorder properties

* Add enrich tests for each ASTVisitor action

* Fix substitution of fragments with multiple top level selections

* Remove meta module

* Explain why SelectionSet visitor is used to substitute fragments

* Extract literal parsing to own module

* Remove unneeded array wrapping

* Shorter function

* Export fragment function directly

* Shorter kind checks

* Shorter if-statement returns

* Remove unneded if-statement

* Add comment about skipping parsing for variable values

* Wording in comment visitor -> visitor functions

* No longer deep clone field nodes

* Slice datetime millis by index instead of regex

* Extract common variables

* Reorder tests

* Prettier format

* Improve comment

* Add newline

* Improve comments about unparsed literals

* Prefix unused arguments with underscores

* Use destructuring assignments

* Rename `editedAST` -> `enrichedAST`

* Reorder function declarations

* Create array of scalar kinds once vs per function call

Co-authored-by: Bob den Os <[email protected]>

* Create array of path kinds once vs per function call

Co-authored-by: Bob den Os <[email protected]>

* Fix simplified path filter

* Extract next path element to variable

* Avoid recreating function

* No array methods to avoid needlessly copying the array

* Improve grammar in comment

---------

Co-authored-by: Bob den Os <[email protected]>

lib/resolvers/parse/ast/enrich.js

@@ -1,91 +1,41 @@
-const { Kind } = require('graphql')
+const { visit, Kind } = require('graphql')
 const fragmentSpreadSelections = require('./fragment')
 const substituteVariable = require('./variable')
-const removeMetaFieldsFromSelections = require('./meta')
-
-const _traverseObjectValue = (info, objectValue, _fields) =>
-  objectValue.fields.forEach(field => {
-    const _field = _fields[field.name.value]
-    _traverseArgumentOrObjectField(info, field, _field)
-  })
-
-const _traverseListValue = (info, listValue, _fields) => {
-  for (let i = 0; i < listValue.values.length; i++) {
-    const value = listValue.values[i]
-    switch (value.kind) {
-      case Kind.VARIABLE:
-        listValue.values[i] = substituteVariable(info, value)
-        break
-      case Kind.OBJECT:
-        _traverseObjectValue(info, value, _fields)
-        break
-    }
-  }
-}
-
-const _isScalarKind = kind => kind === Kind.INT || kind === Kind.FLOAT || kind === Kind.STRING || kind === Kind.BOOLEAN
-
-const _traverseArgumentOrObjectField = (info, argumentOrObjectField, _fieldOr_arg) => {
-  const value = argumentOrObjectField.value
-
-  const type = _getTypeFrom_fieldOr_arg(_fieldOr_arg)
-  if (_isScalarKind(value.kind)) value.value = type.parseLiteral(value)
-
-  switch (value.kind) {
-    case Kind.VARIABLE:
-      argumentOrObjectField.value = substituteVariable(info, value)
-      break
-    case Kind.LIST:
-      _traverseListValue(info, value, type.getFields?.())
-      break
-    case Kind.OBJECT:
-      _traverseObjectValue(info, value, type.getFields())
-      break
-  }
-
-  // Convenience value for both literal and variable values
-  if (argumentOrObjectField.value?.kind === Kind.NULL) argumentOrObjectField.value.value = null
-}
-
-const _traverseSelectionSet = (info, selectionSet, _fields) => {
-  selectionSet.selections = fragmentSpreadSelections(info, selectionSet.selections)
-  selectionSet.selections = removeMetaFieldsFromSelections(selectionSet.selections)
-  selectionSet.selections.forEach(field => {
-    const _field = _fields[field.name.value]
-    _traverseField(info, field, _field)
-  })
-}
-
-const _getTypeFrom_fieldOr_arg = _field => {
-  let type = _field.type
-  while (type.ofType) type = type.ofType
-  return type
-}
-
-const _traverseField = (info, field, _field) => {
-  if (field.selectionSet) {
-    const type = _getTypeFrom_fieldOr_arg(_field)
-    _traverseSelectionSet(info, field.selectionSet, type.getFields())
-  }
-
-  field.arguments.forEach(arg => {
-    const _arg = _field.args.find(a => a.name === arg.name.value)
-    _traverseArgumentOrObjectField(info, arg, _arg)
-  })
-}
-
-const _traverseFieldNodes = (info, fieldNodes, _fields) =>
-  fieldNodes.forEach(fieldNode => {
-    const _field = _fields[fieldNode.name.value]
-    _traverseField(info, fieldNode, _field)
-  })
+const parseLiteral = require('./literal')
 
 module.exports = info => {
-  const deepClonedFieldNodes = JSON.parse(JSON.stringify(info.fieldNodes))
-
   const rootTypeName = info.parentType.name
   const rootType = info.schema.getType(rootTypeName)
-  _traverseFieldNodes(info, deepClonedFieldNodes, rootType.getFields())
+  const rootFields = rootType.getFields()
+
+  const enrichedAST = visit(info.fieldNodes, {
+    [Kind.SELECTION_SET](node) {
+      // Substitute fragment spreads with fragment definitions into the AST as if they were inline fields
+      // Prevents the necessity for special handling of fragments in AST to CQN
+
+      // Note: FragmentSpread visitor function cannot be used to replace fragment spreads with fragment definitions
+      // that contain multiple top level selections, since those must be placed directly into the selection set
+      node.selections = fragmentSpreadSelections(info, node.selections)
+    },
+    [Kind.FIELD](node) {
+      // Remove __typename from selections to prevent field from being interpreted as DB column
+      // Instead let graphql framework determine the type
+      if (node.name?.value === '__typename') return null
+    },
+    // Literals within the AST have not yet been parsed
+    [Kind.ARGUMENT]: parseLiteral(rootFields),
+    // Literals within the AST have not yet been parsed
+    [Kind.OBJECT_FIELD]: parseLiteral(rootFields),
+    [Kind.VARIABLE](node) {
+      // Substitute variable values into the AST as if they were literal values
+      // Prevents the necessity for special handling of variables in AST to CQN
+      return substituteVariable(info, node)
+    },
+    [Kind.NULL](node) {
+      // Convenience value for handling of null values in AST to CQN
+      node.value = null
+    }
+  })
 
-  return deepClonedFieldNodes
+  return enrichedAST
 }

lib/resolvers/parse/ast/fragment.js

@@ -7,7 +7,7 @@ const _substituteFragment = (info, fragmentSpread) =>
 
 const fragmentSpreadSelections = (info, selections) =>
   selections.flatMap(selection =>
-    selection.kind === Kind.FRAGMENT_SPREAD ? _substituteFragment(info, selection) : [selection]
+    selection.kind === Kind.FRAGMENT_SPREAD ? _substituteFragment(info, selection) : selection
   )
 
 module.exports = fragmentSpreadSelections

lib/resolvers/parse/ast/fromObject.js

@@ -3,9 +3,25 @@ const { isPlainObject } = require('../../utils')
 
 const _nullValue = { kind: Kind.NULL }
 
-const _valueToGenericScalarValue = value => ({
-  kind: 'GenericScalarValue',
-  value
+const _valueToGraphQLType = value => {
+  if (typeof value === 'boolean') return Kind.BOOLEAN
+
+  if (typeof value === 'number') {
+    if (Number.isInteger(value)) return Kind.INT
+
+    return Kind.FLOAT
+  }
+
+  // Return below means: (typeof value === 'string' || Buffer.isBuffer(value))
+  return Kind.STRING
+}
+
+const _valueToScalarValue = value => ({
+  kind: _valueToGraphQLType(value),
+  value,
+  // Variable values have already been parsed
+  // -> skip parsing in Argument and ObjectField visitor functions
+  skipParsing: true
 })
 
 const _keyToName = key => ({
@@ -30,23 +46,15 @@ const _arrayToListValue = array => ({
 })
 
 const _variableToValue = variable => {
-  if (Array.isArray(variable)) {
-    return _arrayToListValue(variable)
-  }
+  if (Array.isArray(variable)) return _arrayToListValue(variable)
 
-  if (isPlainObject(variable)) {
-    return _objectToObjectValue(variable)
-  }
+  if (isPlainObject(variable)) return _objectToObjectValue(variable)
 
-  if (variable === null) {
-    return _nullValue
-  }
+  if (variable === null) return _nullValue
 
-  if (variable === undefined) {
-    return undefined
-  }
+  if (variable === undefined) return undefined
 
-  return _valueToGenericScalarValue(variable)
+  return _valueToScalarValue(variable)
 }
 
 module.exports = variableValue => _variableToValue(variableValue)

lib/resolvers/parse/ast/literal.js

@@ -0,0 +1,69 @@
+const { Kind } = require('graphql')
+
+const _getTypeFrom_fieldOr_arg = _field => {
+  let { type } = _field
+  while (type.ofType) type = type.ofType
+  return type
+}
+
+const _getTypeFrom_fields = (_fields, path, index = 0) => {
+  const { name } = path[index++]
+
+  const _field = _fields[name]
+  const type = _getTypeFrom_fieldOr_arg(_field)
+
+  // If type has the parseLiteral function it is a scalar type -> leaf -> end of path
+  if (type.parseLiteral) return type
+
+  const next = path[index]
+  // Is the next path element an argument? If yes, follow the argument
+  if (next.kind === Kind.ARGUMENT) {
+    const arg = _field.args.find(a => a.name === next.name)
+    const type = _getTypeFrom_fieldOr_arg(arg)
+
+    // If type has the parseLiteral function it is a scalar type -> leaf -> end of path
+    // This case occurs when the argument itself is a scalar type, e.g. Books(top: 1)
+    if (type.parseLiteral) return type
+
+    return _getTypeFrom_fields(type.getFields(), path, index + 1)
+  }
+
+  return _getTypeFrom_fields(type.getFields(), path, index)
+}
+
+const _pathKinds = [Kind.FIELD, Kind.ARGUMENT, Kind.OBJECT_FIELD]
+const _isNodePathKind = node => _pathKinds.includes(node.kind)
+// Note: no array methods used to avoid needlessly copying the array
+const _simplifiedPath = (node, ancestors) => {
+  const path = []
+
+  for (const ancestor of ancestors) {
+    if (!_isNodePathKind(ancestor)) continue
+    path.push({ kind: ancestor.kind, name: ancestor.name.value })
+  }
+
+  if (_isNodePathKind(node)) path.push({ kind: node.kind, name: node.name.value })
+
+  return path
+}
+
+const _scalarKinds = [Kind.INT, Kind.FLOAT, Kind.STRING, Kind.BOOLEAN]
+const _isScalarKind = _scalarKinds.includes.bind(_scalarKinds)
+
+// Literals are provided unparsed within the AST, contrary to variable values
+const parseLiteral = rootFields => (node, _key, _parent, _path, ancestors) => {
+  const { value } = node
+  if (!_isScalarKind(value.kind)) return
+
+  // Set for variable values that have been substituted into the AST, which are already parsed
+  if (value.skipParsing) {
+    delete value.skipParsing
+    return
+  }
+
+  const simplifiedPath = _simplifiedPath(node, ancestors)
+  const type = _getTypeFrom_fields(rootFields, simplifiedPath)
+  value.value = type.parseLiteral(value)
+}
+
+module.exports = parseLiteral

lib/schema/types/custom/GraphQLBinary.js

@@ -29,10 +29,15 @@ const parseValue = inputValue => {
 }
 
 const parseLiteral = valueNode => {
-  if (valueNode.kind !== Kind.STRING) throw getGraphQLValueError(ERROR_NON_STRING_VALUE, valueNode)
+  const { kind, value } = valueNode
 
-  const buffer = Buffer.from(valueNode.value, 'base64')
-  _validateBase64String(valueNode.value, buffer, valueNode)
+  if (kind !== Kind.STRING) throw getGraphQLValueError(ERROR_NON_STRING_VALUE, valueNode)
+
+  // WORKAROUND: value could have already been parsed to a Buffer, necessary because of manual parsing in enrich AST
+  if (Buffer.isBuffer(value)) return value
+
+  const buffer = Buffer.from(value, 'base64')
+  _validateBase64String(value, buffer, valueNode)
 
   return buffer
 }

lib/schema/types/custom/GraphQLDateTime.js

@@ -7,7 +7,7 @@ const ERROR_NON_DATE_TIME_VALUE = 'DateTime values must be strings in the ISO 86
 const _parseDate = inputValueOrValueNode => {
   const date = parseDate(inputValueOrValueNode, ERROR_NON_DATE_TIME_VALUE)
   // Cut off milliseconds
-  return date.replace(/\.\d\d\dZ$/, 'Z')
+  return date.slice(0, 19) + 'Z'
 }
 
 const parseValue = inputValue => {

lib/schema/types/custom/GraphQLDecimal.js

@@ -23,12 +23,14 @@ const parseValue = inputValue => {
 }
 
 const parseLiteral = valueNode => {
-  if (valueNode.kind !== Kind.FLOAT && valueNode.kind !== Kind.INT && valueNode.kind !== Kind.STRING)
+  const { kind, value } = valueNode
+
+  if (kind !== Kind.FLOAT && kind !== Kind.INT && kind !== Kind.STRING)
     throw getGraphQLValueError(ERROR_NON_NUMERIC_VALUE, valueNode)
 
   _validateIsDecimal(valueNode)
 
-  return valueNode.value
+  return value
 }
 
 module.exports = new GraphQLScalarType({

lib/schema/types/custom/GraphQLInt64.js

@@ -28,8 +28,9 @@ const parseValue = inputValue => {
 }
 
 const parseLiteral = valueNode => {
-  if (valueNode.kind !== Kind.INT && valueNode.kind !== Kind.STRING)
-    throw getGraphQLValueError(ERROR_NON_INTEGER_VALUE, valueNode)
+  const { kind } = valueNode
+
+  if (kind !== Kind.INT && kind !== Kind.STRING) throw getGraphQLValueError(ERROR_NON_INTEGER_VALUE, valueNode)
 
   const num = _toBigInt(valueNode)
   validateRange(num, MIN_INT64, MAX_INT64, ERROR_NON_64_BIT_INTEGER_VALUE, valueNode)