From 7cfb25c8e467522f68d612f97e4a20a2d3b95dec Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 19 Feb 2019 19:45:20 +0000 Subject: [PATCH] fix: code review --- src/dht.js | 14 +++---- src/error-messages.js | 3 -- src/errors.js | 11 +++++ src/pubsub.js | 26 ++++-------- test/pubsub.node.js | 98 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 31 deletions(-) delete mode 100644 src/error-messages.js create mode 100644 src/errors.js diff --git a/src/dht.js b/src/dht.js index 58ec33a4e4..f727c27108 100644 --- a/src/dht.js +++ b/src/dht.js @@ -3,13 +3,13 @@ const nextTick = require('async/nextTick') const errCode = require('err-code') +const { messages, codes } = require('./errors') + module.exports = (node) => { return { put: (key, value, callback) => { if (!node._dht) { - return nextTick(() => callback( - errCode(new Error('DHT is not available'), 'ERR_DHT_DISABLED') - )) + return nextTick(callback, errCode(new Error(messages.DHT_DISABLED), codes.DHT_DISABLED)) } node._dht.put(key, value, callback) @@ -21,9 +21,7 @@ module.exports = (node) => { } if (!node._dht) { - return nextTick(() => callback( - errCode(new Error('DHT is not available'), 'ERR_DHT_DISABLED') - )) + return nextTick(callback, errCode(new Error(messages.DHT_DISABLED), codes.DHT_DISABLED)) } node._dht.get(key, options, callback) @@ -35,9 +33,7 @@ module.exports = (node) => { } if (!node._dht) { - return nextTick(() => callback( - errCode(new Error('DHT is not available'), 'ERR_DHT_DISABLED') - )) + return nextTick(callback, errCode(new Error(messages.DHT_DISABLED), codes.DHT_DISABLED)) } node._dht.getMany(key, nVals, options, callback) diff --git a/src/error-messages.js b/src/error-messages.js deleted file mode 100644 index e318333024..0000000000 --- a/src/error-messages.js +++ /dev/null @@ -1,3 +0,0 @@ -'use strict' - -exports.NOT_STARTED_YET = 'The libp2p node is not started yet' diff --git a/src/errors.js b/src/errors.js new file mode 100644 index 0000000000..64054bbd14 --- /dev/null +++ b/src/errors.js @@ -0,0 +1,11 @@ +'use strict' + +exports.messages = { + NOT_STARTED_YET: 'The libp2p node is not started yet', + DHT_DISABLED: 'DHT is not available' +} + +exports.codes = { + DHT_DISABLED: 'ERR_DHT_DISABLED', + PUBSUB_NOT_STARTED: 'ERR_PUBSUB_NOT_STARTED' +} diff --git a/src/pubsub.js b/src/pubsub.js index 6a543aab4c..6064aa1c65 100644 --- a/src/pubsub.js +++ b/src/pubsub.js @@ -1,7 +1,7 @@ 'use strict' const nextTick = require('async/nextTick') -const NOT_STARTED_YET = require('./error-messages').NOT_STARTED_YET +const { messages, codes } = require('./errors') const FloodSub = require('libp2p-floodsub') const errCode = require('err-code') @@ -20,9 +20,7 @@ module.exports = (node) => { } if (!node.isStarted() && !floodSub.started) { - return nextTick(() => callback( - errCode(new Error(NOT_STARTED_YET), 'ERR_PUBSUB_NOT_STARTED') - )) + return nextTick(callback, errCode(new Error(messages.NOT_STARTED_YET), codes.PUBSUB_NOT_STARTED)) } function subscribe (cb) { @@ -39,9 +37,7 @@ module.exports = (node) => { unsubscribe: (topic, handler, callback) => { if (!node.isStarted() && !floodSub.started) { - return nextTick(() => callback( - errCode(new Error(NOT_STARTED_YET), 'ERR_PUBSUB_NOT_STARTED') - )) + return nextTick(callback, errCode(new Error(messages.NOT_STARTED_YET), codes.PUBSUB_NOT_STARTED)) } floodSub.removeListener(topic, handler) @@ -57,15 +53,11 @@ module.exports = (node) => { publish: (topic, data, callback) => { if (!node.isStarted() && !floodSub.started) { - return nextTick(() => callback( - errCode(new Error(NOT_STARTED_YET), 'ERR_PUBSUB_NOT_STARTED') - )) + return nextTick(callback, errCode(new Error(messages.NOT_STARTED_YET), codes.PUBSUB_NOT_STARTED)) } if (!Buffer.isBuffer(data)) { - return nextTick(() => callback( - errCode(new Error('data must be a Buffer'), 'ERR_DATA_IS_NOT_A_BUFFER') - )) + return nextTick(callback, errCode(new Error('data must be a Buffer'), 'ERR_DATA_IS_NOT_A_BUFFER')) } floodSub.publish(topic, data) @@ -75,9 +67,7 @@ module.exports = (node) => { ls: (callback) => { if (!node.isStarted() && !floodSub.started) { - return nextTick(() => callback( - errCode(new Error(NOT_STARTED_YET), 'ERR_PUBSUB_NOT_STARTED') - )) + return nextTick(callback, errCode(new Error(messages.NOT_STARTED_YET), codes.PUBSUB_NOT_STARTED)) } const subscriptions = Array.from(floodSub.subscriptions) @@ -87,9 +77,7 @@ module.exports = (node) => { peers: (topic, callback) => { if (!node.isStarted() && !floodSub.started) { - return nextTick(() => callback( - errCode(new Error(NOT_STARTED_YET), 'ERR_PUBSUB_NOT_STARTED') - )) + return nextTick(callback, errCode(new Error(messages.NOT_STARTED_YET), codes.PUBSUB_NOT_STARTED)) } if (typeof topic === 'function') { diff --git a/test/pubsub.node.js b/test/pubsub.node.js index 5cc24db825..f980958ce7 100644 --- a/test/pubsub.node.js +++ b/test/pubsub.node.js @@ -11,6 +11,7 @@ const parallel = require('async/parallel') const series = require('async/series') const _times = require('lodash.times') +const { codes } = require('../src/errors') const createNode = require('./utils/create-node') function startTwo (callback) { @@ -87,6 +88,34 @@ describe('.pubsub', () => { expect(err).to.not.exist().mark() }) }) + + it('publish should fail if data is not a buffer', (done) => { + createNode('/ip4/0.0.0.0/tcp/0', { + config: { + peerDiscovery: { + mdns: { + enabled: false + } + }, + EXPERIMENTAL: { + pubsub: true + } + } + }, (err, node) => { + expect(err).to.not.exist() + + node.start((err) => { + expect(err).to.not.exist() + + node.pubsub.publish('pubsub', 'datastr', (err) => { + expect(err).to.exist() + expect(err.code).to.equal('ERR_DATA_IS_NOT_A_BUFFER') + + done() + }) + }) + }) + }) }) describe('.pubsub off', () => { @@ -109,4 +138,73 @@ describe('.pubsub', () => { }) }) }) + + describe('.pubsub on and node not started', () => { + let libp2pNode + + before(function (done) { + createNode('/ip4/0.0.0.0/tcp/0', { + config: { + peerDiscovery: { + mdns: { + enabled: false + } + }, + EXPERIMENTAL: { + pubsub: true + } + } + }, (err, node) => { + expect(err).to.not.exist() + + libp2pNode = node + done() + }) + }) + + it('fail to subscribe if node not started yet', (done) => { + libp2pNode.pubsub.subscribe('pubsub', () => {}, (err) => { + expect(err).to.exist() + expect(err.code).to.equal(codes.PUBSUB_NOT_STARTED) + + done() + }) + }) + + it('fail to unsubscribe if node not started yet', (done) => { + libp2pNode.pubsub.unsubscribe('pubsub', () => { }, (err) => { + expect(err).to.exist() + expect(err.code).to.equal(codes.PUBSUB_NOT_STARTED) + + done() + }) + }) + + it('fail to publish if node not started yet', (done) => { + libp2pNode.pubsub.publish('pubsub', Buffer.from('data'), (err) => { + expect(err).to.exist() + expect(err.code).to.equal(codes.PUBSUB_NOT_STARTED) + + done() + }) + }) + + it('fail to ls if node not started yet', (done) => { + libp2pNode.pubsub.ls((err) => { + expect(err).to.exist() + expect(err.code).to.equal(codes.PUBSUB_NOT_STARTED) + + done() + }) + }) + + it('fail to get subscribed peers to a topic if node not started yet', (done) => { + libp2pNode.pubsub.peers('pubsub', (err) => { + expect(err).to.exist() + expect(err.code).to.equal(codes.PUBSUB_NOT_STARTED) + + done() + }) + }) + }) })