From 8f6d3efc92b25236f5a3a761ea7ba2f0a7c7f550 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU <ardatanrikulu@gmail.com> Date: Mon, 31 Oct 2022 20:46:50 +0300 Subject: [PATCH] Faster validation (#4801) --- .changeset/green-seas-repair.md | 5 + packages/utils/src/validate-documents.ts | 105 +++--------- .../utils/tests/validate-documents.spec.ts | 159 +++--------------- 3 files changed, 49 insertions(+), 220 deletions(-) create mode 100644 .changeset/green-seas-repair.md diff --git a/.changeset/green-seas-repair.md b/.changeset/green-seas-repair.md new file mode 100644 index 00000000000..e55000868ad --- /dev/null +++ b/.changeset/green-seas-repair.md @@ -0,0 +1,5 @@ +--- +'@graphql-tools/utils': major +--- + +_BREAKING_: `checkValidationErrors` has been dropped and `validateGraphQlDocuments` now accepts `DocumentNode[]` instead and it throws the original `GraphQLError`s with the correct stack trace diff --git a/packages/utils/src/validate-documents.ts b/packages/utils/src/validate-documents.ts index d807868fbdd..4b831dcfcff 100644 --- a/packages/utils/src/validate-documents.ts +++ b/packages/utils/src/validate-documents.ts @@ -2,108 +2,45 @@ import { Kind, validate, GraphQLSchema, - GraphQLError, specifiedRules, - FragmentDefinitionNode, ValidationContext, ASTVisitor, - DefinitionNode, - concatAST, DocumentNode, versionInfo, + DefinitionNode, } from 'graphql'; -import { Source } from './loaders.js'; -import { AggregateError } from './AggregateError.js'; export type ValidationRule = (context: ValidationContext) => ASTVisitor; -export interface LoadDocumentError { - readonly filePath?: string; - readonly errors: ReadonlyArray<GraphQLError>; -} - -export async function validateGraphQlDocuments( +export function validateGraphQlDocuments( schema: GraphQLSchema, - documentFiles: Source[], - effectiveRules: ValidationRule[] = createDefaultRules() -): Promise<ReadonlyArray<LoadDocumentError>> { - const allFragmentMap = new Map<string, FragmentDefinitionNode>(); - const documentFileObjectsToValidate: { - location?: string; - document: DocumentNode; - }[] = []; - - for (const documentFile of documentFiles) { - if (documentFile.document) { - const definitionsToValidate: DefinitionNode[] = []; - for (const definitionNode of documentFile.document.definitions) { - if (definitionNode.kind === Kind.FRAGMENT_DEFINITION) { - allFragmentMap.set(definitionNode.name.value, definitionNode); - } else { - definitionsToValidate.push(definitionNode); - } + documents: DocumentNode[], + rules: ValidationRule[] = createDefaultRules() +) { + const definitionMap = new Map<string, DefinitionNode>(); + for (const document of documents) { + for (const docDefinition of document.definitions) { + if ('name' in docDefinition && docDefinition.name) { + definitionMap.set(docDefinition.name.value, docDefinition); + } else { + definitionMap.set(Date.now().toString(), docDefinition); } - documentFileObjectsToValidate.push({ - location: documentFile.location, - document: { - kind: Kind.DOCUMENT, - definitions: definitionsToValidate, - }, - }); } } - - const allErrors: LoadDocumentError[] = []; - - const allFragmentsDocument: DocumentNode = { + const fullAST: DocumentNode = { kind: Kind.DOCUMENT, - definitions: [...allFragmentMap.values()], + definitions: Array.from(definitionMap.values()), }; - - await Promise.all( - documentFileObjectsToValidate.map(async documentFile => { - const documentToValidate = concatAST([allFragmentsDocument, documentFile.document]); - - const errors = validate(schema, documentToValidate, effectiveRules); - - if (errors.length > 0) { - allErrors.push({ - filePath: documentFile.location, - errors, - }); - } - }) - ); - - return allErrors; -} - -export function checkValidationErrors(loadDocumentErrors: ReadonlyArray<LoadDocumentError>): void | never { - if (loadDocumentErrors.length > 0) { - const errors: Error[] = []; - - for (const loadDocumentError of loadDocumentErrors) { - for (const graphQLError of loadDocumentError.errors) { - const error = new Error(); - error.name = 'GraphQLDocumentError'; - error.message = `${error.name}: ${graphQLError.message}`; - error.stack = error.message; - if (graphQLError.locations) { - for (const location of graphQLError.locations) { - error.stack += `\n at ${loadDocumentError.filePath}:${location.line}:${location.column}`; - } - } - - errors.push(error); + const errors = validate(schema, fullAST, rules); + for (const error of errors) { + error.stack = error.message; + if (error.locations) { + for (const location of error.locations) { + error.stack += `\n at ${error.source?.name}:${location.line}:${location.column}`; } } - - throw new AggregateError( - errors, - `GraphQL Document Validation failed with ${errors.length} errors; - ${errors.map((error, index) => `Error ${index}: ${error.stack}`).join('\n\n')}` - ); } + return errors; } export function createDefaultRules() { diff --git a/packages/utils/tests/validate-documents.spec.ts b/packages/utils/tests/validate-documents.spec.ts index 16f05c4748d..02e7b0f1105 100644 --- a/packages/utils/tests/validate-documents.spec.ts +++ b/packages/utils/tests/validate-documents.spec.ts @@ -1,5 +1,5 @@ -import { checkValidationErrors, validateGraphQlDocuments } from '../src/index.js'; -import { buildSchema, parse, GraphQLError } from 'graphql'; +import { validateGraphQlDocuments } from '../src/index.js'; +import { buildSchema, parse, GraphQLError, Source } from 'graphql'; describe('validateGraphQlDocuments', () => { it('Should throw an informative error when validation errors happens, also check for fragments validation even why they are duplicated', async () => { @@ -25,146 +25,33 @@ describe('validateGraphQlDocuments', () => { } `; - const result = await validateGraphQlDocuments(schema, [ - { - location: 'fragment.graphql', - document: parse(fragment), - }, - { - location: 'query.graphql', - document: parse(/* GraphQL */ ` - query searchPage { - otherStuff { - foo + const result = validateGraphQlDocuments(schema, [ + parse(new Source(fragment, 'packages/client/src/fragments/pizzeriaFragment.fragment.graphql')), + parse( + new Source( + /* GraphQL */ ` + query searchPage { + otherStuff { + foo + } + ...pizzeriaFragment } - ...pizzeriaFragment - } - ${fragment} - `), - }, + ${fragment} + `, + 'packages/client/src/pages/search/searchPage.query.graphql' + ) + ), ]); expect(result).toHaveLength(1); - expect(result[0].filePath).toBe('query.graphql'); - expect(result[0].errors[0] instanceof GraphQLError).toBeTruthy(); - expect(result[0].errors[0].message).toBe( + expect(result[0].source?.name).toBe('packages/client/src/pages/search/searchPage.query.graphql'); + expect(result[0] instanceof GraphQLError).toBeTruthy(); + expect(result[0].message).toBe( 'Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria".' ); - - try { - checkValidationErrors(result); - expect(true).toBeFalsy(); - } catch (aggregateError: any) { - const { errors } = aggregateError; - expect(Symbol.iterator in errors).toBeTruthy(); - const generator = errors[Symbol.iterator](); - - const error = generator.next().value; - - expect(error).toBeInstanceOf(Error); - expect(error.name).toEqual('GraphQLDocumentError'); - expect(error.message).toEqual( - 'GraphQLDocumentError: Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria".' - ); - expect(error.stack).toEqual( - [ - 'GraphQLDocumentError: Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria".', - ' at query.graphql:6:13', - ].join('\n') - ); - } - }); -}); - -describe('checkValidationErrors', () => { - it('Should throw errors source files and locations', async () => { - const loadDocumentErrors = [ - { - filePath: 'packages/server/src/modules/github-check-run/providers/documents/create-check-run.mutation.graphql', - errors: [ - { - message: 'Cannot query field "randomField" on type "CheckRun".', - locations: [ - { - line: 7, - column: 13, - }, - ], - }, - { - message: 'Cannot query field "randomField2" on type "CheckRun".', - locations: [ - { - line: 8, - column: 13, - }, - ], - }, - ], - }, - { - filePath: 'packages/server/src/modules/github-check-run/providers/documents/check-run.query.graphql', - errors: [ - { - message: 'Cannot query field "randomField" on type "CheckRun".', - locations: [ - { - line: 7, - column: 13, - }, - ], - }, - ], - }, - ]; - - let errors; - try { - checkValidationErrors(loadDocumentErrors as any); - } catch (aggregateError: any) { - errors = aggregateError.errors; - } - - expect(Symbol.iterator in errors).toBeTruthy(); - - let error; - const generator = errors[Symbol.iterator](); - - error = generator.next().value; - - expect(error).toBeInstanceOf(Error); - expect(error.name).toEqual('GraphQLDocumentError'); - expect(error.message).toEqual('GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".'); - expect(error.stack).toEqual( - [ - 'GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".', - ' at packages/server/src/modules/github-check-run/providers/documents/create-check-run.mutation.graphql:7:13', - ].join('\n') - ); - - error = generator.next().value; - - expect(error).toBeInstanceOf(Error); - expect(error.name).toEqual('GraphQLDocumentError'); - expect(error.message).toEqual('GraphQLDocumentError: Cannot query field "randomField2" on type "CheckRun".'); - expect(error.stack).toEqual( - [ - 'GraphQLDocumentError: Cannot query field "randomField2" on type "CheckRun".', - ' at packages/server/src/modules/github-check-run/providers/documents/create-check-run.mutation.graphql:8:13', - ].join('\n') - ); - - error = generator.next().value; - - expect(error).toBeInstanceOf(Error); - expect(error.name).toEqual('GraphQLDocumentError'); - expect(error.message).toEqual('GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".'); - expect(error.stack).toEqual( - [ - 'GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".', - ' at packages/server/src/modules/github-check-run/providers/documents/check-run.query.graphql:7:13', - ].join('\n') - ); + expect(result[0].stack) + .toBe(`Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria". + at packages/client/src/pages/search/searchPage.query.graphql:6:15`); }); });