From a2d7c85ff2a3372d91cf2db602d094f81654fcc4 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Thu, 20 Oct 2016 21:16:53 +0300 Subject: [PATCH 01/13] chore(get-support): support for GET method for express and connect --- packages/graphql-server-express/package.json | 1 + .../src/apolloServerHttp.test.ts | 6 +-- .../src/connectApollo.test.ts | 1 + .../src/expressApollo.ts | 47 ++++++++++++------- .../src/index.ts | 38 ++++++++++++++- 5 files changed, 72 insertions(+), 21 deletions(-) diff --git a/packages/graphql-server-express/package.json b/packages/graphql-server-express/package.json index 3fda01eb991..bf204401d89 100644 --- a/packages/graphql-server-express/package.json +++ b/packages/graphql-server-express/package.json @@ -37,6 +37,7 @@ "graphql-server-integration-testsuite": "^0.5.0", "body-parser": "^1.15.2", "connect": "^3.4.1", + "connect-query": "^0.2.0", "express": "^4.14.0", "multer": "^1.2.0" }, diff --git a/packages/graphql-server-express/src/apolloServerHttp.test.ts b/packages/graphql-server-express/src/apolloServerHttp.test.ts index 217c9a8287d..03528cac3bf 100644 --- a/packages/graphql-server-express/src/apolloServerHttp.test.ts +++ b/packages/graphql-server-express/src/apolloServerHttp.test.ts @@ -372,11 +372,11 @@ describe(`GraphQL-HTTP (apolloServer) tests for ${version} express`, () => { app.use(urlString(), graphqlExpress({ schema: TestSchema })); const response = await request(app) - .get(urlString({ query: '{test}' })); + .put(urlString({ query: '{test}' })); expect(response.status).to.equal(405); - expect(response.headers.allow).to.equal('POST'); - return expect(response.text).to.contain('Apollo Server supports only POST requests.'); + expect(response.headers.allow).to.equal('GET, POST'); + return expect(response.text).to.contain('Apollo Server supports only GET/POST requests.'); }); }); diff --git a/packages/graphql-server-express/src/connectApollo.test.ts b/packages/graphql-server-express/src/connectApollo.test.ts index a7a9be333c4..89e77a2b268 100644 --- a/packages/graphql-server-express/src/connectApollo.test.ts +++ b/packages/graphql-server-express/src/connectApollo.test.ts @@ -15,6 +15,7 @@ function createConnectApp(options: CreateAppOptions = {}) { if (options.graphiqlOptions ) { app.use('/graphiql', graphiqlConnect( options.graphiqlOptions )); } + app.use('/graphql', require('connect-query')()); app.use('/graphql', graphqlConnect( options.graphqlOptions )); return app; } diff --git a/packages/graphql-server-express/src/expressApollo.ts b/packages/graphql-server-express/src/expressApollo.ts index bdebad5b107..4ec36d678e1 100644 --- a/packages/graphql-server-express/src/expressApollo.ts +++ b/packages/graphql-server-express/src/expressApollo.ts @@ -42,33 +42,48 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu } const formatErrorFn = optionsObject.formatError || graphql.formatError; + let buffer; + + switch ( req.method ) { + case 'POST': + if ( !req.body ) { + res.statusCode = 500; + res.write('POST body missing. Did you forget "app.use(bodyParser.json())"?'); + res.end(); + return; + } - if (req.method !== 'POST') { - res.setHeader('Allow', 'POST'); - res.statusCode = 405; - res.write('Apollo Server supports only POST requests.'); - res.end(); - return; - } + buffer = req.body; + break; + case 'GET': + if ( !req.query ) { + res.statusCode = 500; + res.write('GET query missing.'); + res.end(); + return; + } - if (!req.body) { - res.statusCode = 500; - res.write('POST body missing. Did you forget "app.use(bodyParser.json())"?'); - res.end(); - return; + buffer = req.query; + break; + + default: + res.setHeader('Allow', 'GET, POST'); + res.statusCode = 405; + res.write('Apollo Server supports only GET/POST requests.'); + res.end(); + return; } - let b = req.body; let isBatch = true; // TODO: do something different here if the body is an array. // Throw an error if body isn't either array or object. - if (!Array.isArray(b)) { + if (!Array.isArray(buffer)) { isBatch = false; - b = [b]; + buffer = [buffer]; } let responses: Array = []; - for (let requestParams of b) { + for (let requestParams of buffer) { try { const query = requestParams.query; const operationName = requestParams.operationName; diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index e0319a0b826..9dd3128fc5d 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; import { stub } from 'sinon'; import 'mocha'; +import * as querystring from 'querystring'; import { GraphQLSchema, @@ -158,10 +159,10 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); - it('rejects the request if the method is not POST', () => { + it('rejects the request if the method is not POST or GET', () => { app = createApp({excludeParser: true}); const req = request(app) - .get('/graphql') + .head('/graphql') .send(); return req.then((res) => { expect(res.status).to.be.oneOf([404, 405]); @@ -181,6 +182,39 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('can handle a basic GET request', () => { + app = createApp(); + const expected = { + testString: 'it works', + }; + const query = { + query: 'query test{ testString }', + }; + const req = request(app) + .get(`/graphql?${querystring.stringify(query)}`); + return req.then((res) => { + expect(res.status).to.equal(200); + return expect(res.body.data).to.deep.equal(expected); + }); + }); + + it('can handle a GET request with variables', () => { + app = createApp(); + const query = { + query: 'query test($echo: String){ testArgument(echo: $echo) }', + variables: JSON.stringify({ echo: 'world' }), + }; + const expected = { + testArgument: 'hello world', + }; + const req = request(app) + .get(`/graphql?${querystring.stringify(query)}`); + return req.then((res) => { + expect(res.status).to.equal(200); + return expect(res.body.data).to.deep.equal(expected); + }); + }); + it('can handle a basic request', () => { app = createApp(); const expected = { From 7abcee839832916282756faed9ea4088167e6409 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Thu, 20 Oct 2016 21:19:52 +0300 Subject: [PATCH 02/13] doc(get-support): update README.md for GET method examples --- README.md | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b7561c617df..b8dcd01a76b 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ where variant is one of the following: - koa - hapi -### Express +### Express ( POST ) ```js import express from 'express'; @@ -49,7 +49,23 @@ app.use('/graphql', bodyParser.json(), graphqlExpress({ schema: myGraphQLSchema app.listen(PORT); ``` -### Connect +### Express ( GET ) + +```js +import express from 'express'; +import { graphqlExpress } from 'graphql-server-express'; + +const myGraphQLSchema = // ... define or import your schema here! +const PORT = 3000; + +var app = express(); + +app.use('/graphql', graphqlExpress({ schema: myGraphQLSchema })); + +app.listen(PORT); +``` + +### Connect ( POST ) ```js import connect from 'connect'; import bodyParser from 'body-parser'; @@ -66,6 +82,22 @@ app.use('/graphql', graphqlConnect({ schema: myGraphQLSchema })); http.createServer(app).listen(PORT); ``` +### Connect ( GET ) +```js +import connect from 'connect'; +import { graphqlConnect } from 'graphql-server-express'; +import http from 'http'; + +const PORT = 3000; + +var app = connect(); + +app.use('/graphql', require('connect-query')()); +app.use('/graphql', graphqlConnect({ schema: myGraphQLSchema })); + +http.createServer(app).listen(PORT); +``` + ### Hapi Now with the Hapi plugins `graphqlHapi` and `graphiqlHapi` you can pass a route object that includes options to be applied to the route. The example below enables CORS on the `/graphql` route. From aa251f0663eb7d5457204df3f5f77f08a73777dc Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Thu, 20 Oct 2016 21:46:52 +0300 Subject: [PATCH 03/13] chore(get-support): support for GET method for hapi --- README.md | 2 +- .../graphql-server-hapi/src/hapiApollo.ts | 53 ++++++++++++++----- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index b8dcd01a76b..5469119d8a1 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ app.use('/graphql', graphqlConnect({ schema: myGraphQLSchema })); http.createServer(app).listen(PORT); ``` -### Hapi +### Hapi ( POST / GET ) Now with the Hapi plugins `graphqlHapi` and `graphiqlHapi` you can pass a route object that includes options to be applied to the route. The example below enables CORS on the `/graphql` route. diff --git a/packages/graphql-server-hapi/src/hapiApollo.ts b/packages/graphql-server-hapi/src/hapiApollo.ts index d1f0482b041..7ae47d443c1 100644 --- a/packages/graphql-server-hapi/src/hapiApollo.ts +++ b/packages/graphql-server-hapi/src/hapiApollo.ts @@ -20,7 +20,8 @@ export interface HapiPluginOptions { } const graphqlHapi: IRegister = function(server: Server, options: HapiPluginOptions, next) { - server.method('verifyPayload', verifyPayload); + server.method('assignIsBatch', assignIsBatch); + server.method('assignBuffer', assignBuffer); server.method('getGraphQLParams', getGraphQLParams); server.method('getGraphQLOptions', getGraphQLOptions); server.method('processQuery', processQuery); @@ -29,12 +30,17 @@ const graphqlHapi: IRegister = function(server: Server, options: HapiPluginOptio plugins: { graphql: isOptionsFunction(options.graphqlOptions) ? options.graphqlOptions : () => options.graphqlOptions, }, - pre: [{ + pre: [ + { + assign: 'buffer', + method: 'assignBuffer(method, payload, query)', + }, + { assign: 'isBatch', - method: 'verifyPayload(payload)', + method: 'assignIsBatch(method, pre.buffer)', }, { assign: 'graphqlParams', - method: 'getGraphQLParams(payload, pre.isBatch)', + method: 'getGraphQLParams(pre.buffer, pre.isBatch)', }, { assign: 'graphqlOptions', method: 'getGraphQLOptions', @@ -45,7 +51,7 @@ const graphqlHapi: IRegister = function(server: Server, options: HapiPluginOptio }); server.route({ - method: 'POST', + method: ['GET', 'POST'], path: options.path || '/graphql', config, handler: function(request, reply) { @@ -71,23 +77,44 @@ graphqlHapi.attributes = { version: '0.0.1', }; -function verifyPayload(payload, reply) { - if (!payload) { - return reply(createErr(500, 'POST body missing.')); - } +function assignBuffer(method, payload, query, reply) { + switch ( method ) { + case 'get': + if (!query) { + return reply(createErr(500, 'GET query missing.')); + } + return reply(query); + case 'post': + if (!payload) { + return reply(createErr(500, 'POST body missing.')); + } + return reply(payload); + default: + return reply(createErr(405, 'Apollo Server supports only GET/POST requests.')); + } +} +function assignIsBatch(method, buffer, reply) { // TODO: do something different here if the body is an array. // Throw an error if body isn't either array or object. - reply(payload && Array.isArray(payload)); + + switch ( method ) { + case 'get': + return reply(false); + case 'post': + return reply(Array.isArray(buffer)); + default: + throw new Error(`Invalid case reached, method is ${method}`); + } } -function getGraphQLParams(payload, isBatch, reply) { +function getGraphQLParams(buffer, isBatch, reply) { if (!isBatch) { - payload = [payload]; + buffer = [buffer]; } const params = []; - for (let query of payload) { + for (let query of buffer) { let variables = query.variables; if (variables && typeof variables === 'string') { try { From d7b724f69dca23ac465e2bde3dde14cda6bd6cf5 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Thu, 20 Oct 2016 22:01:23 +0300 Subject: [PATCH 04/13] chore(get-support): support for GET method for koa --- README.md | 20 ++++++++++-- .../src/index.ts | 2 +- .../graphql-server-koa/src/koaApollo.test.ts | 1 + packages/graphql-server-koa/src/koaApollo.ts | 31 ++++++++++++++----- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 5469119d8a1..1efe1ad35c8 100644 --- a/README.md +++ b/README.md @@ -137,11 +137,11 @@ server.start((err) => { }); ``` -### Koa +### Koa ( POST ) ```js import koa from 'koa'; // koa@2 -import koaBody from 'koa-bodyparser'; // koa-bodyparser@next import koaRouter from 'koa-router'; // koa-router@next +import koaBody from 'koa-bodyparser'; // koa-bodyparser@next import { graphqlKoa } from 'graphql-server-koa'; const app = new koa(); @@ -156,6 +156,22 @@ app.use(router.allowedMethods()); app.listen(PORT); ``` +### Koa ( GET ) +```js +import koa from 'koa'; +import koaRouter from 'koa-router'; +import { graphqlKoa } from 'graphql-server-koa'; + +const app = new koa(); +const router = new koaRouter(); +const PORT = 3000; + +router.get('/graphql', graphqlKoa({ schema: myGraphQLSchema })); +app.use(router.routes()); +app.use(router.allowedMethods()); +app.listen(PORT); +``` + ## Options GraphQL Server can be configured with an options object with the the following fields: diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index 9dd3128fc5d..a3d7125aac8 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -165,7 +165,7 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { .head('/graphql') .send(); return req.then((res) => { - expect(res.status).to.be.oneOf([404, 405]); + expect(res.status).to.equal(405); // Hapi doesn't return allow header, so we can't test this. // return expect(res.headers['allow']).to.equal('POST'); }); diff --git a/packages/graphql-server-koa/src/koaApollo.test.ts b/packages/graphql-server-koa/src/koaApollo.test.ts index a044652ea72..e75a538363b 100644 --- a/packages/graphql-server-koa/src/koaApollo.test.ts +++ b/packages/graphql-server-koa/src/koaApollo.test.ts @@ -20,6 +20,7 @@ function createApp(options: CreateAppOptions = {}) { if (options.graphiqlOptions ) { router.get('/graphiql', graphiqlKoa( options.graphiqlOptions )); } + router.get('/graphql', graphqlKoa( options.graphqlOptions )); router.post('/graphql', graphqlKoa( options.graphqlOptions )); app.use(router.routes()); app.use(router.allowedMethods()); diff --git a/packages/graphql-server-koa/src/koaApollo.ts b/packages/graphql-server-koa/src/koaApollo.ts index f7683eb144b..2b802378b12 100644 --- a/packages/graphql-server-koa/src/koaApollo.ts +++ b/packages/graphql-server-koa/src/koaApollo.ts @@ -34,21 +34,38 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): } const formatErrorFn = optionsObject.formatError || graphql.formatError; + let buffer; - if (!ctx.request.body) { - ctx.status = 500; - return ctx.body = 'POST body missing. Did you forget "app.use(koaBody())"?'; + switch ( ctx.request.method ) { + case 'GET': + if (!ctx.request.query) { + ctx.status = 500; + return ctx.body = 'GET query missing'; + } + + buffer = ctx.request.query; + break; + case 'POST': + if (!ctx.request.body) { + ctx.status = 500; + return ctx.body = 'POST body missing. Did you forget "app.use(koaBody())"?'; + } + + buffer = ctx.request.body; + break; + default: + ctx.status = 405; + return ctx.body = 'Apollo Server supports only GET/POST requests.'; } - let b = ctx.request.body; let isBatch = true; - if (!Array.isArray(b)) { + if (!Array.isArray(buffer)) { isBatch = false; - b = [b]; + buffer = [buffer]; } let responses: Array = []; - for (let requestParams of b) { + for (let requestParams of buffer) { try { const query = requestParams.query; const operationName = requestParams.operationName; From 977f6dfc4393d6c95c01e43e4d2710d233bbe26e Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Mon, 14 Nov 2016 23:45:45 +0200 Subject: [PATCH 05/13] chore(get-support): Update README.md --- README.md | 61 ++++++++----------------------------------------------- 1 file changed, 8 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 1efe1ad35c8..ed4afcef767 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ where variant is one of the following: - koa - hapi -### Express ( POST ) +### Express ```js import express from 'express'; @@ -44,28 +44,13 @@ const PORT = 3000; var app = express(); +// bodyParser is needed just for POST. app.use('/graphql', bodyParser.json(), graphqlExpress({ schema: myGraphQLSchema })); app.listen(PORT); ``` -### Express ( GET ) - -```js -import express from 'express'; -import { graphqlExpress } from 'graphql-server-express'; - -const myGraphQLSchema = // ... define or import your schema here! -const PORT = 3000; - -var app = express(); - -app.use('/graphql', graphqlExpress({ schema: myGraphQLSchema })); - -app.listen(PORT); -``` - -### Connect ( POST ) +### Connect ```js import connect from 'connect'; import bodyParser from 'body-parser'; @@ -76,29 +61,14 @@ const PORT = 3000; var app = connect(); +// bodyParser is needed just for POST. app.use('/graphql', bodyParser.json()); app.use('/graphql', graphqlConnect({ schema: myGraphQLSchema })); http.createServer(app).listen(PORT); ``` -### Connect ( GET ) -```js -import connect from 'connect'; -import { graphqlConnect } from 'graphql-server-express'; -import http from 'http'; - -const PORT = 3000; - -var app = connect(); - -app.use('/graphql', require('connect-query')()); -app.use('/graphql', graphqlConnect({ schema: myGraphQLSchema })); - -http.createServer(app).listen(PORT); -``` - -### Hapi ( POST / GET ) +### Hapi Now with the Hapi plugins `graphqlHapi` and `graphiqlHapi` you can pass a route object that includes options to be applied to the route. The example below enables CORS on the `/graphql` route. @@ -137,7 +107,7 @@ server.start((err) => { }); ``` -### Koa ( POST ) +### Koa ```js import koa from 'koa'; // koa@2 import koaRouter from 'koa-router'; // koa-router@next @@ -148,32 +118,17 @@ const app = new koa(); const router = new koaRouter(); const PORT = 3000; +// koaBody is needed just for POST. app.use(koaBody()); router.post('/graphql', graphqlKoa({ schema: myGraphQLSchema })); -app.use(router.routes()); -app.use(router.allowedMethods()); -app.listen(PORT); -``` - -### Koa ( GET ) -```js -import koa from 'koa'; -import koaRouter from 'koa-router'; -import { graphqlKoa } from 'graphql-server-koa'; - -const app = new koa(); -const router = new koaRouter(); -const PORT = 3000; - router.get('/graphql', graphqlKoa({ schema: myGraphQLSchema })); + app.use(router.routes()); app.use(router.allowedMethods()); app.listen(PORT); ``` -## Options - GraphQL Server can be configured with an options object with the the following fields: * **schema**: the GraphQLSchema to be used From 93d7966df85a6888740daefcda0bfda578d5776d Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Mon, 14 Nov 2016 23:46:09 +0200 Subject: [PATCH 06/13] chore(get-support): rename buffer to requestPayload --- .../src/expressApollo.ts | 12 +++++----- .../graphql-server-hapi/src/hapiApollo.ts | 22 +++++++++---------- packages/graphql-server-koa/src/koaApollo.ts | 12 +++++----- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/graphql-server-express/src/expressApollo.ts b/packages/graphql-server-express/src/expressApollo.ts index 4ec36d678e1..8c03d62ee4d 100644 --- a/packages/graphql-server-express/src/expressApollo.ts +++ b/packages/graphql-server-express/src/expressApollo.ts @@ -42,7 +42,7 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu } const formatErrorFn = optionsObject.formatError || graphql.formatError; - let buffer; + let requestPayload; switch ( req.method ) { case 'POST': @@ -53,7 +53,7 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu return; } - buffer = req.body; + requestPayload = req.body; break; case 'GET': if ( !req.query ) { @@ -63,7 +63,7 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu return; } - buffer = req.query; + requestPayload = req.query; break; default: @@ -77,13 +77,13 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu let isBatch = true; // TODO: do something different here if the body is an array. // Throw an error if body isn't either array or object. - if (!Array.isArray(buffer)) { + if (!Array.isArray(requestPayload)) { isBatch = false; - buffer = [buffer]; + requestPayload = [requestPayload]; } let responses: Array = []; - for (let requestParams of buffer) { + for (let requestParams of requestPayload) { try { const query = requestParams.query; const operationName = requestParams.operationName; diff --git a/packages/graphql-server-hapi/src/hapiApollo.ts b/packages/graphql-server-hapi/src/hapiApollo.ts index 7ae47d443c1..edbaaa02772 100644 --- a/packages/graphql-server-hapi/src/hapiApollo.ts +++ b/packages/graphql-server-hapi/src/hapiApollo.ts @@ -21,7 +21,7 @@ export interface HapiPluginOptions { const graphqlHapi: IRegister = function(server: Server, options: HapiPluginOptions, next) { server.method('assignIsBatch', assignIsBatch); - server.method('assignBuffer', assignBuffer); + server.method('assignRequest', assignRequest); server.method('getGraphQLParams', getGraphQLParams); server.method('getGraphQLOptions', getGraphQLOptions); server.method('processQuery', processQuery); @@ -32,15 +32,15 @@ const graphqlHapi: IRegister = function(server: Server, options: HapiPluginOptio }, pre: [ { - assign: 'buffer', - method: 'assignBuffer(method, payload, query)', + assign: 'requestPayload', + method: 'assignRequest(method, payload, query)', }, { assign: 'isBatch', - method: 'assignIsBatch(method, pre.buffer)', + method: 'assignIsBatch(method, pre.requestPayload)', }, { assign: 'graphqlParams', - method: 'getGraphQLParams(pre.buffer, pre.isBatch)', + method: 'getGraphQLParams(pre.requestPayload, pre.isBatch)', }, { assign: 'graphqlOptions', method: 'getGraphQLOptions', @@ -77,7 +77,7 @@ graphqlHapi.attributes = { version: '0.0.1', }; -function assignBuffer(method, payload, query, reply) { +function assignRequest(method, payload, query, reply) { switch ( method ) { case 'get': if (!query) { @@ -94,7 +94,7 @@ function assignBuffer(method, payload, query, reply) { } } -function assignIsBatch(method, buffer, reply) { +function assignIsBatch(method, requestPayload, reply) { // TODO: do something different here if the body is an array. // Throw an error if body isn't either array or object. @@ -102,19 +102,19 @@ function assignIsBatch(method, buffer, reply) { case 'get': return reply(false); case 'post': - return reply(Array.isArray(buffer)); + return reply(Array.isArray(requestPayload)); default: throw new Error(`Invalid case reached, method is ${method}`); } } -function getGraphQLParams(buffer, isBatch, reply) { +function getGraphQLParams(requestPayload, isBatch, reply) { if (!isBatch) { - buffer = [buffer]; + requestPayload = [requestPayload]; } const params = []; - for (let query of buffer) { + for (let query of requestPayload) { let variables = query.variables; if (variables && typeof variables === 'string') { try { diff --git a/packages/graphql-server-koa/src/koaApollo.ts b/packages/graphql-server-koa/src/koaApollo.ts index 2b802378b12..341568361f3 100644 --- a/packages/graphql-server-koa/src/koaApollo.ts +++ b/packages/graphql-server-koa/src/koaApollo.ts @@ -34,7 +34,7 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): } const formatErrorFn = optionsObject.formatError || graphql.formatError; - let buffer; + let requestPayload; switch ( ctx.request.method ) { case 'GET': @@ -43,7 +43,7 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): return ctx.body = 'GET query missing'; } - buffer = ctx.request.query; + requestPayload = ctx.request.query; break; case 'POST': if (!ctx.request.body) { @@ -51,7 +51,7 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): return ctx.body = 'POST body missing. Did you forget "app.use(koaBody())"?'; } - buffer = ctx.request.body; + requestPayload = ctx.request.body; break; default: ctx.status = 405; @@ -59,13 +59,13 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): } let isBatch = true; - if (!Array.isArray(buffer)) { + if (!Array.isArray(requestPayload)) { isBatch = false; - buffer = [buffer]; + requestPayload = [requestPayload]; } let responses: Array = []; - for (let requestParams of buffer) { + for (let requestParams of requestPayload) { try { const query = requestParams.query; const operationName = requestParams.operationName; From 5b46db4aa00b44c53cd4e058bd35b2f8b7cd309c Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Fri, 16 Dec 2016 15:46:57 +0200 Subject: [PATCH 07/13] fix(get-support): fixed returned status for get without query --- packages/graphql-server-express/src/expressApollo.ts | 4 ++-- packages/graphql-server-hapi/src/hapiApollo.ts | 4 ++-- .../graphql-server-integration-testsuite/src/index.ts | 10 ++++++++++ packages/graphql-server-koa/src/koaApollo.ts | 6 +++--- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/graphql-server-express/src/expressApollo.ts b/packages/graphql-server-express/src/expressApollo.ts index 8c03d62ee4d..75505ae3adc 100644 --- a/packages/graphql-server-express/src/expressApollo.ts +++ b/packages/graphql-server-express/src/expressApollo.ts @@ -56,8 +56,8 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu requestPayload = req.body; break; case 'GET': - if ( !req.query ) { - res.statusCode = 500; + if ( !req.query || (Object.keys(req.query).length === 0) ) { + res.statusCode = 400; res.write('GET query missing.'); res.end(); return; diff --git a/packages/graphql-server-hapi/src/hapiApollo.ts b/packages/graphql-server-hapi/src/hapiApollo.ts index edbaaa02772..0de16e64655 100644 --- a/packages/graphql-server-hapi/src/hapiApollo.ts +++ b/packages/graphql-server-hapi/src/hapiApollo.ts @@ -80,8 +80,8 @@ graphqlHapi.attributes = { function assignRequest(method, payload, query, reply) { switch ( method ) { case 'get': - if (!query) { - return reply(createErr(500, 'GET query missing.')); + if (!query || (Object.keys(query).length === 0)) { + return reply(createErr(400, 'GET query missing.')); } return reply(query); case 'post': diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index a3d7125aac8..d9649a1c2c0 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -182,6 +182,16 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('throws an error if GET query is missing', () => { + app = createApp(); + const req = request(app) + .get(`/graphql`); + return req.then((res) => { + expect(res.status).to.equal(400); + return expect(res.error.text).to.contain('GET query missing.'); + }); + }); + it('can handle a basic GET request', () => { app = createApp(); const expected = { diff --git a/packages/graphql-server-koa/src/koaApollo.ts b/packages/graphql-server-koa/src/koaApollo.ts index 341568361f3..8246b1079ca 100644 --- a/packages/graphql-server-koa/src/koaApollo.ts +++ b/packages/graphql-server-koa/src/koaApollo.ts @@ -38,9 +38,9 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): switch ( ctx.request.method ) { case 'GET': - if (!ctx.request.query) { - ctx.status = 500; - return ctx.body = 'GET query missing'; + if (!ctx.request.query || (Object.keys(ctx.request.query).length === 0)) { + ctx.status = 400; + return ctx.body = 'GET query missing.'; } requestPayload = ctx.request.query; From 32140cb3e72228dc2fb50cb8ed282ba8e985d047 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Fri, 13 Jan 2017 13:49:25 +0200 Subject: [PATCH 08/13] chore(get-support): refactor to share http logic in core module Closes #186 --- .../graphql-server-core/src/graphqlOptions.ts | 6 +- packages/graphql-server-core/src/index.ts | 3 +- .../graphql-server-core/src/runHttpQuery.ts | 127 +++++++++++ .../src/expressApollo.ts | 141 ++---------- .../src/hapiApollo.test.ts | 8 +- .../graphql-server-hapi/src/hapiApollo.ts | 205 ++++-------------- .../src/index.ts | 3 +- packages/graphql-server-koa/src/koaApollo.ts | 129 ++--------- 8 files changed, 221 insertions(+), 401 deletions(-) create mode 100644 packages/graphql-server-core/src/runHttpQuery.ts diff --git a/packages/graphql-server-core/src/graphqlOptions.ts b/packages/graphql-server-core/src/graphqlOptions.ts index 9bc6f3d5a50..9407ce98f3d 100644 --- a/packages/graphql-server-core/src/graphqlOptions.ts +++ b/packages/graphql-server-core/src/graphqlOptions.ts @@ -15,7 +15,7 @@ import { LogFunction } from './runQuery'; * - (optional) debug: a boolean that will print additional debug logging if execution errors occur * */ -interface GraphQLServerOptions { +export interface GraphQLServerOptions { schema: GraphQLSchema; formatError?: Function; rootValue?: any; @@ -28,3 +28,7 @@ interface GraphQLServerOptions { } export default GraphQLServerOptions; + +export function isOptionsFunction(arg: GraphQLServerOptions | Function): arg is Function { + return typeof arg === 'function'; +} diff --git a/packages/graphql-server-core/src/index.ts b/packages/graphql-server-core/src/index.ts index 21389dca72d..38547bf26d3 100644 --- a/packages/graphql-server-core/src/index.ts +++ b/packages/graphql-server-core/src/index.ts @@ -1,2 +1,3 @@ export { runQuery, LogFunction, LogMessage, LogStep, LogAction } from './runQuery' -export { default as GraphQLOptions} from './graphqlOptions' +export { runHttpQuery, HttpQueryRequest, HttpQueryError } from './runHttpQuery'; +export { default as GraphQLOptions } from './graphqlOptions' diff --git a/packages/graphql-server-core/src/runHttpQuery.ts b/packages/graphql-server-core/src/runHttpQuery.ts new file mode 100644 index 00000000000..fdf2641fe50 --- /dev/null +++ b/packages/graphql-server-core/src/runHttpQuery.ts @@ -0,0 +1,127 @@ +import { formatError, ExecutionResult } from 'graphql'; +import { runQuery } from './runQuery'; +import { default as GraphQLOptions, isOptionsFunction } from './graphqlOptions'; + +export interface HttpQueryRequest { + method: string; + query: string; + options: GraphQLOptions | Function; +} + +export class HttpQueryError extends Error { + public statusCode: number; + public isGraphQLError: boolean; + public headers: { [key: string]: string }; + + constructor (statusCode: number, message: string, isGraphQLError: boolean = false, headers?: { [key: string]: string }) { + super(message); + this.statusCode = statusCode; + this.isGraphQLError = isGraphQLError; + this.headers = headers; + } +} + +export async function runHttpQuery(handlerArguments: Array, request: HttpQueryRequest): Promise { + let optionsObject: GraphQLOptions; + if (isOptionsFunction(request.options)) { + try { + optionsObject = await request.options(...handlerArguments); + } catch (e) { + throw new HttpQueryError(500, `Invalid options provided to ApolloServer: ${e.message}`); + } + } else { + optionsObject = request.options; + } + + const formatErrorFn = optionsObject.formatError || formatError; + let requestPayload; + + switch ( request.method ) { + case 'POST': + if ( !request.query ) { + throw new HttpQueryError(500, 'POST body missing. Did you forget use body-parser middleware?'); + } + + requestPayload = request.query; + break; + case 'GET': + if ( !request.query || (Object.keys(request.query).length === 0) ) { + throw new HttpQueryError(400, 'GET query missing.'); + } + + requestPayload = request.query; + break; + + default: + throw new HttpQueryError(405, 'Apollo Server supports only GET/POST requests.', false, { + 'Allow': 'GET, POST', + }); + } + + let isBatch = true; + // TODO: do something different here if the body is an array. + // Throw an error if body isn't either array or object. + if (!Array.isArray(requestPayload)) { + isBatch = false; + requestPayload = [requestPayload]; + } + + let responses: Array = []; + for (let requestParams of requestPayload) { + try { + const query = requestParams.query; + const operationName = requestParams.operationName; + let variables = requestParams.variables; + + if (typeof variables === 'string') { + try { + variables = JSON.parse(variables); + } catch (error) { + throw new HttpQueryError(400, 'Variables are invalid JSON.'); + } + } + + // Shallow clone context for queries in batches. This allows + // users to distinguish multiple queries in the batch and to + // modify the context object without interfering with each other. + let context = optionsObject.context; + if (isBatch) { + context = Object.assign({}, context || {}); + } + + let params = { + schema: optionsObject.schema, + query: query, + variables: variables, + context: context, + rootValue: optionsObject.rootValue, + operationName: operationName, + logFunction: optionsObject.logFunction, + validationRules: optionsObject.validationRules, + formatError: formatErrorFn, + formatResponse: optionsObject.formatResponse, + debug: optionsObject.debug, + }; + + if (optionsObject.formatParams) { + params = optionsObject.formatParams(params); + } + + responses.push(await runQuery(params)); + } catch (e) { + responses.push({ errors: [formatErrorFn(e)] }); + } + } + + if (!isBatch) { + const gqlResponse = responses[0]; + if (gqlResponse.errors && typeof gqlResponse.data === 'undefined') { + throw new HttpQueryError(400, JSON.stringify(gqlResponse), true, { + 'Content-Type': 'application/json', + }); + } + return JSON.stringify(gqlResponse); + } + + return JSON.stringify(responses); +} diff --git a/packages/graphql-server-express/src/expressApollo.ts b/packages/graphql-server-express/src/expressApollo.ts index 75505ae3adc..7be34226e93 100644 --- a/packages/graphql-server-express/src/expressApollo.ts +++ b/packages/graphql-server-express/src/expressApollo.ts @@ -1,7 +1,6 @@ import * as express from 'express'; -import * as graphql from 'graphql'; import * as url from 'url'; -import { GraphQLOptions, runQuery } from 'graphql-server-core'; +import { GraphQLOptions, HttpQueryError, runHttpQuery } from 'graphql-server-core'; import * as GraphiQL from 'graphql-server-module-graphiql'; export interface ExpressGraphQLOptionsFunction { @@ -27,131 +26,33 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu throw new Error(`Apollo Server expects exactly one argument, got ${arguments.length}`); } - return async (req: express.Request, res: express.Response, next) => { - let optionsObject: GraphQLOptions; - if (isOptionsFunction(options)) { - try { - optionsObject = await options(req, res); - } catch (e) { - res.statusCode = 500; - res.write(`Invalid options provided to ApolloServer: ${e.message}`); - res.end(); + return (req: express.Request, res: express.Response): void => { + runHttpQuery([req, res], { + method: req.method, + options: options, + query: req.method === 'POST' ? req.body : req.query, + }).then((gqlResponse) => { + res.setHeader('Content-Type', 'application/json'); + res.write(gqlResponse); + res.end(); + }, (error: HttpQueryError) => { + if ( undefined === error.statusCode ) { + throw error; } - } else { - optionsObject = options; - } - - const formatErrorFn = optionsObject.formatError || graphql.formatError; - let requestPayload; - - switch ( req.method ) { - case 'POST': - if ( !req.body ) { - res.statusCode = 500; - res.write('POST body missing. Did you forget "app.use(bodyParser.json())"?'); - res.end(); - return; - } - - requestPayload = req.body; - break; - case 'GET': - if ( !req.query || (Object.keys(req.query).length === 0) ) { - res.statusCode = 400; - res.write('GET query missing.'); - res.end(); - return; - } - - requestPayload = req.query; - break; - - default: - res.setHeader('Allow', 'GET, POST'); - res.statusCode = 405; - res.write('Apollo Server supports only GET/POST requests.'); - res.end(); - return; - } - - let isBatch = true; - // TODO: do something different here if the body is an array. - // Throw an error if body isn't either array or object. - if (!Array.isArray(requestPayload)) { - isBatch = false; - requestPayload = [requestPayload]; - } - - let responses: Array = []; - for (let requestParams of requestPayload) { - try { - const query = requestParams.query; - const operationName = requestParams.operationName; - let variables = requestParams.variables; - - if (typeof variables === 'string') { - try { - variables = JSON.parse(variables); - } catch (error) { - res.statusCode = 400; - res.write('Variables are invalid JSON.'); - res.end(); - return; - } - } - // Shallow clone context for queries in batches. This allows - // users to distinguish multiple queries in the batch and to - // modify the context object without interfering with each other. - let context = optionsObject.context; - if (isBatch) { - context = Object.assign({}, context || {}); - } - - let params = { - schema: optionsObject.schema, - query: query, - variables: variables, - context: context, - rootValue: optionsObject.rootValue, - operationName: operationName, - logFunction: optionsObject.logFunction, - validationRules: optionsObject.validationRules, - formatError: formatErrorFn, - formatResponse: optionsObject.formatResponse, - debug: optionsObject.debug, - }; - - if (optionsObject.formatParams) { - params = optionsObject.formatParams(params); - } - - responses.push(await runQuery(params)); - } catch (e) { - responses.push({ errors: [formatErrorFn(e)] }); + if ( error.headers ) { + Object.keys(error.headers).forEach((header) => { + res.setHeader(header, error.headers[header]); + }); } - } - res.setHeader('Content-Type', 'application/json'); - if (isBatch) { - res.write(JSON.stringify(responses)); - res.end(); - } else { - const gqlResponse = responses[0]; - if (gqlResponse.errors && typeof gqlResponse.data === 'undefined') { - res.statusCode = 400; - } - res.write(JSON.stringify(gqlResponse)); + res.statusCode = error.statusCode; + res.write(error.message); res.end(); - } - + }); }; } -function isOptionsFunction(arg: GraphQLOptions | ExpressGraphQLOptionsFunction): arg is ExpressGraphQLOptionsFunction { - return typeof arg === 'function'; -} - /* This middleware returns the html for the GraphiQL interactive query UI * * GraphiQLData arguments @@ -164,7 +65,7 @@ function isOptionsFunction(arg: GraphQLOptions | ExpressGraphQLOptionsFunction): */ export function graphiqlExpress(options: GraphiQL.GraphiQLData) { - return (req: express.Request, res: express.Response, next) => { + return (req: express.Request, res: express.Response) => { const q = req.url && url.parse(req.url, true).query || {}; const query = q.query || ''; const variables = q.variables || '{}'; diff --git a/packages/graphql-server-hapi/src/hapiApollo.test.ts b/packages/graphql-server-hapi/src/hapiApollo.test.ts index 3f14fb3d3a2..7540b68833a 100644 --- a/packages/graphql-server-hapi/src/hapiApollo.test.ts +++ b/packages/graphql-server-hapi/src/hapiApollo.test.ts @@ -1,10 +1,10 @@ import * as hapi from 'hapi'; -import { graphqlHapi, graphiqlHapi, HapiPluginOptions } from './hapiApollo'; +import { graphqlHapi, graphiqlHapi } from './hapiApollo'; import 'mocha'; -import testSuite, { Schema } from 'graphql-server-integration-testsuite'; +import testSuite, { Schema, CreateAppOptions } from 'graphql-server-integration-testsuite'; -function createApp(createOptions: HapiPluginOptions) { +function createApp(options: CreateAppOptions) { const server = new hapi.Server(); server.connection({ @@ -15,7 +15,7 @@ function createApp(createOptions: HapiPluginOptions) { server.register({ register: graphqlHapi, options: { - graphqlOptions: createOptions ? createOptions.graphqlOptions : { schema: Schema }, + graphqlOptions: (options && options.graphqlOptions) || { schema: Schema }, path: '/graphql', }, }); diff --git a/packages/graphql-server-hapi/src/hapiApollo.ts b/packages/graphql-server-hapi/src/hapiApollo.ts index 0de16e64655..1e0171001b6 100644 --- a/packages/graphql-server-hapi/src/hapiApollo.ts +++ b/packages/graphql-server-hapi/src/hapiApollo.ts @@ -1,8 +1,7 @@ import * as Boom from 'boom'; -import { Server, Request, IReply } from 'hapi'; -import { ExecutionResult, formatError } from 'graphql'; +import { Server, Response, Request, IReply } from 'hapi'; import * as GraphiQL from 'graphql-server-module-graphiql'; -import { GraphQLOptions, runQuery } from 'graphql-server-core'; +import { GraphQLOptions, runHttpQuery, HttpQueryError } from 'graphql-server-core'; export interface IRegister { (server: Server, options: any, next: any): void; @@ -19,54 +18,48 @@ export interface HapiPluginOptions { graphqlOptions: GraphQLOptions | HapiOptionsFunction; } -const graphqlHapi: IRegister = function(server: Server, options: HapiPluginOptions, next) { - server.method('assignIsBatch', assignIsBatch); - server.method('assignRequest', assignRequest); - server.method('getGraphQLParams', getGraphQLParams); - server.method('getGraphQLOptions', getGraphQLOptions); - server.method('processQuery', processQuery); +function runHttpQueryWrapper(options: GraphQLOptions | HapiOptionsFunction, request: Request, reply: IReply): Promise { + return runHttpQuery([request], { + method: request.method.toUpperCase(), + options: options, + query: request.method === 'post' ? request.payload : request.query, + }).then((gqlResponse) => { + return reply(gqlResponse).type('application/json'); + }, (error: HttpQueryError) => { + if ( undefined === error.statusCode ) { + throw error; + } - const config = Object.assign(options.route || {}, { - plugins: { - graphql: isOptionsFunction(options.graphqlOptions) ? options.graphqlOptions : () => options.graphqlOptions, - }, - pre: [ - { - assign: 'requestPayload', - method: 'assignRequest(method, payload, query)', - }, - { - assign: 'isBatch', - method: 'assignIsBatch(method, pre.requestPayload)', - }, { - assign: 'graphqlParams', - method: 'getGraphQLParams(pre.requestPayload, pre.isBatch)', - }, { - assign: 'graphqlOptions', - method: 'getGraphQLOptions', - }, { - assign: 'graphQL', - method: 'processQuery(pre.graphqlParams, pre.graphqlOptions, pre.isBatch)', - }], + if ( true === error.isGraphQLError ) { + return reply(error.message).code(error.statusCode).type('application/json'); + } + + const err = Boom.create(error.statusCode); + err.output.payload.message = error.message; + if ( error.headers ) { + Object.keys(error.headers).forEach((header) => { + err.output.headers[header] = error.headers[header]; + }); + } + + return reply(err); }); +} + +const graphqlHapi: IRegister = function(server: Server, options: HapiPluginOptions, next) { + if (!options || !options.graphqlOptions) { + throw new Error('Apollo Server requires options.'); + } + + if (arguments.length !== 3) { + throw new Error(`Apollo Server expects exactly 3 argument, got ${arguments.length}`); + } server.route({ method: ['GET', 'POST'], path: options.path || '/graphql', - config, - handler: function(request, reply) { - const responses = request.pre['graphQL']; - if (request.pre['isBatch']) { - return reply(responses); - } else { - const gqlResponse = responses[0]; - if (gqlResponse.errors && typeof gqlResponse.data === 'undefined') { - return reply(gqlResponse).code(400); - } else { - return reply(gqlResponse); - } - } - }, + config: options.route || {}, + handler: (request, reply) => runHttpQueryWrapper(options.graphqlOptions, request, reply), }); return next(); @@ -77,128 +70,6 @@ graphqlHapi.attributes = { version: '0.0.1', }; -function assignRequest(method, payload, query, reply) { - switch ( method ) { - case 'get': - if (!query || (Object.keys(query).length === 0)) { - return reply(createErr(400, 'GET query missing.')); - } - return reply(query); - case 'post': - if (!payload) { - return reply(createErr(500, 'POST body missing.')); - } - return reply(payload); - default: - return reply(createErr(405, 'Apollo Server supports only GET/POST requests.')); - } -} - -function assignIsBatch(method, requestPayload, reply) { - // TODO: do something different here if the body is an array. - // Throw an error if body isn't either array or object. - - switch ( method ) { - case 'get': - return reply(false); - case 'post': - return reply(Array.isArray(requestPayload)); - default: - throw new Error(`Invalid case reached, method is ${method}`); - } -} - -function getGraphQLParams(requestPayload, isBatch, reply) { - if (!isBatch) { - requestPayload = [requestPayload]; - } - - const params = []; - for (let query of requestPayload) { - let variables = query.variables; - if (variables && typeof variables === 'string') { - try { - variables = JSON.parse(variables); - } catch (error) { - return reply(createErr(400, 'Variables are invalid JSON.')); - } - } - - params.push({ - query: query.query, - variables: variables, - operationName: query.operationName, - }); - } - reply(params); -}; - -async function getGraphQLOptions(request: Request, reply: IReply): Promise<{}> { - const options = request.route.settings.plugins['graphql']; - let optionsObject: GraphQLOptions; - if (isOptionsFunction(options)) { - try { - const opsFunc: HapiOptionsFunction = options; - optionsObject = await opsFunc(request); - } catch (e) { - return reply(createErr(500, `Invalid options provided to ApolloServer: ${e.message}`)); - } - } else { - optionsObject = options; - } - reply(optionsObject); -} - -async function processQuery(graphqlParams, optionsObject: GraphQLOptions, isBatch: boolean, reply) { - const formatErrorFn = optionsObject.formatError || formatError; - - let responses: ExecutionResult[] = []; - for (let query of graphqlParams) { - try { - // Shallow clone context for queries in batches. This allows - // users to distinguish multiple queries in the batch and to - // modify the context object without interfering with each other. - let context = optionsObject.context; - if (isBatch) { - context = Object.assign({}, context || {}); - } - - let params = { - schema: optionsObject.schema, - query: query.query, - variables: query.variables, - rootValue: optionsObject.rootValue, - context: context, - operationName: query.operationName, - logFunction: optionsObject.logFunction, - validationRules: optionsObject.validationRules, - formatError: formatErrorFn, - formatResponse: optionsObject.formatResponse, - debug: optionsObject.debug, - }; - - if (optionsObject.formatParams) { - params = optionsObject.formatParams(params); - } - - responses.push(await runQuery(params)); - } catch (e) { - responses.push({ errors: [formatErrorFn(e)] }); - } - } - return reply(responses); -} - -function isOptionsFunction(arg: GraphQLOptions | HapiOptionsFunction): arg is HapiOptionsFunction { - return typeof arg === 'function'; -} - -function createErr(code: number, message: string) { - const err = Boom.create(code); - err.output.payload.message = message; - return err; -} - export interface GraphiQLPluginOptions { path: string; route?: any; diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index d9649a1c2c0..cf7f95df957 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -166,8 +166,7 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { .send(); return req.then((res) => { expect(res.status).to.equal(405); - // Hapi doesn't return allow header, so we can't test this. - // return expect(res.headers['allow']).to.equal('POST'); + expect(res.headers['allow']).to.equal('GET, POST'); }); }); diff --git a/packages/graphql-server-koa/src/koaApollo.ts b/packages/graphql-server-koa/src/koaApollo.ts index 8246b1079ca..11d8b2e0823 100644 --- a/packages/graphql-server-koa/src/koaApollo.ts +++ b/packages/graphql-server-koa/src/koaApollo.ts @@ -1,6 +1,5 @@ import * as koa from 'koa'; -import * as graphql from 'graphql'; -import { GraphQLOptions, runQuery } from 'graphql-server-core'; +import { GraphQLOptions, HttpQueryError, runHttpQuery } from 'graphql-server-core'; import * as GraphiQL from 'graphql-server-module-graphiql'; export interface KoaGraphQLOptionsFunction { @@ -20,118 +19,36 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): throw new Error(`Apollo Server expects exactly one argument, got ${arguments.length}`); } - return async (ctx, next) => { - let optionsObject: GraphQLOptions; - if (isOptionsFunction(options)) { - try { - optionsObject = await options(ctx); - } catch (e) { - ctx.status = 500; - return ctx.body = `Invalid options provided to ApolloServer: ${e.message}`; + return (ctx: koa.Context): Promise => { + return runHttpQuery([ctx], { + method: ctx.request.method, + options: options, + query: ctx.request.method === 'POST' ? ctx.request.body : ctx.request.query, + }).then((gqlResponse) => { + ctx.set('Content-Type', 'application/json'); + ctx.body = gqlResponse; + }, (error: HttpQueryError) => { + if ( undefined === error.statusCode ) { + throw error; } - } else { - optionsObject = options; - } - const formatErrorFn = optionsObject.formatError || graphql.formatError; - let requestPayload; - - switch ( ctx.request.method ) { - case 'GET': - if (!ctx.request.query || (Object.keys(ctx.request.query).length === 0)) { - ctx.status = 400; - return ctx.body = 'GET query missing.'; - } - - requestPayload = ctx.request.query; - break; - case 'POST': - if (!ctx.request.body) { - ctx.status = 500; - return ctx.body = 'POST body missing. Did you forget "app.use(koaBody())"?'; - } - - requestPayload = ctx.request.body; - break; - default: - ctx.status = 405; - return ctx.body = 'Apollo Server supports only GET/POST requests.'; - } - - let isBatch = true; - if (!Array.isArray(requestPayload)) { - isBatch = false; - requestPayload = [requestPayload]; - } - - let responses: Array = []; - for (let requestParams of requestPayload) { - try { - const query = requestParams.query; - const operationName = requestParams.operationName; - let variables = requestParams.variables; - - if (typeof variables === 'string') { - try { - variables = JSON.parse(variables); - } catch (error) { - ctx.status = 400; - return ctx.body = 'Variables are invalid JSON.'; - } - } - - // Shallow clone context for queries in batches. This allows - // users to distinguish multiple queries in the batch and to - // modify the context object without interfering with each other. - let context = optionsObject.context; - if (isBatch) { - context = Object.assign({}, context || {}); - } - - let params = { - schema: optionsObject.schema, - query: query, - variables: variables, - context: context, - rootValue: optionsObject.rootValue, - operationName: operationName, - logFunction: optionsObject.logFunction, - validationRules: optionsObject.validationRules, - formatError: formatErrorFn, - formatResponse: optionsObject.formatResponse, - debug: optionsObject.debug, - }; - - if (optionsObject.formatParams) { - params = optionsObject.formatParams(params); - } - - responses.push(await runQuery(params)); - } catch (e) { - responses.push({ errors: [formatErrorFn(e)] }); - } - } - - ctx.set('Content-Type', 'application/json'); - if (isBatch) { - return ctx.body = JSON.stringify(responses); - } else { - const gqlResponse = responses[0]; - if (gqlResponse.errors && typeof gqlResponse.data === 'undefined') { - ctx.status = 400; + if ( error.headers ) { + Object.keys(error.headers).forEach((header) => { + ctx.set(header, error.headers[header]); + }); } - return ctx.body = JSON.stringify(gqlResponse); - } + ctx.status = error.statusCode; + ctx.body = error.message; + }).then(undefined, (error) => { + console.error(error); + throw error; + }); }; } -function isOptionsFunction(arg: GraphQLOptions | KoaGraphQLOptionsFunction): arg is KoaGraphQLOptionsFunction { - return typeof arg === 'function'; -} - export function graphiqlKoa(options: GraphiQL.GraphiQLData) { - return (ctx, next) => { + return (ctx: koa.Context) => { const q = ctx.request.query || {}; const query = q.query || ''; From aa3a96aaa9eddae2f8bf7f15e26b37538d4ec036 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Fri, 13 Jan 2017 16:25:37 +0200 Subject: [PATCH 09/13] chore(get-support): blocked GET for queries only --- packages/graphql-server-core/src/runHttpQuery.ts | 9 +++++++++ .../src/index.ts | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/packages/graphql-server-core/src/runHttpQuery.ts b/packages/graphql-server-core/src/runHttpQuery.ts index fdf2641fe50..205f2c87fe6 100644 --- a/packages/graphql-server-core/src/runHttpQuery.ts +++ b/packages/graphql-server-core/src/runHttpQuery.ts @@ -22,6 +22,7 @@ export class HttpQueryError extends Error { } export async function runHttpQuery(handlerArguments: Array, request: HttpQueryRequest): Promise { + let isGetRequest: boolean = false; let optionsObject: GraphQLOptions; if (isOptionsFunction(request.options)) { try { @@ -49,6 +50,7 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu throw new HttpQueryError(400, 'GET query missing.'); } + isGetRequest = true; requestPayload = request.query; break; @@ -68,6 +70,13 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu let responses: Array = []; for (let requestParams of requestPayload) { + if ( isGetRequest && !requestParams.query.trim().startsWith('query')) { + const errorMsg = `GET supports only query operation`; + throw new HttpQueryError(405, errorMsg, false, { + 'Allow': 'POST', + }); + } + try { const query = requestParams.query; const operationName = requestParams.operationName; diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index cf7f95df957..9c4e0202e9e 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -207,6 +207,20 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('throws error if trying to use mutation with GET request', () => { + app = createApp(); + const query = { + query: 'mutation test{ testMutation(echo: "ping") }', + }; + const req = request(app) + .get(`/graphql?${querystring.stringify(query)}`); + return req.then((res) => { + expect(res.status).to.equal(405); + expect(res.headers['allow']).to.equal('POST'); + return expect(res.error.text).to.contain('GET supports only query operation'); + }); + }); + it('can handle a GET request with variables', () => { app = createApp(); const query = { From 0ad6d424139e2960b5405187e3bbf6f1245ff458 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Fri, 13 Jan 2017 16:35:10 +0200 Subject: [PATCH 10/13] chore(package): Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d38dc279d45..7fa97b868c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ### VNEXT +* add support for HTTP GET Method ([@DxCx](https://github.com/DxCx)) on [#180](https://github.com/apollostack/graphql-server/pull/180) ### v0.5.0 * Switch graphql typings for typescript to @types/graphql [#260](https://github.com/apollostack/graphql-server/pull/260) From 11bd61065c26eca0979119f3a2516b29d00235db Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Fri, 13 Jan 2017 16:42:49 +0200 Subject: [PATCH 11/13] fix(get-support): unnamed operation was not treated as query --- packages/graphql-server-core/src/runHttpQuery.ts | 6 +++++- .../src/index.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/graphql-server-core/src/runHttpQuery.ts b/packages/graphql-server-core/src/runHttpQuery.ts index 205f2c87fe6..1a24e20c30a 100644 --- a/packages/graphql-server-core/src/runHttpQuery.ts +++ b/packages/graphql-server-core/src/runHttpQuery.ts @@ -21,6 +21,10 @@ export class HttpQueryError extends Error { } } +function isQueryOperation(query: string) { + return query.trim().startsWith('{') || query.trim().startsWith('query'); +} + export async function runHttpQuery(handlerArguments: Array, request: HttpQueryRequest): Promise { let isGetRequest: boolean = false; let optionsObject: GraphQLOptions; @@ -70,7 +74,7 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu let responses: Array = []; for (let requestParams of requestPayload) { - if ( isGetRequest && !requestParams.query.trim().startsWith('query')) { + if ( isGetRequest && !isQueryOperation(requestParams.query)) { const errorMsg = `GET supports only query operation`; throw new HttpQueryError(405, errorMsg, false, { 'Allow': 'POST', diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index 9c4e0202e9e..1eb09908237 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -207,6 +207,22 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('can handle a basic implicit GET request', () => { + app = createApp(); + const expected = { + testString: 'it works', + }; + const query = { + query: '{ testString }', + }; + const req = request(app) + .get(`/graphql?${querystring.stringify(query)}`); + return req.then((res) => { + expect(res.status).to.equal(200); + return expect(res.body.data).to.deep.equal(expected); + }); + }); + it('throws error if trying to use mutation with GET request', () => { app = createApp(); const query = { From be573a6ade2aad452945165a9a5ad2f8bc02ae92 Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Fri, 13 Jan 2017 16:50:23 +0200 Subject: [PATCH 12/13] chore(get-support): cleanup --- packages/graphql-server-core/src/runHttpQuery.ts | 3 +-- packages/graphql-server-koa/src/koaApollo.ts | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/graphql-server-core/src/runHttpQuery.ts b/packages/graphql-server-core/src/runHttpQuery.ts index 1a24e20c30a..25a30658e06 100644 --- a/packages/graphql-server-core/src/runHttpQuery.ts +++ b/packages/graphql-server-core/src/runHttpQuery.ts @@ -75,8 +75,7 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu let responses: Array = []; for (let requestParams of requestPayload) { if ( isGetRequest && !isQueryOperation(requestParams.query)) { - const errorMsg = `GET supports only query operation`; - throw new HttpQueryError(405, errorMsg, false, { + throw new HttpQueryError(405, `GET supports only query operation`, false, { 'Allow': 'POST', }); } diff --git a/packages/graphql-server-koa/src/koaApollo.ts b/packages/graphql-server-koa/src/koaApollo.ts index 11d8b2e0823..af8ba9cf44e 100644 --- a/packages/graphql-server-koa/src/koaApollo.ts +++ b/packages/graphql-server-koa/src/koaApollo.ts @@ -40,9 +40,6 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): ctx.status = error.statusCode; ctx.body = error.message; - }).then(undefined, (error) => { - console.error(error); - throw error; }); }; } From 01e3010323750d136524d397d83db19bc036542a Mon Sep 17 00:00:00 2001 From: Hagai Cohen Date: Wed, 18 Jan 2017 10:49:13 +0200 Subject: [PATCH 13/13] fix(get-support): preparse query for GET requests to handle query properly --- .../graphql-server-core/src/runHttpQuery.ts | 35 +++++++++---- .../src/expressApollo.ts | 2 +- .../graphql-server-hapi/src/hapiApollo.ts | 2 +- .../src/index.ts | 51 ++++++++++++++++++- packages/graphql-server-koa/src/koaApollo.ts | 2 +- 5 files changed, 78 insertions(+), 14 deletions(-) diff --git a/packages/graphql-server-core/src/runHttpQuery.ts b/packages/graphql-server-core/src/runHttpQuery.ts index 25a30658e06..aaf6fe79520 100644 --- a/packages/graphql-server-core/src/runHttpQuery.ts +++ b/packages/graphql-server-core/src/runHttpQuery.ts @@ -1,4 +1,4 @@ -import { formatError, ExecutionResult } from 'graphql'; +import { parse, getOperationAST, DocumentNode, formatError, ExecutionResult } from 'graphql'; import { runQuery } from './runQuery'; import { default as GraphQLOptions, isOptionsFunction } from './graphqlOptions'; @@ -15,14 +15,16 @@ export class HttpQueryError extends Error { constructor (statusCode: number, message: string, isGraphQLError: boolean = false, headers?: { [key: string]: string }) { super(message); + this.name = 'HttpQueryError'; this.statusCode = statusCode; this.isGraphQLError = isGraphQLError; this.headers = headers; } } -function isQueryOperation(query: string) { - return query.trim().startsWith('{') || query.trim().startsWith('query'); +function isQueryOperation(query: DocumentNode, operationName: string) { + const operationAST = getOperationAST(query, operationName); + return operationAST.operation === 'query'; } export async function runHttpQuery(handlerArguments: Array, request: HttpQueryRequest): Promise { @@ -74,14 +76,21 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu let responses: Array = []; for (let requestParams of requestPayload) { - if ( isGetRequest && !isQueryOperation(requestParams.query)) { - throw new HttpQueryError(405, `GET supports only query operation`, false, { - 'Allow': 'POST', - }); - } - try { - const query = requestParams.query; + let query = requestParams.query; + if ( isGetRequest ) { + if (typeof query === 'string') { + // preparse the query incase of GET so we can assert the operation. + query = parse(query); + } + + if ( ! isQueryOperation(query, requestParams.operationName) ) { + throw new HttpQueryError(405, `GET supports only query operation`, false, { + 'Allow': 'POST', + }); + } + } + const operationName = requestParams.operationName; let variables = requestParams.variables; @@ -121,6 +130,12 @@ export async function runHttpQuery(handlerArguments: Array, request: HttpQu responses.push(await runQuery(params)); } catch (e) { + // Populate any HttpQueryError to our handler which should + // convert it to Http Error. + if ( e.name === 'HttpQueryError' ) { + throw e; + } + responses.push({ errors: [formatErrorFn(e)] }); } } diff --git a/packages/graphql-server-express/src/expressApollo.ts b/packages/graphql-server-express/src/expressApollo.ts index 7be34226e93..048f6df6128 100644 --- a/packages/graphql-server-express/src/expressApollo.ts +++ b/packages/graphql-server-express/src/expressApollo.ts @@ -36,7 +36,7 @@ export function graphqlExpress(options: GraphQLOptions | ExpressGraphQLOptionsFu res.write(gqlResponse); res.end(); }, (error: HttpQueryError) => { - if ( undefined === error.statusCode ) { + if ( 'HttpQueryError' !== error.name ) { throw error; } diff --git a/packages/graphql-server-hapi/src/hapiApollo.ts b/packages/graphql-server-hapi/src/hapiApollo.ts index 1e0171001b6..0973707077e 100644 --- a/packages/graphql-server-hapi/src/hapiApollo.ts +++ b/packages/graphql-server-hapi/src/hapiApollo.ts @@ -26,7 +26,7 @@ function runHttpQueryWrapper(options: GraphQLOptions | HapiOptionsFunction, requ }).then((gqlResponse) => { return reply(gqlResponse).type('application/json'); }, (error: HttpQueryError) => { - if ( undefined === error.statusCode ) { + if ( 'HttpQueryError' !== error.name ) { throw error; } diff --git a/packages/graphql-server-integration-testsuite/src/index.ts b/packages/graphql-server-integration-testsuite/src/index.ts index 1eb09908237..0ab809014b7 100644 --- a/packages/graphql-server-integration-testsuite/src/index.ts +++ b/packages/graphql-server-integration-testsuite/src/index.ts @@ -8,6 +8,7 @@ import { GraphQLObjectType, GraphQLString, GraphQLError, + GraphQLNonNull, introspectionQuery, BREAK, } from 'graphql'; @@ -60,6 +61,18 @@ const QueryType = new GraphQLObjectType({ }, }); +const PersonType = new GraphQLObjectType({ + name: 'PersonType', + fields: { + firstName: { + type: GraphQLString, + }, + lastName: { + type: GraphQLString, + }, + }, +}); + const MutationType = new GraphQLObjectType({ name: 'MutationType', fields: { @@ -70,6 +83,20 @@ const MutationType = new GraphQLObjectType({ return `not really a mutation, but who cares: ${echo}`; }, }, + testPerson: { + type: PersonType, + args: { + firstName: { + type: new GraphQLNonNull(GraphQLString), + }, + lastName: { + type: new GraphQLNonNull(GraphQLString), + }, + }, + resolve(root, args) { + return args; + }, + }, }, }); @@ -223,7 +250,7 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); - it('throws error if trying to use mutation with GET request', () => { + it('throws error if trying to use mutation using GET request', () => { app = createApp(); const query = { query: 'mutation test{ testMutation(echo: "ping") }', @@ -237,6 +264,28 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('throws error if trying to use mutation with fragment using GET request', () => { + app = createApp(); + const query = { + query: `fragment PersonDetails on PersonType { + firstName + } + + mutation test { + testPerson(firstName: "Test", lastName: "Me") { + ...PersonDetails + } + }`, + }; + const req = request(app) + .get(`/graphql?${querystring.stringify(query)}`); + return req.then((res) => { + expect(res.status).to.equal(405); + expect(res.headers['allow']).to.equal('POST'); + return expect(res.error.text).to.contain('GET supports only query operation'); + }); + }); + it('can handle a GET request with variables', () => { app = createApp(); const query = { diff --git a/packages/graphql-server-koa/src/koaApollo.ts b/packages/graphql-server-koa/src/koaApollo.ts index af8ba9cf44e..759c6492f85 100644 --- a/packages/graphql-server-koa/src/koaApollo.ts +++ b/packages/graphql-server-koa/src/koaApollo.ts @@ -28,7 +28,7 @@ export function graphqlKoa(options: GraphQLOptions | KoaGraphQLOptionsFunction): ctx.set('Content-Type', 'application/json'); ctx.body = gqlResponse; }, (error: HttpQueryError) => { - if ( undefined === error.statusCode ) { + if ( 'HttpQueryError' !== error.name ) { throw error; }