From 248cd137359d679a1bfd6317fa49e990c56c380d Mon Sep 17 00:00:00 2001 From: David Dias Date: Tue, 9 Aug 2016 10:36:07 +0100 Subject: [PATCH] fix(files.get): - Move path.join call out of readStream to not mess with the AST parser - Fix big file test --- package.json | 3 +- src/add-to-dagnode-transform.js | 1 + src/api/add-files.js | 5 ++++ src/request-api.js | 38 +++++++++++++++-------- test/api/get.spec.js | 53 +++++++++++++++++++-------------- 5 files changed, 64 insertions(+), 36 deletions(-) diff --git a/package.json b/package.json index d14eb10534..6f8915b596 100644 --- a/package.json +++ b/package.json @@ -33,8 +33,9 @@ "aegir": "^6.0.0", "chai": "^3.5.0", "gulp": "^3.9.1", - "interface-ipfs-core": "^0.5.0", + "interface-ipfs-core": "^0.6.0", "ipfsd-ctl": "^0.14.0", + "passthrough-counter": "^1.0.0", "pre-commit": "^1.1.3", "stream-equal": "^0.1.8", "stream-http": "^2.3.1", diff --git a/src/add-to-dagnode-transform.js b/src/add-to-dagnode-transform.js index 46e92b6fb6..f70d869f92 100644 --- a/src/add-to-dagnode-transform.js +++ b/src/add-to-dagnode-transform.js @@ -8,6 +8,7 @@ module.exports = function (err, res, send, done) { if (err) { return done(err) } + async.map(res, function map (entry, next) { getDagNode(send, entry.Hash, function (err, node) { if (err) { diff --git a/src/api/add-files.js b/src/api/add-files.js index c5362f6439..26f71097df 100644 --- a/src/api/add-files.js +++ b/src/api/add-files.js @@ -1,5 +1,6 @@ 'use strict' +const isNode = require('detect-node') const addToDagNodesTransform = require('../add-to-dagnode-transform') module.exports = (send) => { @@ -9,6 +10,10 @@ module.exports = (send) => { opts = {} } + if (!isNode) { + return cb(new Error('Recursive uploads are not supported in the browser')) + } + if (typeof (path) !== 'string') { return cb(new Error('"path" must be a string')) } diff --git a/src/request-api.js b/src/request-api.js index bef4fafcfa..a5c01ea4c5 100644 --- a/src/request-api.js +++ b/src/request-api.js @@ -4,6 +4,7 @@ const Wreck = require('wreck') const Qs = require('qs') const ndjson = require('ndjson') const getFilesStream = require('./get-files-stream') +const Counter = require('passthrough-counter') const isNode = require('detect-node') @@ -11,13 +12,19 @@ const isNode = require('detect-node') function parseChunkedJson (res, cb) { const parsed = [] + const c = new Counter() res + .pipe(c) .pipe(ndjson.parse()) - .on('data', parsed.push.bind(parsed)) - .on('end', () => cb(null, parsed)) + .on('data', (obj) => { + parsed.push(obj) + }) + .on('end', () => { + cb(null, parsed) + }) } -function onRes (buffer, cb) { +function onRes (buffer, cb, uri) { return (err, res) => { if (err) { return cb(err) @@ -42,10 +49,14 @@ function onRes (buffer, cb) { }) } - if (stream && !buffer) return cb(null, res) + if (stream && !buffer) { + return cb(null, res) + } if (chunkedObjects) { - if (isJson) return parseChunkedJson(res, cb) + if (isJson) { + return parseChunkedJson(res, cb) + } return Wreck.read(res, null, cb) } @@ -56,6 +67,11 @@ function onRes (buffer, cb) { function requestAPI (config, path, args, qs, files, buffer, cb) { qs = qs || {} + + if (Array.isArray(files)) { + qs.recursive = true + } + if (Array.isArray(path)) path = path.join('/') if (args && !Array.isArray(args)) args = [args] if (args) qs.arg = args @@ -67,10 +83,6 @@ function requestAPI (config, path, args, qs, files, buffer, cb) { delete qs.r } - if (!isNode && qs.recursive && path === 'add') { - return cb(new Error('Recursive uploads are not supported in the browser')) - } - qs['stream-channels'] = true let stream @@ -104,7 +116,7 @@ function requestAPI (config, path, args, qs, files, buffer, cb) { opts.payload = stream } - return Wreck.request(opts.method, opts.uri, opts, onRes(buffer, cb)) + return Wreck.request(opts.method, opts.uri, opts, onRes(buffer, cb, opts.uri)) } // -- Interface @@ -128,9 +140,9 @@ exports = module.exports = function getRequestAPI (config) { return requestAPI(config, path, args, qs, files, buffer, cb) } - // Wraps the 'send' function such that an asynchronous transform may be - // applied to its result before passing it on to either its callback or - // promise. + // Wraps the 'send' function such that an asynchronous + // transform may be applied to its result before + // passing it on to either its callback or promise. send.withTransform = function (transform) { return function (path, args, qs, files, buffer, cb) { if (typeof buffer === 'function') { diff --git a/test/api/get.spec.js b/test/api/get.spec.js index 42c5b852e7..77a9f7451d 100644 --- a/test/api/get.spec.js +++ b/test/api/get.spec.js @@ -17,13 +17,15 @@ const path = require('path') // const extract = require('tar-stream').extract const testfile = fs.readFileSync(path.join(__dirname, '/../testfile.txt')) + let testfileBig if (isNode) { - testfileBig = fs.createReadStream(path.join(__dirname, '/../15mb.random'), { bufferSize: 128 }) + const tfbPath = path.join(__dirname, '/../15mb.random') + testfileBig = fs.createReadStream(tfbPath, { bufferSize: 128 }) } -describe.skip('.get', () => { +describe('.get', () => { it('get with no compression args', (done) => { apiClients.a .get('Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP', (err, res) => { @@ -92,35 +94,42 @@ describe.skip('.get', () => { return done() } - apiClients.a.get('Qme79tX2bViL26vNjPsF3DP1R9rMKMvnPYJiKTTKPrXJjq', (err, res) => { + apiClients.a.get('Qme79tX2bViL26vNjPsF3DP1R9rMKMvnPYJiKTTKPrXJjq', (err, files) => { expect(err).to.not.exist - // Do not blow out the memory of nodejs :) - streamEqual(res, testfileBig, (err, equal) => { - expect(err).to.not.exist - expect(equal).to.be.true - done() + files.on('data', (file) => { + // Do not blow out the memory of nodejs :) + streamEqual(file.content, testfileBig, (err, equal) => { + expect(err).to.not.exist + expect(equal).to.be.true + done() + }) }) }) }) - describe.skip('promise', () => { + describe('promise', () => { it('get', (done) => { apiClients.a.get('Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP') - .then((res) => { - let buf = '' - res - .on('error', (err) => { - throw err - }) - .on('data', (data) => { - buf += data - }) - .on('end', () => { - expect(buf).to.contain(testfile.toString()) - done() - }) + .then((files) => { + files.on('data', (file) => { + let buf = '' + file.content + .on('error', (err) => { + throw err + }) + .on('data', (data) => { + buf += data.toString() + }) + .on('end', () => { + expect(buf).to.contain(testfile.toString()) + done() + }) + }) }) + .catch((err) => { + expect(err).to.not.exist + }) }) }) })