Skip to content

Commit

Permalink
feat: stop using ipfs-repo
Browse files Browse the repository at this point in the history
* feat: dont call spawn cb more than once on error

* fix: add flag to signal callback was already called

* docs: adding TODO

* chore: fix deps semvers

* test: skip inProc for add-retrieve until js-ipfs 0-29

* feat: let js-ipfs create a repo for itself, no need to import ipfs-repo

* Revert "feat: let js-ipfs create a repo for itself, no need to import ipfs-repo"

This reverts commit 42597c1.

* fix: use once to call cb once

* test: skip in proc tests untill js-ipfs 0.29 release is out

* chore: use 8.11.1 for circle

* feat: detect inited repo, remove ipfs-repo

* fix: remove TODOs
  • Loading branch information
dryajov authored and daviddias committed May 17, 2018
1 parent 46416ff commit ca44996
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 115 deletions.
2 changes: 1 addition & 1 deletion circle.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Warning: This file is automatically synced from https://github.com/ipfs/ci-sync so if you want to change it, please change it there and ask someone to sync all repositories.
machine:
node:
version: stable
version: 8.11.1

test:
post:
Expand Down
16 changes: 6 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"joi": false,
"stream": "readable-stream",
"http": "stream-http",
"./src/utils/repo/create-nodejs.js": "./src/utils/repo/create-browser.js",
"./src/utils/repo/nodejs.js": "./src/utils/repo/browser.js",
"./src/utils/exec.js": false,
"./src/utils/find-ipfs-executable.js": false,
"./src/utils/tmp-dir.js": "./src/utils/tmp-dir-browser.js",
Expand Down Expand Up @@ -83,21 +83,21 @@
"boom": "^7.2.0",
"debug": "^3.1.0",
"detect-node": "^2.0.3",
"dexie": "^1.5.1",
"hapi": "^16.6.2",
"hat": "0.0.3",
"ipfs-api": "^21.0.0",
"ipfs-repo": "^0.19.0",
"joi": "^13.1.2",
"lodash.clone": "^4.5.0",
"lodash.defaults": "^4.2.0",
"lodash.defaultsdeep": "^4.6.0",
"multiaddr": "^4.0.0",
"multiaddr": "^5.0.0",
"once": "^1.4.0",
"readable-stream": "^2.3.6",
"rimraf": "^2.6.2",
"safe-json-parse": "^4.0.0",
"safe-json-stringify": "^1.1.0",
"shutdown": "^0.3.0",
"shutdown": "~0.3.0",
"stream-http": "^2.8.1",
"subcomandante": "^1.0.5",
"superagent": "^3.8.2",
Expand All @@ -115,7 +115,7 @@
"mkdirp": "^0.5.1",
"proxyquire": "^2.0.1",
"sinon": "^4.5.0",
"superagent-mocker": "^0.5.2",
"superagent-mocker": "~0.5.2",
"supertest": "^3.0.0"
},
"repository": {
Expand All @@ -125,9 +125,5 @@
"bugs": {
"url": "https://github.com/ipfs/js-ipfsd-ctl/issues"
},
"homepage": "https://github.com/ipfs/js-ipfsd-ctl",
"directories": {
"example": "examples",
"test": "test"
}
"homepage": "https://github.com/ipfs/js-ipfsd-ctl"
}
19 changes: 13 additions & 6 deletions src/factory-in-proc.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const clone = require('lodash.clone')
const series = require('async/series')
const path = require('path')
const tmpDir = require('./utils/tmp-dir')
const once = require('once')
const repoUtils = require('./utils/repo/nodejs')

const Node = require('./ipfsd-in-proc')
const defaultConfig = require('./defaults/config')
Expand Down Expand Up @@ -130,14 +132,19 @@ class FactoryInProc {
}

const node = new Node(options)
node.once('error', err => callback(err, node))
const callbackOnce = once((err) => {
if (err) {
return callback(err)
}
callback(null, node)
})
node.once('error', callbackOnce)

series([
(cb) => node.once('ready', cb),
(cb) => node.repo._isInitialized(err => {
// if err exists, repo failed to find config or the ipfs-repo package
// version is different than that of the existing repo.
node.initialized = !err
(cb) => repoUtils.repoExists(node.path, (err, initialized) => {
if (err) { return cb(err) }
node.initialized = initialized
cb()
}),
(cb) => options.init
Expand All @@ -146,7 +153,7 @@ class FactoryInProc {
(cb) => options.start
? node.start(options.args, cb)
: cb()
], (err) => callback(err, node))
], callbackOnce)
}
}

