fix(postgraphql): fix HTTP server

graphile · Oct 9, 2016 · 62c4212 · 62c4212
1 parent 1615982
commit 62c4212
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 24 deletions.
diff --git a/src/postgraphql/http/__tests__/createPostGraphQLHTTPRequestHandler-test.js b/src/postgraphql/http/__tests__/createPostGraphQLHTTPRequestHandler-test.js
@@ -1,5 +1,6 @@
 import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql'
-import createPostGraphQLHTTPRequestHandler from '../createPostGraphQLHTTPRequestHandler'
+import { $$pgClient } from '../../../postgres/inventory/pgClientFromContext'
+import createPostGraphQLHTTPRequestHandler, { $$pgClientOrigQuery } from '../createPostGraphQLHTTPRequestHandler'
 
 const http = require('http')
 const request = require('supertest-as-promised')
@@ -21,7 +22,12 @@ const graphqlSchema = new GraphQLSchema({
           name: { type: GraphQLString },
         },
         resolve: (source, { name }) => `Hello, ${name}!`,
-      }
+      },
+      query: {
+        type: GraphQLString,
+        resolve: (source, args, context) =>
+          context[$$pgClient].query()
+      },
     },
   }),
   mutation: new GraphQLObjectType({
@@ -36,7 +42,7 @@ const graphqlSchema = new GraphQLSchema({
 })
 
 const pgClient = {
-  query: jest.fn(),
+  [$$pgClientOrigQuery]: jest.fn(() => Promise.resolve()),
   release: jest.fn(),
 }
 
@@ -225,7 +231,7 @@ for (const [name, createServerFromHandler] of serverCreators) {
 
     test('will connect and release a Postgres client from the pool on every request', async () => {
       pgPool.connect.mockClear()
-      pgClient.query.mockClear()
+      pgClient[$$pgClientOrigQuery].mockClear()
       pgClient.release.mockClear()
       const server = createServer()
       await (
@@ -237,7 +243,25 @@ for (const [name, createServerFromHandler] of serverCreators) {
         .expect({ data: { hello: 'world' } })
       )
       expect(pgPool.connect.mock.calls).toEqual([[]])
-      expect(pgClient.query.mock.calls).toEqual([['begin'], ['commit']])
+      expect(pgClient[$$pgClientOrigQuery].mock.calls).toEqual([])
+      expect(pgClient.release.mock.calls).toEqual([[]])
+    })
+
+    test('will call `begin` and `commit` for requests that use the Postgres client', async () => {
+      pgPool.connect.mockClear()
+      pgClient[$$pgClientOrigQuery].mockClear()
+      pgClient.release.mockClear()
+      const server = createServer()
+      await (
+        request(server)
+        .post('/graphql')
+        .send({ query: '{query}' })
+        .expect(200)
+        .expect('Content-Type', /json/)
+        .expect({ data: { query: null } })
+      )
+      expect(pgPool.connect.mock.calls).toEqual([[]])
+      expect(pgClient[$$pgClientOrigQuery].mock.calls).toEqual([['begin'], [], ['commit']])
       expect(pgClient.release.mock.calls).toEqual([[]])
     })
 

diff --git a/src/postgraphql/http/createPostGraphQLHTTPRequestHandler.js b/src/postgraphql/http/createPostGraphQLHTTPRequestHandler.js
@@ -19,7 +19,7 @@ const parseUrl = require('parseurl')
 const finalHandler = require('finalhandler')
 const bodyParser = require('body-parser')
 
-const $$pgClientOrigQuery = Symbol()
+export const $$pgClientOrigQuery = Symbol()
 
 const debugGraphql = new Debugger('postgraphql:graphql')
 const debugPG = new Debugger('postgraphql:postgres')
@@ -263,26 +263,24 @@ export default function createPostGraphQLHTTPRequestHandler (options) {
       const pgClient = await pgPool.connect()
       let pgClientInTransaction = false
 
-      // If the user has enabled the debugging of Postgres queries, enhance the
-      // query function to log SQL queries.
-      if (debugPG.enabled) {
-        // Set the original query method to a key on our client. If that key is
-        // already set, use that.
-        pgClient[$$pgClientOrigQuery] = pgClient[$$pgClientOrigQuery] || pgClient.query
-
-        pgClient.query = function (...args) {
-          // If the client is not currently in a transaction, run the `begin`
-          // command and then run the actual query.
-          if (!pgClientInTransaction) {
-            pgClientInTransaction = true
-            return pgClient.query('begin').then(() => pgClient.query(...args))
-          }
+      // Set the original query method to a key on our client. If that key is
+      // already set, use that.
+      pgClient[$$pgClientOrigQuery] = pgClient[$$pgClientOrigQuery] || pgClient.query
 
-          // Debug just the query text.
-          debugPG(args[0].text || args[0])
-          // Call the original query method.
-          return pgClient[$$pgClientOrigQuery].apply(this, args)
+      pgClient.query = function (...args) {
+        // If the client is not currently in a transaction, run the `begin`
+        // command and then run the actual query.
+        if (!pgClientInTransaction) {
+          pgClientInTransaction = true
+          return pgClient.query('begin').then(() => pgClient.query(...args))
         }
+
+        // Debug just the query text. We don’t want to debug variables because
+        // there may be passwords in there.
+        debugPG(args[0] && args[0].text ? args[0].text : args[0])
+
+        // Call the original query method.
+        return pgClient[$$pgClientOrigQuery].apply(this, args)
       }
 
       try {