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

feat(files.add): update files.add API #56

Merged
merged 2 commits into from
Aug 12, 2016
Merged

feat(files.add): update files.add API #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5ee5d2
feat(files.add): update files.add API
daviddias Aug 10, 2016
dfa0094
feat(files.add): update tests to new files add API
daviddias Aug 12, 2016
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
6 changes: 4 additions & 2 deletions API/files/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ If no `content` is passed, then the path is treated as an empty directory
```js
{
path: '/tmp/myfile.txt',
node: DAGNode
hash: 'QmHash', // base58 encoded multihash
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be a base58 encoded multihash. Looking at the future we are introducing cid + ipld soon. That means we have to break this interface at that point to a degree. This should be mulithash buffer/object something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I was thinking of that too, specially with the introduction of CID.

However, there will be a CID string version anyway, so the question emerges, do we have a raw multihash/CID or the string version

Copy link
Contributor

Choose a reason for hiding this comment

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

CIDs will allow for different versions of encoding, i.e. non base58. This interface though binds itself to using base58 which I find the most concerning part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it. Right now, the content Ids are presented as base58 Encoded multihash, because there is no multibase nor multicodec. Once we have CID, the description will change to 'String representation of the CID', cause it is a self described Id :D

size: 123
}
```

Expand Down Expand Up @@ -79,7 +80,8 @@ ipfs.files.createAddStream(function (err, stream) {
// 'file' will be of the form
// {
// path: '/tmp/myfile.txt',
// node: DAGNode
// hash: 'QmHash' // base58 encoded multihash
// size: 123
// }
})

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@
"greenkeeperio-bot <[email protected]>",
"nginnever <[email protected]>"
]
}
}
86 changes: 50 additions & 36 deletions src/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,33 @@ module.exports = (common) => {
describe('.add', () => {
it('stream', (done) => {
const buffered = new Buffer('some data')
const expectedMultihash = 'QmVv4Wz46JaZJeH5PMV4LGbRiiMKEmszPYY3g6fjGnVXBS'

const rs = new Readable()
rs.push(buffered)
rs.push(null)

const arr = []
const filePair = {path: 'data.txt', content: rs}
const filePair = {
path: 'data.txt',
content: rs
}

arr.push(filePair)

ipfs.files.add(arr, (err, res) => {
expect(err).to.not.exist
expect(res).to.be.length(1)
expect(res[0].path).to.equal('data.txt')
expect(res[0].node.size()).to.equal(17)
const mh = 'QmVv4Wz46JaZJeH5PMV4LGbRiiMKEmszPYY3g6fjGnVXBS'
expect(bs58.encode(res[0].node.multihash()).toString()).to.equal(mh)
const file = res[0]
expect(file).to.be.eql({
path: 'data.txt',
size: 17,
hash: expectedMultihash
})
expect(file).to.eqlk
expect(file.path).to.equal('data.txt')
expect(file.size).to.equal(17)
expect(file.hash).to.equal(expectedMultihash)
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nicer to write this like so

expect(file).to.be.eql({
  path: 'data.txt',
  size: 17,
  hash: expectedMultihash
})

or

expect(file).to.have.key('path', 'data.txt')
expect(file).to.have.key('size', 17)
expect(file).to.have.key('hash', expectedMultihash)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the first case to the first one, then I changed the remaining ones to the second options, but those got angry at me:

      Uncaught AssertionError: expected { Object (path, hash, ...) } to have key 'QmVvjDy7yF7hdnqE8Hrf4MHo5ABDtb5AbX6hWbD3Y42bXP'
      + expected - actual

       [
      -  "hash"
      -  "path"
      -  "size"
      +  "QmVvjDy7yF7hdnqE8Hrf4MHo5ABDtb5AbX6hWbD3Y42bXP"
       ]

It seems that .to.have.key expects that the arguments are the keys to be looked inside the object and not their values. Checked the Chai API docs and didn't find an example to check for a key and then assert its value (http://chaijs.com/api/bdd/), keeping those how they were :)

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh sorry, it should have been .to.have.property('key', 'value')

done()
})
})
Expand All @@ -77,41 +89,40 @@ module.exports = (common) => {
path: 'testfile.txt',
content: smallFile
}
const expectedMultihash = 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP'

ipfs.files.add([file], (err, res) => {
expect(err).to.not.exist

const added = res[0] != null ? res[0] : res
const mh = bs58.encode(added.node.multihash()).toString()
expect(mh).to.equal('Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP')
expect(added.path).to.equal('testfile.txt')
expect(added.node.links).to.have.length(0)
const file = res[0]
expect(file.hash).to.equal(expectedMultihash)
expect(file.path).to.equal('testfile.txt')
done()
})
})

it('buffer', (done) => {
const expectedMultihash = 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP'

ipfs.files.add(smallFile, (err, res) => {
expect(err).to.not.exist

expect(res).to.have.length(1)
const mh = bs58.encode(res[0].node.multihash()).toString()
expect(mh).to.equal('Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP')
expect(res[0].path).to.equal(mh)
expect(res[0].node.links).to.have.length(0)
const file = res[0]
expect(file.hash).to.equal(expectedMultihash)
expect(file.path).to.equal(file.hash)
done()
})
})

it('BIG buffer', (done) => {
const expectedMultihash = 'Qme79tX2bViL26vNjPsF3DP1R9rMKMvnPYJiKTTKPrXJjq'

ipfs.files.add(bigFile, (err, res) => {
expect(err).to.not.exist

expect(res).to.have.length(1)
expect(res[0].node.links).to.have.length(58)
const mh = bs58.encode(res[0].node.multihash()).toString()
expect(res[0].path).to.equal(mh)
expect(mh).to.equal('Qme79tX2bViL26vNjPsF3DP1R9rMKMvnPYJiKTTKPrXJjq')
const file = res[0]
expect(file.hash).to.equal(expectedMultihash)
expect(file.path).to.equal(file.hash)
done()
})
})
Expand All @@ -121,9 +132,13 @@ module.exports = (common) => {
path: `test-folder/${name}`,
content: directoryContent[name]
})

const emptyDir = (name) => ({
path: `test-folder/${name}`
})

const expectedRootMultihash = 'QmVvjDy7yF7hdnqE8Hrf4MHo5ABDtb5AbX6hWbD3Y42bXP'

const dirs = [
content('pp.txt'),
content('holmes.txt'),
Expand All @@ -137,13 +152,10 @@ module.exports = (common) => {

ipfs.files.add(dirs, (err, res) => {
expect(err).to.not.exist
const root = res[res.length - 1]

const added = res[res.length - 1]
const mh = bs58.encode(added.node.multihash()).toString()
expect(added.node.links).to.have.length(6)
expect(added.path).to.equal('test-folder')
expect(mh).to.equal('QmVvjDy7yF7hdnqE8Hrf4MHo5ABDtb5AbX6hWbD3Y42bXP')

expect(root.path).to.equal('test-folder')
expect(root.hash).to.equal(expectedRootMultihash)
done()
})
})
Expand All @@ -154,9 +166,13 @@ module.exports = (common) => {
path: `test-folder/${name}`,
content: directoryContent[name]
})

const emptyDir = (name) => ({
path: `test-folder/${name}`
})

const expectedRootMultihash = 'QmVvjDy7yF7hdnqE8Hrf4MHo5ABDtb5AbX6hWbD3Y42bXP'

const files = [
content('pp.txt'),
content('holmes.txt'),
Expand All @@ -171,11 +187,9 @@ module.exports = (common) => {
ipfs.files.createAddStream((err, stream) => {
expect(err).to.not.exist

stream.on('data', (tuple) => {
if (tuple.path === 'test-folder') {
const mh = bs58.encode(tuple.node.multihash()).toString()
expect(mh).to.equal('QmVvjDy7yF7hdnqE8Hrf4MHo5ABDtb5AbX6hWbD3Y42bXP')
expect(tuple.node.links).to.have.length(6)
stream.on('data', (file) => {
if (file.path === 'test-folder') {
expect(file.hash).to.equal(expectedRootMultihash)
}
})

Expand Down Expand Up @@ -232,14 +246,14 @@ module.exports = (common) => {

describe('promise API', () => {
describe('.add', () => {
const expectedMultihash = 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP'

it('buffer', () => {
return ipfs.files.add(smallFile)
.then((res) => {
const added = res[0] != null ? res[0] : res
const mh = bs58.encode(added.node.multihash()).toString()
expect(mh).to.equal('Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP')
expect(added.path).to.equal(mh)
expect(added.node.links).to.have.length(0)
const file = res[0]
expect(file.hash).to.equal(expectedMultihash)
expect(file.path).to.equal(file.hash)
})
})
})
Expand Down