From 86ea629ac7bbd0a9b249471d1f0841da21bb2224 Mon Sep 17 00:00:00 2001 From: Alex Potsides Date: Thu, 2 Jul 2020 14:47:25 +0100 Subject: [PATCH] fix: optional arguments go in the options object (#3118) We have a few older APIs that take multiple optional arguments, which makes our code more complicated as it has to guess the users' intent, sometimes by inspecting properties on the passed args to see if they happen to correspond with properties on the actual options object. The options object was recently added to all API calls and is the right place for optional arguments to go, so the change here is to move all optional arguments into the options object, except where the presence of an optional argument dramatically changes the behaviour of the call (`ipfs.bootstrap` I'm mostly looking at you), in which case the methods are split out into multiple versions that do distinct things. Only the programatic API is affected, the CLI and HTTP APIs do not change. BREAKING CHANGES: - `ipfs.bitswap.wantlist([peer], [options])` is split into: - `ipfs.bitswap.wantlist([options])` - `ipfs.bitswap.wantlistForPeer(peer, [options])` - `ipfs.bootstrap.add([addr], [options])` is split into: - `ipfs.bootstrap.add(addr, [options])` - add a bootstrap node - `ipfs.bootstrap.reset()` - restore the default list of bootstrap nodes - `ipfs.bootstrap.rm([addr], [options])` is split into: - `ipfs.bootstrap.rm(addr, [options])` - remove a bootstrap node - `ipfs.bootstrap.clear([options])` - empty the bootstrap list - `ipfs.dag.get(cid, [path], [options])` becomes `ipfs.dag.get(cid, [options])` - `path` is moved into the `options` object - `ipfs.dag.tree(cid, [path], [options])` becomes `ipfs.dag.tree(cid, [options])` - `path` is moved into the `options` object - `ipfs.dag.resolve(cid, [path], [options])` becomes `ipfs.dag.resolve(cid, [options])` - `path` is moved into the `options` object - `ipfs.files.flush([path], [options])` becomes `ipfs.files.flush(path, [options])` - `ipfs.files.ls([path], [options])` becomes `ipfs.files.ls(path, [options])` - `ipfs.object.new([template], [options])` becomes `ipfs.object.new([options])` - `template` is moved into the `options` object - `ipfs.pin.ls([paths], [options])` becomes `ipfs.pin.ls([options])` - `paths` is moved into the `options` object Co-authored-by: Hugo Dias --- README.md | 2 +- src/bitswap/index.js | 1 + src/bitswap/wantlist-for-peer.js | 23 +++++++++++++++++++++++ src/bitswap/wantlist.js | 11 +---------- src/bootstrap/clear.js | 20 ++++++++++++++++++++ src/bootstrap/index.js | 2 ++ src/bootstrap/reset.js | 20 ++++++++++++++++++++ src/dag/get.js | 9 ++------- src/dag/resolve.js | 9 ++------- src/files/flush.js | 5 ++--- src/files/ls.js | 5 ++--- src/object/new.js | 9 ++------- src/pin/ls.js | 11 ++++------- 13 files changed, 82 insertions(+), 45 deletions(-) create mode 100644 src/bitswap/wantlist-for-peer.js create mode 100644 src/bootstrap/clear.js create mode 100644 src/bootstrap/reset.js diff --git a/README.md b/README.md index 01e5ecc8a..1a9b837ff 100644 --- a/README.md +++ b/README.md @@ -280,7 +280,7 @@ const ipfs = ipfsClient({ host: '1.1.1.1', port: '80', apiPath: '/ipfs/api/v0' } ```javascript const bitswap = require('ipfs-http-client/src/bitswap')('/ip4/127.0.0.1/tcp/5001') -const list = await bitswap.wantlist(key) +const list = await bitswap.wantlist() // ... ``` diff --git a/src/bitswap/index.js b/src/bitswap/index.js index 5c8354498..36d70b7db 100644 --- a/src/bitswap/index.js +++ b/src/bitswap/index.js @@ -2,6 +2,7 @@ module.exports = config => ({ wantlist: require('./wantlist')(config), + wantlistForPeer: require('./wantlist-for-peer')(config), stat: require('./stat')(config), unwant: require('./unwant')(config) }) diff --git a/src/bitswap/wantlist-for-peer.js b/src/bitswap/wantlist-for-peer.js new file mode 100644 index 000000000..b777ee7d6 --- /dev/null +++ b/src/bitswap/wantlist-for-peer.js @@ -0,0 +1,23 @@ +'use strict' + +const CID = require('cids') +const configure = require('../lib/configure') +const toUrlSearchParams = require('../lib/to-url-search-params') + +module.exports = configure(api => { + return async (peerId, options = {}) => { + peerId = typeof peerId === 'string' ? peerId : new CID(peerId).toString() + + const res = await (await api.post('bitswap/wantlist', { + timeout: options.timeout, + signal: options.signal, + searchParams: toUrlSearchParams({ + ...options, + peer: peerId + }), + headers: options.headers + })).json() + + return (res.Keys || []).map(k => new CID(k['/'])) + } +}) diff --git a/src/bitswap/wantlist.js b/src/bitswap/wantlist.js index d8357e7ac..2599c7ed7 100644 --- a/src/bitswap/wantlist.js +++ b/src/bitswap/wantlist.js @@ -5,16 +5,7 @@ const configure = require('../lib/configure') const toUrlSearchParams = require('../lib/to-url-search-params') module.exports = configure(api => { - return async (peer, options = {}) => { - if (peer && (peer.timeout || peer.signal)) { - options = peer - peer = undefined - } - - if (peer) { - options.peer = typeof peer === 'string' ? peer : new CID(peer).toString() - } - + return async (options = {}) => { const res = await (await api.post('bitswap/wantlist', { timeout: options.timeout, signal: options.signal, diff --git a/src/bootstrap/clear.js b/src/bootstrap/clear.js new file mode 100644 index 000000000..99d809142 --- /dev/null +++ b/src/bootstrap/clear.js @@ -0,0 +1,20 @@ +'use strict' + +const configure = require('../lib/configure') +const toUrlSearchParams = require('../lib/to-url-search-params') + +module.exports = configure(api => { + return async (options = {}) => { + const res = await api.post('bootstrap/rm', { + timeout: options.timeout, + signal: options.signal, + searchParams: toUrlSearchParams({ + ...options, + all: true + }), + headers: options.headers + }) + + return res.json() + } +}) diff --git a/src/bootstrap/index.js b/src/bootstrap/index.js index 519a7161e..3a1021d34 100644 --- a/src/bootstrap/index.js +++ b/src/bootstrap/index.js @@ -2,6 +2,8 @@ module.exports = config => ({ add: require('./add')(config), + clear: require('./clear')(config), rm: require('./rm')(config), + reset: require('./reset')(config), list: require('./list')(config) }) diff --git a/src/bootstrap/reset.js b/src/bootstrap/reset.js new file mode 100644 index 000000000..9332aafc7 --- /dev/null +++ b/src/bootstrap/reset.js @@ -0,0 +1,20 @@ +'use strict' + +const configure = require('../lib/configure') +const toUrlSearchParams = require('../lib/to-url-search-params') + +module.exports = configure(api => { + return async (options = {}) => { + const res = await api.post('bootstrap/add', { + timeout: options.timeout, + signal: options.signal, + searchParams: toUrlSearchParams({ + ...options, + default: true + }), + headers: options.headers + }) + + return res.json() + } +}) diff --git a/src/dag/get.js b/src/dag/get.js index 2875aea8f..cbc8e76f3 100644 --- a/src/dag/get.js +++ b/src/dag/get.js @@ -15,13 +15,8 @@ module.exports = configure((api, options) => { const getBlock = require('../block/get')(options) const dagResolve = require('./resolve')(options) - return async (cid, path, options = {}) => { - if (path && typeof path === 'object') { - options = path - path = null - } - - const resolved = await dagResolve(cid, path, options) + return async (cid, options = {}) => { + const resolved = await dagResolve(cid, options) const block = await getBlock(resolved.cid, options) const dagResolver = resolvers[block.cid.codec] diff --git a/src/dag/resolve.js b/src/dag/resolve.js index acced6a42..ba8c90384 100644 --- a/src/dag/resolve.js +++ b/src/dag/resolve.js @@ -5,17 +5,12 @@ const configure = require('../lib/configure') const toUrlSearchParams = require('../lib/to-url-search-params') module.exports = configure(api => { - return async (cid, path, options = {}) => { - if (path && typeof path === 'object') { - options = path - path = null - } - + return async (cid, options = {}) => { const res = await api.post('dag/resolve', { timeout: options.timeout, signal: options.signal, searchParams: toUrlSearchParams({ - arg: path ? [cid, path].join(path.startsWith('/') ? '' : '/') : `${cid}`, + arg: `${cid}${options.path ? `/${options.path}`.replace(/\/[/]+/g, '/') : ''}`, ...options }), headers: options.headers diff --git a/src/files/flush.js b/src/files/flush.js index cb97a6d98..d9934be9a 100644 --- a/src/files/flush.js +++ b/src/files/flush.js @@ -6,9 +6,8 @@ const toUrlSearchParams = require('../lib/to-url-search-params') module.exports = configure(api => { return async (path, options = {}) => { - if (typeof path !== 'string') { - options = path || {} - path = '/' + if (!path || typeof path !== 'string') { + throw new Error('ipfs.files.flush requires a path') } const res = await api.post('files/flush', { diff --git a/src/files/ls.js b/src/files/ls.js index 2e22ddcdb..2420955e7 100644 --- a/src/files/ls.js +++ b/src/files/ls.js @@ -7,9 +7,8 @@ const toUrlSearchParams = require('../lib/to-url-search-params') module.exports = configure(api => { return async function * ls (path, options = {}) { - if (typeof path !== 'string') { - options = path || {} - path = '/' + if (!path || typeof path !== 'string') { + throw new Error('ipfs.files.ls requires a path') } const res = await api.post('files/ls', { diff --git a/src/object/new.js b/src/object/new.js index 6b3bb04eb..f0ae4e557 100644 --- a/src/object/new.js +++ b/src/object/new.js @@ -5,17 +5,12 @@ const configure = require('../lib/configure') const toUrlSearchParams = require('../lib/to-url-search-params') module.exports = configure(api => { - return async (template, options = {}) => { - if (typeof template !== 'string') { - options = template || {} - template = null - } - + return async (options = {}) => { const res = await api.post('object/new', { timeout: options.timeout, signal: options.signal, searchParams: toUrlSearchParams({ - arg: template, + arg: options.template, ...options }), headers: options.headers diff --git a/src/pin/ls.js b/src/pin/ls.js index 5fdc17386..5133e3a56 100644 --- a/src/pin/ls.js +++ b/src/pin/ls.js @@ -5,20 +5,17 @@ const configure = require('../lib/configure') const toUrlSearchParams = require('../lib/to-url-search-params') module.exports = configure(api => { - return async function * ls (path, options = {}) { - if (path && (path.type || path.timeout)) { - options = path || {} - path = [] + return async function * ls (options = {}) { + if (options.paths) { + options.paths = Array.isArray(options.paths) ? options.paths : [options.paths] } - path = Array.isArray(path) ? path : [path] - const res = await api.post('pin/ls', { timeout: options.timeout, signal: options.signal, searchParams: toUrlSearchParams({ - arg: path.map(p => `${p}`), ...options, + arg: (options.paths || []).map(path => `${path}`), stream: true }), headers: options.headers