Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Commit

Permalink
fix: allow null/undefined options (#1581)
Browse files Browse the repository at this point in the history
Options should be optional! Our API should be flexible enough to allow passing null or undefined in place of an options object. This PR fixes cases where we assume an options object has been passed.

fixes: #1574
  • Loading branch information
Alan Shaw authored Sep 19, 2018
1 parent 88c735a commit c73bd2f
Show file tree
Hide file tree
Showing 15 changed files with 211 additions and 9 deletions.
4 changes: 4 additions & 0 deletions src/core/components/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ module.exports = function block (self) {
options = {}
}

options = options || {}

if (Array.isArray(block)) {
return callback(new Error('Array is not supported'))
}
Expand Down Expand Up @@ -93,6 +95,8 @@ module.exports = function block (self) {
options = {}
}

options = options || {}

try {
cid = cleanCid(cid)
} catch (err) {
Expand Down
9 changes: 6 additions & 3 deletions src/core/components/dag.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@ module.exports = function dag (self) {
if (typeof options === 'function') {
callback = options
options = {}
} else if (options && options.cid && (options.format || options.hashAlg)) {
}

options = options || {}

if (options.cid && (options.format || options.hashAlg)) {
return callback(new Error('Can\'t put dag node. Please provide either `cid` OR `format` and `hashAlg` options.'))
} else if (options && ((options.format && !options.hashAlg) || (!options.format && options.hashAlg))) {
} else if (((options.format && !options.hashAlg) || (!options.format && options.hashAlg))) {
return callback(new Error('Can\'t put dag node. Please provide `format` AND `hashAlg` options.'))
}
options = options || {}

const optionDefaults = {
format: 'dag-cbor',
Expand Down
4 changes: 4 additions & 0 deletions src/core/components/dht.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ module.exports = (self) => {
options = {}
}

options = options || {}

self._libp2pNode.dht.get(key, options.timeout, callback)
}),

Expand Down Expand Up @@ -134,6 +136,8 @@ module.exports = (self) => {
options = {}
}

options = options || {}

// ensure blocks are actually local
every(keys, (key, cb) => {
self._repo.blocks.has(key, cb)
Expand Down
2 changes: 2 additions & 0 deletions src/core/components/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ module.exports = () => {
opts = {}
}

opts = opts || {}

dns(domain, opts, callback)
})
}
6 changes: 3 additions & 3 deletions src/core/components/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,14 @@ module.exports = function files (self) {

return {
add: (() => {
const add = promisify((data, options = {}, callback) => {
const add = promisify((data, options, callback) => {
if (typeof options === 'function') {
callback = options
options = {}
} else if (!callback || typeof callback !== 'function') {
callback = noop
}

options = options || {}

const ok = Buffer.isBuffer(data) ||
isStream.readable(data) ||
Array.isArray(data) ||
Expand Down
1 change: 1 addition & 0 deletions src/core/components/key.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const promisify = require('promisify-es6')
module.exports = function key (self) {
return {
gen: promisify((name, opts, callback) => {
opts = opts || {}
self._keychain.createKey(name, opts.type, opts.size, callback)
}),

Expand Down
6 changes: 6 additions & 0 deletions src/core/components/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ module.exports = function object (self) {
options = {}
}

options = options || {}

waterfall([
(cb) => {
self.object.get(multihash, options, cb)
Expand Down Expand Up @@ -151,6 +153,8 @@ module.exports = function object (self) {
options = {}
}

options = options || {}

const encoding = options.enc
let node

Expand Down Expand Up @@ -217,6 +221,8 @@ module.exports = function object (self) {
options = {}
}

options = options || {}

let mh, cid

try {
Expand Down
2 changes: 2 additions & 0 deletions src/core/components/stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ module.exports = function stats (self) {
opts = {}
}

opts = opts || {}

bandwidthStats(self, opts)
.then((stats) => callback(null, stats))
.catch((err) => callback(err))
Expand Down
2 changes: 2 additions & 0 deletions src/core/components/swarm.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ module.exports = function swarm (self) {
opts = {}
}

opts = opts || {}

if (!self.isOnline()) {
return callback(new Error(OFFLINE_ERROR))
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/components/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = function version (self) {

self.repo.version((err, repoVersion) => {
if (err) {
callback(err)
return callback(err)
}

callback(null, {
Expand Down
21 changes: 21 additions & 0 deletions test/core/block.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)
const hat = require('hat')

const IPFSFactory = require('ipfsd-ctl')
const IPFS = require('../../src/core')
Expand Down Expand Up @@ -47,6 +48,15 @@ describe('block', () => {
})
})

describe('put', () => {
it('should not error when passed null options', (done) => {
ipfs.block.put(Buffer.from(hat()), null, (err) => {
expect(err).to.not.exist()
done()
})
})
})

describe('rm', () => {
it('should callback with error for invalid CID input', (done) => {
ipfs.block.rm('INVALID CID', (err) => {
Expand All @@ -65,5 +75,16 @@ describe('block', () => {
done()
})
})

it('should not error when passed null options', (done) => {
ipfs.block.put(Buffer.from(hat()), (err, block) => {
expect(err).to.not.exist()

ipfs.block.stat(block.cid, null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})
})
11 changes: 10 additions & 1 deletion test/core/files.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)

const hat = require('hat')
const pull = require('pull-stream')
const IPFSFactory = require('ipfsd-ctl')
const IPFS = require('../../src/core')
Expand Down Expand Up @@ -75,4 +75,13 @@ describe('files', () => {
)
})
})

describe('add', () => {
it('should not error when passed null options', (done) => {
ipfs.files.add(Buffer.from(hat()), null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})
93 changes: 92 additions & 1 deletion test/core/object.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)

const hat = require('hat')
const IPFSFactory = require('ipfsd-ctl')
const auto = require('async/auto')
const IPFS = require('../../src/core')

describe('object', () => {
Expand Down Expand Up @@ -45,6 +46,17 @@ describe('object', () => {
done()
})
})

it('should not error when passed null options', (done) => {
ipfs.object.put(Buffer.from(hat()), (err, dagNode) => {
expect(err).to.not.exist()

ipfs.object.get(dagNode.multihash, null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})

describe('put', () => {
Expand All @@ -55,5 +67,84 @@ describe('object', () => {
done()
})
})

it('should not error when passed null options', (done) => {
ipfs.object.put(Buffer.from(hat()), null, (err) => {
expect(err).to.not.exist()
done()
})
})
})

describe('patch.addLink', () => {
it('should not error when passed null options', (done) => {
auto({
a: (cb) => ipfs.object.put(Buffer.from(hat()), cb),
b: (cb) => ipfs.object.put(Buffer.from(hat()), cb)
}, (err, nodes) => {
expect(err).to.not.exist()

const link = {
name: nodes.b.name,
multihash: nodes.b.multihash,
size: nodes.b.size
}

ipfs.object.patch.addLink(nodes.a.multihash, link, null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})

describe('patch.rmLink', () => {
it('should not error when passed null options', (done) => {
auto({
nodeA: (cb) => ipfs.object.put(Buffer.from(hat()), cb),
nodeB: (cb) => ipfs.object.put(Buffer.from(hat()), cb),
nodeAWithLink: ['nodeA', 'nodeB', (res, cb) => {
ipfs.object.patch.addLink(res.nodeA.multihash, {
name: res.nodeB.name,
multihash: res.nodeB.multihash,
size: res.nodeB.size
}, cb)
}]
}, (err, res) => {
expect(err).to.not.exist()

const link = res.nodeAWithLink.links[0]
ipfs.object.patch.rmLink(res.nodeAWithLink.multihash, link, null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})

describe('patch.appendData', () => {
it('should not error when passed null options', (done) => {
ipfs.object.put(Buffer.from(hat()), null, (err, dagNode) => {
expect(err).to.not.exist()

ipfs.object.patch.appendData(dagNode.multihash, Buffer.from(hat()), null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})

describe('patch.setData', () => {
it('should not error when passed null options', (done) => {
ipfs.object.put(Buffer.from(hat()), null, (err, dagNode) => {
expect(err).to.not.exist()

ipfs.object.patch.setData(dagNode.multihash, Buffer.from(hat()), null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})
})
9 changes: 9 additions & 0 deletions test/core/stats.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,13 @@ describe('stats', () => {
)
})
})

describe('bw', () => {
it('should not error when passed null options', (done) => {
ipfs.stats.bw(null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})
48 changes: 48 additions & 0 deletions test/core/swarm.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/* eslint max-nested-callbacks: ["error", 8] */
/* eslint-env mocha */
'use strict'

const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)

const IPFSFactory = require('ipfsd-ctl')
const IPFS = require('../../src/core')

describe('swarm', () => {
let ipfsd, ipfs

before(function (done) {
this.timeout(20 * 1000)

const factory = IPFSFactory.create({ type: 'proc' })

factory.spawn({
exec: IPFS,
initOptions: { bits: 512 }
}, (err, _ipfsd) => {
expect(err).to.not.exist()
ipfsd = _ipfsd
ipfs = _ipfsd.api
done()
})
})

after((done) => {
if (ipfsd) {
ipfsd.stop(done)
} else {
done()
}
})

describe('peers', () => {
it('should not error when passed null options', (done) => {
ipfs.swarm.peers(null, (err) => {
expect(err).to.not.exist()
done()
})
})
})
})

0 comments on commit c73bd2f

Please sign in to comment.