Skip to content
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

refactor(enrich): replace manual AST traversal with ASTVisitor from graphql-js #139

Merged
merged 54 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
19dfa84
WIP: Replace AST traversal with `ASTVisitor`
schwma Aug 25, 2023
3b9ba39
WIP: literal parsing
schwma Aug 25, 2023
bc07760
Add WIP comments
schwma Aug 25, 2023
3ff1c53
Merge branch 'main' into ASTVisitor
schwma Sep 21, 2023
d27d7f7
WIP visitWithTypeInfo
schwma Sep 21, 2023
38ee532
WIP
schwma Sep 21, 2023
b580262
WIP get types for scalar input literals
schwma Sep 21, 2023
7f74758
Cleanup
schwma Sep 21, 2023
78fae20
Fix parsing of top level argument literal values
schwma Sep 21, 2023
973631f
Remove unneeded top level if-statement
schwma Sep 22, 2023
e86ad0b
Rename variable and argument
schwma Sep 22, 2023
dbd886c
Rename `_directPath` -> `_simplifiedPath`
schwma Sep 22, 2023
cb2b442
Add comments for function that finds type
schwma Sep 22, 2023
8000d0b
Add comments explaining enriching of AST
schwma Sep 22, 2023
9fe16c0
Remove `skipParsing` flag from substituted variable values
schwma Sep 22, 2023
c18ec08
Improve comment and reorder properties
schwma Sep 22, 2023
2d0bd87
Add enrich tests for each ASTVisitor action
schwma Sep 22, 2023
ab7058b
Fix substitution of fragments with multiple top level selections
schwma Sep 22, 2023
1c72c16
Remove meta module
schwma Sep 22, 2023
0c5e96b
Explain why SelectionSet visitor is used to substitute fragments
schwma Sep 22, 2023
2593718
Extract literal parsing to own module
schwma Sep 22, 2023
6d86847
Remove unneeded array wrapping
schwma Sep 22, 2023
ef14b58
Shorter function
schwma Sep 22, 2023
049a572
Export fragment function directly
schwma Sep 22, 2023
f35bf32
Shorter kind checks
schwma Sep 22, 2023
08e83a4
Shorter if-statement returns
schwma Sep 22, 2023
575d4b0
Remove unneded if-statement
schwma Sep 22, 2023
e3c3b8a
Add comment about skipping parsing for variable values
schwma Sep 22, 2023
27c94ad
Wording in comment visitor -> visitor functions
schwma Sep 22, 2023
bfb21d4
No longer deep clone field nodes
schwma Oct 18, 2023
5021f59
Slice datetime millis by index instead of regex
schwma Oct 18, 2023
f9224c6
Extract common variables
schwma Oct 18, 2023
4096349
Reorder tests
schwma Oct 18, 2023
6745205
Merge branch 'main' into ASTVisitor
schwma Oct 18, 2023
d537037
Prettier format
schwma Oct 18, 2023
e0c76f3
Improve comment
schwma Oct 18, 2023
b170ffd
Add newline
schwma Oct 18, 2023
73bfcf8
Improve comments about unparsed literals
schwma Oct 18, 2023
d15ce6e
Prefix unused arguments with underscores
schwma Oct 18, 2023
7e3f4e1
Use destructuring assignments
schwma Oct 18, 2023
ab5f872
Rename `editedAST` -> `enrichedAST`
schwma Oct 18, 2023
7a33f83
Merge branch 'main' into ASTVisitor
schwma Nov 21, 2023
5f7f394
Merge branch 'main' into ASTVisitor
schwma Nov 23, 2023
ccc9a76
Reorder function declarations
schwma Nov 23, 2023
a601999
Merge branch 'main' into ASTVisitor
schwma Nov 28, 2023
b3021cc
Create array of scalar kinds once vs per function call
schwma Dec 1, 2023
ecb411d
Create array of path kinds once vs per function call
schwma Dec 1, 2023
bc1858d
Fix simplified path filter
schwma Dec 1, 2023
22c4ded
Extract next path element to variable
schwma Dec 1, 2023
5fd3c65
Avoid recreating function
schwma Dec 6, 2023
2fa986b
Merge branch 'main' into ASTVisitor
schwma Dec 7, 2023
b5a9385
Merge branch 'main' into ASTVisitor
schwma Jan 5, 2024
5ae3356
No array methods to avoid needlessly copying the array
schwma Jan 5, 2024
739a241
Improve grammar in comment
schwma Jan 5, 2024
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
116 changes: 33 additions & 83 deletions lib/resolvers/parse/ast/enrich.js
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion lib/resolvers/parse/ast/fragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
40 changes: 24 additions & 16 deletions lib/resolvers/parse/ast/fromObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => ({
Expand All @@ -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)
61 changes: 61 additions & 0 deletions lib/resolvers/parse/ast/literal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
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 _isPathKind = _pathKinds.includes.bind(_pathKinds)
const _simplifiedPath = (node, ancestors) =>
ancestors
.concat(node)
.filter(e => _isPathKind(e.kind))
.map(e => ({ kind: e.kind, name: e.name.value }))
schwma marked this conversation as resolved.
Show resolved Hide resolved

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
4 changes: 0 additions & 4 deletions lib/resolvers/parse/ast/meta.js

This file was deleted.

11 changes: 8 additions & 3 deletions lib/schema/types/custom/GraphQLBinary.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 can 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
}
Expand Down
2 changes: 1 addition & 1 deletion lib/schema/types/custom/GraphQLDateTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
6 changes: 4 additions & 2 deletions lib/schema/types/custom/GraphQLDecimal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
5 changes: 3 additions & 2 deletions lib/schema/types/custom/GraphQLInt64.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading