From bb91008ddd0430facc10b7d8bdddc51e894ecd24 Mon Sep 17 00:00:00 2001 From: David Dias Date: Mon, 4 Sep 2017 18:10:39 +0100 Subject: [PATCH 1/8] chore: update deps --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f92fa066..592fa091 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,7 @@ "dependencies": { "async": "^2.5.0", "go-ipfs-dep": "0.4.10", - "ipfs-api": "^14.3.1", + "ipfs-api": "^14.3.2", "multiaddr": "^3.0.0", "once": "^1.4.0", "rimraf": "^2.6.1", From 2bec42ec79eae2853b9a2bf32279ada991fb04cd Mon Sep 17 00:00:00 2001 From: David Dias Date: Mon, 4 Sep 2017 18:32:53 +0100 Subject: [PATCH 2/8] test: check tests --- test/index.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/index.spec.js b/test/index.spec.js index 2bb5020d..885d4ede 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -482,11 +482,11 @@ describe('daemons', () => { expect(daemon).to.have.property('apiAddr') expect(daemon).to.have.property('gatewayAddr') - expect(multiaddr.isMultiaddr(daemon.apiAddr)).to.equal(true) - expect(multiaddr.isMultiaddr(daemon.gatewayAddr)).to.equal(true) + expect(daemon.apiAddr).to.not.equal(null) + expect(multiaddr.isMultiaddr(daemon.apiAddr)).to.equal(true) expect(daemon.gatewayAddr).to.not.equal(null) - + expect(multiaddr.isMultiaddr(daemon.gatewayAddr)).to.equal(true) expect(res).to.have.property('apiHost') expect(res).to.have.property('apiPort') expect(res).to.have.property('gatewayHost') From 3179881a59ed70415ef0efb273bf5c99e86a08bd Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 6 Sep 2017 10:03:16 +0100 Subject: [PATCH 3/8] fix --- test/index.spec.js | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/test/index.spec.js b/test/index.spec.js index 885d4ede..b27907af 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -467,34 +467,32 @@ describe('daemons', () => { }) it('starts the daemon and returns valid API and gateway addresses', (done) => { - let daemon const dir = `${os.tmpdir()}/tmp-${Date.now() + '-' + Math.random().toString(36)}` async.waterfall([ (cb) => ipfsd.local(dir, cb), - (node, cb) => { - daemon = node - node.init((err) => cb(err, node)) - }, - (node, cb) => node.startDaemon((err, api) => cb(err, api)) - ], (err, res) => { + (daemon, cb) => daemon.init((err) => cb(err, daemon)), + (daemon, cb) => daemon.startDaemon((err, api) => cb(err, daemon, api)) + ], (err, daemon, api) => { expect(err).to.not.exist() + // Check for props in daemon expect(daemon).to.have.property('apiAddr') expect(daemon).to.have.property('gatewayAddr') - expect(daemon.apiAddr).to.not.equal(null) expect(multiaddr.isMultiaddr(daemon.apiAddr)).to.equal(true) expect(daemon.gatewayAddr).to.not.equal(null) expect(multiaddr.isMultiaddr(daemon.gatewayAddr)).to.equal(true) - expect(res).to.have.property('apiHost') - expect(res).to.have.property('apiPort') - expect(res).to.have.property('gatewayHost') - expect(res).to.have.property('gatewayPort') - expect(res.apiHost).to.equal('127.0.0.1') - expect(res.apiPort).to.equal('5001') - expect(res.gatewayHost).to.equal('127.0.0.1') - expect(res.gatewayPort).to.equal('8080') + + // Check for props in ipfs-api instance + expect(api).to.have.property('apiHost') + expect(api).to.have.property('apiPort') + expect(api).to.have.property('gatewayHost') + expect(api).to.have.property('gatewayPort') + expect(api.apiHost).to.equal('127.0.0.1') + expect(api.apiPort).to.equal('5001') + expect(api.gatewayHost).to.equal('127.0.0.1') + expect(api.gatewayPort).to.equal('8080') daemon.stopDaemon(done) }) From c3605735907f68ca7cbb5153f87fe48a54b63307 Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 6 Sep 2017 10:31:11 +0100 Subject: [PATCH 4/8] refactor npm installs tests, make sure to capture both api and gw match --- src/{node.js => daemon.js} | 20 +++++++------- src/index.js | 2 +- test/index.spec.js | 47 --------------------------------- test/npm-installs.spec.js | 53 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 58 deletions(-) rename src/{node.js => daemon.js} (93%) create mode 100644 test/npm-installs.spec.js diff --git a/src/node.js b/src/daemon.js similarity index 93% rename from src/node.js rename to src/daemon.js index 9c409767..e81fd360 100644 --- a/src/node.js +++ b/src/daemon.js @@ -226,22 +226,22 @@ class Node { }, data: (data) => { const str = String(data).trim() - const match = str.match(/API server listening on (.*)/) - const gwmatch = str.match(/Gateway (.*) listening on (.*)/) + const apiMatch = str.match(/API server listening on (.*)/) + const gwMatch = str.match(/Gateway (.*) listening on (.*)/) - if (match) { - this._apiAddr = multiaddr(match[1]) - this.api = ipfs(match[1]) + if (apiMatch && gwMatch) { + this._apiAddr = multiaddr(apiMatch[1]) + this.api = ipfs(apiMatch[1]) this.api.apiHost = this.apiAddr.nodeAddress().address this.api.apiPort = this.apiAddr.nodeAddress().port - if (gwmatch) { - this._gatewayAddr = multiaddr(gwmatch[2]) - this.api.gatewayHost = this.gatewayAddr.nodeAddress().address - this.api.gatewayPort = this.gatewayAddr.nodeAddress().port - } + this._gatewayAddr = multiaddr(gwMatch[2]) + this.api.gatewayHost = this.gatewayAddr.nodeAddress().address + this.api.gatewayPort = this.gatewayAddr.nodeAddress().port callback(null, this.api) + } else { + callback(new Error('Daemon did not start properly')) } } }) diff --git a/src/index.js b/src/index.js index 23cf7222..adac9c9a 100644 --- a/src/index.js +++ b/src/index.js @@ -3,7 +3,7 @@ const os = require('os') const join = require('path').join -const Node = require('./node') +const Node = require('./daemon') const defaultOptions = { 'Addresses.Swarm': ['/ip4/0.0.0.0/tcp/0'], diff --git a/test/index.spec.js b/test/index.spec.js index b27907af..551bbe89 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -9,10 +9,8 @@ const expect = chai.expect chai.use(dirtyChai) const ipfsApi = require('ipfs-api') const multiaddr = require('multiaddr') -const Buffer = require('safe-buffer').Buffer const fs = require('fs') const rimraf = require('rimraf') -const mkdirp = require('mkdirp') const path = require('path') const once = require('once') const os = require('os') @@ -22,51 +20,6 @@ const ipfsd = require('../src') const isWindows = os.platform() === 'win32' -describe('ipfs executable path', () => { - let Node - - it('has the correct path when installed with npm3', (done) => { - process.env.testpath = '/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/lib' // fake __dirname - let npm3Path = '/tmp/ipfsd-ctl-test/node_modules/go-ipfs-dep/go-ipfs' - - mkdirp(npm3Path, (err) => { - if (err) { - throw err - } - - fs.writeFileSync(path.join(npm3Path, 'ipfs')) - delete require.cache[require.resolve('../src/node.js')] - Node = require('../src/node.js') - var node = new Node() - expect(node.exec) - .to.eql(path.normalize('/tmp/ipfsd-ctl-test/node_modules/go-ipfs-dep/go-ipfs/ipfs')) - rimraf('/tmp/ipfsd-ctl-test', done) - }) - }) - - it('has the correct path when installed with npm2', (done) => { - process.env.testpath = '/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/lib' // fake __dirname - let npm2Path = '/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/node_modules/go-ipfs-dep/go-ipfs' - - mkdirp(npm2Path, (err) => { - if (err) { - throw err - } - - fs.writeFileSync(path.join(npm2Path, 'ipfs')) - delete require.cache[require.resolve('../src/node.js')] - Node = require('../src/node.js') - var node = new Node() - expect( - node.exec - ).to.be.eql( - path.normalize('/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/node_modules/go-ipfs-dep/go-ipfs/ipfs') - ) - rimraf('/tmp/ipfsd-ctl-test', done) - }) - }) -}) - describe('daemons', () => { describe('local node', () => { const repoPath = '/tmp/ipfsd-ctl-test' diff --git a/test/npm-installs.spec.js b/test/npm-installs.spec.js new file mode 100644 index 00000000..028ae9f6 --- /dev/null +++ b/test/npm-installs.spec.js @@ -0,0 +1,53 @@ +/* eslint-env mocha */ +/* eslint max-nested-callbacks: ["error", 8] */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) +const fs = require('fs') +const rimraf = require('rimraf') +const mkdirp = require('mkdirp') +const path = require('path') + +describe('ipfs executable path', () => { + it('has the correct path when installed with npm3', (done) => { + process.env.testpath = '/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/lib' // fake __dirname + let npm3Path = '/tmp/ipfsd-ctl-test/node_modules/go-ipfs-dep/go-ipfs' + + mkdirp(npm3Path, (err) => { + expect(err).to.not.exist() + + fs.writeFileSync(path.join(npm3Path, 'ipfs')) + delete require.cache[require.resolve('../src/daemon.js')] + const Daemon = require('../src/daemon.js') + + const node = new Daemon() + expect(node.exec) + .to.eql(path.normalize('/tmp/ipfsd-ctl-test/node_modules/go-ipfs-dep/go-ipfs/ipfs')) + rimraf('/tmp/ipfsd-ctl-test', done) + }) + }) + + it('has the correct path when installed with npm2', (done) => { + process.env.testpath = '/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/lib' // fake __dirname + let npm2Path = '/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/node_modules/go-ipfs-dep/go-ipfs' + + mkdirp(npm2Path, (err) => { + expect(err).to.not.exist() + + fs.writeFileSync(path.join(npm2Path, 'ipfs')) + delete require.cache[require.resolve('../src/daemon.js')] + const Daemon = require('../src/daemon.js') + + const node = new Daemon() + + expect(node.exec) + .to.eql( + path.normalize('/tmp/ipfsd-ctl-test/node_modules/ipfsd-ctl/node_modules/go-ipfs-dep/go-ipfs/ipfs') + ) + rimraf('/tmp/ipfsd-ctl-test', done) + }) + }) +}) From 7dc07b703550a457befc0544f01ccffb4e998703 Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 6 Sep 2017 10:36:49 +0100 Subject: [PATCH 5/8] moar refactoring --- test/index.spec.js | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/test/index.spec.js b/test/index.spec.js index 551bbe89..09d3a9f7 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -20,8 +20,8 @@ const ipfsd = require('../src') const isWindows = os.platform() === 'win32' -describe('daemons', () => { - describe('local node', () => { +describe('daemon spawning', () => { + describe('local daemon', () => { const repoPath = '/tmp/ipfsd-ctl-test' const addr = '/ip4/127.0.0.1/tcp/5678' const config = { @@ -47,7 +47,7 @@ describe('daemons', () => { }) }) - describe('disposable node', () => { + describe('disposable daemon', () => { const blorb = Buffer.from('blorb') let ipfs let store @@ -67,7 +67,7 @@ describe('daemons', () => { ], done) }) - describe('with local api', () => { + describe('without api instance (.disposable)', () => { before((done) => { async.waterfall([ (cb) => ipfsd.disposable(cb), @@ -98,12 +98,10 @@ describe('daemons', () => { }) }) - describe('disposableApi', () => { + describe('with api instance (.disposableApi)', () => { before((done) => { ipfsd.disposableApi((err, api) => { - if (err) { - done(err) - } + expect(err).to.not.exist() ipfs = api done() @@ -118,11 +116,11 @@ describe('daemons', () => { it('should be able to store objects', () => { expect(store) - .to.eql('QmPv52ekjS75L4JmHpXVeuJ5uX2ecSfSZo88NSyxwA3rAQ') + .to.equal('QmPv52ekjS75L4JmHpXVeuJ5uX2ecSfSZo88NSyxwA3rAQ') }) it('should be able to retrieve objects', () => { - expect(retrieve.toString()).to.be.eql('blorb') + expect(retrieve.toString()).to.equal('blorb') }) }) }) @@ -157,7 +155,7 @@ describe('daemons', () => { before((done) => { node.startDaemon((err, res) => { - if (err) throw err + expect(err).to.not.exist() pid = node.daemonPid() ipfs = res @@ -178,11 +176,10 @@ describe('daemons', () => { before((done) => { node.stopDaemon((err) => { - if (err) { - return done(err) - } + expect(err).to.not.exist() stopped = true }) + // make sure it's not still running const poll = setInterval(() => { exec('kill', ['-0', pid], {cleanup: true}, { @@ -298,9 +295,7 @@ describe('daemons', () => { it('should give an error if setting an invalid config value', (done) => { ipfsNode.setConfig('Bootstrap', 'true', (err) => { - expect(err.message).to.match( - /failed to set config value/ - ) + expect(err.message).to.match(/failed to set config value/) done() }) }) @@ -330,9 +325,9 @@ describe('daemons', () => { before((done) => { ipfsd.disposable((err, node) => { - if (err) throw err + expect(err).to.not.exist() node.startDaemon((err, ignore) => { - if (err) throw err + expect(err).to.not.exist() ipfs = ipfsApi(node.apiAddr) done() }) @@ -342,15 +337,13 @@ describe('daemons', () => { // skip on windows for now // https://github.com/ipfs/js-ipfsd-ctl/pull/155#issuecomment-326970190 // fails on windows see https://github.com/ipfs/js-ipfs-api/issues/408 - if (isWindows) { - it.skip('uses the correct ipfs-api') - return // does not continue this test on win - } + if (isWindows) { return it.skip('uses the correct ipfs-api') } - // NOTE: if you change ./fixtures, the hash will need to be changed it('uses the correct ipfs-api', (done) => { - ipfs.util.addFromFs(path.join(__dirname, 'fixtures/'), { recursive: true }, (err, res) => { - if (err) throw err + ipfs.util.addFromFs(path.join(__dirname, 'fixtures/'), { + recursive: true + }, (err, res) => { + expect(err).to.not.exist() const added = res[res.length - 1] From 657b028da1d8157c406723443f33929b0a60f2da Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 6 Sep 2017 11:21:10 +0100 Subject: [PATCH 6/8] slayed two of the dragons, one to go --- src/daemon.js | 62 ++++++++++++++----- src/index.js | 53 +++++++++------- ...index.spec.js => spawning-daemons.spec.js} | 4 +- 3 files changed, 75 insertions(+), 44 deletions(-) rename test/{index.spec.js => spawning-daemons.spec.js} (99%) diff --git a/src/daemon.js b/src/daemon.js index e81fd360..3337a12f 100644 --- a/src/daemon.js +++ b/src/daemon.js @@ -32,12 +32,15 @@ function findIpfsExecutable () { const npm3Path = path.join(appRoot, '../', depPath) const npm2Path = path.join(appRoot, 'node_modules', depPath) + let npmPath try { fs.statSync(npm3Path) - return npm3Path + npmPath = npm3Path } catch (e) { - return npm2Path + npmPath = npm2Path } + + return npmPath } function setConfigValue (node, key, value, callback) { @@ -201,11 +204,26 @@ class Node { callback = once(callback) + // Check if there were explicit options to want or not want. Otherwise, + // assume values will be in the local daemon config + // TODO: This should check the local daemon config + const want = { + gateway: typeof this.opts['Addresses.Gateway'] === 'string' + ? this.opts['Addresses.Gateway'].length > 0 + : true, + api: typeof this.opts['Addresses.API'] === 'string' + ? this.opts['Addresses.API'].length > 0 + : true + } + parseConfig(this.path, (err, conf) => { if (err) { return callback(err) } + let output = '' + let returned = false + this.subprocess = this._run(args, {env: this.env}, { error: (err) => { // Only look at the last error @@ -225,23 +243,35 @@ class Node { } }, data: (data) => { - const str = String(data).trim() - const apiMatch = str.match(/API server listening on (.*)/) - const gwMatch = str.match(/Gateway (.*) listening on (.*)/) + output += String(data).trim() + + const apiMatch = want.api + ? output.match(/API server listening on (.*)/) + : true - if (apiMatch && gwMatch) { - this._apiAddr = multiaddr(apiMatch[1]) - this.api = ipfs(apiMatch[1]) - this.api.apiHost = this.apiAddr.nodeAddress().address - this.api.apiPort = this.apiAddr.nodeAddress().port + const gwMatch = want.gateway + ? output.match(/Gateway (.*) listening on (.*)/) + : true - this._gatewayAddr = multiaddr(gwMatch[2]) - this.api.gatewayHost = this.gatewayAddr.nodeAddress().address - this.api.gatewayPort = this.gatewayAddr.nodeAddress().port + if (apiMatch && gwMatch && !returned) { + returned = true + + if (want.api) { + this._apiAddr = multiaddr(apiMatch[1]) + this.api = ipfs(apiMatch[1]) + this.api.apiHost = this.apiAddr.nodeAddress().address + this.api.apiPort = this.apiAddr.nodeAddress().port + } + + if (want.gateway) { + this._gatewayAddr = multiaddr(gwMatch[2]) + this.api.gatewayHost = this.gatewayAddr.nodeAddress().address + this.api.gatewayPort = this.gatewayAddr.nodeAddress().port + } callback(null, this.api) } else { - callback(new Error('Daemon did not start properly')) + console.log('bit more') } } }) @@ -255,9 +285,7 @@ class Node { * @returns {undefined} */ stopDaemon (callback) { - if (!callback) { - callback = () => {} - } + callback = callback || function noop () {} if (!this.subprocess) { return callback() diff --git a/src/index.js b/src/index.js index adac9c9a..7dc7e6ba 100644 --- a/src/index.js +++ b/src/index.js @@ -5,6 +5,7 @@ const join = require('path').join const Node = require('./daemon') +// Note how defaultOptions are Addresses.Swarm and not Addresses: { Swarm : <> } const defaultOptions = { 'Addresses.Swarm': ['/ip4/0.0.0.0/tcp/0'], 'Addresses.Gateway': '', @@ -33,6 +34,7 @@ const IpfsDaemonController = { version (callback) { (new Node()).version(callback) }, + /** * Create a new local node. * @@ -47,37 +49,17 @@ const IpfsDaemonController = { callback = opts opts = {} } + if (!callback) { callback = path path = process.env.IPFS_PATH || join(process.env.HOME || process.env.USERPROFILE, '.ipfs') } - process.nextTick(() => { - callback(null, new Node(path, opts)) - }) - }, - /** - * Create a new disposable node and already start the daemon. - * - * @memberof IpfsDaemonController - * @param {Object} [opts={}] - * @param {function(Error, Node)} callback - * @returns {undefined} - */ - disposableApi (opts, callback) { - if (typeof opts === 'function') { - callback = opts - opts = {} - } - this.disposable(opts, (err, node) => { - if (err) { - return callback(err) - } - node.startDaemon(callback) - }) + process.nextTick(() => callback(null, new Node(path, opts))) }, + /** * Create a new disposable node. * This means the repo is created in a temporary location and cleaned up on process exit. @@ -90,7 +72,7 @@ const IpfsDaemonController = { disposable (opts, callback) { if (typeof opts === 'function') { callback = opts - opts = {} + opts = defaultOptions } let options = {} @@ -109,6 +91,29 @@ const IpfsDaemonController = { } else { node.init((err) => callback(err, node)) } + }, + + /** + * Create a new disposable node and already start the daemon. + * + * @memberof IpfsDaemonController + * @param {Object} [opts={}] + * @param {function(Error, Node)} callback + * @returns {undefined} + */ + disposableApi (opts, callback) { + if (typeof opts === 'function') { + callback = opts + opts = defaultOptions + } + + this.disposable(opts, (err, node) => { + if (err) { + return callback(err) + } + + node.startDaemon(callback) + }) } } diff --git a/test/index.spec.js b/test/spawning-daemons.spec.js similarity index 99% rename from test/index.spec.js rename to test/spawning-daemons.spec.js index 09d3a9f7..c401eaae 100644 --- a/test/index.spec.js +++ b/test/spawning-daemons.spec.js @@ -73,9 +73,7 @@ describe('daemon spawning', () => { (cb) => ipfsd.disposable(cb), (node, cb) => { node.startDaemon((err) => { - if (err) { - return cb(err) - } + expect(err).to.not.exist() ipfs = ipfsApi(node.apiAddr) cb() }) From 89a43315f153d9b777a2799107f520f10eb1d43e Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 6 Sep 2017 11:28:03 +0100 Subject: [PATCH 7/8] what a beautiful dragon to be slayed --- src/daemon.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/daemon.js b/src/daemon.js index 3337a12f..3e7c5851 100644 --- a/src/daemon.js +++ b/src/daemon.js @@ -243,14 +243,14 @@ class Node { } }, data: (data) => { - output += String(data).trim() + output += String(data) const apiMatch = want.api - ? output.match(/API server listening on (.*)/) + ? output.trim().match(/API server listening on (.*)/) : true const gwMatch = want.gateway - ? output.match(/Gateway (.*) listening on (.*)/) + ? output.trim().match(/Gateway (.*) listening on (.*)/) : true if (apiMatch && gwMatch && !returned) { @@ -270,8 +270,6 @@ class Node { } callback(null, this.api) - } else { - console.log('bit more') } } }) From cce06743eceedfc6261624381617e074d4aa30d5 Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 6 Sep 2017 13:00:26 +0100 Subject: [PATCH 8/8] apply cr --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 7dc7e6ba..9e4fb0c3 100644 --- a/src/index.js +++ b/src/index.js @@ -94,7 +94,7 @@ const IpfsDaemonController = { }, /** - * Create a new disposable node and already start the daemon. + * Create a new disposable node and already started the daemon. * * @memberof IpfsDaemonController * @param {Object} [opts={}]