From 42eab951af554f0a347e05b3d7eb8969635d73f3 Mon Sep 17 00:00:00 2001 From: dirkmc Date: Fri, 16 Aug 2019 05:23:47 -0400 Subject: [PATCH] refactor: convert from callbacks to async (#13) BREAKING CHANGE: All places in the API that used callbacks are now replaced with async/await --- .travis.yml | 2 +- package.json | 17 +++---- src/selection.js | 6 +-- src/validator.js | 12 ++--- src/validators/public-key.js | 32 +++++-------- test/validator.spec.js | 88 ++++++++++++++++-------------------- 6 files changed, 68 insertions(+), 89 deletions(-) diff --git a/.travis.yml b/.travis.yml index 727be57..680a5c4 100644 --- a/.travis.yml +++ b/.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 diff --git a/package.json b/package.json index 4fdea6b..887ea4f 100644 --- a/package.json +++ b/package.json @@ -27,9 +27,8 @@ "node": ">=6.0.0", "npm": ">=3.0.0" }, - "pre-commit": [ - "lint", - "test" + "pre-push": [ + "lint" ], "author": "Friedel Ziegelmayer ", "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": [ diff --git a/src/selection.js b/src/selection.js index b92171e..b03b0af 100644 --- a/src/selection.js +++ b/src/selection.js @@ -15,7 +15,7 @@ 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('/')) @@ -23,7 +23,7 @@ const bestRecord = (selectors, k, records) => { 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()] @@ -31,7 +31,7 @@ const bestRecord = (selectors, k, records) => { 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) diff --git a/src/validator.js b/src/validator.js index 2529418..95ade5e 100644 --- a/src/validator.js +++ b/src/validator.js @@ -6,19 +6,19 @@ 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()] @@ -26,10 +26,10 @@ const verifyRecord = (validators, record, callback) => { 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 = { diff --git a/src/validators/public-key.js b/src/validators/public-key.js index 5de337d..95e2d41 100644 --- a/src/validators/public-key.js +++ b/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/'` * @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 = { diff --git a/test/validator.spec.js b/test/validator.spec.js index ea16ee3..a043941 100644 --- a/test/validator.spec.js +++ b/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) }) }) })