refactor: convert from callbacks to async (#13)

BREAKING CHANGE: All places in the API that used callbacks are now replaced with async/await

Author: dirkmc

.travis.yml

@@ -8,6 +8,7 @@ stages:
 
 node_js:
   - '10'
+  - '12'
 
 os:
   - linux
@@ -21,7 +22,6 @@ jobs:
   include:
     - stage: check
       script:
-        - npx aegir commitlint --travis
         - npx aegir dep-check
         - npm run lint
 

package.json

@@ -27,9 +27,8 @@
     "node": ">=6.0.0",
     "npm": ">=3.0.0"
   },
-  "pre-commit": [
-    "lint",
-    "test"
+  "pre-push": [
+    "lint"
   ],
   "author": "Friedel Ziegelmayer <dignifiedquire@gmail.com>",
   "license": "MIT",
@@ -38,19 +37,17 @@
   },
   "homepage": "https://github.com/libp2p/js-libp2p-record",
   "devDependencies": {
-    "aegir": "^18.2.2",
+    "aegir": "^20.0.0",
     "chai": "^4.2.0",
     "dirty-chai": "^2.0.1",
-    "libp2p-crypto": "~0.16.1",
-    "peer-id": "~0.12.2",
-    "pre-commit": "^1.2.2"
+    "libp2p-crypto": "~0.17.0",
+    "peer-id": "~0.13.2"
   },
   "dependencies": {
-    "async": "^2.6.2",
     "buffer-split": "^1.0.0",
     "err-code": "^1.1.2",
-    "multihashes": "~0.4.14",
-    "multihashing-async": "~0.6.0",
+    "multihashes": "~0.4.15",
+    "multihashing-async": "~0.7.0",
     "protons": "^1.0.1"
   },
   "contributors": [

src/selection.js

@@ -15,23 +15,23 @@ const bestRecord = (selectors, k, records) => {
   if (records.length === 0) {
     const errMsg = `No records given`
 
-    throw errcode(new Error(errMsg), 'ERR_NO_RECORDS_RECEIVED')
+    throw errcode(errMsg, 'ERR_NO_RECORDS_RECEIVED')
   }
 
   const parts = bsplit(k, Buffer.from('/'))
 
   if (parts.length < 3) {
     const errMsg = `Record key does not have a selector function`
 
-    throw errcode(new Error(errMsg), 'ERR_NO_SELECTOR_FUNCTION_FOR_RECORD_KEY')
+    throw errcode(errMsg, 'ERR_NO_SELECTOR_FUNCTION_FOR_RECORD_KEY')
   }
 
   const selector = selectors[parts[1].toString()]
 
   if (!selector) {
     const errMsg = `Unrecognized key prefix: ${parts[1]}`
 
-    throw errcode(new Error(errMsg), 'ERR_UNRECOGNIZED_KEY_PREFIX')
+    throw errcode(errMsg, 'ERR_UNRECOGNIZED_KEY_PREFIX')
   }
 
   return selector(k, records)

src/validator.js

@@ -6,30 +6,30 @@ const errcode = require('err-code')
 /**
  * Checks a record and ensures it is still valid.
  * It runs the needed validators.
+ * If verification fails the returned Promise will reject with the error.
  *
  * @param {Object} validators
  * @param {Record} record
- * @param {function(Error)} callback
- * @returns {undefined}
+ * @returns {Promise}
  */
-const verifyRecord = (validators, record, callback) => {
+const verifyRecord = (validators, record) => {
   const key = record.key
   const parts = bsplit(key, Buffer.from('/'))
 
   if (parts.length < 3) {
     // No validator available
-    return callback()
+    return
   }
 
   const validator = validators[parts[1].toString()]
 
   if (!validator) {
     const errMsg = `Invalid record keytype`
 
-    return callback(errcode(new Error(errMsg), 'ERR_INVALID_RECORD_KEY_TYPE'))
+    throw errcode(errMsg, 'ERR_INVALID_RECORD_KEY_TYPE')
   }
 
-  validator.func(key, record.value, callback)
+  return validator.func(key, record.value)
 }
 
 module.exports = {

src/validators/public-key.js

@@ -1,48 +1,40 @@
 'use strict'
 
-const setImmediate = require('async/setImmediate')
 const multihashing = require('multihashing-async')
+const errcode = require('err-code')
 
 /**
  * Validator for publick key records.
  * Verifies that the passed in record value is the PublicKey
  * that matches the passed in key.
+ * If validation fails the returned Promise will reject with the error.
  *
  * @param {Buffer} key - A valid key is of the form `'/pk/<keymultihash>'`
  * @param {Buffer} publicKey - The public key to validate against (protobuf encoded).
- * @param {function(Error)} callback
- * @returns {undefined}
+ * @returns {Promise}
  */
-const validatePublicKeyRecord = (key, publicKey, callback) => {
-  const done = (err) => setImmediate(() => callback(err))
-
+const validatePublicKeyRecord = async (key, publicKey) => {
   if (!Buffer.isBuffer(key)) {
-    return done(new Error('"key" must be a Buffer'))
+    throw errcode('"key" must be a Buffer', 'ERR_INVALID_RECORD_KEY_NOT_BUFFER')
   }
 
-  if (key.length < 3) {
-    return done(new Error('invalid public key record'))
+  if (key.length < 5) {
+    throw errcode('invalid public key record', 'ERR_INVALID_RECORD_KEY_TOO_SHORT')
   }
 
   const prefix = key.slice(0, 4).toString()
 
   if (prefix !== '/pk/') {
-    return done(new Error('key was not prefixed with /pk/'))
+    throw errcode('key was not prefixed with /pk/', 'ERR_INVALID_RECORD_KEY_BAD_PREFIX')
   }
 
   const keyhash = key.slice(4)
 
-  multihashing(publicKey, 'sha2-256', (err, publicKeyHash) => {
-    if (err) {
-      return done(err)
-    }
-
-    if (!keyhash.equals(publicKeyHash)) {
-      return done(new Error('public key does not match passed in key'))
-    }
+  const publicKeyHash = await multihashing(publicKey, 'sha2-256')
 
-    done()
-  })
+  if (!keyhash.equals(publicKeyHash)) {
+    throw errcode('public key does not match passed in key', 'ERR_INVALID_RECORD_HASH_MISMATCH')
+  }
 }
 
 module.exports = {

test/validator.spec.js

@@ -5,8 +5,6 @@
 const chai = require('chai')
 chai.use(require('dirty-chai'))
 const expect = chai.expect
-const waterfall = require('async/waterfall')
-const each = require('async/each')
 const crypto = require('libp2p-crypto')
 const PeerId = require('peer-id')
 
@@ -29,14 +27,16 @@ const generateCases = (hash) => {
     invalid: {
       publicKey: [
         // missing hashkey
-        Buffer.from('/pk/'),
+        [Buffer.from('/pk/'), 'ERR_INVALID_RECORD_KEY_TOO_SHORT'],
         // not the hash of a key
-        Buffer.concat([
+        [Buffer.concat([
           Buffer.from('/pk/'),
           Buffer.from('random')
-        ]),
+        ]), 'ERR_INVALID_RECORD_HASH_MISMATCH'],
         // missing prefix
-        hash
+        [hash, 'ERR_INVALID_RECORD_KEY_BAD_PREFIX'],
+        // not a buffer
+        ['not a buffer', 'ERR_INVALID_RECORD_KEY_NOT_BUFFER']
       ]
     }
   }
@@ -47,57 +47,47 @@ describe('validator', () => {
   let hash
   let cases
 
-  before((done) => {
-    waterfall([
-      (cb) => crypto.keys.generateKeyPair('rsa', 1024, cb),
-      (pair, cb) => {
-        key = pair
-        pair.public.hash(cb)
-      },
-      (_hash, cb) => {
-        hash = _hash
-        cases = generateCases(hash)
-        cb()
-      }
-    ], done)
+  before(async () => {
+    key = await crypto.keys.generateKeyPair('rsa', 1024)
+    hash = await key.public.hash()
+    cases = generateCases(hash)
   })
 
   describe('verifyRecord', () => {
-    it('calls matching validator', (done) => {
+    it('calls matching validator', () => {
       const k = Buffer.from('/hello/you')
       const rec = new Record(k, Buffer.from('world'), new PeerId(hash))
 
       const validators = {
         hello: {
-          func (key, value, cb) {
+          func (key, value) {
             expect(key).to.eql(k)
             expect(value).to.eql(Buffer.from('world'))
-            cb()
           },
           sign: false
         }
       }
-      validator.verifyRecord(validators, rec, done)
+      return validator.verifyRecord(validators, rec)
     })
 
-    it('calls not matching any validator', (done) => {
+    it('calls not matching any validator', () => {
       const k = Buffer.from('/hallo/you')
       const rec = new Record(k, Buffer.from('world'), new PeerId(hash))
 
       const validators = {
         hello: {
-          func (key, value, cb) {
+          func (key, value) {
             expect(key).to.eql(k)
             expect(value).to.eql(Buffer.from('world'))
-            cb()
           },
           sign: false
         }
       }
-      validator.verifyRecord(validators, rec, (err) => {
-        expect(err).to.exist()
-        done()
-      })
+      return expect(
+        () => validator.verifyRecord(validators, rec)
+      ).to.throw(
+        /Invalid record keytype/
+      )
     })
   })
 
@@ -107,40 +97,40 @@ describe('validator', () => {
     })
 
     describe('public key', () => {
-      it('exports func and sing', () => {
+      it('exports func and sign', () => {
         const pk = validator.validators.pk
 
         expect(pk).to.have.property('func')
         expect(pk).to.have.property('sign', false)
       })
 
-      it('does not error on valid record', (done) => {
-        each(cases.valid.publicKey, (k, cb) => {
-          validator.validators.pk.func(k, key.public.bytes, cb)
-        }, done)
+      it('does not error on valid record', () => {
+        return Promise.all(cases.valid.publicKey.map((k) => {
+          return validator.validators.pk.func(k, key.public.bytes)
+        }))
       })
 
-      it('throws on invalid records', (done) => {
-        each(cases.invalid.publicKey, (k, cb) => {
-          validator.validators.pk.func(k, key.public.bytes, (err) => {
-            expect(err).to.exist()
-            cb()
-          })
-        }, done)
+      it('throws on invalid records', () => {
+        return Promise.all(cases.invalid.publicKey.map(async ([k, errCode]) => {
+          try {
+            await validator.validators.pk.func(k, key.public.bytes)
+          } catch (err) {
+            expect(err.code).to.eql(errCode)
+            return
+          }
+          expect.fail('did not throw an error with code ' + errCode)
+        }))
       })
     })
   })
 
   describe('go interop', () => {
-    it('record with key from from go', (done) => {
+    it('record with key from from go', async () => {
       const pubKey = crypto.keys.unmarshalPublicKey(fixture.publicKey)
 
-      pubKey.hash((err, hash) => {
-        expect(err).to.not.exist()
-        const k = Buffer.concat([Buffer.from('/pk/'), hash])
-
-        validator.validators.pk.func(k, pubKey.bytes, done)
-      })
+      const hash = await pubKey.hash()
+      const k = Buffer.concat([Buffer.from('/pk/'), hash])
+      return validator.validators.pk.func(k, pubKey.bytes)
     })
   })
 })