Don't validate if query is already an AST.

apollographql · May 8, 2018 · 7ef423a · 7ef423a
1 parent e685acb
commit 7ef423a
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@ 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)
+
 ### v1.3.6
 
 * Recognize requests with Apollo Persisted Queries and return `PersistedQueryNotSupported` to the client instead of a confusing error. [PR #982](https://github.com/apollographql/apollo-server/pull/982)

diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts
@@ -102,6 +102,7 @@ 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
@@ -138,7 +139,7 @@ export async function runHttpQuery(
       }
 
       if (isGetRequest) {
-        if (typeof query === 'string') {
+        if (isQueryString) {
           // 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
@@ -193,6 +194,7 @@ 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
@@ -72,6 +72,7 @@ export interface QueryOptions {
   debug?: boolean;
   tracing?: boolean;
   cacheControl?: boolean | CacheControlExtensionOptions;
+  isQueryString?: boolean;
 }
 
 export function runQuery(options: QueryOptions): Promise<GraphQLResponse> {
@@ -171,17 +172,19 @@ 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, options.formatError),
-    });
+  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, options.formatError),
+      });
+    }
   }
 
   if (extensionStack) {

diff --git a/packages/apollo-server-express/src/apolloServerHttp.test.ts b/packages/apollo-server-express/src/apolloServerHttp.test.ts
@@ -37,7 +37,9 @@ import {
   GraphQLScalarType,
   GraphQLError,
   BREAK,
+  parse,
 } from 'graphql';
+import { LogAction } from 'apollo-server-core';
 
 const QueryRootType = new GraphQLObjectType({
   name: 'QueryRoot',
@@ -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);
+    });
+  });
 });