Merge pull request #2 from node-modli/fix/connections

Fix/connections

Author: psvet

binci.yml

@@ -26,6 +26,9 @@ tasks:
   test: |
     node ./scripts/postgres_conn.js
     yarn run test
+  test:watch: |
+    node ./scripts/postgres_conn.js
+    yarn run test:watch
   cover: |
     node ./scripts/postgres_conn.js
     yarn run cover

package.json

@@ -18,23 +18,45 @@
   "author": "TechnologyAdvice <devteam@technologyadvice.com>",
   "scripts": {
     "test": "yarn run lint && yarn run cover",
+    "test:watch": "nodemon --exec \"./node_modules/.bin/mocha ./test/src || exit 1\"",
     "clean": "rm -rf ./node_modules rm -rf ./build",
     "mocha": "mocha ./test/src --recursive",
     "cover": "istanbul cover ./node_modules/.bin/_mocha ./test/src",
     "lint": "standard --fix --verbose"
   },
   "license": "MIT",
   "devDependencies": {
-    "assert": "^1.3.0",
     "chai": "^4.1.2",
-    "express": "^4.13.3",
+    "chai-as-promised": "^7.1.1",
+    "chai-subset": "^1.6.0",
+    "dirty-chai": "^2.0.1",
+    "fixd": "^1.0.1",
     "istanbul": "^0.4.5",
     "mocha": "^3.5.3",
+    "nodemon": "^1.12.1",
+    "require-dir": "^0.3.2",
     "should": "^13.0.1",
+    "sinon": "^3.3.0",
+    "sinon-chai": "^2.13.0",
     "standard": "^10.0.3"
   },
   "dependencies": {
     "bluebird": "^3.5.0",
     "pg": "^7.3.0"
+  },
+  "standard": {
+    "ignore": [
+      "coverage"
+    ],
+    "env": [
+      "node",
+      "mocha"
+    ],
+    "globals": [
+      "sinon",
+      "fixd",
+      "expect",
+      "sandbox"
+    ]
   }
 }

src/index.js

