Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

interface-ipfs-core config API spec compliant #307

Merged
merged 1 commit into from
Jul 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"ndjson": "^1.4.3",
"promisify-es6": "^1.0.1",
"qs": "^6.1.0",
"streamifier": "^0.1.1",
"wreck": "^7.0.2"
},
"engines": {
Expand All @@ -45,7 +46,7 @@
"aegir": "^3.2.0",
"chai": "^3.5.0",
"gulp": "^3.9.1",
"interface-ipfs-core": "^0.3.0",
"interface-ipfs-core": "^0.4.2",
"ipfsd-ctl": "^0.14.0",
"pre-commit": "^1.1.2",
"stream-equal": "^0.1.8",
Expand Down
59 changes: 46 additions & 13 deletions src/api/config.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,64 @@
'use strict'

const argCommand = require('../cmd-helpers').argCommand
const streamifier = require('streamifier')

module.exports = (send) => {
return {
get: argCommand(send, 'config'),
set (key, value, opts, cb) {
if (typeof (opts) === 'function') {
cb = opts
get (key, callback) {
if (typeof key === 'function') {
callback = key
key = undefined
}

if (!key) {
return send('config/show', null, null, null, true, callback)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably be valuable for the send method to accept a config argument rather than having a parameter list filled with nulls

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 agree. In fact, what I'm very much thinking is after merging the config-api PR into ipfs-api/ng branch, just make a PR to update how request-api.js works, break the tests (so that we don't have to run 300 tests while developing a single feature) and clean up whatever else I find.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is happening :) 43a1dfa

}

return send('config', key, null, null, (err, result) => {
if (err) {
return callback(err)
}
callback(null, result.Value)
})
},
set (key, value, opts, callback) {
if (typeof opts === 'function') {
callback = opts
opts = {}
}
if (typeof key !== 'string') {
return callback(new Error('Invalid key type'))
}

if (typeof value !== 'object' &&
typeof value !== 'boolean' &&
typeof value !== 'string') {
return callback(new Error('Invalid value type'))
}

if (typeof (value) === 'object') {
if (typeof value === 'object') {
value = JSON.stringify(value)
opts = { json: true }
} else if (typeof (value) === 'boolean') {
}

if (typeof value === 'boolean') {
value = value.toString()
opts = { bool: true }
}

return send('config', [key, value], opts, null, cb)
},
show (cb) {
return send('config/show', null, null, null, true, cb)
return send('config', [key, value], opts, null, callback)
},
replace (file, cb) {
return send('config/replace', null, null, file, cb)
replace (config, callback) {
// Its a path
if (typeof config === 'string') {
return send('config/replace', null, null, config, callback)
}

// Its a config obj
if (typeof config === 'object') {
config = streamifier.createReadStream(new Buffer(JSON.stringify(config)))
return send('config/replace', null, null, config, callback)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just missing to make the config.replace tests to work. It seems that this was never working (see https://github.com/ipfs/js-ipfs-api/blob/master/test/api/config.spec.js#L123-L133), unfortunately.

The current problem is that the config never gets replaced and after we send the replace, the content type of the following config.get requests becomes text/plain instead of application/json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learned that this is an error in actual go-ipfs ipfs/kubo#2927

}
}
}
4 changes: 3 additions & 1 deletion src/get-files-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ function loadPaths (opts, file) {
}

function getFilesStream (files, opts) {
if (!files) return null
if (!files) {
return null
}

const mp = new Multipart()

Expand Down
6 changes: 3 additions & 3 deletions src/request-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ function onRes (buffer, cb) {

const stream = Boolean(res.headers['x-stream-output'])
const chunkedObjects = Boolean(res.headers['x-chunked-output'])
const isJson = res.headers['content-type'] && res.headers['content-type'].indexOf('application/json') === 0
const isJson = res.headers['content-type'] &&
res.headers['content-type'].indexOf('application/json') === 0

if (res.statusCode >= 400 || !res.statusCode) {
const error = new Error(`Server responded with ${res.statusCode}`)
Expand Down Expand Up @@ -84,7 +85,7 @@ function requestAPI (config, path, args, qs, files, buffer, cb) {
const port = config.port ? `:${config.port}` : ''

const opts = {
method: files ? 'POST' : 'GET',
method: 'POST',
uri: `${config.protocol}://${config.host}${port}${config['api-path']}${path}?${Qs.stringify(qs, {arrayFormat: 'repeat'})}`,
headers: {}
}
Expand All @@ -100,7 +101,6 @@ function requestAPI (config, path, args, qs, files, buffer, cb) {
}

opts.headers['Content-Type'] = `multipart/form-data; boundary=${stream.boundary}`
opts.downstreamRes = stream
opts.payload = stream
}

Expand Down
142 changes: 12 additions & 130 deletions test/api/config.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,133 +2,15 @@
/* globals apiClients */
'use strict'

const expect = require('chai').expect
const isNode = require('detect-node')
const path = require('path')

describe('.config', () => {
describe('.config.{set, get}', () => {
it('string', (done) => {
const confKey = 'arbitraryKey'
const confVal = 'arbitraryVal'

apiClients.a.config.set(confKey, confVal, (err, res) => {
expect(err).to.not.exist
apiClients.a.config.get(confKey, (err, res) => {
expect(err).to.not.exist
expect(res).to.have.a.property('Value', confVal)
done()
})
})
})

it('bool', (done) => {
const confKey = 'otherKey'
const confVal = true

apiClients.a.config.set(confKey, confVal, (err, res) => {
expect(err).to.not.exist
apiClients.a.config.get(confKey, (err, res) => {
expect(err).to.not.exist
expect(res.Value).to.deep.equal(confVal)
done()
})
})
})

it('json', (done) => {
const confKey = 'API.HTTPHeaders.Access-Control-Allow-Origin'
const confVal = ['http://example.io']

apiClients.a.config.set(confKey, confVal, (err, res) => {
expect(err).to.not.exist
apiClients.a.config.get(confKey, (err, res) => {
expect(err).to.not.exist
expect(res.Value).to.deep.equal(confVal)
done()
})
})
})
})

it('.config.show', (done) => {
apiClients.c.config.show((err, res) => {
expect(err).to.not.exist
expect(res).to.exist
done()
})
})

it('.config.replace', (done) => {
if (!isNode) {
return done()
}

apiClients.c.config.replace(path.join(__dirname, '/../r-config.json'), (err, res) => {
expect(err).to.not.exist
expect(res).to.be.equal(null)
done()
})
})

describe('promise', () => {
describe('.config.{set, get}', () => {
it('string', () => {
const confKey = 'arbitraryKey'
const confVal = 'arbitraryVal'

return apiClients.a.config.set(confKey, confVal)
.then((res) => {
return apiClients.a.config.get(confKey)
})
.then((res) => {
expect(res).to.have.a.property('Value', confVal)
})
})

it('bool', () => {
const confKey = 'otherKey'
const confVal = true

return apiClients.a.config.set(confKey, confVal)
.then((res) => {
return apiClients.a.config.get(confKey)
})
.then((res) => {
expect(res.Value).to.deep.equal(confVal)
})
})

it('json', () => {
const confKey = 'API.HTTPHeaders.Access-Control-Allow-Origin'
const confVal = ['http://example.com']

return apiClients.a.config.set(confKey, confVal)
.then((res) => {
return apiClients.a.config.get(confKey)
})
.then((res) => {
expect(res.Value).to.deep.equal(confVal)
})
})
})

it('.config.show', () => {
return apiClients.c.config.show()
.then((res) => {
expect(res).to.exist
})
})

it('.config.replace', () => {
if (!isNode) {
return
}

return apiClients.c.config.replace(path.join(__dirname, '/../r-config.json'))
.then((res) => {
expect(res).to.be.equal(null)
})
})
})
})
const test = require('interface-ipfs-core')

const common = {
setup: function (cb) {
cb(null, apiClients.a)
},
teardown: function (cb) {
cb()
}
}

test.config(common)