From a5b7a6d6b08d29b4860f6a16bcf08dd51a368ce7 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 9 Apr 2018 11:30:22 +0100 Subject: [PATCH] chore: Conform to spec for stream slices - see ipfs/interface-ipfs-core#242 --- README.md | 13 ++-- src/exporter/file.js | 69 ++++++++++--------- src/exporter/index.js | 4 +- src/exporter/resolve.js | 12 ++-- test/exporter.js | 143 +++++++++++++++------------------------- 5 files changed, 102 insertions(+), 139 deletions(-) diff --git a/README.md b/README.md index cf959ab1..9a9ebd0f 100644 --- a/README.md +++ b/README.md @@ -178,13 +178,9 @@ Creates a new readable stream in object mode that outputs objects of the form } ``` -#### `begin` and `end` +#### `offset` and `length` -`begin` and `end` arguments can optionally be passed to the reader function. These follow the same semantics as the JavaScript [`Array.slice(begin, end)`][] method. - -That is: `begin` is the index in the stream to start sending data, `end` is the index *before* which to stop sending data. - -A negative `begin` starts the slice from the end of the stream and a negative `end` ends the slice by subtracting `end` from the total stream length. +`offset` and `length` arguments can optionally be passed to the reader function. These will cause the returned stream to only emit bytes starting at `offset` and with length of `length`. See [the tests](test/reader.js) for examples of using these arguments. @@ -195,8 +191,8 @@ const drain = require('pull-stream/sinks/drain') pull( exporter(cid, ipldResolver, { - begin: 0, - end: 10 + offset: 0, + length: 10 }) drain((file) => { // file.content() is a pull stream containing only the first 10 bytes of the file @@ -225,7 +221,6 @@ pull( [ipld-resolver instance]: https://github.com/ipld/js-ipld-resolver [UnixFS]: https://github.com/ipfs/specs/tree/master/unixfs [pull-stream]: https://www.npmjs.com/package/pull-stream -[`Array.slice(begin, end)`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice ## Contribute diff --git a/src/exporter/file.js b/src/exporter/file.js index 50787fc9..6a0d801f 100644 --- a/src/exporter/file.js +++ b/src/exporter/file.js @@ -7,7 +7,7 @@ const pull = require('pull-stream') const paramap = require('pull-paramap') // Logic to export a single (possibly chunked) unixfs file. -module.exports = (node, name, path, pathRest, resolve, size, dag, parent, depth, begin, end) => { +module.exports = (node, name, path, pathRest, resolve, size, dag, parent, depth, offset, length) => { const accepts = pathRest[0] if (accepts !== undefined && accepts !== path) { @@ -16,46 +16,50 @@ module.exports = (node, name, path, pathRest, resolve, size, dag, parent, depth, const file = UnixFS.unmarshal(node.data) const fileSize = size || file.fileSize() - const content = streamBytes(dag, node, fileSize, findByteRange(fileSize, begin, end)) - return pull.values([{ - depth: depth, - content: content, - name: name, - path: path, - hash: node.multihash, - size: fileSize, - type: 'file' - }]) -} + if (offset < 0) { + return pull.error(new Error('Offset must be greater than 0')) + } -function findByteRange (fileSize, begin, end) { - if (!begin) { - begin = 0 + if (offset > fileSize) { + return pull.error(new Error('Offset must be less than the file size')) } - if (!end || end > fileSize) { - end = fileSize + if (length < 0) { + return pull.error(new Error('Length must be greater than or equal to 0')) } - if (begin < 0) { - begin = fileSize + begin + if (length === 0) { + return pull.empty() } - if (end < 0) { - end = fileSize + end + if (!offset) { + offset = 0 } - return { - begin, end + if (!length || (offset + length > fileSize)) { + length = fileSize - offset } + + const content = streamBytes(dag, node, fileSize, offset, length) + + return pull.values([{ + depth: depth, + content: content, + name: name, + path: path, + hash: node.multihash, + size: fileSize, + type: 'file' + }]) } -function streamBytes (dag, node, fileSize, { begin, end }) { - if (begin === end) { +function streamBytes (dag, node, fileSize, offset, length) { + if (offset === fileSize || length === 0) { return pull.empty() } + const end = offset + length let streamPosition = 0 function getData ({ node, start }) { @@ -70,11 +74,13 @@ function streamBytes (dag, node, fileSize, { begin, end }) { return } - const block = extractDataFromBlock(file.data, start, begin, end) + const block = extractDataFromBlock(file.data, start, offset, end) + + streamPosition += block.length return block - } catch (err) { - throw new Error('Failed to unmarshal node') + } catch (error) { + throw new Error(`Failed to unmarshal node - ${error.message}`) } } @@ -95,9 +101,9 @@ function streamBytes (dag, node, fileSize, { begin, end }) { return child }) .filter((child, index) => { - return (begin >= child.start && begin < child.end) || // child has begin byte + return (offset >= child.start && offset < child.end) || // child has offset byte (end > child.start && end <= child.end) || // child has end byte - (begin < child.start && end > child.end) // child is between begin and end bytes + (offset < child.start && end > child.end) // child is between offset and end bytes }) if (filteredLinks.length) { @@ -111,7 +117,8 @@ function streamBytes (dag, node, fileSize, { begin, end }) { dag.get(new CID(child.link.multihash), (error, result) => cb(error, { start: child.start, end: child.end, - node: result && result.value + node: result && result.value, + size: child.size })) }) ) diff --git a/src/exporter/index.js b/src/exporter/index.js index 365257b8..76c55dd4 100644 --- a/src/exporter/index.js +++ b/src/exporter/index.js @@ -37,8 +37,8 @@ function pathBaseAndRest (path) { const defaultOptions = { maxDepth: Infinity, - begin: undefined, - end: undefined + offset: undefined, + length: undefined } module.exports = (path, dag, options) => { diff --git a/src/exporter/resolve.js b/src/exporter/resolve.js index 369d063c..3f40ee06 100644 --- a/src/exporter/resolve.js +++ b/src/exporter/resolve.js @@ -32,14 +32,14 @@ function createResolver (dag, options, depth, parent) { return pull.error(new Error('no depth')) } if (item.object) { - return cb(null, resolveItem(item.object, item, options.begin, options.end)) + return cb(null, resolveItem(item.object, item, options.offset, options.length)) } dag.get(new CID(item.multihash), (err, node) => { if (err) { return cb(err) } // const name = item.fromPathRest ? item.name : item.path - cb(null, resolveItem(node.value, item, options.begin, options.end)) + cb(null, resolveItem(node.value, item, options.offset, options.length)) }) }), pull.flatten(), @@ -47,18 +47,18 @@ function createResolver (dag, options, depth, parent) { pull.filter((node) => node.depth <= options.maxDepth) ) - function resolveItem (node, item, begin, end) { - return resolve(node, item.name, item.path, item.pathRest, item.size, dag, item.parent || parent, item.depth, begin, end) + function resolveItem (node, item, offset, length) { + return resolve(node, item.name, item.path, item.pathRest, item.size, dag, item.parent || parent, item.depth, offset, length) } - function resolve (node, name, path, pathRest, size, dag, parentNode, depth, begin, end) { + function resolve (node, name, path, pathRest, size, dag, parentNode, depth, offset, length) { const type = typeOf(node) const nodeResolver = resolvers[type] if (!nodeResolver) { return pull.error(new Error('Unkown node type ' + type)) } const resolveDeep = createResolver(dag, options, depth, node) - return nodeResolver(node, name, path, pathRest, resolveDeep, size, dag, parentNode, depth, begin, end) + return nodeResolver(node, name, path, pathRest, resolveDeep, size, dag, parentNode, depth, offset, length) } } diff --git a/test/exporter.js b/test/exporter.js index 02ac4e2f..d165e7e4 100644 --- a/test/exporter.js +++ b/test/exporter.js @@ -43,7 +43,7 @@ module.exports = (repo) => { ) } - function addAndReadTestFile ({file, begin, end, strategy = 'balanced', path = '/foo', maxChunkSize}, cb) { + function addAndReadTestFile ({file, offset, length, strategy = 'balanced', path = '/foo', maxChunkSize}, cb) { addTestFile({file, strategy, path, maxChunkSize}, (error, multihash) => { if (error) { return cb(error) @@ -51,7 +51,7 @@ module.exports = (repo) => { pull( exporter(multihash, ipld, { - begin, end + offset, length }), pull.collect((error, files) => { if (error) { @@ -74,8 +74,8 @@ module.exports = (repo) => { addAndReadTestFile({ file: bytes, - begin: bytesInABlock - 1, - end: bytesInABlock + 2, + offset: bytesInABlock - 1, + length: 3, strategy }, (error, data) => { if (error) { @@ -141,8 +141,8 @@ module.exports = (repo) => { it('exports a chunk of a file with no links', (done) => { const hash = 'QmQmZQxSKQppbsWfVzBvg59Cn3DKtsNVQ94bjAxg2h3Lb8' - const begin = 0 - const end = 5 + const offset = 0 + const length = 5 pull( zip( @@ -151,8 +151,8 @@ module.exports = (repo) => { pull.map((res) => UnixFS.unmarshal(res.value.data)) ), exporter(hash, ipld, { - begin, - end + offset, + length }) ), pull.collect((err, values) => { @@ -161,7 +161,7 @@ module.exports = (repo) => { const unmarsh = values[0][0] const file = values[0][1] - fileEql(file, unmarsh.data.slice(begin, end), done) + fileEql(file, unmarsh.data.slice(offset, offset + length), done) }) ) }) @@ -183,18 +183,18 @@ module.exports = (repo) => { it('exports a chunk of a small file with links', function (done) { this.timeout(30 * 1000) const hash = 'QmW7BDxEbGqxxSYVtn3peNPQgdDXbWkoQ6J1EFYAEuQV3Q' - const begin = 0 - const end = 5 + const offset = 0 + const length = 5 pull( exporter(hash, ipld, { - begin, - end + offset, + length }), pull.collect((err, files) => { expect(err).to.not.exist() - fileEql(files[0], bigFile.slice(begin, end), done) + fileEql(files[0], bigFile.slice(offset, offset + length), done) }) ) }) @@ -216,18 +216,18 @@ module.exports = (repo) => { it('exports a chunk of a small file with links using CID instead of multihash', function (done) { this.timeout(30 * 1000) const cid = new CID('QmW7BDxEbGqxxSYVtn3peNPQgdDXbWkoQ6J1EFYAEuQV3Q') - const begin = 0 - const end = 5 + const offset = 0 + const length = 5 pull( exporter(cid, ipld, { - begin, - end + offset, + length }), pull.collect((err, files) => { expect(err).to.not.exist() - fileEql(files[0], bigFile.slice(begin, end), done) + fileEql(files[0], bigFile.slice(offset, offset + length), done) }) ) }) @@ -249,13 +249,13 @@ module.exports = (repo) => { it('exports a chunk of a large file > 5mb', function (done) { this.timeout(30 * 1000) const hash = 'QmRQgufjp9vLE8XK2LGKZSsPCFCF6e4iynCQtNB5X2HBKE' - const begin = 0 - const end = 5 + const offset = 0 + const length = 5 pull( exporter(hash, ipld, { - begin, - end + offset, + length }), pull.collect((err, files) => { expect(err).to.not.exist() @@ -270,13 +270,13 @@ module.exports = (repo) => { this.timeout(30 * 1000) const hash = 'QmRQgufjp9vLE8XK2LGKZSsPCFCF6e4iynCQtNB5X2HBKE' const bytesInABlock = 262144 - const begin = bytesInABlock - 1 - const end = bytesInABlock + 1 + const offset = bytesInABlock - 1 + const length = bytesInABlock + 1 pull( exporter(hash, ipld, { - begin, - end + offset, + length }), pull.collect((err, files) => { expect(err).to.not.exist() @@ -378,10 +378,10 @@ module.exports = (repo) => { ) }) - it('reads bytes with a begin', (done) => { + it('reads bytes with an offset', (done) => { addAndReadTestFile({ file: Buffer.from([0, 1, 2, 3]), - begin: 1 + offset: 1 }, (error, data) => { expect(error).to.not.exist() expect(data).to.deep.equal(Buffer.from([1, 2, 3])) @@ -390,23 +390,23 @@ module.exports = (repo) => { }) }) - it('reads bytes with a negative begin', (done) => { + it('reads bytes with a negative offset', (done) => { addAndReadTestFile({ file: Buffer.from([0, 1, 2, 3]), - begin: -1 + offset: -1 }, (error, data) => { - expect(error).to.not.exist() - expect(data).to.deep.equal(Buffer.from([3])) + expect(error).to.be.ok() + expect(error.message).to.contain('Offset must be greater than 0') done() }) }) - it('reads bytes with an end', (done) => { + it('reads bytes with an offset and a length', (done) => { addAndReadTestFile({ file: Buffer.from([0, 1, 2, 3]), - begin: 0, - end: 1 + offset: 0, + length: 1 }, (error, data) => { expect(error).to.not.exist() expect(data).to.deep.equal(Buffer.from([0])) @@ -415,66 +415,27 @@ module.exports = (repo) => { }) }) - it('reads bytes with a negative end', (done) => { - addAndReadTestFile({ - file: Buffer.from([0, 1, 2, 3, 4]), - begin: 2, - end: -1 - }, (error, data) => { - expect(error).to.not.exist() - expect(data).to.deep.equal(Buffer.from([2, 3])) - - done() - }) - }) - - it('reads bytes with an begin and an end', (done) => { - addAndReadTestFile({ - file: Buffer.from([0, 1, 2, 3, 4]), - begin: 1, - end: 4 - }, (error, data) => { - expect(error).to.not.exist() - expect(data).to.deep.equal(Buffer.from([1, 2, 3])) - - done() - }) - }) - - it('reads bytes with a negative begin and a negative end that point to the same byte', (done) => { + it('reads bytes with a negative length', (done) => { addAndReadTestFile({ file: Buffer.from([0, 1, 2, 3, 4]), - begin: -1, - end: -1 + offset: 2, + length: -1 }, (error, data) => { - expect(error).to.not.exist() - expect(data).to.deep.equal(Buffer.from([])) + expect(error).to.be.ok() + expect(error.message).to.contain('Length must be greater than or equal to 0') done() }) }) - it('reads bytes with a negative begin and a negative end', (done) => { + it('reads bytes with an offset and a length', (done) => { addAndReadTestFile({ file: Buffer.from([0, 1, 2, 3, 4]), - begin: -2, - end: -1 - }, (error, data) => { - expect(error).to.not.exist() - expect(data).to.deep.equal(Buffer.from([3])) - - done() - }) - }) - - it('reads bytes to the end of the file when end is larger than the file', (done) => { - addAndReadTestFile({ - file: Buffer.from([0, 1, 2, 3]), - begin: 0, - end: 100 + offset: 1, + length: 4 }, (error, data) => { expect(error).to.not.exist() - expect(data).to.deep.equal(Buffer.from([0, 1, 2, 3])) + expect(data).to.deep.equal(Buffer.from([1, 2, 3, 4])) done() }) @@ -485,8 +446,8 @@ module.exports = (repo) => { addAndReadTestFile({ file: bigFile, - begin: 0, - end: bigFile.length, + offset: 0, + length: bigFile.length, maxChunkSize: 1024 }, (error, data) => { expect(error).to.not.exist() @@ -501,7 +462,7 @@ module.exports = (repo) => { let results = [] let chunkSize = 1024 - let begin = 0 + let offset = 0 addTestFile({ file: bigFile, @@ -515,8 +476,8 @@ module.exports = (repo) => { (next) => { pull( exporter(multihash, ipld, { - begin, - end: begin + chunkSize + offset, + length: chunkSize }), pull.collect(next) ) @@ -527,9 +488,9 @@ module.exports = (repo) => { (result) => { results.push(result) - begin += result.length + offset += result.length - return begin >= bigFile.length + return offset >= bigFile.length }, (error) => { expect(error).to.not.exist()