fix: code review for tests

package.json

@@ -69,7 +69,7 @@
     "execa": "^1.0.0",
     "form-data": "^2.3.3",
     "hat": "0.0.3",
-    "interface-ipfs-core": "~0.88.0",
+    "interface-ipfs-core": "ipfs/interface-ipfs-core#fix/ipns-over-pubsub-tests",
     "ipfsd-ctl": "~0.40.1",
     "ncp": "^2.0.0",
     "qs": "^6.5.2",
@@ -80,7 +80,6 @@
   "dependencies": {
     "@nodeutils/defaults-deep": "^1.1.0",
     "async": "^2.6.1",
-    "base32.js": "~0.1.0",
     "big.js": "^5.2.2",
     "binary-querystring": "~0.1.2",
     "bl": "^2.1.2",
@@ -89,16 +88,9 @@
     "byteman": "^1.3.5",
     "cid-tool": "~0.2.0",
     "cids": "~0.5.5",
-<<<<<<< HEAD
-<<<<<<< HEAD
-    "datastore-core": "~0.6.0",
-=======
-=======
     "class-is": "^1.1.0",
-    "datastore-core": "~0.4.0",
->>>>>>> fix: second pass of code review
-    "datastore-pubsub": "~0.0.2",
->>>>>>> feat: ipns over pubsub
+    "datastore-core": "~0.6.0",
+    "datastore-pubsub": "ipfs/js-datastore-pubsub#feat/encode-record-store-keys",
     "debug": "^4.1.0",
     "deep-extend": "~0.6.0",
     "err-code": "^1.1.2",

src/core/components/init.js

@@ -15,9 +15,6 @@ const UnixFs = require('ipfs-unixfs')
 const IPNS = require('../ipns')
 const OfflineDatastore = require('../ipns/routing/offline-datastore')
 
-const IPNS = require('../ipns')
-const OfflineDatastore = require('../ipns/routing/offline-datastore')
-
 const addDefaultAssets = require('./init-assets')
 
 module.exports = function init (self) {

src/core/components/pre-start.js

@@ -7,10 +7,6 @@ const waterfall = require('async/waterfall')
 const Keychain = require('libp2p-keychain')
 const defaultsDeep = require('@nodeutils/defaults-deep')
 const NoKeychain = require('./no-keychain')
-
-const IPNS = require('../ipns')
-const OfflineDatastore = require('../ipns/routing/offline-datastore')
-
 /*
  * Load stuff from Repo into memory
  */
@@ -99,13 +95,6 @@ module.exports = function preStart (self) {
 
         cb()
       },
-      // Setup offline routing for IPNS.
-      (cb) => {
-        const offlineDatastore = new OfflineDatastore(self._repo)
-
-        self._ipns = new IPNS(offlineDatastore, self)
-        cb()
-      },
       (cb) => self.pin._load(cb)
     ], callback)
   }

src/core/components/start.js

