From e47938bdd5e3ea16133cdafc9e5e670405210cce Mon Sep 17 00:00:00 2001 From: damour Date: Mon, 19 Feb 2018 20:00:20 +0200 Subject: [PATCH] Skip validation option. --- README.md | 3 ++ .../apollo-server-core/src/runHttpQuery.ts | 5 +-- packages/apollo-server-core/src/runQuery.ts | 4 +- .../src/apolloServerHttp.test.ts | 38 ------------------- .../src/index.ts | 33 ++++++++++++++++ 5 files changed, 39 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index c51f367390f..b73cb16c464 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,7 @@ Apollo Server can be configured with an options object with the following fields * **context**: the context value passed to resolvers during GraphQL execution * **rootValue**: the value passed to the first resolve function * **formatError**: a function to apply to every error before sending the response to clients +* **skipValidation**: skip query validation (increase performance, use carefully, only with whitelisting) * **validationRules**: additional GraphQL validation rules to be applied to client-specified queries * **formatParams**: a function applied for each query in a batch to format parameters before execution * **formatResponse**: a function applied to each response after execution @@ -224,6 +225,7 @@ All options except for `schema` are optional. ### Whitelisting The `formatParams` function can be used in combination with the `OperationStore` to enable whitelisting. +In this case query parsing and validation will be called only once when saving to store. ```js const store = new OperationStore(Schema); @@ -232,6 +234,7 @@ graphqlOptions = { schema: Schema, formatParams(params) { params['query'] = store.get(params.operationName); + params['skipValidation'] = true; return params; }, }; diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index c9deba735cb..780ca7f97c3 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -105,10 +105,8 @@ export async function runHttpQuery( const requests: Array = requestPayload.map(requestParams => { try { let query = requestParams.query; - const isQueryString = typeof query === 'string'; - if (isGetRequest) { - if (isQueryString) { + if (typeof query === 'string') { // preparse the query incase of GET so we can assert the operation. query = parse(query); } @@ -151,7 +149,6 @@ export async function runHttpQuery( query: query, variables: variables, context, - isQueryString, rootValue: optionsObject.rootValue, operationName: operationName, logFunction: optionsObject.logFunction, diff --git a/packages/apollo-server-core/src/runQuery.ts b/packages/apollo-server-core/src/runQuery.ts index 65b2ba63b1f..d79e8f5592b 100644 --- a/packages/apollo-server-core/src/runQuery.ts +++ b/packages/apollo-server-core/src/runQuery.ts @@ -69,7 +69,7 @@ export interface QueryOptions { debug?: boolean; tracing?: boolean; cacheControl?: boolean; - isQueryString?: boolean; + skipValidation?: boolean; } function runQuery(options: QueryOptions): Promise { @@ -166,7 +166,7 @@ function doRunQuery(options: QueryOptions): Promise { documentAST = options.query as DocumentNode; } - if (options.isQueryString !== false) { + if (options.skipValidation !== true) { let rules = specifiedRules; if (options.validationRules) { rules = rules.concat(options.validationRules); diff --git a/packages/apollo-server-express/src/apolloServerHttp.test.ts b/packages/apollo-server-express/src/apolloServerHttp.test.ts index 56e609c910a..24c6ac89e4b 100644 --- a/packages/apollo-server-express/src/apolloServerHttp.test.ts +++ b/packages/apollo-server-express/src/apolloServerHttp.test.ts @@ -37,9 +37,7 @@ import { GraphQLScalarType, GraphQLError, BREAK, - parse, } from 'graphql'; -import { LogAction } from 'apollo-server-core'; const QueryRootType = new GraphQLObjectType({ name: 'QueryRoot', @@ -540,40 +538,4 @@ describe(`GraphQL-HTTP (apolloServer) tests for ${version} express`, () => { }); }); }); - - describe('Query is an AST', () => { - it('Do not validate if query is already an AST.', async () => { - const app = express(); - let validationCalled = false; - - app.use('/graphql', bodyParser.json()); - app.use('/graphql', (req, res, next) => { - req.body.query = parse(req.body.query); - - next(); - }); - app.use( - '/graphql', - graphqlExpress({ - schema: TestSchema, - logFunction: ({ action }) => { - if (action == LogAction.validation) { - validationCalled = true; - } - }, - }), - ); - - const response = await request(app) - .post('/graphql') - .send({ - query: '{ test(who: "World") }', - }); - - expect( - validationCalled, - 'Validation should not be called if query is already an AST', - ).to.equal(false); - }); - }); }); diff --git a/packages/apollo-server-integration-testsuite/src/index.ts b/packages/apollo-server-integration-testsuite/src/index.ts index 9a4d88cb451..4e52d431715 100644 --- a/packages/apollo-server-integration-testsuite/src/index.ts +++ b/packages/apollo-server-integration-testsuite/src/index.ts @@ -19,6 +19,7 @@ const request = require('supertest'); import { GraphQLOptions } from 'apollo-server-core'; import * as GraphiQL from 'apollo-server-module-graphiql'; import { OperationStore } from 'apollo-server-module-operation-store'; +import { LogAction } from '../../apollo-server-core/dist'; const queryType = new GraphQLObjectType({ name: 'QueryType', @@ -962,6 +963,38 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { return expect(res.body).to.deep.equal(expected); }); }); + + it('do not validate if query is already an AST', async () => { + const store = new OperationStore(schema); + let validationCalled = false; + store.put('query testquery{ testString }'); + app = await createApp({ + graphqlOptions: { + schema, + formatParams(params) { + params['query'] = store.get(params.operationName); + params['skipValidation'] = true; + return params; + }, + logFunction: ({ action }) => { + if (action == LogAction.validation) { + validationCalled = true; + } + }, + }, + }); + const req = request(app) + .post('/graphql') + .send({ + operationName: 'testquery', + }); + return req.then(res => { + return expect( + validationCalled, + 'Validation should not be called if skipValidation option provided', + ).to.equal(false); + }); + }); }); describe('server setup', () => {