fix: code review

Co-Authored-By: vasco-santos <[email protected]>

Author: hugomrdias

package.json

@@ -93,6 +93,7 @@
     "datastore-pubsub": "~0.1.1",
     "debug": "^4.1.0",
     "deep-extend": "~0.6.0",
+    "dlv": "^1.1.2",
     "err-code": "^1.1.2",
     "file-type": "^10.2.0",
     "fnv1a": "^1.0.1",
@@ -120,7 +121,7 @@
     "ipld-ethereum": "^2.0.1",
     "ipld-git": "~0.2.2",
     "ipld-zcash": "~0.1.6",
-    "ipns": "~0.4.2",
+    "ipns": "~0.4.3",
     "is-ipfs": "~0.4.7",
     "is-pull-stream": "~0.0.0",
     "is-stream": "^1.1.0",

src/core/components/name-pubsub.js

@@ -11,22 +11,18 @@ log.error = debug('jsipfs:name-pubsub:error')
 
 // Is pubsub enabled
 const isNamePubsubEnabled = (node) => {
-  let pubsub
   try {
-    pubsub = getPubsubRouting(node)
+    return Boolean(getPubsubRouting(node))
   } catch (err) {
     return false
   }
-
-  return Boolean(pubsub)
 }
 
 // Get pubsub from IPNS routing
 const getPubsubRouting = (node) => {
   if (!node._ipns || !node._options.EXPERIMENTAL.ipnsPubsub) {
     const errMsg = 'IPNS pubsub subsystem is not enabled'
 
-    log.error(errMsg)
     throw errcode(errMsg, 'ERR_IPNS_PUBSUB_NOT_ENABLED')
   }
 
@@ -41,9 +37,9 @@ const getPubsubRouting = (node) => {
   if (!pubsub) {
     const errMsg = 'IPNS pubsub datastore not found'
 
-    log.error(errMsg)
     throw errcode(errMsg, 'ERR_PUBSUB_DATASTORE_NOT_FOUND')
   }
+
   return pubsub
 }
 

src/core/components/start.js

@@ -2,7 +2,7 @@
 
 const series = require('async/series')
 const Bitswap = require('ipfs-bitswap')
-const get = require('lodash/get')
+const get = require('dlv')
 const setImmediate = require('async/setImmediate')
 const promisify = require('promisify-es6')
 const { TieredDatastore } = require('datastore-core')

src/core/config.js

@@ -29,7 +29,7 @@ const schema = Joi.object().keys({
   }).allow(null),
   EXPERIMENTAL: Joi.object().keys({
     pubsub: Joi.boolean(),
-    namesysPubsub: Joi.boolean(),
+    ipnsPubsub: Joi.boolean(),
     sharding: Joi.boolean(),
     dht: Joi.boolean()
   }).allow(null),

src/core/ipns/routing/offline-datastore.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const { Key } = require('interface-datastore')
-const Record = require('libp2p-record').Record
+const { Record } = require('libp2p-record')
 const { encodeBase32 } = require('./utils')
 
 const errcode = require('err-code')

src/core/ipns/routing/pubsub-datastore.js

@@ -11,10 +11,7 @@ const debug = require('debug')
 const log = debug('jsipfs:ipns:pubsub')
 log.error = debug('jsipfs:ipns:pubsub:error')
 
-const ipnsNS = '/ipns/'
-const ipnsNSLength = ipnsNS.length
-
-// Pubsub aims to manage the pubsub subscriptions for IPNS
+// Pubsub datastore aims to manage the pubsub subscriptions for IPNS
 class IpnsPubsubDatastore {
   constructor (pubsub, localDatastore, peerId) {
     this._pubsub = pubsub
@@ -68,11 +65,11 @@ class IpnsPubsubDatastore {
 
     this._pubsubDs.get(key, (err, res) => {
       // Add topic subscribed
-      const ns = key.slice(0, ipnsNSLength)
+      const ns = key.slice(0, ipns.namespaceLength)
 
-      if (ns.toString() === ipnsNS) {
+      if (ns.toString() === ipns.namespace) {
         const stringifiedTopic = key.toString()
-        const id = toB58String(key.slice(ipnsNSLength))
+        const id = toB58String(key.slice(ipns.namespaceLength))
 
         this._subscriptions[stringifiedTopic] = id
 
@@ -116,9 +113,9 @@ class IpnsPubsubDatastore {
    * @returns {void}
    */
   getSubscriptions (callback) {
-    const subscriptions = Object.values(this._subscriptions)
+    const subscriptions = Object.values(this._subscriptions).filter(Boolean)
 
-    return callback(null, subscriptions.map((sub) => `/ipns/${sub}`))
+    return callback(null, subscriptions.map((sub) => `${ipns.namespace}${sub}`))
   }
 
   /**
@@ -136,8 +133,8 @@ class IpnsPubsubDatastore {
     }
 
     // Trim /ipns/ prefix from the name
-    if (name.startsWith(ipnsNS)) {
-      name = name.substring(ipnsNSLength)
+    if (name.startsWith(ipns.namespace)) {
+      name = name.substring(ipns.namespaceLength)
     }
 
     const stringifiedTopic = Object.keys(this._subscriptions).find((key) => this._subscriptions[key] === name)
@@ -158,7 +155,7 @@ class IpnsPubsubDatastore {
       return callback(err)
     }
 
-    delete this._subscriptions[stringifiedTopic]
+    this._subscriptions[stringifiedTopic] = undefined
     log(`unsubscribed pubsub ${stringifiedTopic}: ${name}`)
 
     callback(null, {
@@ -167,5 +164,4 @@ class IpnsPubsubDatastore {
   }
 }
 
-// exports = module.exports = IpnsPubsubDatastore
 exports = module.exports = withIs(IpnsPubsubDatastore, { className: 'IpnsPubsubDatastore', symbolName: '@js-ipfs/ipns/IpnsPubsubDatastore' })

test/core/name-pubsub.js

@@ -64,6 +64,8 @@ describe('name-pubsub', function () {
   after((done) => parallel(nodes.map((node) => (cb) => node.stop(cb)), done))
 
   it('should publish and then resolve correctly', function (done) {
+    this.timeout(50 * 1000)
+
     nodeB.name.resolve(idA.id, (err) => {
       expect(err).to.exist()