@@ -1,4 +1,4 @@
-const pg = require('pg')
+const { Pool } = require('pg')
 
 /**
  * @class postgres
@@ -13,7 +13,7 @@ module.exports = class {
    * @param {String} config.database The connection database
    */
   constructor (config) {
-    this.pg = new pg.Client(config)
+    this.pg = new Pool(config)
   }
 
   /**
@@ -22,18 +22,19 @@ module.exports = class {
    * @returns {Object} promise
    */
   query (query) {
-    let res
-    // TODO: overlapping connections
+    let client
     return this.pg.connect()
-      .then(() => this.pg.query(query))
-      .then(data => {
-        res = data
-        return this.pg.end()
+      .then((cli) => {
+        client = cli
+        return client.query(query)
+      })
+      .then(res => {
+        client.release()
+        return res
       })
-      .then(() => res)
       .catch((err) => {
-        return this.pg.end()
-          .then(() => { throw err })
+        client.release()
+        throw err
       })
   }
 
@@ -67,14 +68,22 @@ module.exports = class {
   create (body, version = false) {
     return this.validate(body)
       .then(data => {
+        const res = Object.assign({}, data)
         const vals = []
         const cols = Object.keys(data).reduce((acc, key) => {
+          if (typeof data[key] === 'object' && data[key] !== null) {
+            data[key] = JSON.stringify(data[key])
+          }
           vals.push(`'${data[key]}'`)
           acc.push(key)
           return acc
         }, [])
         const query = `INSERT INTO ${this.tableName} (${cols.join(',')}) VALUES (${vals.join(',')})`
         return this.query(query)
+          .then(data => {
+            if (data.rowCount) return res
+            throw new Error('Unable to create record')
+          })
       })
   }
 
@@ -103,17 +112,26 @@ module.exports = class {
   update (query, body, version = false) {
     return this.validate(body)
       .then(data => {
+        const res = Object.assign({}, data)
         let i = 1
         let changes = ''
-        let len = Object.keys(body).length
-        for (let prop in body) {
-          if ({}.hasOwnProperty.call(body, prop)) {
+        let len = Object.keys(data).length
+        for (let key in data) {
+          /* istanbul ignore else: should not happen */
+          if ({}.hasOwnProperty.call(data, key)) {
+            if (typeof data[key] === 'object' && data[key] !== null) {
+              data[key] = JSON.stringify(data[key])
+            }
             let comma = (i !== len) ? ', ' : ''
-            changes += `${prop}='${body[prop]}'${comma}`
+            changes += `${key}='${data[key]}'${comma}`
             i++
           }
         }
         return this.query(`UPDATE ${this.tableName} SET ${changes} WHERE ${query}`)
+          .then(data => {
+            if (data.rowCount) return res
+            throw new Error('Unable to update record(s)')
+          })
       })
   }
 

test/fixtures/postgres.fixture.js

@@ -0,0 +1,24 @@
+fixd.postgres = {
+  config: {
+    host: process.env.MODLI_POSTGRES_HOST,
+    user: process.env.POSTGRES_USER,
+    password: process.env.POSTGRES_PASSWORD,
+    database: process.env.POSTGRES_DB
+  },
+  testData: {
+    fname: 'John',
+    lname: 'Smith',
+    email: 'jsmith@gmail.com',
+    address: {
+      street: '123 Fake St',
+      city: 'Nashville'
+    }
+  },
+  createTable: {
+    id: [ 'serial', 'NOT NULL', 'PRIMARY KEY' ],
+    fname: [ 'varchar(255)' ],
+    lname: [ 'varchar(255)' ],
+    email: [ 'varchar(255)' ],
+    address: [ 'jsonb' ]
+  }
+}

test/setup.js

@@ -1,4 +1,37 @@
 const chai = require('chai')
-global.assert = require('assert')
-global.should = require('should')
+const dirtyChai = require('dirty-chai')
+const chaiAsPromised = require('chai-as-promised')
+const chaiSubset = require('chai-subset')
+const mod = require('module')
+const path = require('path')
+const sinon = require('sinon')
+const sinonChai = require('sinon-chai')
+const fixd = require('fixd')
+
+// Chai
+// ----------------------------------------
 global.expect = chai.expect
+chai.use(chaiAsPromised)
+chai.use(dirtyChai)
+chai.use(chaiSubset)
+chai.use(sinonChai)
+
+// Sinon
+// ----------------------------------------
+global.sinon = sinon
+global.sandbox = sinon.sandbox.create()
+
+if (global.afterEach) {
+  afterEach(() => global.sandbox.restore())
+}
+
+// Fixd
+// ----------------------------------------
+global.fixd = fixd
+require('require-dir')(path.resolve(__dirname, 'fixtures'))
+
+// Node
+// ----------------------------------------
+// allow require() from project root
+process.env.NODE_PATH = path.join(process.cwd(), process.env.NODE_PATH || '')
+mod._initPaths()

test/src/index.spec.js

@@ -1,110 +1,102 @@
 /* eslint no-unused-expressions: 0 */
-/* global expect, describe, it, beforeEach, afterEach */
 require('../setup')
-const PostgresAdapter = require('../../src/index')
-
-const config = {
-  host: process.env.MODLI_POSTGRES_HOST,
-  user: process.env.POSTGRES_USER,
-  password: process.env.POSTGRES_PASSWORD,
-  database: process.env.POSTGRES_DB
-}
-
-// Test record
-const testData = {
-  fname: 'John',
-  lname: 'Smith',
-  email: 'jsmith@gmail.com'
-}
+const PostgresAdapter = require('src')
 
 describe('postgres', () => {
   let inst
-  beforeEach(() => {
-    inst = new PostgresAdapter(config)
+  before(() => {
+    inst = new PostgresAdapter(fixd.postgres.config)
     inst.tableName = 'foo'
     // Mock validation method, this is automatically done by the model
     inst.validate = (body) => Promise.resolve(body)
     // Mock sanitize method, this is automatically done by the model
     inst.sanitize = (body) => body
   })
-  afterEach(() => {
-    inst.query(`DROP TABLE ${inst.tableName}`)
-  })
+  after(() => inst.pg.end())
   describe('query', () => {
-    it('fails when invalid query is run', () => {
-      return inst.query('`')
-        .catch((err) => {
-          expect(err).to.be.an.instanceof(Error)
-        })
-    })
     it('runs a query against the database when connection is good', () => {
       return inst.query('SELECT 1 + 1 AS number')
-        .then((result) => {
-          expect(result.rows[0].number).to.equal(2)
+        .then((res) => {
+          expect(res.rows[0].number).to.equal(2)
         })
     })
+    it('fails when invalid query is run', () => {
+      return expect(inst.query('`')).to.be.rejectedWith(Error)
+    })
   })
   describe('createTable', () => {
     it('creates a new table based on object passed (if not exists)', () => {
-      return inst.createTable({
-        'id': [ 'serial', 'NOT NULL', 'PRIMARY KEY' ],
-        'fname': [ 'varchar(255)' ],
-        'lname': [ 'varchar(255)' ],
-        'email': [ 'varchar(255)' ]
-      })
-      .then((result) => {
-        expect(result.command).to.equal('CREATE')
-      })
+      return inst.createTable(fixd.postgres.createTable)
+        .then((res) => {
+          expect(res.command).to.equal('CREATE')
+        })
     })
   })
   describe('create', () => {
+    let data
+    beforeEach(() => {
+      data = Object.assign({}, fixd.postgres.testData)
+    })
     it('creates a new record based on object passed', () => {
-      return inst.create(testData)
-        .then((result) => {
-          expect(result.command).to.equal('INSERT')
-          expect(result.rowCount).to.equal(1)
+      return inst.create(data)
+        .then((res) => {
+          expect(res).to.containSubset(fixd.postgres.testData)
         })
     })
+    it('rejects if record is not created', () => {
+      sandbox.stub(inst, 'query').resolves({ rowCount: 0 })
+      return expect(inst.create(data))
+        .to.be.rejectedWith('Unable to create record')
+    })
   })
   describe('read', () => {
     it('reads all when no query specified', () => {
       return inst.read()
-        .then((result) => {
-          expect(result.length).to.equal(1)
-          expect(result[0].email).to.equal(testData.email)
+        .then((res) => {
+          expect(res.length).to.equal(1)
+          expect(res[0].email).to.equal(fixd.postgres.testData.email)
         })
     })
     it('reads specific records when query supplied', () => {
       return inst.read('fname=\'John\'', 1)
-        .then((result) => {
-          expect(result.length).to.equal(1)
-          expect(result[0].email).to.equal(testData.email)
+        .then((res) => {
+          expect(res.length).to.equal(1)
+          expect(res[0].email).to.equal(fixd.postgres.testData.email)
         })
     })
     it('fails when a bad query is provided', () => {
-      return inst.read('`fart=`knocker')
-        .catch((err) => {
-          expect(err).to.be.an.instanceof(Error)
-        })
+      return expect(inst.read('`')).to.be.rejectedWith(Error)
     })
   })
   describe('update', () => {
     it('updates record(s) based on query and body', () => {
-      return inst.update('fname=\'John\'', {
+      const query = 'fname=\'John\''
+      const body = {
         fname: 'Bob',
         email: 'bsmith@gmail.com'
-      }, 1)
-      .then((result) => {
-        expect(result.command).to.equal('UPDATE')
-        expect(result.rowCount).to.equal(1)
-      })
+      }
+      return inst.update(query, body)
+        .then((res) => {
+          expect(res.fname).to.equal('Bob')
+        })
+    })
+    it('rejects if records are not updated', () => {
+      sandbox.stub(inst, 'query').resolves({ rowCount: 0 })
+      const query = 'fname=\'John\''
+      const body = {
+        fname: 'Bob',
+        email: 'bsmith@gmail.com',
+        address: { city: 'Fakeville' }
+      }
+      return expect(inst.update(query, body))
+        .to.be.rejectedWith('Unable to update record(s)')
     })
   })
   describe('delete', () => {
     it('deletes record(s) based on query', () => {
       return inst.delete('fname=\'Bob\'')
-        .then((result) => {
-          expect(result.rowCount).to.equal(1)
+        .then((res) => {
+          expect(res.rowCount).to.equal(1)
         })
     })
   })