From ad4ce9adb2528148c68da27bf20deee938818be5 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Mon, 23 Jan 2017 09:21:22 +0200 Subject: [PATCH 1/6] test(batching#176): add failing test for serial batching --- .../src/index.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index f87f6834c59..763b72dcde5 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -7,6 +7,7 @@ import { GraphQLSchema, GraphQLObjectType, GraphQLString, + GraphQLInt, GraphQLError, GraphQLNonNull, introspectionQuery, @@ -29,6 +30,17 @@ const queryType = new GraphQLObjectType({ return 'it works'; }, }, + testStringWithDelay: { + type: GraphQLString, + args: { + delay: { type: new GraphQLNonNull(GraphQLInt) }, + }, + resolve(root, args) { + return new Promise((resolve, reject) => { + setTimeout(() => resolve('it works'), args['delay']); + }); + }, + }, testContext: { type: GraphQLString, resolve(_, args, context) { @@ -457,6 +469,27 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('can handle batch requests in parallel', function() { + // this test will fail due to timeout if running serially. + this.timeout(300); + + app = createApp(); + const expected = Array(10).fill({ + data: { testStringWithDelay: 'it works' }, + }); + const req = request(app) + .post('/graphql') + .send(Array(10).fill({ + query: `query test($delay: Int!) { testStringWithDelay(delay: $delay) }`, + operationName: 'test', + variables: { delay: 40 }, + })); + return req.then((res) => { + expect(res.status).to.equal(200); + return expect(res.body).to.deep.equal(expected); + }); + }); + it('clones batch context', () => { app = createApp({graphqlOptions: { schema, From 47e341f5c230aec63e320bb63658521f133cd084 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Mon, 23 Jan 2017 09:39:42 +0200 Subject: [PATCH 2/6] fix(batching#176): run batched requests in parallel --- packages/graphql-server-core/src/runHttpQuery.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/graphql-server-core/src/runHttpQuery.ts b/packages/graphql-server-core/src/runHttpQuery.ts index aaf6fe79520..efef17e71e1 100644 --- a/packages/graphql-server-core/src/runHttpQuery.ts +++ b/packages/graphql-server-core/src/runHttpQuery.ts @@ -74,8 +74,7 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu requestPayload = [requestPayload]; } - let responses: Array = []; - for (let requestParams of requestPayload) { + const requests: Array = requestPayload.map(requestParams => { try { let query = requestParams.query; if ( isGetRequest ) { @@ -128,17 +127,18 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu params = optionsObject.formatParams(params); } - responses.push(await runQuery(params)); + return runQuery(params); } catch (e) { // Populate any HttpQueryError to our handler which should // convert it to Http Error. if ( e.name === 'HttpQueryError' ) { - throw e; + return Promise.reject(e); } - responses.push({ errors: [formatErrorFn(e)] }); + return Promise.resolve({ errors: [formatErrorFn(e)] }); } - } + }); + const responses = await Promise.all(requests); if (!isBatch) { const gqlResponse = responses[0]; From 123454165c120ede98a6fb8742189d250019fc3d Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Mon, 23 Jan 2017 09:45:43 +0200 Subject: [PATCH 3/6] chore(package): Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b710567df8..081e5855f02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ### VNEXT +* run batched requests in parallel ([@DxCx](https://github.com/DxCx)) on [#273](https://github.com/apollostack/graphql-server/pull/273) * Fix GraphiQL options variables. Issue #193. ([@alanchristensen](https://github.com/alanchristensen)) on [PR #255](https://github.com/apollostack/apollo-server/pull/255) From 0a7b56643121a37f9955c16de4d65d36805f6168 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Mon, 23 Jan 2017 10:43:53 +0200 Subject: [PATCH 4/6] test(batching#176): increase parallel test timeout while increasing threads as well, for Node v4 --- .../graphql-server-integration-testsuite/src/index.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index 763b72dcde5..0bf4062f41d 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -471,18 +471,20 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { it('can handle batch requests in parallel', function() { // this test will fail due to timeout if running serially. - this.timeout(300); + const parallels = 100; + const delayPerReq = 40; + this.timeout(2000); app = createApp(); - const expected = Array(10).fill({ + const expected = Array(parallels).fill({ data: { testStringWithDelay: 'it works' }, }); const req = request(app) .post('/graphql') - .send(Array(10).fill({ + .send(Array(parallels).fill({ query: `query test($delay: Int!) { testStringWithDelay(delay: $delay) }`, operationName: 'test', - variables: { delay: 40 }, + variables: { delay: delayPerReq }, })); return req.then((res) => { expect(res.status).to.equal(200); From 3c249eb540fa4d9d6d73158d7dc9f1ce056922b4 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Mon, 23 Jan 2017 12:36:28 +0200 Subject: [PATCH 5/6] test(batching#176): increase timeout for deflat/gzip tests, for slow Node 4 --- .../graphql-server-express/src/apolloServerHttp.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/graphql-server-express/src/apolloServerHttp.test.ts b/packages/graphql-server-express/src/apolloServerHttp.test.ts index 03528cac3bf..50e3658a7ac 100644 --- a/packages/graphql-server-express/src/apolloServerHttp.test.ts +++ b/packages/graphql-server-express/src/apolloServerHttp.test.ts @@ -154,7 +154,9 @@ const version = 'modern'; describe(`GraphQL-HTTP (apolloServer) tests for ${version} express`, () => { describe('POST functionality', () => { - it('allows gzipped POST bodies', async () => { + it('allows gzipped POST bodies', async function () { + // Increase timeout for slow node 4 + this.timeout(3000); const app = express(); app.use(urlString(), bodyParser.json()); @@ -181,7 +183,9 @@ describe(`GraphQL-HTTP (apolloServer) tests for ${version} express`, () => { }); }); - it('allows deflated POST bodies', async () => { + it('allows deflated POST bodies', async function () { + // Increase timeout for slow node 4 + this.timeout(3000); const app = express(); app.use(urlString(), bodyParser.json()); From f0127a471c0f67e56fa50adf023093a3f09b4da1 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Tue, 24 Jan 2017 12:26:41 +0200 Subject: [PATCH 6/6] test(batching#176): increase timeout for parallel, was still not enough for slow Node 4 --- packages/graphql-server-integration-testsuite/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index 0bf4062f41d..f48dc6c1cc7 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -473,7 +473,7 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { // this test will fail due to timeout if running serially. const parallels = 100; const delayPerReq = 40; - this.timeout(2000); + this.timeout(3000); app = createApp(); const expected = Array(parallels).fill({