From d216fd04c92ec594fb9b448025fc3e23fe6dfdad Mon Sep 17 00:00:00 2001 From: Mitchell Hamilton Date: Thu, 29 Apr 2021 12:44:26 +1000 Subject: [PATCH] Implement db lists API in terms of GraphQLSchema (#5569) --- .changeset/real-apricots-kick.md | 5 + .../keystone/src/lib/context/createContext.ts | 29 ++- .../context/executeGraphQLFieldToRootVal.ts | 209 ++++++++++++++++++ ...ceAndValidateArgumentsFnForGraphQLField.ts | 75 ------- .../keystone/src/lib/context/itemAPI.ts | 182 ++++++--------- 5 files changed, 298 insertions(+), 202 deletions(-) create mode 100644 .changeset/real-apricots-kick.md create mode 100644 packages-next/keystone/src/lib/context/executeGraphQLFieldToRootVal.ts delete mode 100644 packages-next/keystone/src/lib/context/getCoerceAndValidateArgumentsFnForGraphQLField.ts diff --git a/.changeset/real-apricots-kick.md b/.changeset/real-apricots-kick.md new file mode 100644 index 00000000000..4c563636be4 --- /dev/null +++ b/.changeset/real-apricots-kick.md @@ -0,0 +1,5 @@ +--- +'@keystone-next/keystone': patch +--- + +Refactored implementation of db lists API diff --git a/packages-next/keystone/src/lib/context/createContext.ts b/packages-next/keystone/src/lib/context/createContext.ts index 11a3d64b851..86906bd5ea9 100644 --- a/packages-next/keystone/src/lib/context/createContext.ts +++ b/packages-next/keystone/src/lib/context/createContext.ts @@ -8,7 +8,7 @@ import type { ImagesConfig, } from '@keystone-next/types'; -import { itemDbAPIForList, itemAPIForList, getArgsFactory } from './itemAPI'; +import { getDbAPIFactory, itemAPIForList } from './itemAPI'; import { accessControlContext, skipAccessControlContext } from './createAccessControlContext'; import { createImagesContext } from './createImagesContext'; @@ -25,15 +25,20 @@ export function makeCreateContext({ }) { const images = createImagesContext(imagesConfig); // We precompute these helpers here rather than every time createContext is called - // because they require parsing the entire schema, which is potentially expensive. - const publicGetArgsByList: Record> = {}; + // because they involve creating a new GraphQLSchema, creating a GraphQL document AST(programmatically, not by parsing) and validating the + // note this isn't as big of an optimisation as you would imagine(at least in comparison with the rest of the system), + // the regular non-db lists api does more expensive things on every call + // like parsing the generated GraphQL document, and validating it against the schema on _every_ call + // is that really that bad? no not really. this has just been more optimised because the cost of what it's + // doing is more obvious(even though in reality it's much smaller than the alternative) + const publicDbApiFactories: Record> = {}; for (const [listKey, list] of Object.entries(keystone.lists)) { - publicGetArgsByList[listKey] = getArgsFactory(list, graphQLSchema); + publicDbApiFactories[listKey] = getDbAPIFactory(list.gqlNames, graphQLSchema); } - const internalGetArgsByList: Record> = {}; + const internalDbApiFactories: Record> = {}; for (const [listKey, list] of Object.entries(keystone.lists)) { - internalGetArgsByList[listKey] = getArgsFactory(list, internalSchema); + internalDbApiFactories[listKey] = getDbAPIFactory(list.gqlNames, internalSchema); } const createContext = ({ @@ -62,8 +67,8 @@ export function makeCreateContext({ } return result.data as Record; }; - const dbAPI: Record> = {}; - const itemAPI: Record> = {}; + const dbAPI: KeystoneContext['db']['lists'] = {}; + const itemAPI: KeystoneContext['lists'] = {}; const contextToReturn: KeystoneContext = { schemaName, ...(skipAccessControl ? skipAccessControlContext : accessControlContext), @@ -90,10 +95,10 @@ export function makeCreateContext({ gqlNames: (listKey: string) => keystone.lists[listKey].gqlNames, images, }; - const getArgsByList = schemaName === 'public' ? publicGetArgsByList : internalGetArgsByList; - for (const [listKey, list] of Object.entries(keystone.lists)) { - dbAPI[listKey] = itemDbAPIForList(list, contextToReturn, getArgsByList[listKey]); - itemAPI[listKey] = itemAPIForList(list, contextToReturn, getArgsByList[listKey]); + const dbAPIFactories = schemaName === 'public' ? publicDbApiFactories : internalDbApiFactories; + for (const listKey of Object.keys(keystone.lists)) { + dbAPI[listKey] = dbAPIFactories[listKey](contextToReturn); + itemAPI[listKey] = itemAPIForList(listKey, contextToReturn, dbAPI[listKey]); } return contextToReturn; }; diff --git a/packages-next/keystone/src/lib/context/executeGraphQLFieldToRootVal.ts b/packages-next/keystone/src/lib/context/executeGraphQLFieldToRootVal.ts new file mode 100644 index 00000000000..e110a52fc5a --- /dev/null +++ b/packages-next/keystone/src/lib/context/executeGraphQLFieldToRootVal.ts @@ -0,0 +1,209 @@ +import { KeystoneContext } from '@keystone-next/types'; +import { + GraphQLScalarType, + GraphQLObjectType, + GraphQLArgument, + GraphQLArgumentConfig, + GraphQLSchema, + DocumentNode, + validate, + execute, + GraphQLField, + VariableDefinitionNode, + TypeNode, + GraphQLType, + GraphQLNonNull, + GraphQLList, + GraphQLEnumType, + GraphQLInputObjectType, + GraphQLInterfaceType, + GraphQLUnionType, + ListTypeNode, + NamedTypeNode, + ArgumentNode, + GraphQLFieldConfig, + GraphQLOutputType, +} from 'graphql'; + +function getNamedOrListTypeNodeForType( + type: + | GraphQLScalarType + | GraphQLObjectType + | GraphQLInterfaceType + | GraphQLUnionType + | GraphQLEnumType + | GraphQLInputObjectType + | GraphQLList +): NamedTypeNode | ListTypeNode { + if (type instanceof GraphQLList) { + return { kind: 'ListType', type: getTypeNodeForType(type.ofType) }; + } + return { kind: 'NamedType', name: { kind: 'Name', value: type.name } }; +} + +function getTypeNodeForType(type: GraphQLType): TypeNode { + if (type instanceof GraphQLNonNull) { + return { kind: 'NonNullType', type: getNamedOrListTypeNodeForType(type.ofType) }; + } + return getNamedOrListTypeNodeForType(type); +} + +function getVariablesForGraphQLField(field: GraphQLField) { + const variableDefinitions: VariableDefinitionNode[] = field.args.map(arg => ({ + kind: 'VariableDefinition', + type: getTypeNodeForType(arg.type), + variable: { kind: 'Variable', name: { kind: 'Name', value: arg.name } }, + })); + + const argumentNodes: ArgumentNode[] = field.args.map(arg => ({ + kind: 'Argument', + name: { kind: 'Name', value: arg.name }, + value: { kind: 'Variable', name: { kind: 'Name', value: arg.name } }, + })); + + return { variableDefinitions, argumentNodes }; +} + +const rawField = 'raw'; + +const RawScalar = new GraphQLScalarType({ name: 'RawThingPlsDontRelyOnThisAnywhere' }); + +const ReturnRawValueObjectType = new GraphQLObjectType({ + name: 'ReturnRawValue', + fields: { + [rawField]: { + type: RawScalar, + resolve(rootVal) { + return rootVal; + }, + }, + }, +}); + +type RequiredButStillAllowUndefined< + T extends Record, + // this being a param is important and is what makes this work, + // please do not move it inside the mapped type. + // i can't find a place that explains this but the tldr is that + // having the keyof T _inside_ the mapped type means TS will keep modifiers + // like readonly and optionality and we want to remove those here + Key extends keyof T = keyof T +> = { + [K in Key]: T[K]; +}; + +function argsToArgsConfig(args: GraphQLArgument[]) { + return Object.fromEntries( + args.map(arg => { + const argConfig: RequiredButStillAllowUndefined = { + astNode: arg.astNode, + defaultValue: arg.defaultValue, + deprecationReason: arg.deprecationReason, + description: arg.description, + extensions: arg.extensions, + type: arg.type, + }; + return [arg.name, argConfig]; + }) + ); +} + +type OutputTypeWithoutNonNull = GraphQLObjectType | GraphQLList; + +type OutputType = OutputTypeWithoutNonNull | GraphQLNonNull; + +// note the GraphQLNonNull and GraphQLList constructors are incorrectly +// not generic over their inner type which is why we have to use as +// (the classes are generic but not the constructors) +function getTypeForField(originalType: GraphQLOutputType): OutputType { + if (originalType instanceof GraphQLNonNull) { + return new GraphQLNonNull(getTypeForField(originalType.ofType)) as OutputType; + } + if (originalType instanceof GraphQLList) { + return new GraphQLList(getTypeForField(originalType.ofType)) as OutputType; + } + return ReturnRawValueObjectType; +} + +function getRootValGivenOutputType(originalType: OutputType, value: any): any { + if (originalType instanceof GraphQLNonNull) { + return getRootValGivenOutputType(originalType.ofType, value); + } + if (originalType instanceof GraphQLList) { + if (value === null) return null; + return value.map((x: any) => getRootValGivenOutputType(originalType.ofType, x)); + } + return value[rawField]; +} + +export function executeGraphQLFieldToRootVal(field: GraphQLField) { + const { argumentNodes, variableDefinitions } = getVariablesForGraphQLField(field); + const document: DocumentNode = { + kind: 'Document', + definitions: [ + { + kind: 'OperationDefinition', + operation: 'query', + selectionSet: { + kind: 'SelectionSet', + selections: [ + { + kind: 'Field', + name: { kind: 'Name', value: field.name }, + arguments: argumentNodes, + selectionSet: { + kind: 'SelectionSet', + selections: [{ kind: 'Field', name: { kind: 'Name', value: rawField } }], + }, + }, + ], + }, + variableDefinitions, + }, + ], + }; + + const type = getTypeForField(field.type); + + const fieldConfig: RequiredButStillAllowUndefined> = { + args: argsToArgsConfig(field.args), + astNode: undefined, + deprecationReason: field.deprecationReason, + description: field.description, + extensions: field.extensions, + resolve: field.resolve, + subscribe: field.subscribe, + type, + }; + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + [field.name]: fieldConfig, + }, + }), + }); + + const validationErrors = validate(schema, document); + + if (validationErrors.length > 0) { + throw validationErrors[0]; + } + return async ( + args: Record, + context: KeystoneContext, + rootValue: Record = {} + ) => { + const result = await execute({ + schema, + document, + contextValue: context, + variableValues: args, + rootValue, + }); + if (result.errors?.length) { + throw result.errors[0]; + } + return getRootValGivenOutputType(type, result.data![field.name]); + }; +} diff --git a/packages-next/keystone/src/lib/context/getCoerceAndValidateArgumentsFnForGraphQLField.ts b/packages-next/keystone/src/lib/context/getCoerceAndValidateArgumentsFnForGraphQLField.ts deleted file mode 100644 index f80ad8eb337..00000000000 --- a/packages-next/keystone/src/lib/context/getCoerceAndValidateArgumentsFnForGraphQLField.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { - GraphQLField, - GraphQLSchema, - VariableDefinitionNode, - TypeNode, - GraphQLType, - GraphQLNonNull, - GraphQLList, - GraphQLEnumType, - GraphQLInputObjectType, - GraphQLInterfaceType, - GraphQLObjectType, - GraphQLScalarType, - GraphQLUnionType, - ListTypeNode, - NamedTypeNode, - FieldNode, -} from 'graphql'; -import { getArgumentValues, getVariableValues } from 'graphql/execution/values'; - -function getNamedOrListTypeNodeForType( - type: - | GraphQLScalarType - | GraphQLObjectType - | GraphQLInterfaceType - | GraphQLUnionType - | GraphQLEnumType - | GraphQLInputObjectType - | GraphQLList -): NamedTypeNode | ListTypeNode { - if (type instanceof GraphQLList) { - return { kind: 'ListType', type: getTypeNodeForType(type.ofType) }; - } - return { kind: 'NamedType', name: { kind: 'Name', value: type.name } }; -} - -function getTypeNodeForType(type: GraphQLType): TypeNode { - if (type instanceof GraphQLNonNull) { - return { kind: 'NonNullType', type: getNamedOrListTypeNodeForType(type.ofType) }; - } - return getNamedOrListTypeNodeForType(type); -} - -export function getCoerceAndValidateArgumentsFnForGraphQLField( - schema: GraphQLSchema, - field?: GraphQLField -) { - if (!field) return; - const variableDefintions: VariableDefinitionNode[] = []; - - for (const arg of field.args) { - variableDefintions.push({ - kind: 'VariableDefinition', - type: getTypeNodeForType(arg.type), - variable: { kind: 'Variable', name: { kind: 'Name', value: `${arg.name}` } }, - }); - } - - const fieldNode: FieldNode = { - kind: 'Field', - name: { kind: 'Name', value: field.name }, - arguments: field.args.map(arg => ({ - kind: 'Argument', - name: { kind: 'Name', value: arg.name }, - value: { kind: 'Variable', name: { kind: 'Name', value: `${arg.name}` } }, - })), - }; - return (args: Record) => { - const coercedVariableValues = getVariableValues(schema, variableDefintions, args); - if (coercedVariableValues.errors) { - throw coercedVariableValues.errors[0]; - } - return getArgumentValues(field, fieldNode, coercedVariableValues.coerced); - }; -} diff --git a/packages-next/keystone/src/lib/context/itemAPI.ts b/packages-next/keystone/src/lib/context/itemAPI.ts index b5e3daef5b0..239506b5cd8 100644 --- a/packages-next/keystone/src/lib/context/itemAPI.ts +++ b/packages-next/keystone/src/lib/context/itemAPI.ts @@ -1,10 +1,10 @@ -import { GraphQLSchema } from 'graphql'; +import { GraphQLSchema, GraphQLField } from 'graphql'; import { BaseGeneratedListTypes, - BaseKeystoneList, KeystoneDbAPI, KeystoneListsAPI, KeystoneContext, + GqlNames, } from '@keystone-next/types'; import { getItem, @@ -16,22 +16,48 @@ import { deleteItem, deleteItems, } from './server-side-graphql-client'; -import { getCoerceAndValidateArgumentsFnForGraphQLField } from './getCoerceAndValidateArgumentsFnForGraphQLField'; +import { executeGraphQLFieldToRootVal } from './executeGraphQLFieldToRootVal'; -export function getArgsFactory(list: BaseKeystoneList, schema: GraphQLSchema) { +// this is generally incorrect because types are open in TS but is correct in the specific usage here. +// (i mean it's not really any more incorrect than TS is generally is but let's ignore that) +const objectEntriesButUsingKeyof: >( + obj: T +) => [keyof T, T[keyof T]][] = Object.entries as any; + +export function getDbAPIFactory( + gqlNames: GqlNames, + schema: GraphQLSchema +): (context: KeystoneContext) => KeystoneDbAPI>[string] { const queryFields = schema.getQueryType()!.getFields(); const mutationFields = schema.getMutationType()!.getFields(); - const f = getCoerceAndValidateArgumentsFnForGraphQLField; - return { - findOne: f(schema, queryFields[list.gqlNames.itemQueryName]), - findMany: f(schema, queryFields[list.gqlNames.listQueryName]), - count: f(schema, queryFields[list.gqlNames.listQueryMetaName]), - createOne: f(schema, mutationFields[list.gqlNames.createMutationName]), - createMany: f(schema, mutationFields[list.gqlNames.createManyMutationName]), - updateOne: f(schema, mutationFields[list.gqlNames.updateMutationName]), - updateMany: f(schema, mutationFields[list.gqlNames.updateManyMutationName]), - deleteOne: f(schema, mutationFields[list.gqlNames.deleteMutationName]), - deleteMany: f(schema, mutationFields[list.gqlNames.deleteManyMutationName]), + const f = (field: GraphQLField | undefined) => { + if (field === undefined) { + return (): never => { + throw new Error('You do not have access to this resource'); + }; + } + return executeGraphQLFieldToRootVal(field); + }; + const api = { + findOne: f(queryFields[gqlNames.itemQueryName]), + findMany: f(queryFields[gqlNames.listQueryName]), + count: f(queryFields[gqlNames.listQueryMetaName]), + createOne: f(mutationFields[gqlNames.createMutationName]), + createMany: f(mutationFields[gqlNames.createManyMutationName]), + updateOne: f(mutationFields[gqlNames.updateMutationName]), + updateMany: f(mutationFields[gqlNames.updateManyMutationName]), + deleteOne: f(mutationFields[gqlNames.deleteMutationName]), + deleteMany: f(mutationFields[gqlNames.deleteManyMutationName]), + }; + return (context: KeystoneContext) => { + let obj = Object.fromEntries( + objectEntriesButUsingKeyof(api).map(([key, impl]) => [ + key, + (args: Record) => impl(args, context), + ]) + ); + obj.count = async (args: Record) => (await api.count(args, context)).getCount(); + return obj; }; } @@ -53,161 +79,87 @@ function defaultQueryParam(query?: string, resolveFields?: string | false) { * We'll be removing the option to use `resolveFields` entirely in a future release. */ export function itemAPIForList( - list: BaseKeystoneList, + listKey: string, context: KeystoneContext, - getArgs: ReturnType + dbAPI: KeystoneDbAPI>[string] ): KeystoneListsAPI>[string] { - const listKey = list.key; return { - findOne({ query, resolveFields, ...rawArgs }) { - if (!getArgs.findOne) throw new Error('You do not have access to this resource'); + findOne({ query, resolveFields, ...args }) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const args = rawArgs; return getItem({ listKey, context, returnFields, itemId: args.where.id }); } else { - const args = getArgs.findOne(rawArgs) as { where: { id: string } }; - return list.itemQuery(args, context); + return dbAPI.findOne(args); } }, - findMany({ query, resolveFields, ...rawArgs } = {}) { - if (!getArgs.findMany) throw new Error('You do not have access to this resource'); + findMany({ query, resolveFields, ...args } = {}) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const args = rawArgs; return getItems({ listKey, context, returnFields, ...args }); } else { - const args = getArgs.findMany(rawArgs); - return list.listQuery(args, context); + return dbAPI.findMany(args); } }, - async count(rawArgs = {}) { - if (!getArgs.count) throw new Error('You do not have access to this resource'); - const { first, skip, where } = rawArgs; + async count(args = {}) { + const { first, skip, where } = args; const { listQueryMetaName, whereInputName } = context.gqlNames(listKey); const query = `query ($first: Int, $skip: Int, $where: ${whereInputName}) { ${listQueryMetaName}(first: $first, skip: $skip, where: $where) { count } }`; const response = await context.graphql.run({ query, variables: { first, skip, where } }); return response[listQueryMetaName].count; }, - createOne({ query, resolveFields, ...rawArgs }) { - if (!getArgs.createOne) throw new Error('You do not have access to this resource'); + createOne({ query, resolveFields, ...args }) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const { data } = rawArgs; + const { data } = args; return createItem({ listKey, context, returnFields, item: data }); } else { - const { data } = getArgs.createOne(rawArgs); - return list.createMutation(data, context); + return dbAPI.createOne(args); } }, - createMany({ query, resolveFields, ...rawArgs }) { - if (!getArgs.createMany) throw new Error('You do not have access to this resource'); + createMany({ query, resolveFields, ...args }) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const { data } = rawArgs; + const { data } = args; return createItems({ listKey, context, returnFields, items: data }); } else { - const { data } = getArgs.createMany(rawArgs); - return list.createManyMutation(data, context); + return dbAPI.createMany(args); } }, - updateOne({ query, resolveFields, ...rawArgs }) { - if (!getArgs.updateOne) throw new Error('You do not have access to this resource'); + updateOne({ query, resolveFields, ...args }) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const { id, data } = rawArgs; + const { id, data } = args; return updateItem({ listKey, context, returnFields, item: { id, data } }); } else { - const { id, data } = getArgs.updateOne(rawArgs); - return list.updateMutation(id, data, context); + return dbAPI.updateOne(args); } }, - updateMany({ query, resolveFields, ...rawArgs }) { - if (!getArgs.updateMany) throw new Error('You do not have access to this resource'); + updateMany({ query, resolveFields, ...args }) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const { data } = rawArgs; + const { data } = args; return updateItems({ listKey, context, returnFields, items: data }); } else { - const { data } = getArgs.updateMany(rawArgs); - return list.updateManyMutation(data, context); + return dbAPI.updateMany(args); } }, - deleteOne({ query, resolveFields, ...rawArgs }) { - if (!getArgs.deleteOne) throw new Error('You do not have access to this resource'); + deleteOne({ query, resolveFields, ...args }) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const { id } = rawArgs; + const { id } = args; return deleteItem({ listKey, context, returnFields, itemId: id }); } else { - const { id } = getArgs.deleteOne(rawArgs); - return list.deleteMutation(id, context); + return dbAPI.deleteOne(args); } }, - deleteMany({ query, resolveFields, ...rawArgs }) { - if (!getArgs.deleteMany) throw new Error('You do not have access to this resource'); + deleteMany({ query, resolveFields, ...args }) { const returnFields = defaultQueryParam(query, resolveFields); if (returnFields) { - const { ids } = rawArgs; + const { ids } = args; return deleteItems({ listKey, context, returnFields, items: ids }); } else { - const { ids } = getArgs.deleteMany(rawArgs); - return list.deleteManyMutation(ids, context); + return dbAPI.deleteMany(args); } }, }; } - -export function itemDbAPIForList( - list: BaseKeystoneList, - context: KeystoneContext, - getArgs: ReturnType -): KeystoneDbAPI>[string] { - return { - findOne(rawArgs) { - if (!getArgs.findOne) throw new Error('You do not have access to this resource'); - const args = getArgs.findOne(rawArgs) as { where: { id: string } }; - return list.itemQuery(args, context); - }, - findMany(rawArgs = {}) { - if (!getArgs.findMany) throw new Error('You do not have access to this resource'); - const args = getArgs.findMany(rawArgs); - return list.listQuery(args, context); - }, - async count(rawArgs = {}) { - if (!getArgs.count) throw new Error('You do not have access to this resource'); - const args = getArgs.count(rawArgs!); - return (await list.listQueryMeta(args, context)).getCount(); - }, - createOne(rawArgs) { - if (!getArgs.createOne) throw new Error('You do not have access to this resource'); - const { data } = getArgs.createOne(rawArgs); - return list.createMutation(data, context); - }, - createMany(rawArgs) { - if (!getArgs.createMany) throw new Error('You do not have access to this resource'); - const { data } = getArgs.createMany(rawArgs); - return list.createManyMutation(data, context); - }, - updateOne(rawArgs) { - if (!getArgs.updateOne) throw new Error('You do not have access to this resource'); - const { id, data } = getArgs.updateOne(rawArgs); - return list.updateMutation(id, data, context); - }, - updateMany(rawArgs) { - if (!getArgs.updateMany) throw new Error('You do not have access to this resource'); - const { data } = getArgs.updateMany(rawArgs); - return list.updateManyMutation(data, context); - }, - deleteOne(rawArgs) { - if (!getArgs.deleteOne) throw new Error('You do not have access to this resource'); - const { id } = getArgs.deleteOne(rawArgs); - return list.deleteMutation(id, context); - }, - deleteMany(rawArgs) { - if (!getArgs.deleteMany) throw new Error('You do not have access to this resource'); - const { ids } = getArgs.deleteMany(rawArgs); - return list.deleteManyMutation(ids, context); - }, - }; -}