@@ -61,7 +61,7 @@ module.exports = (self) => {
 
         // Create ipns routing with a set of datastores
         const routing = new TieredDatastore(ipnsStores)
-        self._ipns = new IPNS(routing, self._repo, self._peerInfo, self._keychain, self._options, pubsubDs)
+        self._ipns = new IPNS(routing, self._repo, self._peerInfo, self._keychain, self._options)
 
         self._bitswap = new Bitswap(
           self._libp2pNode,

src/core/ipns/index.js

@@ -17,13 +17,12 @@ const path = require('./path')
 const defaultRecordTtl = 60 * 1000
 
 class IPNS {
-  constructor (routing, repo, peerInfo, keychain, options, pubsub) {
+  constructor (routing, repo, peerInfo, keychain, options) {
     this.publisher = new IpnsPublisher(routing, repo)
     this.republisher = new IpnsRepublisher(this.publisher, repo, peerInfo, keychain, options)
     this.resolver = new IpnsResolver(routing)
     this.cache = new Receptacle({ max: 1000 }) // Create an LRU cache with max 1000 items
     this.routing = routing
-    this.pubsub = pubsub
   }
 
   // Publish
@@ -72,7 +71,7 @@ class IPNS {
     options = options || {}
 
     // If recursive, we should not try to get the cached value
-    if (options.nocache && !options.recursive) {
+    if (!options.nocache && !options.recursive) {
       // Try to get the record from cache
       const id = name.split('/')[2]
       const result = this.cache.get(id)

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

@@ -3,6 +3,11 @@
 const { Key } = require('interface-datastore')
 const { encodeBase32 } = require('./utils')
 
+const errcode = require('err-code')
+const debug = require('debug')
+const log = debug('jsipfs:ipns:offline-datastore')
+log.error = debug('jsipfs:ipns:offline-datastore:error')
+
 // Offline datastore aims to mimic the same encoding as routing when storing records
 // to the local datastore
 class OfflineDatastore {

test/cli/name-pubsub.js

@@ -83,24 +83,21 @@ describe('name-pubsub', () => {
 
     // Connect
     before(function () {
-      return ipfsA('swarm', 'connect', bMultiaddr).then((out) => {
-        expect(out).to.eql(`connect ${bMultiaddr} success\n`)
-      })
+      return ipfsA('swarm', 'connect', bMultiaddr)
+        .then((out) => {
+          expect(out).to.eql(`connect ${bMultiaddr} success\n`)
+        })
     })
 
     after((done) => parallel(nodes.map((node) => (cb) => node.stop(cb)), done))
 
     describe('pubsub commands', () => {
-      before(function (done) {
-        this.timeout(50 * 1000)
-        done()
-      })
-
       it('should get enabled state of pubsub', function () {
-        return ipfsA('name pubsub state').then((res) => {
-          expect(res).to.exist()
-          expect(res).to.have.string('enabled') // enabled
-        })
+        return ipfsA('name pubsub state')
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.have.string('enabled') // enabled
+          })
       })
 
       it('should subscribe on name resolve', function () {
@@ -110,19 +107,17 @@ describe('name-pubsub', () => {
           .catch((err) => {
             expect(err).to.exist() // Not available (subscribed)
 
-            return Promise.all([
-              ipfsB('pubsub ls'),
-              ipfsB('name pubsub subs')
-            ])
-              .then((res) => {
-                expect(res).to.exist()
-
-                expect(res[0]).to.exist()
-                expect(res[0]).to.have.string('/ipns/') // have an ipns subscribtion
+            return ipfsB('pubsub ls')
+          })
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.have.string('/record/') // have a record ipns subscribtion
 
-                expect(res[1]).to.exist()
-                expect(res[1]).to.have.string(`/ipns/${nodeAId.id}`) // have subscription
-              })
+            return ipfsB('name pubsub subs')
+          })
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.have.string(`/ipns/${nodeAId.id}`) // have subscription
           })
       })
 
@@ -135,29 +130,27 @@ describe('name-pubsub', () => {
             expect(res).to.have.string('no subscription') // tried to cancel a not yet subscribed id
 
             return ipfsA(`name resolve ${nodeBId.id}`)
-              .catch((err) => {
-                expect(err).to.exist() // Not available (subscribed now)
-
-                return ipfsA(`name pubsub cancel /ipns/${nodeBId.id}`)
-                  .then((res) => {
-                    expect(res).to.exist()
-                    expect(res).to.have.string('canceled') // canceled now
-
-                    return Promise.all([
-                      ipfsA('pubsub ls'),
-                      ipfsA('name pubsub subs')
-                    ])
-                      .then((res) => {
-                        expect(res).to.exist()
-
-                        expect(res[0]).to.exist()
-                        expect(res[0]).to.not.have.string('/ipns/') // ipns subscribtion not available
-
-                        expect(res[1]).to.exist()
-                        expect(res[1]).to.not.have.string(`/ipns/${nodeBId.id}`) // ipns subscribtion not available
-                      })
-                  })
-              })
+          })
+          .catch((err) => {
+            expect(err).to.exist() // Not available (subscribed now)
+
+            return ipfsA(`name pubsub cancel /ipns/${nodeBId.id}`)
+          })
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.have.string('canceled') // canceled now
+
+            return ipfsA('pubsub ls')
+          })
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.not.have.string('/ipns/') // ipns subscribtion not available
+
+            return ipfsA('name pubsub subs')
+          })
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.not.have.string(`/ipns/${nodeBId.id}`) // ipns subscribtion not available
           })
       })
     })
