From 69366b9ede2ad9a20081c9c1082aa0436a584e0a Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sat, 5 Oct 2019 07:34:06 +0100 Subject: [PATCH 01/10] feat: support block.rm over http api Also, validate that a block is not pinned before removing it. --- package.json | 2 +- src/cli/commands/block/rm.js | 39 ++++++++++---- src/core/components/block.js | 94 +++++++++++++++++++++++---------- src/http/api/resources/block.js | 62 ++++++++++++++++++---- src/http/api/routes/block.js | 5 +- test/cli/block.js | 2 +- 6 files changed, 151 insertions(+), 53 deletions(-) diff --git a/package.json b/package.json index a32cd13d41..809e5a247b 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "ipfs-bitswap": "^0.26.0", "ipfs-block": "~0.8.1", "ipfs-block-service": "~0.16.0", - "ipfs-http-client": "^38.1.0", + "js-ipfs-http-client": "ipfs/js-ipfs-http-client#5ec138edfd18a239b763ac13d35adbe4ed55fc6f", "ipfs-http-response": "~0.3.1", "ipfs-mfs": "^0.13.0", "ipfs-multipart": "^0.2.0", diff --git a/src/cli/commands/block/rm.js b/src/cli/commands/block/rm.js index e7640ec83d..4b55f0e6fc 100644 --- a/src/cli/commands/block/rm.js +++ b/src/cli/commands/block/rm.js @@ -1,22 +1,41 @@ 'use strict' module.exports = { - command: 'rm ', + command: 'rm ', - describe: 'Remove a raw IPFS block', + describe: 'Remove IPFS block(s)', - builder: {}, + builder: { + force: { + alias: 'f', + describe: 'Ignore nonexistent blocks', + type: 'boolean', + default: false + }, + quiet: { + alias: 'q', + describe: 'Write minimal output', + type: 'boolean', + default: false + } + }, - handler ({ getIpfs, print, isDaemonOn, key, resolve }) { + handler ({ getIpfs, print, hash, resolve }) { resolve((async () => { - if (isDaemonOn()) { - // TODO implement this once `js-ipfs-http-client` supports it - throw new Error('rm block with daemon running is not yet implemented') + const ipfs = await getIpfs() + let errored = false + + for await (const result of ipfs.block._rmAsyncIterator(hash)) { + if (result.error) { + errored = true + } + + print(result.error || 'removed ' + result.hash) } - const ipfs = await getIpfs() - await ipfs.block.rm(key) - print('removed ' + key) + if (errored) { + print('Error: some blocks not removed') + } })()) } } diff --git a/src/core/components/block.js b/src/core/components/block.js index 205dcedcdc..c2f600d042 100644 --- a/src/core/components/block.js +++ b/src/core/components/block.js @@ -5,17 +5,67 @@ const multihashing = require('multihashing-async') const CID = require('cids') const callbackify = require('callbackify') const errCode = require('err-code') +const all = require('async-iterator-all') +const { PinTypes } = require('./pin/pin-manager') module.exports = function block (self) { - return { - get: callbackify.variadic(async (cid, options) => { // eslint-disable-line require-await - options = options || {} + async function * rmAsyncIterator (cids, options) { + options = options || {} - try { + if (!Array.isArray(cids)) { + cids = [] + } + + // We need to take a write lock here to ensure that adding and removing + // blocks are exclusive operations + const release = await self._gcLock.writeLock() + + try { + for (let cid of cids) { cid = cleanCid(cid) - } catch (err) { - throw errCode(err, 'ERR_INVALID_CID') + + const result = { + hash: cid.toString() + } + + try { + const pinResult = await self.pin.pinManager.isPinnedWithType(cid, PinTypes.all) + + if (pinResult.pinned) { + if (CID.isCID(pinResult.reason)) { // eslint-disable-line max-depth + throw errCode(new Error(`pinned via ${pinResult.reason}`)) + } + + throw errCode(new Error(`pinned: ${pinResult.reason}`)) + } + + // remove has check when https://github.com/ipfs/js-ipfs-block-service/pull/88 is merged + const has = await self._blockService._repo.blocks.has(cid) + + if (!has) { + throw errCode(new Error('block not found'), 'ERR_BLOCK_NOT_FOUND') + } + + await self._blockService.delete(cid) + } catch (err) { + if (!options.force) { + result.error = `cannot remove ${cid}: ${err.message}` + } + } + + if (!options.quiet) { + yield result + } } + } finally { + release() + } + } + + return { + get: callbackify.variadic(async (cid, options) => { // eslint-disable-line require-await + options = options || {} + cid = cleanCid(cid) if (options.preload !== false) { self._preload(cid) @@ -66,31 +116,13 @@ module.exports = function block (self) { release() } }), - rm: callbackify(async (cid) => { - try { - cid = cleanCid(cid) - } catch (err) { - throw errCode(err, 'ERR_INVALID_CID') - } - - // We need to take a write lock here to ensure that adding and removing - // blocks are exclusive operations - const release = await self._gcLock.writeLock() - - try { - await self._blockService.delete(cid) - } finally { - release() - } + rm: callbackify.variadic(async (cids, options) => { // eslint-disable-line require-await + return all(rmAsyncIterator(cids, options)) }), + _rmAsyncIterator: rmAsyncIterator, stat: callbackify.variadic(async (cid, options) => { options = options || {} - - try { - cid = cleanCid(cid) - } catch (err) { - throw errCode(err, 'ERR_INVALID_CID') - } + cid = cleanCid(cid) if (options.preload !== false) { self._preload(cid) @@ -112,5 +144,9 @@ function cleanCid (cid) { } // CID constructor knows how to do the cleaning :) - return new CID(cid) + try { + return new CID(cid) + } catch (err) { + throw errCode(err, 'ERR_INVALID_CID') + } } diff --git a/src/http/api/resources/block.js b/src/http/api/resources/block.js index d72f4b78bc..e03a23c8e4 100644 --- a/src/http/api/resources/block.js +++ b/src/http/api/resources/block.js @@ -8,6 +8,7 @@ const Boom = require('@hapi/boom') const { cidToString } = require('../../../utils/cid') const debug = require('debug') const all = require('async-iterator-all') +const { PassThrough } = require('readable-stream') const log = debug('ipfs:http-api:block') log.error = debug('ipfs:http-api:block:error') @@ -102,20 +103,61 @@ exports.put = { } exports.rm = { - // uses common parseKey method that returns a `key` - parseArgs: exports.parseKey, + validate: { + query: Joi.object().keys({ + arg: Joi.array().items(Joi.string()).single().required(), + force: Joi.boolean().default(false), + quiet: Joi.boolean().default(false) + }).unknown() + }, - // main route handler which is called after the above `parseArgs`, but only if the args were valid - async handler (request, h) { - const { key } = request.pre.args + parseArgs: (request, h) => { + let { arg } = request.query + arg = arg.map(thing => new CID(thing)) - try { - await request.server.app.ipfs.block.rm(key) - } catch (err) { - throw Boom.boomify(err, { message: 'Failed to delete block' }) + return { + ...request.query, + arg } + }, + + // main route handler which is called after the above `parseArgs`, but only if the args were valid + handler (request, h) { + const { arg, force, quiet } = request.pre.args + const output = new PassThrough() + + Promise.resolve() + .then(async () => { + try { + for await (const result of request.server.app.ipfs.block._rmAsyncIterator(arg, { + force, + quiet + })) { + output.write(JSON.stringify({ + Hash: result.hash, + Error: result.error + }) + '\n') + } + } catch (err) { + throw Boom.boomify(err, { message: 'Failed to delete block' }) + } + }) + .catch(err => { + request.raw.res.addTrailers({ + 'X-Stream-Error': JSON.stringify({ + Message: err.message, + Code: 0 + }) + }) + }) + .then(() => { + output.end() + }) - return h.response() + return h.response(output) + .header('x-chunked-output', '1') + .header('content-type', 'application/json') + .header('Trailer', 'X-Stream-Error') } } diff --git a/src/http/api/routes/block.js b/src/http/api/routes/block.js index 1e25b98543..ae45ce8bed 100644 --- a/src/http/api/routes/block.js +++ b/src/http/api/routes/block.js @@ -31,10 +31,11 @@ module.exports = [ { method: '*', path: '/api/v0/block/rm', - config: { + options: { pre: [ { method: resources.block.rm.parseArgs, assign: 'args' } - ] + ], + validate: resources.block.rm.validate }, handler: resources.block.rm.handler }, diff --git a/test/cli/block.js b/test/cli/block.js index ee0d47b86f..09036fde1f 100644 --- a/test/cli/block.js +++ b/test/cli/block.js @@ -72,7 +72,7 @@ describe('block', () => runOnAndOff((thing) => { ].join('\n') + '\n') }) - it.skip('rm', async function () { + it('rm', async function () { this.timeout(40 * 1000) const out = await ipfs('block rm QmZjTnYw2TFhn9Nn7tjmPSoTBoY7YRkwPzwSrSbabY24Kp') From b284518b9d5ae59165677135d56bb86553a18619 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sat, 5 Oct 2019 10:24:11 +0100 Subject: [PATCH 02/10] chore: skip config endpoint tests --- test/core/interface.spec.js | 7 +------ test/http-api/interface.js | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/test/core/interface.spec.js b/test/core/interface.spec.js index 5ac674782c..0ffe937ab1 100644 --- a/test/core/interface.spec.js +++ b/test/core/interface.spec.js @@ -12,12 +12,7 @@ describe('interface-ipfs-core tests', function () { tests.bitswap(defaultCommonFactory, { skip: !isNode }) - tests.block(defaultCommonFactory, { - skip: [{ - name: 'rm', - reason: 'Not implemented' - }] - }) + tests.block(defaultCommonFactory) tests.bootstrap(defaultCommonFactory) diff --git a/test/http-api/interface.js b/test/http-api/interface.js index 7469be1564..c73a5d0c6e 100644 --- a/test/http-api/interface.js +++ b/test/http-api/interface.js @@ -12,12 +12,7 @@ describe('interface-ipfs-core over ipfs-http-client tests', () => { tests.bitswap(defaultCommonFactory) - tests.block(defaultCommonFactory, { - skip: [{ - name: 'rm', - reason: 'Not implemented' - }] - }) + tests.block(defaultCommonFactory) tests.bootstrap(defaultCommonFactory) From c769ea2902ac790f7fa62ecce9fd2baadbca5435 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sat, 5 Oct 2019 12:52:45 +0100 Subject: [PATCH 03/10] chore: fix up interface tests --- src/core/components/block.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/components/block.js b/src/core/components/block.js index c2f600d042..87b00fc52c 100644 --- a/src/core/components/block.js +++ b/src/core/components/block.js @@ -13,7 +13,7 @@ module.exports = function block (self) { options = options || {} if (!Array.isArray(cids)) { - cids = [] + cids = [cids] } // We need to take a write lock here to ensure that adding and removing From ca24f365e1e3b26eb0394c00dca579e3ceb3c75f Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sat, 5 Oct 2019 13:00:45 +0100 Subject: [PATCH 04/10] chore: rev http client --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 809e5a247b..03a082102d 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "ipfs-bitswap": "^0.26.0", "ipfs-block": "~0.8.1", "ipfs-block-service": "~0.16.0", - "js-ipfs-http-client": "ipfs/js-ipfs-http-client#5ec138edfd18a239b763ac13d35adbe4ed55fc6f", + "js-ipfs-http-client": "ipfs/js-ipfs-http-client#f6e648262e51b3f1b54770efa2d170dc7088e982", "ipfs-http-response": "~0.3.1", "ipfs-mfs": "^0.13.0", "ipfs-multipart": "^0.2.0", From 2e613ea7e2acdfa09b2dc83c7b0cfe0439b8995f Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sun, 6 Oct 2019 07:28:41 +0100 Subject: [PATCH 05/10] test: add more cli tests --- src/cli/commands/block/rm.js | 15 ++++++++++----- test/cli/block.js | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/cli/commands/block/rm.js b/src/cli/commands/block/rm.js index 4b55f0e6fc..1f92ed1a06 100644 --- a/src/cli/commands/block/rm.js +++ b/src/cli/commands/block/rm.js @@ -20,21 +20,26 @@ module.exports = { } }, - handler ({ getIpfs, print, hash, resolve }) { + handler ({ getIpfs, print, hash, force, quiet, resolve }) { resolve((async () => { const ipfs = await getIpfs() let errored = false - for await (const result of ipfs.block._rmAsyncIterator(hash)) { + for await (const result of ipfs.block._rmAsyncIterator(hash, { + force, + quiet + })) { if (result.error) { errored = true } - print(result.error || 'removed ' + result.hash) + if (!quiet) { + print(result.error || 'removed ' + result.hash) + } } - if (errored) { - print('Error: some blocks not removed') + if (errored && !quiet) { + throw new Error('some blocks not removed') } })()) } diff --git a/test/cli/block.js b/test/cli/block.js index 09036fde1f..3304c8fcb9 100644 --- a/test/cli/block.js +++ b/test/cli/block.js @@ -75,7 +75,33 @@ describe('block', () => runOnAndOff((thing) => { it('rm', async function () { this.timeout(40 * 1000) + await ipfs('block put test/fixtures/test-data/hello') + const out = await ipfs('block rm QmZjTnYw2TFhn9Nn7tjmPSoTBoY7YRkwPzwSrSbabY24Kp') expect(out).to.eql('removed QmZjTnYw2TFhn9Nn7tjmPSoTBoY7YRkwPzwSrSbabY24Kp\n') }) + + it('rm quietly', async function () { + this.timeout(40 * 1000) + + await ipfs('block put test/fixtures/test-data/hello') + + const out = await ipfs('block rm --quiet QmZjTnYw2TFhn9Nn7tjmPSoTBoY7YRkwPzwSrSbabY24Kp') + expect(out).to.eql('') + }) + + it('rm force', async function () { + this.timeout(40 * 1000) + + const out = await ipfs('block rm --force QmZjTnYw2TFhn9Nn7tjmPSoTBoY7YRkwPzwSrSbabY24Kh') + expect(out).to.eql('removed QmZjTnYw2TFhn9Nn7tjmPSoTBoY7YRkwPzwSrSbabY24Kh\n') + }) + + it('fails to remove non-existent block', async function () { + this.timeout(40 * 1000) + + const out = await ipfs.fail('block rm QmZjTnYw2TFhn9Nn7tjmPSoTBoY7YRkwPzwSrSbabY24Kh') + expect(out.stdout).to.include('block not found') + expect(out.stdout).to.include('some blocks not removed') + }) })) From 53db381bb393fef2e88f5178c883d7b0a0f007a5 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sun, 6 Oct 2019 17:16:18 +0100 Subject: [PATCH 06/10] chore: update deps --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 03a082102d..294f7b9d18 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "ipfs-bitswap": "^0.26.0", "ipfs-block": "~0.8.1", "ipfs-block-service": "~0.16.0", - "js-ipfs-http-client": "ipfs/js-ipfs-http-client#f6e648262e51b3f1b54770efa2d170dc7088e982", + "js-ipfs-http-client": "^38.2.0", "ipfs-http-response": "~0.3.1", "ipfs-mfs": "^0.13.0", "ipfs-multipart": "^0.2.0", From 5f9ff3223021a64daa6785478cdc3da71e96f5f7 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sun, 6 Oct 2019 17:33:25 +0100 Subject: [PATCH 07/10] chore: give dep correct name --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 294f7b9d18..c05c28d201 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "ipfs-bitswap": "^0.26.0", "ipfs-block": "~0.8.1", "ipfs-block-service": "~0.16.0", - "js-ipfs-http-client": "^38.2.0", + "ipfs-http-client": "^38.2.0", "ipfs-http-response": "~0.3.1", "ipfs-mfs": "^0.13.0", "ipfs-multipart": "^0.2.0", From d04ced963742e40b8481f972750e3553c06a7f35 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sun, 6 Oct 2019 17:59:42 +0100 Subject: [PATCH 08/10] chore: fix failing test --- src/http/api/resources/block.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/http/api/resources/block.js b/src/http/api/resources/block.js index e03a23c8e4..57526e61fa 100644 --- a/src/http/api/resources/block.js +++ b/src/http/api/resources/block.js @@ -113,7 +113,12 @@ exports.rm = { parseArgs: (request, h) => { let { arg } = request.query - arg = arg.map(thing => new CID(thing)) + + try { + arg = arg.map(thing => new CID(thing)) + } catch (err) { + throw Boom.badRequest('Not a valid hash') + } return { ...request.query, From a552aa4261356e61de47327a8335cfeedd990bb7 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 7 Oct 2019 17:47:45 +0100 Subject: [PATCH 09/10] chore: address PR comments --- src/http/api/resources/block.js | 50 ++++++++++--------------------- src/http/utils/stream-response.js | 29 ++++++++++++++++++ 2 files changed, 45 insertions(+), 34 deletions(-) create mode 100644 src/http/utils/stream-response.js diff --git a/src/http/api/resources/block.js b/src/http/api/resources/block.js index 57526e61fa..c88b25b15f 100644 --- a/src/http/api/resources/block.js +++ b/src/http/api/resources/block.js @@ -8,7 +8,7 @@ const Boom = require('@hapi/boom') const { cidToString } = require('../../../utils/cid') const debug = require('debug') const all = require('async-iterator-all') -const { PassThrough } = require('readable-stream') +const streamResponse = require('../../utils/stream-response') const log = debug('ipfs:http-api:block') log.error = debug('ipfs:http-api:block:error') @@ -129,40 +129,22 @@ exports.rm = { // main route handler which is called after the above `parseArgs`, but only if the args were valid handler (request, h) { const { arg, force, quiet } = request.pre.args - const output = new PassThrough() - - Promise.resolve() - .then(async () => { - try { - for await (const result of request.server.app.ipfs.block._rmAsyncIterator(arg, { - force, - quiet - })) { - output.write(JSON.stringify({ - Hash: result.hash, - Error: result.error - }) + '\n') - } - } catch (err) { - throw Boom.boomify(err, { message: 'Failed to delete block' }) - } - }) - .catch(err => { - request.raw.res.addTrailers({ - 'X-Stream-Error': JSON.stringify({ - Message: err.message, - Code: 0 - }) - }) - }) - .then(() => { - output.end() - }) - return h.response(output) - .header('x-chunked-output', '1') - .header('content-type', 'application/json') - .header('Trailer', 'X-Stream-Error') + return streamResponse(request, h, async (output) => { + try { + for await (const result of request.server.app.ipfs.block._rmAsyncIterator(arg, { + force, + quiet + })) { + output.write(JSON.stringify({ + Hash: result.hash, + Error: result.error + }) + '\n') + } + } catch (err) { + throw Boom.boomify(err, { message: 'Failed to delete block' }) + } + }) } } diff --git a/src/http/utils/stream-response.js b/src/http/utils/stream-response.js new file mode 100644 index 0000000000..a029daaaa9 --- /dev/null +++ b/src/http/utils/stream-response.js @@ -0,0 +1,29 @@ +'use strict' + +const { PassThrough } = require('readable-stream') + +function streamResponse (request, h, fn) { + const output = new PassThrough() + const errorTrailer = 'X-Stream-Error' + + Promise.resolve() + .then(() => fn(output)) + .catch(err => { + request.raw.res.addTrailers({ + [errorTrailer]: JSON.stringify({ + Message: err.message, + Code: 0 + }) + }) + }) + .finally(() => { + output.end() + }) + + return h.response(output) + .header('x-chunked-output', '1') + .header('content-type', 'application/json') + .header('Trailer', errorTrailer) +} + +module.exports = streamResponse From f8470656650ace171fe3bb6307d4a58226475fb3 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Tue, 8 Oct 2019 09:19:41 +0100 Subject: [PATCH 10/10] chore: update interop dep --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c05c28d201..216fc76085 100644 --- a/package.json +++ b/package.json @@ -204,7 +204,7 @@ "form-data": "^2.5.1", "hat": "0.0.3", "interface-ipfs-core": "^0.117.2", - "ipfs-interop": "~0.1.0", + "ipfs-interop": "^0.1.1", "ipfsd-ctl": "^0.47.2", "libp2p-websocket-star": "~0.10.2", "ncp": "^2.0.0",