Skip to content

Commit

Permalink
Skip validation option.
Browse files Browse the repository at this point in the history
  • Loading branch information
damour committed May 8, 2018
1 parent 7ef423a commit 52f1589
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 44 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ All of the packages in the `apollo-server` repo are released with the same versi

### vNEXT

* Don't validate if query is already an AST. [PR #839](https://github.com/apollographql/apollo-server/pull/839)
* Add skipValidation option [PR #839](https://github.com/apollographql/apollo-server/pull/839)

### v1.3.6

Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -228,6 +229,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);
Expand All @@ -236,6 +238,7 @@ graphqlOptions = {
schema: Schema,
formatParams(params) {
params['query'] = store.get(params.operationName);
params['skipValidation'] = true;
return params;
},
};
Expand Down
4 changes: 1 addition & 3 deletions packages/apollo-server-core/src/runHttpQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ export async function runHttpQuery(
try {
let query = requestParams.query;
let extensions = requestParams.extensions;
const isQueryString = typeof query === 'string';

if (isGetRequest && extensions) {
// For GET requests, we have to JSON-parse extensions. (For POST
Expand Down Expand Up @@ -139,7 +138,7 @@ export async function runHttpQuery(
}

if (isGetRequest) {
if (isQueryString) {
if (typeof query === 'string') {
// preparse the query incase of GET so we can assert the operation.
// XXX This makes the type of 'query' in this function confused
// which has led to us accidentally supporting GraphQL AST over
Expand Down Expand Up @@ -194,7 +193,6 @@ export async function runHttpQuery(
query: query,
variables: variables,
context,
isQueryString,
rootValue: optionsObject.rootValue,
operationName: operationName,
logFunction: optionsObject.logFunction,
Expand Down
4 changes: 2 additions & 2 deletions packages/apollo-server-core/src/runQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export interface QueryOptions {
debug?: boolean;
tracing?: boolean;
cacheControl?: boolean | CacheControlExtensionOptions;
isQueryString?: boolean;
skipValidation?: boolean;
}

export function runQuery(options: QueryOptions): Promise<GraphQLResponse> {
Expand Down Expand Up @@ -172,7 +172,7 @@ function doRunQuery(options: QueryOptions): Promise<GraphQLResponse> {
documentAST = options.query as DocumentNode;
}

if (options.isQueryString !== false) {
if (options.skipValidation !== true) {
let rules = specifiedRules;
if (options.validationRules) {
rules = rules.concat(options.validationRules);
Expand Down
38 changes: 0 additions & 38 deletions packages/apollo-server-express/src/apolloServerHttp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ import {
GraphQLScalarType,
GraphQLError,
BREAK,
parse,
} from 'graphql';
import { LogAction } from 'apollo-server-core';

const QueryRootType = new GraphQLObjectType({
name: 'QueryRoot',
Expand Down Expand Up @@ -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);
});
});
});
33 changes: 33 additions & 0 deletions packages/apollo-server-integration-testsuite/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 personType = new GraphQLObjectType({
name: 'PersonType',
Expand Down Expand Up @@ -1054,6 +1055,38 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => {
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', () => {
Expand Down

0 comments on commit 52f1589

Please sign in to comment.