@@ -167,10 +160,11 @@ describe('name-pubsub', () => {
 
       before(function (done) {
         this.timeout(50 * 1000)
-        ipfsA('add src/init-files/init-docs/readme').then((out) => {
-          cidAdded = out.split(' ')[1]
-          done()
-        })
+        ipfsA('add src/init-files/init-docs/readme')
+          .then((out) => {
+            cidAdded = out.split(' ')[1]
+            done()
+          })
       })
 
       it('should publish the received record to the subscriber', function () {
@@ -182,30 +176,28 @@ describe('name-pubsub', () => {
             expect(res).to.satisfy(checkAll([emptyDirCid])) // Empty dir received (subscribed)
 
             return ipfsA(`name resolve ${nodeBId.id}`)
-              .catch((err) => {
-                expect(err).to.exist() // Not available (subscribed now)
-
-                return ipfsB(`name publish ${cidAdded}`)
-                  .then((res) => {
-                    // published to IpfsB and published through pubsub to ipfsa
-                    expect(res).to.exist()
-                    expect(res).to.satisfy(checkAll([cidAdded, nodeBId.id]))
-
-                    return Promise.all([
-                      ipfsB(`name resolve ${nodeBId.id}`),
-                      ipfsA(`name resolve ${nodeBId.id}`)
-                    ])
-                      .then((res) => {
-                        expect(res).to.exist()
-
-                        expect(res[0]).to.exist()
-                        expect(res[0]).to.satisfy(checkAll([cidAdded]))
-
-                        expect(res[1]).to.exist()
-                        expect(res[1]).to.satisfy(checkAll([cidAdded])) // value propagated to node B
-                      })
-                  })
-              })
+          })
+          .catch((err) => {
+            expect(err).to.exist() // Not available (subscribed now)
+
+            return ipfsB(`name publish ${cidAdded}`)
+          })
+          .then((res) => {
+            // published to IpfsB and published through pubsub to ipfsa
+            expect(res).to.exist()
+            expect(res).to.satisfy(checkAll([cidAdded, nodeBId.id]))
+
+            return ipfsB(`name resolve ${nodeBId.id}`)
+          })
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.satisfy(checkAll([cidAdded]))
+
+            return ipfsA(`name resolve ${nodeBId.id}`)
+          })
+          .then((res) => {
+            expect(res).to.exist()
+            expect(res).to.satisfy(checkAll([cidAdded])) // value propagated to node B
           })
       })
     })
@@ -235,30 +227,28 @@ describe('name-pubsub', () => {
 
     after((done) => parallel(nodes.map((node) => (cb) => node.stop(cb)), done))
 
-    it('should get disabled state of pubsub', function (done) {
-      ipfsA('name pubsub state').then((res) => {
-        expect(res).to.exist()
-        expect(res).to.have.string('disabled')
-
-        done()
-      })
+    it('should get disabled state of pubsub', function () {
+      return ipfsA('name pubsub state')
+        .then((res) => {
+          expect(res).to.exist()
+          expect(res).to.have.string('disabled')
+        })
     })
 
-    it('should get error getting the available subscriptions', function (done) {
-      ipfsA('name pubsub subs').catch((err) => {
-        expect(err).to.exist() // error as it is disabled
-        expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled')
-        done()
-      })
+    it('should get error getting the available subscriptions', function () {
+      return ipfsA('name pubsub subs')
+        .catch((err) => {
+          expect(err).to.exist() // error as it is disabled
+          expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled')
+        })
     })
 
-    it('should get error canceling a subscription', function (done) {
-      ipfsA('name pubsub cancel /ipns/QmSWxaPcGgf4TDnFEBDWz2JnbHywF14phmY9hNcAeBEK5v').catch((err) => {
-        expect(err).to.exist() // error as it is disabled
-        expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled')
-
-        done()
-      })
+    it('should get error canceling a subscription', function () {
+      return ipfsA('name pubsub cancel /ipns/QmSWxaPcGgf4TDnFEBDWz2JnbHywF14phmY9hNcAeBEK5v')
+        .catch((err) => {
+          expect(err).to.exist() // error as it is disabled
+          expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled')
+        })
     })
   })
 })