Expand Down
13 changes: 8 additions & 5 deletions src/ipfsd-in-proc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const multiaddr = require('multiaddr')
const defaultsDeep = require('lodash.defaultsdeep')
const createRepo = require('./utils/repo/create-nodejs')
const repoUtils = require('./utils/repo/nodejs')
const defaults = require('lodash.defaults')
const waterfall = require('async/waterfall')
const debug = require('debug')
Expand Down Expand Up @@ -30,14 +30,14 @@ class Node extends EventEmitter {
IPFS = this.opts.exec

this.opts.args = this.opts.args || []
this.path = this.opts.repoPath
this.repo = createRepo(this.path)
this.path = this.opts.repoPath || repoUtils.createTempRepoPath()
this.disposable = this.opts.disposable
this.clean = true
this._apiAddr = null
this._gatewayAddr = null
this._started = false
this.api = null
this.initialized = false
this.bits = this.opts.initOptions ? this.opts.initOptions.bits : null

this.opts.EXPERIMENTAL = defaultsDeep({}, opts.EXPERIMENTAL, {
Expand All @@ -64,14 +64,16 @@ class Node extends EventEmitter {
})

this.exec = new IPFS({
repo: this.repo,
repo: this.path,
init: false,
start: false,
pass: this.opts.pass,
EXPERIMENTAL: this.opts.EXPERIMENTAL,
libp2p: this.opts.libp2p
})

// TODO: should this be wrapped in a process.nextTick(), for context:
// https://nodejs.org/en/docs/guides/event-loop-timers-and-nexttick/#why-use-process-nexttick
this.exec.once('error', err => this.emit('error', err))
this.exec.once('ready', () => this.emit('ready'))
}
Expand Down Expand Up @@ -176,7 +178,8 @@ class Node extends EventEmitter {
return callback()
}

this.repo.teardown(callback)
repoUtils.removeRepo(this.path)
callback()
}

/**
Expand Down
24 changes: 24 additions & 0 deletions src/utils/repo/browser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* global self */
'use strict'

const hat = require('hat')
const Dexie = require('dexie')

exports.createTempRepoPath = function createTempPathRepo () {
return '/ipfs-' + hat()
}

exports.removeRepo = function removeRepo (repoPath) {
Dexie.delete(repoPath)
}

exports.repoExists = function repoExists (repoPath, cb) {
const db = new Dexie(repoPath)
db.open(repoPath)
.then((store) => {
const table = store.table(repoPath)
return table
.count((cnt) => cb(null, cnt > 0))
.catch(cb)
}).catch(cb)
}
28 changes: 0 additions & 28 deletions src/utils/repo/create-browser.js

This file was deleted.

41 changes: 0 additions & 41 deletions src/utils/repo/create-nodejs.js

This file was deleted.

29 changes: 29 additions & 0 deletions src/utils/repo/nodejs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict'

const os = require('os')
const path = require('path')
const hat = require('hat')
const rimraf = require('rimraf')
const fs = require('fs')

exports.removeRepo = function removeRepo (dir) {
try {
fs.accessSync(dir)
} catch (err) {
// Does not exist so all good
return
}

return rimraf.sync(dir)
}

exports.createTempRepoPath = function createTempRepo () {
return path.join(os.tmpdir(), '/ipfs-test-' + hat())
}

exports.repoExists = function (repoPath, cb) {
fs.access(`${repoPath}/config`, (err) => {
if (err) { return cb(null, false) }
cb(null, true)
})
}
33 changes: 32 additions & 1 deletion test/spawn-options.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)

const fs = require('fs')
const isNode = require('detect-node')
const hat = require('hat')
const IPFSFactory = require('../src')
const JSIPFS = require('ipfs')
const once = require('once')

const tests = [
{ type: 'go', bits: 1024 },
Expand Down Expand Up @@ -160,7 +162,6 @@ describe('Spawn options', function () {
})
})

// TODO ?? What is this test trying to prove?
describe('spawn a node and attach api', () => {
let ipfsd

Expand Down Expand Up @@ -440,5 +441,35 @@ describe('Spawn options', function () {
})
})
})

describe(`don't callback twice on error`, () => {
if (fOpts.type !== 'proc') { return }
it('spawn with error', (done) => {
this.timeout(20 * 1000)
// `once.strict` should throw if its called more than once
const callback = once.strict((err, ipfsd) => {
if (err) { return done(err) }

ipfsd.once('error', () => {}) // avoid EventEmitter throws

// Do an operation, just to make sure we're working
ipfsd.api.id((err) => {
if (err) {
return done(err)
}

// Do something to make stopping fail
ipfsd.exec._repo.close((err) => {
if (err) { return done(err) }
ipfsd.stop((err) => {
expect(err).to.exist()
done()
})
})
})
})
f.spawn(callback)
})
})
}))
})
23 changes: 0 additions & 23 deletions test/utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ const path = require('path')
const flatten = require('../src/utils/flatten')
const tempDir = require('../src/utils/tmp-dir')
const findIpfsExecutable = require('../src/utils/find-ipfs-executable')
const createRepo = require('../src/utils/repo/create-nodejs')

const IPFSRepo = require('ipfs-repo')

describe('utils', () => {
describe('.flatten', () => {
Expand Down Expand Up @@ -63,25 +60,5 @@ describe('utils', () => {
expect(fs.existsSync(execPath)).to.be.ok()
})
})

describe('.createRepo', () => {
let repo = null
let repoPath = tempDir()

it('should create repo', () => {
repo = createRepo(repoPath)
expect(repo).to.exist()
expect(repo).to.be.instanceOf(IPFSRepo)
expect(fs.existsSync(repoPath)).to.be.ok()
})

it('should cleanup repo', (done) => {
repo.teardown((err) => {
expect(err).to.not.exist()
expect(!fs.existsSync(repoPath)).to.be.ok()
done()
})
})
})
}
})

0 comments on commit ca44996

Please sign in to comment.