From 33124ba077bd3bd2c1dfccaec52233a1aa914864 Mon Sep 17 00:00:00 2001 From: Richard Schneider Date: Sat, 3 Feb 2018 09:37:12 +1300 Subject: [PATCH 1/6] feat: no need to poll --- README.md | 6 +- src/index.js | 9 +-- src/query.js | 20 +++--- test/multicast-dns.spec.js | 122 +++++++++++++++++++++---------------- 4 files changed, 82 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index 5c271d9..9bd8901 100644 --- a/README.md +++ b/README.md @@ -22,12 +22,12 @@ const mdns = new MDNS(peerInfo, options) mdns.on('peer', (peerInfo) => { console.log('Found a peer in the local network', peerInfo.id.toB58String()) }) +process.on('exit', mdns.stop) -// Broadcast for 20 seconds -mdns.start(() => setTimeout(() => mdns.stop(() => {}), 20 * 1000)) +// Listen for peers +mdns.start() ``` - options - `broadcast` - (true/false) announce our presence through mDNS, default false - - `interval` - query interval, default 10 * 1000 (10 seconds) - `serviceTag` - name of the service announcedm, default 'ipfs.local` diff --git a/src/index.js b/src/index.js index 04f8b95..5bc39fc 100644 --- a/src/index.js +++ b/src/index.js @@ -12,11 +12,9 @@ class MulticastDNS extends EventEmitter { options = options || {} this.broadcast = options.broadcast !== false - this.interval = options.interval || (1e3 * 10) this.serviceTag = options.serviceTag || 'ipfs.local' this.port = options.port || 5353 this.peerInfo = peerInfo - this._queryInterval = null } start (callback) { @@ -25,8 +23,6 @@ class MulticastDNS extends EventEmitter { this.mdns = mdns - this._queryInterval = query.queryLAN(this.mdns, this.serviceTag, this.interval) - mdns.on('response', (event) => { query.gotResponse(event, this.peerInfo, this.serviceTag, (err, foundPeer) => { if (err) { @@ -41,15 +37,14 @@ class MulticastDNS extends EventEmitter { query.gotQuery(event, this.mdns, this.peerInfo, this.serviceTag, this.broadcast) }) - setImmediate(() => callback()) + query.queryLAN(this.mdns, this.serviceTag) + setImmediate(callback) } stop (callback) { if (!this.mdns) { callback(new Error('MulticastDNS service had not started yet')) } else { - clearInterval(this._queryInterval) - this._queryInterval = null this.mdns.destroy(callback) this.mdns = undefined } diff --git a/src/query.js b/src/query.js index ee39386..551ad46 100644 --- a/src/query.js +++ b/src/query.js @@ -12,19 +12,13 @@ const tcp = new TCP() module.exports = { queryLAN: function (mdns, serviceTag, interval) { - const query = () => { - log('query', serviceTag) - mdns.query({ - questions: [{ - name: serviceTag, - type: 'PTR' - }] - }) - } - - // Immediately start a query, then do it every interval. - query() - return setInterval(query, interval) + log('query', serviceTag) + mdns.query({ + questions: [{ + name: serviceTag, + type: 'PTR' + }] + }) }, gotResponse: function (rsp, peerInfo, serviceTag, callback) { diff --git a/test/multicast-dns.spec.js b/test/multicast-dns.spec.js index f7f1a7b..794cb19 100644 --- a/test/multicast-dns.spec.js +++ b/test/multicast-dns.spec.js @@ -61,8 +61,6 @@ describe('MulticastDNS', () => { }) it('find another peer', function (done) { - this.timeout(40 * 1000) - const options = { port: 50001 // port must be the same } @@ -72,25 +70,22 @@ describe('MulticastDNS', () => { }) const mdnsB = new MulticastDNS(pB, options) + mdnsA.once('peer', (peerInfo) => { + expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String()) + parallel([ + (cb) => mdnsA.stop(cb), + (cb) => mdnsB.stop(cb) + ], done) + }) + + mdnsB.once('peer', (peerInfo) => {}) parallel([ (cb) => mdnsA.start(cb), (cb) => mdnsB.start(cb) - ], () => { - mdnsA.once('peer', (peerInfo) => { - expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String()) - parallel([ - (cb) => mdnsA.stop(cb), - (cb) => mdnsB.stop(cb) - ], done) - }) - - mdnsB.once('peer', (peerInfo) => {}) - }) + ], () => {}) }) it('only announce TCP multiaddrs', function (done) { - this.timeout(40 * 1000) - const options = { port: 50003 // port must be the same } @@ -102,63 +97,55 @@ describe('MulticastDNS', () => { const mdnsC = new MulticastDNS(pC, options) const mdnsD = new MulticastDNS(pD, options) + mdnsA.once('peer', (peerInfo) => { + expect(pC.id.toB58String()).to.eql(peerInfo.id.toB58String()) + expect(peerInfo.multiaddrs.size).to.equal(1) + parallel([ + (cb) => mdnsA.stop(cb), + (cb) => mdnsC.stop(cb), + (cb) => mdnsD.stop(cb) + ], done) + }) + mdnsC.once('peer', (peerInfo) => {}) parallel([ (cb) => mdnsA.start(cb), (cb) => mdnsC.start(cb), (cb) => mdnsD.start(cb) - ], () => { - mdnsA.once('peer', (peerInfo) => { - expect(pC.id.toB58String()).to.eql(peerInfo.id.toB58String()) - expect(peerInfo.multiaddrs.size).to.equal(1) - parallel([ - (cb) => mdnsA.stop(cb), - (cb) => mdnsC.stop(cb), - (cb) => mdnsD.stop(cb) - ], done) - }) - - mdnsC.once('peer', (peerInfo) => {}) - }) + ], () => {}) }) it('announces IP6 addresses', function (done) { - this.timeout(40 * 1000) - const options = { port: 50001 // port must be the same } - const mdnsA = new MulticastDNS(pA, { - broadcast: false, // do not talk to ourself - port: 50001 - }) + const mdnsA = new MulticastDNS(pA, options) const mdnsB = new MulticastDNS(pB, options) + mdnsA.once('peer', (peerInfo) => { + expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String()) + expect(peerInfo.multiaddrs.size).to.equal(2) + parallel([ + (cb) => mdnsA.stop(cb), + (cb) => mdnsB.stop(cb) + ], done) + }) + mdnsB.once('peer', (peerInfo) => {}) series([ (cb) => mdnsB.start(cb), (cb) => mdnsA.start(cb) - ], () => { - mdnsA.once('peer', (peerInfo) => { - expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String()) - expect(peerInfo.multiaddrs.size).to.equal(2) - parallel([ - (cb) => mdnsA.stop(cb), - (cb) => mdnsB.stop(cb) - ], done) - }) - - mdnsB.once('peer', (peerInfo) => {}) - }) + ], () => {}) }) it('doesn\'t emit peers after stop', function (done) { - this.timeout(40 * 1000) - const options = { port: 50004 // port must be the same } const mdnsA = new MulticastDNS(pA, options) const mdnsC = new MulticastDNS(pC, options) + mdnsC.once('peer', (peerInfo) => { + done(new Error('Should not receive new peer.')) + }) series([ (cb) => mdnsA.start(cb), @@ -166,10 +153,41 @@ describe('MulticastDNS', () => { (cb) => mdnsA.stop(cb), (cb) => mdnsC.start(cb) ], () => { - setTimeout(() => mdnsC.stop(done), 5000) - mdnsC.once('peer', (peerInfo) => { - done(new Error('Should not receive new peer.')) - }) + setTimeout(() => mdnsC.stop(done), 1000) }) }) + + it('find all peers', function (done) { + const options = { + port: 50001 // port must be the same + } + const mdnsA = new MulticastDNS(pA, options) + const mdnsB = new MulticastDNS(pB, options) + const mdnsC = new MulticastDNS(pC, options) + const peersA = {} + const peersB = {} + const peersC = {} + + // After all the peers have started, each peer should see two other peers. + function check (receiver, peerInfo) { + receiver[peerInfo.id.toB58String()] = true + if (Object.keys(peersA).length === 2 && Object.keys(peersB).length === 2 && Object.keys(peersC).length === 2) { + parallel([ + (cb) => mdnsA.stop(cb), + (cb) => mdnsB.stop(cb), + (cb) => mdnsC.stop(cb) + ], done) + } + } + mdnsA.on('peer', (peerInfo) => check(peersA, peerInfo)) + mdnsB.on('peer', (peerInfo) => check(peersB, peerInfo)) + mdnsC.on('peer', (peerInfo) => check(peersC, peerInfo)) + series([ + (cb) => mdnsA.start(cb), + (cb) => setTimeout(cb, 500), + (cb) => mdnsB.start(cb), + (cb) => setTimeout(cb, 500), + (cb) => mdnsC.start(cb) + ], () => {}) + }) }) From 18b791879117bec11d9e086e1da70e17761cf210 Mon Sep 17 00:00:00 2001 From: Richard Schneider Date: Sat, 3 Feb 2018 09:57:11 +1300 Subject: [PATCH 2/6] docs: broadcast defaults to true --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9bd8901..eb6381b 100644 --- a/README.md +++ b/README.md @@ -29,5 +29,5 @@ mdns.start() ``` - options - - `broadcast` - (true/false) announce our presence through mDNS, default false + - `broadcast` - (true/false) announce our presence through mDNS, default true - `serviceTag` - name of the service announcedm, default 'ipfs.local` From 8f5d4e40852d9dc7e6e68534a5b0b052761d492f Mon Sep 17 00:00:00 2001 From: Richard Schneider Date: Sat, 3 Feb 2018 12:13:53 +1300 Subject: [PATCH 3/6] test: faster tests with smaller RSA key --- test/multicast-dns.spec.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/multicast-dns.spec.js b/test/multicast-dns.spec.js index 794cb19..88d445d 100644 --- a/test/multicast-dns.spec.js +++ b/test/multicast-dns.spec.js @@ -7,11 +7,19 @@ const expect = chai.expect chai.use(dirtyChai) const multiaddr = require('multiaddr') const PeerInfo = require('peer-info') +const PeerId = require('peer-id') const parallel = require('async/parallel') const series = require('async/series') const MulticastDNS = require('./../src') +function createPeer (callback) { + PeerId.create({ bits: 512}, (err, id) => { + if (err) throw err + PeerInfo.create(id, callback) + }) +} + describe('MulticastDNS', () => { let pA let pB @@ -19,10 +27,9 @@ describe('MulticastDNS', () => { let pD before(function (done) { - this.timeout(80 * 1000) parallel([ (cb) => { - PeerInfo.create((err, peer) => { + createPeer((err, peer) => { expect(err).to.not.exist() pA = peer @@ -31,7 +38,7 @@ describe('MulticastDNS', () => { }) }, (cb) => { - PeerInfo.create((err, peer) => { + createPeer((err, peer) => { expect(err).to.not.exist() pB = peer @@ -41,7 +48,7 @@ describe('MulticastDNS', () => { }) }, (cb) => { - PeerInfo.create((err, peer) => { + createPeer((err, peer) => { expect(err).to.not.exist() pC = peer pC.multiaddrs.add(multiaddr('/ip4/127.0.0.1/tcp/20003')) @@ -50,7 +57,7 @@ describe('MulticastDNS', () => { }) }, (cb) => { - PeerInfo.create((err, peer) => { + createPeer((err, peer) => { if (err) { cb(err) } pD = peer pD.multiaddrs.add(multiaddr('/ip4/127.0.0.1/tcp/30003/ws')) From 4cae8fab736369be1c96eec1b054a777f1a39699 Mon Sep 17 00:00:00 2001 From: Richard Schneider Date: Sat, 3 Feb 2018 12:41:56 +1300 Subject: [PATCH 4/6] test: linting --- test/multicast-dns.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/multicast-dns.spec.js b/test/multicast-dns.spec.js index 88d445d..c672d25 100644 --- a/test/multicast-dns.spec.js +++ b/test/multicast-dns.spec.js @@ -14,7 +14,7 @@ const series = require('async/series') const MulticastDNS = require('./../src') function createPeer (callback) { - PeerId.create({ bits: 512}, (err, id) => { + PeerId.create({ bits: 512 }, (err, id) => { if (err) throw err PeerInfo.create(id, callback) }) From 8d057c9831211354984634ecfaceb0b1abd2bd4c Mon Sep 17 00:00:00 2001 From: Richard Schneider Date: Mon, 5 Feb 2018 23:42:30 +1300 Subject: [PATCH 5/6] fix: racey stop --- src/index.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/index.js b/src/index.js index 5bc39fc..a9f5403 100644 --- a/src/index.js +++ b/src/index.js @@ -24,7 +24,10 @@ class MulticastDNS extends EventEmitter { this.mdns = mdns mdns.on('response', (event) => { - query.gotResponse(event, this.peerInfo, this.serviceTag, (err, foundPeer) => { + if (!self.mdns) { + return; + } + query.gotResponse(event, self.peerInfo, self.serviceTag, (err, foundPeer) => { if (err) { return log('Error processing peer response', err) } @@ -34,7 +37,10 @@ class MulticastDNS extends EventEmitter { }) mdns.on('query', (event) => { - query.gotQuery(event, this.mdns, this.peerInfo, this.serviceTag, this.broadcast) + if (!self.mdns) { + return; + } + query.gotQuery(event, self.mdns, self.peerInfo, self.serviceTag, self.broadcast) }) query.queryLAN(this.mdns, this.serviceTag) @@ -42,12 +48,12 @@ class MulticastDNS extends EventEmitter { } stop (callback) { - if (!this.mdns) { - callback(new Error('MulticastDNS service had not started yet')) - } else { - this.mdns.destroy(callback) - this.mdns = undefined + const mdns = this.mdns + if (!mdns) { + return callback(new Error('MulticastDNS service had not started yet')) } + this.mdns = undefined + mdns.destroy(callback) } } From 9dec0158976a4eca0dcbc0989cef84d1e4bc20be Mon Sep 17 00:00:00 2001 From: Richard Schneider Date: Mon, 5 Feb 2018 23:46:25 +1300 Subject: [PATCH 6/6] fix: lint --- src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index a9f5403..a9d5293 100644 --- a/src/index.js +++ b/src/index.js @@ -25,7 +25,7 @@ class MulticastDNS extends EventEmitter { mdns.on('response', (event) => { if (!self.mdns) { - return; + return } query.gotResponse(event, self.peerInfo, self.serviceTag, (err, foundPeer) => { if (err) { @@ -38,7 +38,7 @@ class MulticastDNS extends EventEmitter { mdns.on('query', (event) => { if (!self.mdns) { - return; + return } query.gotQuery(event, self.mdns, self.peerInfo, self.serviceTag, self.broadcast) })