Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: pin type filtering in pin.ls #2228

Merged
merged 4 commits into from
Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 3 additions & 3 deletions src/core/components/pin.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ module.exports = (self) => {
// check the pinned state of specific hashes
waterfall([
(cb) => resolvePath(self.object, paths, cb),
(hashes, cb) => mapSeries(hashes, (hash, done) => pin._isPinnedWithType(hash, types.all, done), cb),
(hashes, cb) => mapSeries(hashes, (hash, done) => pin._isPinnedWithType(hash, type, done), cb),
Copy link
Member Author

@lidel lidel Jul 9, 2019

Choose a reason for hiding this comment

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

This is the fix for filtering 🙃

(results, cb) => {
results = results
.filter(result => result.pinned)
Expand All @@ -348,12 +348,12 @@ module.exports = (self) => {
})

if (!results.length) {
return cb(new Error(`Path is not pinned`))
return cb(new Error(`path '${paths}' is not pinned`))
Copy link
Member Author

Choose a reason for hiding this comment

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

}

cb(null, results)
}
], callback)
], (err, results) => err ? callback(err) : callback(null, results)) // we don't want results equal [undefined] when err is present
} else {
// show all pinned items of type
let pins = []
Expand Down
2 changes: 1 addition & 1 deletion src/http/api/resources/pin.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ exports.ls = {
try {
result = await ipfs.pin.ls(path, { type })
} catch (err) {
throw Boom.boomify(err, { message: 'Failed to list pins' })
throw Boom.boomify(err)
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed message prefix to match error from go-ipfs.
Interop test for this is added in https://github.com/ipfs/interface-js-ipfs-core/pull/375/files#diff-6a00eb1307f801bb37999815584589bdR199

}

return h.response({
Expand Down
98 changes: 90 additions & 8 deletions test/core/pin.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ chai.use(dirtyChai)

const fs = require('fs')

const CID = require('cids')
const IPFS = require('../../src/core')
const createTempRepo = require('../utils/create-repo-nodejs')
const expectTimeout = require('../utils/expect-timeout')
Expand Down Expand Up @@ -44,14 +45,22 @@ describe('pin', function () {
let pin
let repo

function expectPinned (hash, type, pinned = true) {
function expectPinned (hash, type = pinTypes.all, pinned = true) {
Copy link
Member Author

Choose a reason for hiding this comment

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

expectPinned was not checking if returned type match requested one, so I've added double-check for that.

if (typeof type === 'boolean') {
pinned = type
type = undefined
type = pinTypes.all
}

return pin._isPinnedWithType(hash, type || pinTypes.all)
.then(result => expect(result.pinned).to.eql(pinned))
return pin._isPinnedWithType(hash, type)
.then(result => {
expect(result.pinned).to.eql(pinned)
if (type === pinTypes.indirect) {
// indirect pins return a CID of recursively pinned root instead of 'indirect' string
expect(CID.isCID(result.reason)).to.be.true()
} else if (type !== pinTypes.all) {
expect(result.reason).to.eql(type)
}
})
}

async function clearPins () {
Expand Down Expand Up @@ -159,6 +168,7 @@ describe('pin', function () {
it('recursive', function () {
return pin.add(pins.root)
.then(() => {
expectPinned(pins.root, pinTypes.recursive)
const pinChecks = Object.values(pins)
.map(hash => expectPinned(hash))

Expand All @@ -169,7 +179,7 @@ describe('pin', function () {
it('direct', function () {
return pin.add(pins.root, { recursive: false })
.then(() => Promise.all([
expectPinned(pins.root),
expectPinned(pins.root, pinTypes.direct),
expectPinned(pins.solarWiki, false)
]))
})
Expand Down Expand Up @@ -242,7 +252,7 @@ describe('pin', function () {
)
})

it('direct', function () {
it('all direct', function () {
return pin.ls({ type: 'direct' })
.then(out =>
expect(out).to.deep.include.members([
Expand All @@ -252,7 +262,7 @@ describe('pin', function () {
)
})

it('recursive', function () {
it('all recursive', function () {
return pin.ls({ type: 'recursive' })
.then(out =>
expect(out).to.deep.include.members([
Expand All @@ -262,7 +272,7 @@ describe('pin', function () {
)
})

it('indirect', function () {
it('all indirect', function () {
return pin.ls({ type: 'indirect' })
.then(out =>
expect(out).to.deep.include.members([
Expand All @@ -275,6 +285,78 @@ describe('pin', function () {
])
)
})

it('direct for CID', function () {
return pin.ls(pins.mercuryDir, { type: 'direct' })
.then(out =>
expect(out).to.have.deep.members([
{ type: 'direct',
hash: pins.mercuryDir }
])
)
})

it('direct for path', function () {
return pin.ls(`/ipfs/${pins.root}/mercury/`, { type: 'direct' })
.then(out =>
expect(out).to.have.deep.members([
{ type: 'direct',
hash: pins.mercuryDir }
])
)
})

it('direct for path (no match)', function (done) {
pin.ls(`/ipfs/${pins.root}/mercury/wiki.md`, { type: 'direct' }, (err, pinset) => {
expect(err).to.exist()
Copy link
Member Author

Choose a reason for hiding this comment

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

To make it easier to maintain the error message is not hardcoded here, but checked in interop tests:

expect(pinset).to.not.exist()
done()
})
})

it('direct for CID (no match)', function (done) {
pin.ls(pins.root, { type: 'direct' }, (err, pinset) => {
expect(err).to.exist()
expect(pinset).to.not.exist()
done()
})
})

it('recursive for CID', function () {
return pin.ls(pins.root, { type: 'recursive' })
.then(out =>
expect(out).to.have.deep.members([
{ type: 'recursive',
hash: pins.root }
])
)
})

it('recursive for CID (no match)', function (done) {
return pin.ls(pins.mercuryDir, { type: 'recursive' }, (err, pinset) => {
expect(err).to.exist()
expect(pinset).to.not.exist()
done()
})
})

it('indirect for CID', function () {
return pin.ls(pins.solarWiki, { type: 'indirect' })
.then(out =>
expect(out).to.have.deep.members([
{ type: `indirect through ${pins.root}`,
hash: pins.solarWiki }
])
)
})

it('indirect for CID (no match)', function (done) {
pin.ls(pins.root, { type: 'indirect' }, (err, pinset) => {
expect(err).to.exist()
expect(pinset).to.not.exist()
done()
})
})
})
})

Expand Down