fix: code review

Author: vasco-santos

SPEC/NAME.md

@@ -1,10 +1,10 @@
 # Name API
 
 * [name.publish](#namepublish)
-* [name.resolve](#nameresolve)
 * [name.pubsub.cancel](#namepubsubcancel)
 * [name.pubsub.state](#namepubsubstate)
 * [name.pubsub.subs](#namepubsubsubs)
+* [name.resolve](#nameresolve)
 
 #### `name.publish`
 
@@ -58,41 +58,6 @@ ipfs.name.publish(addr, function (err, res) {
 
 This way, you can republish a new version of your website under the same address. By default, `ipfs.name.publish` will use the Peer ID. If you want to have multiple websites (for example) under the same IPFS module, you can always check the [key API](./KEY.md).
 
-#### `name.resolve`
-
-> Resolve an IPNS name.
-
-##### `Go` **WIP**
-
-##### `JavaScript` - ipfs.name.resolve(value, [options, callback])
-
-`value` is a IPNS address, such as: `/ipns/ipfs.io`.
-
-`options` is an object that may contain:
-
-```JavaScript
-{
-  recursive: // bool - Resolve until the result is not an IPNS name. Default: false.
-  nocache: // bool - Do not use cached entries. Default: false.
-}
-```
-
-`callback` must follow `function (err, result) {}` signature, where `err` is an error if the operation was not successful. `result` is an object that contains the resulting path.
-
-If no `callback` is passed, a promise is returned.
-
-**Example:**
-
-```JavaScript
-// The IPNS address you want to resolve.
-const addr = '/ipns/ipfs.io'
-
-ipfs.name.resolve(addr, function (err, result) {
-    console.log(result.path)
-    // /ipfs/QmQrX8hka2BtNHa8N8arAq16TCVx5qHcb46c5yPewRycLm
-})
-```
-
 #### `name.pubsub.cancel`
 
 > Cancel a name subscription.
@@ -159,12 +124,10 @@ ipfs.name.pubsub.state(function (err, result) {
 
 ##### `JavaScript` - ipfs.name.pubsub.subs([callback])
 
-`callback` must follow `function (err, result) {}` signature, where `err` is an error if the operation was not successful. `result` is an object that contains the result of the operation, such as:
+`callback` must follow `function (err, result) {}` signature, where `err` is an error if the operation was not successful. `result` is an array of subscriptions, such as:
 
 ```JavaScript
-{
-  strings: ['/ipns/QmQrX8hka2BtNHa8N8arAq16TCVx5qHcb46c5yPewRycLm']
-}
+['/ipns/QmQrX8hka2BtNHa8N8arAq16TCVx5qHcb46c5yPewRycLm']
 ```
 
 If no `callback` is passed, a promise is returned.
@@ -173,7 +136,42 @@ If no `callback` is passed, a promise is returned.
 
 ```JavaScript
 ipfs.name.pubsub.subs(function (err, result) {
-    console.log(result.strings)
+    console.log(result)
     // ['/ipns/QmQrX8hka2BtNHa8N8arAq16TCVx5qHcb46c5yPewRycLm']
 })
 ```
+
+#### `name.resolve`
+
+> Resolve an IPNS name.
+
+##### `Go` **WIP**
+
+##### `JavaScript` - ipfs.name.resolve(value, [options, callback])
+
+`value` is a IPNS address, such as: `/ipns/ipfs.io`.
+
+`options` is an object that may contain:
+
+```JavaScript
+{
+  recursive: // bool - Resolve until the result is not an IPNS name. Default: false.
+  nocache: // bool - Do not use cached entries. Default: false.
+}
+```
+
+`callback` must follow `function (err, name) {}` signature, where `err` is an error if the operation was not successful. `name` is a string that contains the IPFS hash.
+
+If no `callback` is passed, a promise is returned.
+
+**Example:**
+
+```JavaScript
+// The IPNS address you want to resolve.
+const addr = '/ipns/ipfs.io'
+
+ipfs.name.resolve(addr, function (err, name) {
+    console.log(name)
+    // /ipfs/QmQrX8hka2BtNHa8N8arAq16TCVx5qHcb46c5yPewRycLm
+})
+```

js/src/name-pubsub/cancel.js

@@ -2,14 +2,14 @@
 /* eslint-env mocha */
 'use strict'
 
+const series = require('async/series')
 const loadFixture = require('aegir/fixtures')
 
 const { spawnNodeWithId } = require('../utils/spawn')
 const { getDescribe, getIt, expect } = require('../utils/mocha')
 
 const fixture = Object.freeze({
-  data: loadFixture('js/test/fixtures/testfile.txt', 'interface-ipfs-core'),
-  cid: 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP'
+  data: loadFixture('js/test/fixtures/testfile.txt', 'interface-ipfs-core')
 })
 
 module.exports = (createCommon, options) => {
@@ -20,6 +20,7 @@ module.exports = (createCommon, options) => {
   describe('.name.pubsub.cancel', function () {
     let ipfs
     let nodeId
+    let value
 
     before(function (done) {
       // CI takes longer to instantiate the daemon, so we need to increase the
@@ -35,7 +36,12 @@ module.exports = (createCommon, options) => {
           ipfs = node
           nodeId = node.peerId.id
 
-          ipfs.files.add(fixture.data, { pin: false }, done)
+          ipfs.files.add(fixture.data, { pin: false }, (err, res) => {
+            expect(err).to.not.exist()
+
+            value = res[0].path
+            done()
+          })
         })
       })
     })
@@ -56,24 +62,25 @@ module.exports = (createCommon, options) => {
     })
 
     it('should cancel a subscription correctly returning true', function (done) {
-      this.timeout(140 * 1000)
-      const value = fixture.cid
-
-      ipfs.name.publish(value, { resolve: false }, (err, res) => {
+      this.timeout(300 * 1000)
+      const ipnsPath = `/ipns/${nodeId}`
+
+      series([
+        (cb) => ipfs.name.pubsub.subs(cb),
+        (cb) => ipfs.name.publish(value, { resolve: false }, cb),
+        (cb) => ipfs.name.resolve(nodeId, cb),
+        (cb) => ipfs.name.pubsub.subs(cb),
+        (cb) => ipfs.name.pubsub.cancel(ipnsPath, cb),
+        (cb) => ipfs.name.pubsub.subs(cb)
+      ], (err, res) => {
         expect(err).to.not.exist()
+        expect(res).to.exist()
+        expect(res[0]).to.eql([]) // initally empty
+        expect(res[4]).to.have.property('canceled')
+        expect(res[4].canceled).to.eql(true)
+        expect(res[5]).to.be.an('array').that.does.not.include(ipnsPath)
 
-        ipfs.name.resolve(nodeId, (err) => {
-          expect(err).to.not.exist()
-
-          ipfs.name.pubsub.cancel(nodeId, (err, res) => {
-            expect(err).to.not.exist()
-            expect(res).to.exist()
-            expect(res).to.have.property('canceled')
-            expect(res.canceled).to.eql(true)
-
-            done()
-          })
-        })
+        done()
       })
     })
   })

js/src/name-pubsub/subs.js

@@ -2,14 +2,14 @@
 /* eslint-env mocha */
 'use strict'
 
+const series = require('async/series')
 const loadFixture = require('aegir/fixtures')
 
 const { spawnNodeWithId } = require('../utils/spawn')
 const { getDescribe, getIt, expect } = require('../utils/mocha')
 
 const fixture = Object.freeze({
-  data: loadFixture('js/test/fixtures/testfile.txt', 'interface-ipfs-core'),
-  cid: 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP'
+  data: loadFixture('js/test/fixtures/testfile.txt', 'interface-ipfs-core')
 })
 
 module.exports = (createCommon, options) => {
@@ -20,6 +20,7 @@ module.exports = (createCommon, options) => {
   describe('.name.pubsub.subs', function () {
     let ipfs
     let nodeId
+    let value
 
     before(function (done) {
       // CI takes longer to instantiate the daemon, so we need to increase the
@@ -35,45 +36,45 @@ module.exports = (createCommon, options) => {
           ipfs = node
           nodeId = node.peerId.id
 
-          ipfs.files.add(fixture.data, { pin: false }, done)
+          ipfs.files.add(fixture.data, { pin: false }, (err, res) => {
+            expect(err).to.not.exist()
+
+            value = res[0].path
+            done()
+          })
         })
       })
     })
 
     after((done) => common.teardown(done))
 
-    it('should get a null result of subscriptions before any resolve', function (done) {
+    it('should get an empty array as a result of subscriptions before any resolve', function (done) {
       this.timeout(60 * 1000)
 
       ipfs.name.pubsub.subs((err, res) => {
         expect(err).to.not.exist()
         expect(res).to.exist()
-        expect(res).to.have.property('strings')
-        expect(res.strings).to.eql(null)
+        expect(res).to.eql([])
 
         done()
       })
     })
 
     it('should get the list of subscriptions updated after a resolve', function (done) {
-      this.timeout(140 * 1000)
-      const value = fixture.cid
-
-      ipfs.name.publish(value, { resolve: false }, (err, res) => {
+      this.timeout(300 * 1000)
+
+      series([
+        (cb) => ipfs.name.pubsub.subs(cb),
+        (cb) => ipfs.name.publish(value, { resolve: false }, cb),
+        (cb) => ipfs.name.resolve(nodeId, cb),
+        (cb) => ipfs.name.pubsub.subs(cb)
+      ], (err, res) => {
         expect(err).to.not.exist()
+        expect(res).to.exist()
+        expect(res[0]).to.eql([]) // initally empty
+        expect(res[3]).to.be.an('array').that.does.include(`/ipns/${nodeId}`)
 
-        ipfs.name.resolve(nodeId, (err) => {
-          expect(err).to.not.exist()
-
-          ipfs.name.pubsub.subs((err, res) => {
-            expect(err).to.not.exist()
-            expect(res).to.exist()
-            expect(res).to.have.property('strings')
-            expect(res.strings).to.be.an('array').that.does.include(`/ipns/${nodeId}`)
-
-            done()
-          })
-        })
+        done()
       })
     })
   })