From 6a6ffed42ec7fa77f7cd35bc7440cea9629f0658 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Mon, 14 May 2018 03:53:30 +0100 Subject: [PATCH 1/9] fix(ping): convert the ping messages to lowercase --- src/ping-pull-stream.js | 7 ++++- src/ping-readable-stream.js | 14 +++++----- src/ping.js | 20 +++++++++------ src/utils/ping-message-converter.js | 23 +++++++++++++++++ src/utils/ping-message-stream.js | 23 +++++++++++++++++ test/ping.spec.js | 40 +++++++++++++++++------------ 6 files changed, 94 insertions(+), 33 deletions(-) create mode 100644 src/utils/ping-message-converter.js create mode 100644 src/utils/ping-message-stream.js diff --git a/src/ping-pull-stream.js b/src/ping-pull-stream.js index 9c18140e2..a7871faa3 100644 --- a/src/ping-pull-stream.js +++ b/src/ping-pull-stream.js @@ -2,7 +2,9 @@ const toPull = require('stream-to-pull-stream') const deferred = require('pull-defer') +const pump = require('pump') const moduleConfig = require('./utils/module-config') +const PingMessageStream = require('./utils/ping-message-stream') module.exports = (arg) => { const send = moduleConfig(arg) @@ -18,10 +20,13 @@ module.exports = (arg) => { qs: opts } const p = deferred.source() + const response = new PingMessageStream() send(request, (err, stream) => { if (err) { return p.abort(err) } - p.resolve(toPull.source(stream)) + + pump(stream, response) + p.resolve(toPull.source(response)) }) return p diff --git a/src/ping-readable-stream.js b/src/ping-readable-stream.js index 6281a44de..6d2a0e606 100644 --- a/src/ping-readable-stream.js +++ b/src/ping-readable-stream.js @@ -1,8 +1,8 @@ 'use strict' -const Stream = require('readable-stream') const pump = require('pump') const moduleConfig = require('./utils/module-config') +const PingMessageStream = require('./utils/ping-message-stream') module.exports = (arg) => { const send = moduleConfig(arg) @@ -17,17 +17,15 @@ module.exports = (arg) => { args: id, qs: opts } - // ndjson streams objects - const pt = new Stream.PassThrough({ - objectMode: true - }) + + const response = new PingMessageStream() send(request, (err, stream) => { - if (err) { return pt.destroy(err) } + if (err) { return response.destroy(err) } - pump(stream, pt) + pump(stream, response) }) - return pt + return response } } diff --git a/src/ping.js b/src/ping.js index 2682e9752..7372b5761 100644 --- a/src/ping.js +++ b/src/ping.js @@ -1,8 +1,10 @@ 'use strict' const promisify = require('promisify-es6') +const pump = require('pump') +const concat = require('concat-stream') const moduleConfig = require('./utils/module-config') -const streamToValue = require('./utils/stream-to-value') +const PingMessageStream = require('./utils/ping-message-stream') module.exports = (arg) => { const send = moduleConfig(arg) @@ -30,14 +32,16 @@ module.exports = (arg) => { // Transform the response stream to a value: // [{ Success: , Time: , Text: }] - const transform = (res, callback) => { - streamToValue(res, (err, res) => { - if (err) { - return callback(err) + const transform = (stream, callback) => { + const messageConverter = new PingMessageStream() + pump( + stream, + messageConverter, + concat({encoding: 'object'}, (data) => callback(null, data)), + (err) => { + if (err) callback(err) } - - callback(null, res) - }) + ) } send.andTransform(request, transform, callback) diff --git a/src/utils/ping-message-converter.js b/src/utils/ping-message-converter.js new file mode 100644 index 000000000..79d9fc3be --- /dev/null +++ b/src/utils/ping-message-converter.js @@ -0,0 +1,23 @@ +'use strict' + +// Converts IPFS API ping messages to lowercase +// +// { +// Success: true, +// Text: 'foobar', +// Time: 0 +// } +// + +module.exports = function pingMessageConverter (obj) { + if (!isPingMessage(obj)) throw new Error('Invalid ping message received') + return { + success: obj.Success, + time: obj.Time, + text: obj.Text + } +} + +function isPingMessage (obj) { + return obj && typeof obj.Success === 'boolean' +} diff --git a/src/utils/ping-message-stream.js b/src/utils/ping-message-stream.js new file mode 100644 index 000000000..96516fe9b --- /dev/null +++ b/src/utils/ping-message-stream.js @@ -0,0 +1,23 @@ +'use strict' + +const TransformStream = require('readable-stream').Transform +const pingMessageConverter = require('./ping-message-converter') + +class PingMessageStream extends TransformStream { + constructor (options) { + const opts = Object.assign(options || {}, { objectMode: true }) + super(opts) + } + + _transform (obj, enc, callback) { + try { + const msg = pingMessageConverter(obj) + this.push(msg) + } catch (err) { + return callback(err) + } + callback() + } +} + +module.exports = PingMessageStream diff --git a/test/ping.spec.js b/test/ping.spec.js index 49e96a8b0..e0ec03dc5 100644 --- a/test/ping.spec.js +++ b/test/ping.spec.js @@ -12,6 +12,7 @@ const parallel = require('async/parallel') const series = require('async/series') const IPFSApi = require('../src') +const PingMessageStream = require('../src/utils/ping-message-stream') const f = require('./utils/factory') describe('.ping', function () { @@ -77,10 +78,10 @@ describe('.ping', function () { expect(res).to.be.an('array') expect(res).to.have.lengthOf(3) res.forEach(packet => { - expect(packet).to.have.keys('Success', 'Time', 'Text') - expect(packet.Time).to.be.a('number') + expect(packet).to.have.keys('success', 'time', 'text') + expect(packet.time).to.be.a('number') }) - const resultMsg = res.find(packet => packet.Text.includes('Average latency')) + const resultMsg = res.find(packet => packet.text.includes('Average latency')) expect(resultMsg).to.exist() done() }) @@ -92,10 +93,10 @@ describe('.ping', function () { expect(res).to.be.an('array') expect(res).to.have.lengthOf(4) res.forEach(packet => { - expect(packet).to.have.keys('Success', 'Time', 'Text') - expect(packet.Time).to.be.a('number') + expect(packet).to.have.keys('success', 'time', 'text') + expect(packet.time).to.be.a('number') }) - const resultMsg = res.find(packet => packet.Text.includes('Average latency')) + const resultMsg = res.find(packet => packet.text.includes('Average latency')) expect(resultMsg).to.exist() done() }) @@ -107,10 +108,10 @@ describe('.ping', function () { expect(res).to.be.an('array') expect(res).to.have.lengthOf(4) res.forEach(packet => { - expect(packet).to.have.keys('Success', 'Time', 'Text') - expect(packet.Time).to.be.a('number') + expect(packet).to.have.keys('success', 'time', 'text') + expect(packet.time).to.be.a('number') }) - const resultMsg = res.find(packet => packet.Text.includes('Average latency')) + const resultMsg = res.find(packet => packet.text.includes('Average latency')) expect(resultMsg).to.exist() done() }) @@ -131,10 +132,10 @@ describe('.ping', function () { expect(res).to.be.an('array') expect(res).to.have.lengthOf(3) res.forEach(packet => { - expect(packet).to.have.keys('Success', 'Time', 'Text') - expect(packet.Time).to.be.a('number') + expect(packet).to.have.keys('success', 'time', 'text') + expect(packet.time).to.be.a('number') }) - const resultMsg = res.find(packet => packet.Text.includes('Average latency')) + const resultMsg = res.find(packet => packet.text.includes('Average latency')) expect(resultMsg).to.exist() }) }) @@ -147,10 +148,10 @@ describe('.ping', function () { expect(data).to.be.an('array') expect(data).to.have.lengthOf(3) data.forEach(packet => { - expect(packet).to.have.keys('Success', 'Time', 'Text') - expect(packet.Time).to.be.a('number') + expect(packet).to.have.keys('success', 'time', 'text') + expect(packet.time).to.be.a('number') }) - const resultMsg = data.find(packet => packet.Text.includes('Average latency')) + const resultMsg = data.find(packet => packet.text.includes('Average latency')) expect(resultMsg).to.exist() done() }) @@ -162,7 +163,7 @@ describe('.ping', function () { ipfs.pingReadableStream(otherId) .on('data', data => { expect(data).to.be.an('object') - expect(data).to.have.keys('Success', 'Time', 'Text') + expect(data).to.have.keys('success', 'time', 'text') packetNum++ }) .on('error', err => { @@ -173,4 +174,11 @@ describe('.ping', function () { done() }) }) + + it('message conversion fails if invalid message is received', () => { + const messageConverter = new PingMessageStream() + expect(() => { + messageConverter.write({some: 'InvalidMessage'}) + }).to.throw('Invalid ping message received') + }) }) From fdf3c4bd998ab2eef9c996483a9a93e7d294d049 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 16 May 2018 14:09:15 +0100 Subject: [PATCH 2/9] test: add interface-core-api ping tests and fix impl License: MIT Signed-off-by: Alan Shaw --- src/ping-readable-stream.js | 3 +-- src/ping.js | 18 ++++++++++++------ src/utils/ping-message-stream.js | 4 ++++ test/interface/ping.spec.js | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 test/interface/ping.spec.js diff --git a/src/ping-readable-stream.js b/src/ping-readable-stream.js index 6d2a0e606..df4f70401 100644 --- a/src/ping-readable-stream.js +++ b/src/ping-readable-stream.js @@ -21,8 +21,7 @@ module.exports = (arg) => { const response = new PingMessageStream() send(request, (err, stream) => { - if (err) { return response.destroy(err) } - + if (err) { return response.emit('error', err) } pump(stream, response) }) diff --git a/src/ping.js b/src/ping.js index 7372b5761..8b9081967 100644 --- a/src/ping.js +++ b/src/ping.js @@ -2,7 +2,7 @@ const promisify = require('promisify-es6') const pump = require('pump') -const concat = require('concat-stream') +const Writable = require('readable-stream').Writable const moduleConfig = require('./utils/module-config') const PingMessageStream = require('./utils/ping-message-stream') @@ -31,16 +31,22 @@ module.exports = (arg) => { } // Transform the response stream to a value: - // [{ Success: , Time: , Text: }] + // [{ success: , time: , text: }] const transform = (stream, callback) => { const messageConverter = new PingMessageStream() + const responses = [] + pump( stream, messageConverter, - concat({encoding: 'object'}, (data) => callback(null, data)), - (err) => { - if (err) callback(err) - } + new Writable({ + objectMode: true, + write (chunk, enc, cb) { + responses.push(chunk) + cb() + } + }), + (err) => callback(err, responses) ) } diff --git a/src/utils/ping-message-stream.js b/src/utils/ping-message-stream.js index 96516fe9b..944e2d9cf 100644 --- a/src/utils/ping-message-stream.js +++ b/src/utils/ping-message-stream.js @@ -13,6 +13,10 @@ class PingMessageStream extends TransformStream { try { const msg = pingMessageConverter(obj) this.push(msg) + + if (!msg.success) { + throw new Error(msg.text) + } } catch (err) { return callback(err) } diff --git a/test/interface/ping.spec.js b/test/interface/ping.spec.js new file mode 100644 index 000000000..910bf3820 --- /dev/null +++ b/test/interface/ping.spec.js @@ -0,0 +1,32 @@ +/* eslint-env mocha */ +/* eslint max-nested-callbacks: ["error", 8] */ +'use strict' + +const test = require('interface-ipfs-core') +const parallel = require('async/parallel') + +const IPFSApi = require('../../src') +const f = require('../utils/factory') + +const nodes = [] +const common = { + setup: function (callback) { + callback(null, { + spawnNode: (cb) => { + f.spawn({ initOptions: { bits: 1024 } }, (err, _ipfsd) => { + if (err) { + return cb(err) + } + + nodes.push(_ipfsd) + cb(null, IPFSApi(_ipfsd.apiAddr)) + }) + } + }) + }, + teardown: function (callback) { + parallel(nodes.map((node) => (cb) => node.stop(cb)), callback) + } +} + +test.ping(common) From 47950a9c89475a6d37fb3e1edf804de296c84674 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 16 May 2018 16:18:25 +0100 Subject: [PATCH 3/9] chore: update interface-ipfs-core dependency License: MIT Signed-off-by: Alan Shaw --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b47fa2e68..a72f194d5 100644 --- a/package.json +++ b/package.json @@ -79,7 +79,7 @@ "eslint-plugin-react": "^7.8.1", "go-ipfs-dep": "^0.4.14", "gulp": "^3.9.1", - "interface-ipfs-core": "~0.65.5", + "interface-ipfs-core": "^0.66.0", "ipfs": "~0.28.2", "ipfsd-ctl": "~0.33.2", "pull-stream": "^3.6.8", From a3bc3dd2b1cb4748e2c48453f0db49595814be69 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 18 May 2018 10:02:54 +0100 Subject: [PATCH 4/9] fix: update asserted error message License: MIT Signed-off-by: Alan Shaw --- test/get.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/get.spec.js b/test/get.spec.js index a8a2adbf5..91d16f3fe 100644 --- a/test/get.spec.js +++ b/test/get.spec.js @@ -72,7 +72,7 @@ describe('.get (specific go-ipfs features)', function () { 'compression-level': 10 }, (err, files) => { expect(err).to.exist() - expect(err.toString()).to.equal('Error: Compression level must be between 1 and 9') + expect(err.toString()).to.equal('Error: compression level must be between 1 and 9') done() }) }) From da791f69dba92abc59bca69f62ed02b79ac9e36f Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 18 May 2018 10:03:45 +0100 Subject: [PATCH 5/9] chore: update interface-ipfs-core dependency License: MIT Signed-off-by: Alan Shaw --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index a72f194d5..fc71a0e78 100644 --- a/package.json +++ b/package.json @@ -72,14 +72,14 @@ }, "devDependencies": { "aegir": "^13.1.0", - "browser-process-platform": "^0.1.1", + "browser-process-platform": "~0.1.1", "chai": "^4.1.2", "cross-env": "^5.1.5", "dirty-chai": "^2.0.1", "eslint-plugin-react": "^7.8.1", - "go-ipfs-dep": "^0.4.14", + "go-ipfs-dep": "~0.4.14", "gulp": "^3.9.1", - "interface-ipfs-core": "^0.66.0", + "interface-ipfs-core": "~0.66.1", "ipfs": "~0.28.2", "ipfsd-ctl": "~0.33.2", "pull-stream": "^3.6.8", From 7bc7d3ef6f565f1fee6f83abc954ec23be101145 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 18 May 2018 10:41:33 +0100 Subject: [PATCH 6/9] chore: upgrade ipfsd-ctl and go-ipfs-dep dependencies License: MIT Signed-off-by: Alan Shaw --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index fc71a0e78..423b44483 100644 --- a/package.json +++ b/package.json @@ -77,11 +77,11 @@ "cross-env": "^5.1.5", "dirty-chai": "^2.0.1", "eslint-plugin-react": "^7.8.1", - "go-ipfs-dep": "~0.4.14", + "go-ipfs-dep": "~0.4.15", "gulp": "^3.9.1", "interface-ipfs-core": "~0.66.1", "ipfs": "~0.28.2", - "ipfsd-ctl": "~0.33.2", + "ipfsd-ctl": "~0.36.0", "pull-stream": "^3.6.8", "socket.io": "^2.1.0", "socket.io-client": "^2.1.0", From 6b291e331160da09e55a632258a52743544a533a Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 18 May 2018 14:36:57 +0100 Subject: [PATCH 7/9] chore: upgrade interface-ipfs-core dependency License: MIT Signed-off-by: Alan Shaw --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 423b44483..b9e66ee8b 100644 --- a/package.json +++ b/package.json @@ -79,7 +79,7 @@ "eslint-plugin-react": "^7.8.1", "go-ipfs-dep": "~0.4.15", "gulp": "^3.9.1", - "interface-ipfs-core": "~0.66.1", + "interface-ipfs-core": "~0.66.2", "ipfs": "~0.28.2", "ipfsd-ctl": "~0.36.0", "pull-stream": "^3.6.8", From ea3abf2efdea2530ad6a470c9ef2cc421fa6eab2 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 18 May 2018 15:18:17 +0100 Subject: [PATCH 8/9] fix: more robust ping tests License: MIT Signed-off-by: Alan Shaw --- test/ping.spec.js | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/test/ping.spec.js b/test/ping.spec.js index e0ec03dc5..39c1320ee 100644 --- a/test/ping.spec.js +++ b/test/ping.spec.js @@ -15,7 +15,12 @@ const IPFSApi = require('../src') const PingMessageStream = require('../src/utils/ping-message-stream') const f = require('./utils/factory') -describe('.ping', function () { +// Determine if a ping response object is a pong, or something else, like a status message +function isPong (pingResponse) { + return Boolean(pingResponse && pingResponse.time) +} + +describe.only('.ping', function () { let ipfs let ipfsd let other @@ -76,7 +81,7 @@ describe('.ping', function () { ipfs.ping(otherId, (err, res) => { expect(err).to.not.exist() expect(res).to.be.an('array') - expect(res).to.have.lengthOf(3) + expect(res.filter(isPong)).to.have.lengthOf(1) res.forEach(packet => { expect(packet).to.have.keys('success', 'time', 'text') expect(packet.time).to.be.a('number') @@ -91,7 +96,7 @@ describe('.ping', function () { ipfs.ping(otherId, { count: 2 }, (err, res) => { expect(err).to.not.exist() expect(res).to.be.an('array') - expect(res).to.have.lengthOf(4) + expect(res.filter(isPong)).to.have.lengthOf(2) res.forEach(packet => { expect(packet).to.have.keys('success', 'time', 'text') expect(packet.time).to.be.a('number') @@ -106,7 +111,7 @@ describe('.ping', function () { ipfs.ping(otherId, { n: 2 }, (err, res) => { expect(err).to.not.exist() expect(res).to.be.an('array') - expect(res).to.have.lengthOf(4) + expect(res.filter(isPong)).to.have.lengthOf(2) res.forEach(packet => { expect(packet).to.have.keys('success', 'time', 'text') expect(packet.time).to.be.a('number') @@ -130,7 +135,7 @@ describe('.ping', function () { return ipfs.ping(otherId) .then((res) => { expect(res).to.be.an('array') - expect(res).to.have.lengthOf(3) + expect(res.filter(isPong)).to.have.lengthOf(1) res.forEach(packet => { expect(packet).to.have.keys('success', 'time', 'text') expect(packet.time).to.be.a('number') @@ -146,7 +151,7 @@ describe('.ping', function () { collect((err, data) => { expect(err).to.not.exist() expect(data).to.be.an('array') - expect(data).to.have.lengthOf(3) + expect(data.filter(isPong)).to.have.lengthOf(1) data.forEach(packet => { expect(packet).to.have.keys('success', 'time', 'text') expect(packet.time).to.be.a('number') @@ -164,13 +169,13 @@ describe('.ping', function () { .on('data', data => { expect(data).to.be.an('object') expect(data).to.have.keys('success', 'time', 'text') - packetNum++ + if (isPong(data)) packetNum++ }) .on('error', err => { expect(err).not.to.exist() }) .on('end', () => { - expect(packetNum).to.be.above(2) + expect(packetNum).to.equal(1) done() }) }) From 71bf65ec1cd2ef4d187b9019cdcc89020308bd9c Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 18 May 2018 15:33:15 +0100 Subject: [PATCH 9/9] fix: remove .only License: MIT Signed-off-by: Alan Shaw --- test/ping.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ping.spec.js b/test/ping.spec.js index 39c1320ee..a80f3a9d4 100644 --- a/test/ping.spec.js +++ b/test/ping.spec.js @@ -20,7 +20,7 @@ function isPong (pingResponse) { return Boolean(pingResponse && pingResponse.time) } -describe.only('.ping', function () { +describe('.ping', function () { let ipfs let ipfsd let other