chore: apply suggestions from code review

Co-Authored-By: Jacob Heun <jacobheun@gmail.com>

package.json

@@ -38,7 +38,7 @@
     "chai": "^4.2.0",
     "delay": "^4.3.0",
     "dirty-chai": "^2.0.1",
-    "libp2p-interfaces": "libp2p/js-interfaces#v0.3.x",
+    "libp2p-interfaces": "^0.3.0",
     "p-defer": "^3.0.0"
   },
   "dependencies": {

src/compat/index.js

@@ -7,11 +7,13 @@ const Responder = require('./responder')
 const Querier = require('./querier')
 
 class GoMulticastDNS extends EE {
-  constructor (peerId, multiaddrs) {
+  constructor ({ peerId, multiaddrs, queryPeriod, queryInterval }) {
     super()
     this._started = false
     this._peerId = peerId
     this._multiaddrs = multiaddrs
+    this._queryPeriod = queryPeriod
+    this._queryInterval = queryInterval
     this._onPeer = this._onPeer.bind(this)
   }
 
@@ -21,8 +23,15 @@ class GoMulticastDNS extends EE {
     }
 
     this._started = true
-    this._responder = new Responder(this._peerId, this._multiaddrs)
-    this._querier = new Querier(this._peerId)
+    this._responder = new Responder({
+      peerId: this._peerId,
+      multiaddrs: this._multiaddrs
+    })
+    this._querier = new Querier({
+      peerId: this._peerId,
+      queryInterval: this._queryInterval,
+      queryPeriod: this._queryPeriod
+    })
 
     this._querier.on('peer', this._onPeer)
 
@@ -32,8 +41,8 @@ class GoMulticastDNS extends EE {
     ])
   }
 
-  _onPeer (peerInfo) {
-    this.emit('peer', peerInfo)
+  _onPeer (peerData) {
+    this.emit('peer', peerData)
   }
 
   stop () {

src/compat/querier.js

@@ -10,24 +10,24 @@ log.error = debug('libp2p:mdns:compat:querier:error')
 const { SERVICE_TAG_LOCAL, MULTICAST_IP, MULTICAST_PORT } = require('./constants')
 
 class Querier extends EE {
-  constructor (peerId, options) {
+  constructor ({ peerId, queryInterval = 60000, queryPeriod }) {
     super()
 
     if (!peerId) {
       throw new Error('missing peerId parameter')
     }
 
-    options = options || {}
     this._peerIdStr = peerId.toB58String()
-    // Re-query every 60s, in leu of network change detection
-    options.queryInterval = options.queryInterval || 60000
-    // Time for which the MDNS server will stay alive waiting for responses
-    // Must be less than options.queryInterval!
-    options.queryPeriod = Math.min(
-      options.queryInterval,
-      options.queryPeriod == null ? 5000 : options.queryPeriod
-    )
-    this._options = options
+    this._options = {
+      // Re-query in leu of network change detection (every 60s by default)
+      queryInterval: queryInterval,
+      // Time for which the MDNS server will stay alive waiting for responses
+      // Must be less than options.queryInterval!
+      queryPeriod: Math.min(
+        queryInterval,
+        queryPeriod == null ? 5000 : queryPeriod
+      )
+    }
     this._onResponse = this._onResponse.bind(this)
   }
 

src/compat/responder.js

@@ -6,7 +6,7 @@ const log = require('debug')('libp2p:mdns:compat:responder')
 const { SERVICE_TAG_LOCAL } = require('./constants')
 
 class Responder {
-  constructor (peerId, multiaddrs) {
+  constructor ({ peerId, multiaddrs }) {
     if (!peerId) {
       throw new Error('missing peerId parameter')
     }

src/index.js

@@ -25,7 +25,9 @@ class MulticastDNS extends EventEmitter {
     this._onPeer = this._onPeer.bind(this)
 
     if (options.compat !== false) {
-      this._goMdns = new GoMulticastDNS(options.peerId, this.peerMultiaddrs, {
+      this._goMdns = new GoMulticastDNS({
+        multiaddrs: this.peerMultiaddrs,
+        peerId: options.peerId,
         queryPeriod: options.compatQueryPeriod,
         queryInterval: options.compatQueryInterval
       })
@@ -56,9 +58,9 @@ class MulticastDNS extends EventEmitter {
     query.gotQuery(event, this.mdns, this.peerId, this.peerMultiaddrs, this.serviceTag, this.broadcast)
   }
 
-  async _onMdnsResponse (event) {
+  _onMdnsResponse (event) {
     try {
-      const foundPeer = await query.gotResponse(event, this.peerId, this.serviceTag)
+      const foundPeer = query.gotResponse(event, this.peerId, this.serviceTag)
 
       if (foundPeer) {
         this.emit('peer', foundPeer)
@@ -68,8 +70,8 @@ class MulticastDNS extends EventEmitter {
     }
   }
 
-  _onPeer (peerInfo) {
-    this.emit('peer', peerInfo)
+  _onPeer (peerData) {
+    this.emit('peer', peerData)
   }
 
   /**

test/compat/go-multicast-dns.spec.js

@@ -27,14 +27,20 @@ describe('GoMulticastDNS', () => {
   })
 
   it('should start and stop', async () => {
-    const mdns = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])
+    const mdns = new GoMulticastDNS({
+      peerId: peerIds[0],
+      multiaddrs: [peerAddrs[0]]
+    })
 
     await mdns.start()
     return mdns.stop()
   })
 
   it('should ignore multiple start calls', async () => {
-    const mdns = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])
+    const mdns = new GoMulticastDNS({
+      peerId: peerIds[0],
+      multiaddrs: [peerAddrs[0]]
+    })
 
     await mdns.start()
     await mdns.start()
@@ -43,13 +49,22 @@ describe('GoMulticastDNS', () => {
   })
 
   it('should ignore unnecessary stop calls', async () => {
-    const mdns = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])
+    const mdns = new GoMulticastDNS({
+      peerId: peerIds[0],
+      multiaddrs: [peerAddrs[0]]
+    })
     await mdns.stop()
   })
 
-  it('should emit peer info when peer is discovered', async () => {
-    const mdnsA = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])
-    const mdnsB = new GoMulticastDNS(peerIds[1], [peerAddrs[1]])
+  it('should emit peer data when peer is discovered', async () => {
+    const mdnsA = new GoMulticastDNS({
+      peerId: peerIds[0],
+      multiaddrs: [peerAddrs[0]]
+    })
+    const mdnsB = new GoMulticastDNS({
+      peerId: peerIds[1],
+      multiaddrs: [peerAddrs[1]]
+    })
     const defer = pDefer()
 
     mdnsA.on('peer', ({ id, multiaddrs }) => {

test/compat/querier.spec.js

@@ -36,14 +36,14 @@ describe('Querier', () => {
   })
 
   it('should start and stop', async () => {
-    const querier = new Querier(peerIds[0])
+    const querier = new Querier({ peerId: peerIds[0] })
 
     await querier.start()
     await querier.stop()
   })
 
   it('should query on interval', async () => {
-    querier = new Querier(peerIds[0], { queryPeriod: 0, queryInterval: 10 })
+    querier = new Querier({ peerId: peerIds[0], queryPeriod: 0, queryInterval: 10 })
     mdns = MDNS()
 
     let queryCount = 0
@@ -243,7 +243,7 @@ describe('Querier', () => {
    * @param {Function} getResponse Given a query, construct a response to test the querier
    */
   async function ensurePeer (getResponse) {
-    querier = new Querier(peerIds[0])
+    querier = new Querier({ peerId: peerIds[0] })
     mdns = MDNS()
 
     mdns.on('query', (event, info) => {
@@ -266,11 +266,11 @@ describe('Querier', () => {
   }
 
   /**
-   * Ensure none of peerInfos are emitted from `querier`
+   * Ensure none of peerIds are emitted from `querier`
    * @param {Function} getResponse Given a query, construct a response to test the querier
    */
   async function ensureNoPeer (getResponse) {
-    querier = new Querier(peerIds[0])
+    querier = new Querier({ peerId: peerIds[0] })
     mdns = MDNS()
 
     mdns.on('query', (event, info) => {

test/compat/responder.spec.js

@@ -38,15 +38,21 @@ describe('Responder', () => {
   })
 
   it('should start and stop', async () => {
-    const responder = new Responder(peerIds[0], [peerAddrs[0]])
+    const responder = new Responder({
+      peerId: peerIds[0],
+      multiaddrs: [peerAddrs[0]]
+    })
 
     await responder.start()
     await responder.stop()
   })
 
   it('should not respond to a query if no TCP addresses', async () => {
     const peerId = await PeerId.create()
-    responder = new Responder(peerId, [])
+    responder = new Responder({
+      peerId,
+      multiaddrs: []
+    })
     mdns = MDNS({ multicast: false, interface: '0.0.0.0', port: 0 })
 
     await responder.start()
@@ -72,7 +78,10 @@ describe('Responder', () => {
   })
 
   it('should not respond to a query with non matching service tag', async () => {
-    responder = new Responder(peerIds[0], [peerAddrs[0]])
+    responder = new Responder({
+      peerId: peerIds[0],
+      multiaddrs: [peerAddrs[0]]
+    })
     mdns = MDNS({ multicast: false, interface: '0.0.0.0', port: 0 })
 
     await responder.start()
@@ -100,7 +109,10 @@ describe('Responder', () => {
   })
 
   it('should respond correctly', async () => {
-    responder = new Responder(peerIds[0], [peerAddrs[0]])
+    responder = new Responder({
+      peerId: peerIds[0],
+      multiaddrs: [peerAddrs[0]]
+    })
     mdns = MDNS({ multicast: false, interface: '0.0.0.0', port: 0 })
 
     await responder.start()

test/multicast-dns.spec.js

@@ -191,17 +191,17 @@ describe('MulticastDNS', () => {
     await mdns.start()
 
     return new Promise((resolve, reject) => {
-      mdns.on('peer', (peerInfo) => {
-        if (!peerInfo) {
-          reject(new Error('peerInfo was not set'))
+      mdns.on('peer', (peerData) => {
+        if (!peerData) {
+          reject(new Error('peerData was not set'))
         }
       })
 
       mdns.mdns.on('response', () => {
         // query.gotResponse is async - we'll bail from that method when
         // comparing the senders PeerId to our own but it'll happen later
         // so allow enough time for the test to have failed if we emit
-        // empty PeerInfo objects
+        // empty peerData objects
         setTimeout(() => {
           resolve()
         }, 100)