chore: refactor ping component and some cleanup

Author: JGAntunes

src/cli/commands/ping.js

@@ -9,7 +9,7 @@ const print = require('../utils').print
 module.exports = {
   command: 'ping <peerId>',
 
-  describe: 'Measure the latency of a connection',
+  description: 'Measure the latency of a connection',
 
   builder: {
     count: {

src/core/components/ping.js

@@ -3,21 +3,14 @@
 const promisify = require('promisify-es6')
 const debug = require('debug')
 const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR
-const PeerId = require('peer-id')
-const PeerInfo = require('peer-info')
 const pull = require('pull-stream/pull')
 const Pushable = require('pull-pushable')
 const ndjson = require('pull-ndjson')
+const waterfall = require('async/waterfall')
 
 const log = debug('jsipfs:ping')
 log.error = debug('jsipfs:ping:error')
 
-function getPacket (msg) {
-  // Default msg
-  const basePacket = {Success: false, Time: 0, Text: ''}
-  return Object.assign({}, basePacket, msg)
-}
-
 module.exports = function ping (self) {
   return promisify((peerId, count, cb) => {
     if (!self.isOnline()) {
@@ -31,49 +24,73 @@ module.exports = function ping (self) {
       ndjson.serialize()
     )
 
-    let peer
-    try {
-      peer = self._libp2pNode.peerBook.get(peerId)
-    } catch (err) {
-      // Conforming with go implemmentation, not sure if makes sense to log this
-      // since we perform no `findPeer`
-      source.push(getPacket({Success: true, Text: `Looking up peer ${peerId}`}))
-      peer = new PeerInfo(PeerId.createFromB58String(peerId))
-    }
+    waterfall([
+      getPeer.bind(null, self._libp2pNode, source, peerId),
+      runPing.bind(null, self._libp2pNode, source, count)
+    ], (err) => {
+      log.error(err)
+      source.push(getPacket({Text: err.toString()}))
+      return source.end(err)
+    })
 
-    self._libp2pNode.ping(peer, (err, p) => {
-      if (err) {
-        log.error(err)
-        source.push(getPacket({Text: err.toString()}))
-        return source.end(err)
-      }
+    cb(null, response)
+  })
+}
+
+function getPacket (msg) {
+  // Default msg
+  const basePacket = {Success: false, Time: 0, Text: ''}
+  return Object.assign({}, basePacket, msg)
+}
 
-      let packetCount = 0
-      let totalTime = 0
-      source.push(getPacket({Success: true, Text: `PING ${peerId}`}))
+function getPeer (libp2pNode, statusStream, peerId, cb) {
+  let peer
+  try {
+    peer = libp2pNode.peerBook.get(peerId)
+    console.log(peer)
+    return cb(null, peer)
+  } catch (err) {
+    // Check if we have support for peerRouting
+    if (!libp2pNode.peerRouting) {
+      return cb(new Error('Peer not found in peer book and no peer routing mechanism enabled'))
+    }
+    // Share lookup status just as in the go implemmentation
+    statusStream.push(getPacket({Success: true, Text: `Looking up peer ${peerId}`}))
+    libp2pNode.peerRouting.findPeer(peerId, cb)
+  }
+}
 
-      p.on('ping', (time) => {
-        source.push(getPacket({ Success: true, Time: time }))
-        totalTime += time
-        packetCount++
-        if (packetCount >= count) {
-          const average = totalTime / count
-          p.stop()
-          source.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` }))
-          source.end()
-        }
-      })
+function runPing (libp2pNode, statusStream, count, peer, cb) {
+  libp2pNode.ping(peer, (err, p) => {
+    if (err) {
+      return cb(err)
+    }
+
+    let packetCount = 0
+    let totalTime = 0
+    statusStream.push(getPacket({Success: true, Text: `PING ${peer.id.toB58String()}`}))
 
-      p.on('error', (err) => {
-        log.error(err)
+    p.on('ping', (time) => {
+      statusStream.push(getPacket({ Success: true, Time: time }))
+      totalTime += time
+      packetCount++
+      if (packetCount >= count) {
+        const average = totalTime / count
         p.stop()
-        source.push(getPacket({Text: err.toString()}))
-        source.end(err)
-      })
+        statusStream.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` }))
+        statusStream.end()
+      }
+    })
 
-      p.start()
+    p.on('error', (err) => {
+      log.error(err)
+      p.stop()
+      statusStream.push(getPacket({Text: err.toString()}))
+      statusStream.end(err)
     })
 
-    cb(null, response)
+    p.start()
+
+    return cb()
   })
 }

test/cli/ping.js

@@ -23,7 +23,7 @@ const config = {
   }
 }
 
-describe.only('ping', function () {
+describe('ping', function () {
   this.timeout(60 * 1000)
   let ipfsdA
   let ipfsdB