Skip to content

Commit

Permalink
Don't validate if query is already an AST.
Browse files Browse the repository at this point in the history
  • Loading branch information
damour committed Feb 19, 2018
1 parent 5b90120 commit 446441e
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### vNEXT

* Fix apollo-server-core runQuery breaks async_hooks tracking [PR #733](https://github.com/apollographql/apollo-server/pull/733)
* Don't validate if query is already an AST. [PR #839](https://github.com/apollographql/apollo-server/pull/839)

### v1.3.0

Expand Down
5 changes: 4 additions & 1 deletion packages/apollo-server-core/src/runHttpQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,10 @@ export async function runHttpQuery(
const requests: Array<ExecutionResult> = requestPayload.map(requestParams => {
try {
let query = requestParams.query;
const isQueryString = typeof query === 'string';

if (isGetRequest) {
if (typeof query === 'string') {
if (isQueryString) {
// preparse the query incase of GET so we can assert the operation.
query = parse(query);
}
Expand Down Expand Up @@ -149,6 +151,7 @@ export async function runHttpQuery(
query: query,
variables: variables,
context,
isQueryString,
rootValue: optionsObject.rootValue,
operationName: operationName,
logFunction: optionsObject.logFunction,
Expand Down
21 changes: 12 additions & 9 deletions packages/apollo-server-core/src/runQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export interface QueryOptions {
debug?: boolean;
tracing?: boolean;
cacheControl?: boolean;
isQueryString?: boolean;
}

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

let rules = specifiedRules;
if (options.validationRules) {
rules = rules.concat(options.validationRules);
}
logFunction({ action: LogAction.validation, step: LogStep.start });
const validationErrors = validate(options.schema, documentAST, rules);
logFunction({ action: LogAction.validation, step: LogStep.end });
if (validationErrors.length) {
return Promise.resolve({ errors: format(validationErrors) });
if (options.isQueryString !== false) {
let rules = specifiedRules;
if (options.validationRules) {
rules = rules.concat(options.validationRules);
}
logFunction({ action: LogAction.validation, step: LogStep.start });
const validationErrors = validate(options.schema, documentAST, rules);
logFunction({ action: LogAction.validation, step: LogStep.end });
if (validationErrors.length) {
return Promise.resolve({ errors: format(validationErrors) });
}
}

if (extensionStack) {
Expand Down
38 changes: 38 additions & 0 deletions packages/apollo-server-express/src/apolloServerHttp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ import {
GraphQLScalarType,
GraphQLError,
BREAK,
parse,
} from 'graphql';
import { LogAction } from 'apollo-server-core';

const QueryRootType = new GraphQLObjectType({
name: 'QueryRoot',
Expand Down Expand Up @@ -538,4 +540,40 @@ 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);
});
});
});

0 comments on commit 446441e

Please sign in to comment.