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

feat: show which link does not exist on ipfs.cat #1270

Closed
wants to merge 5 commits into from
Closed

feat: show which link does not exist on ipfs.cat #1270

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3b2d9fe
Show which link does not exist on ipfs.cat
youngnicks Mar 18, 2018
87ab4bb
Determine which node in path is missing.
youngnicks Mar 19, 2018
aa2d6a6
Add test for cat nested error.
youngnicks Mar 20, 2018
4f3ec22
Change cat error to match go-ipfs
youngnicks Mar 21, 2018
8b6f79b
More specific cat errors.
youngnicks Mar 21, 2018
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
20 changes: 17 additions & 3 deletions src/core/components/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,22 +131,36 @@ module.exports = function files (self) {
throw new Error('You must supply an ipfsPath')
}

let bestMatch = 0

ipfsPath = normalizePath(ipfsPath)
const pathComponents = ipfsPath.split('/')
const restPath = normalizePath(pathComponents.slice(1).join('/'))
const filterFile = (file) => (restPath && file.path === restPath) || (file.path === ipfsPath)
const filterFile = (file) => {
if (file.path === ipfsPath.substring(0, file.path.length)) {
const matchedNodes = file.path.split('/').length

if (matchedNodes > bestMatch) {
bestMatch = matchedNodes
}
}

return (restPath && file.path === restPath) || (file.path === ipfsPath)
}

const d = deferred.source()

pull(
exporter(ipfsPath, self._ipld),
exporter(pathComponents[0], self._ipld),
pull.collect((err, files) => {
if (err) { return d.abort(err) }
if (files && files.length > 1) {
files = files.filter(filterFile)
}
if (!files || !files.length) {
return d.abort(new Error('No such file'))
const parent = pathComponents.slice(0, bestMatch).join('/')
const link = pathComponents.slice(bestMatch).join('/')
return d.abort(new Error(`no link named "${link}" under ${parent}`))
}

const file = files[0]
Expand Down
4 changes: 4 additions & 0 deletions test/cli/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,11 @@ describe('files', () => runOnAndOff((thing) => {
return ipfs('cat QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB/dummy')
.then(() => expect.fail(0, 1, 'Should have thrown an error'))
.catch((err) => {
const message = err.stderr.match(/^Error:(?: Failed to cat file: Error:)? (.*)$/m)[1]
expect(err).to.exist()
expect(message).to.eql(
'no link named "dummy" under QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB'
)
Copy link
Member

Choose a reason for hiding this comment

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

This feature is still not complete. Please add tests for multiple levels of nestedness (i.e /a/b/c/d) and you will see that after the first level, this PR fails.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I was more asking if this would be an acceptable location for this logic, knowing that it could possibly have a performance hit when there is a large amount of files on a node.

What should the error message show when a middle level is missing? I am thinking the message should be no link named "c/d" under a/b to provide more context to as to what the requested file is.

})
})

Expand Down