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

fix: enable preload on MFS commands that accept IPFS paths #2355

Merged
merged 3 commits into from
Aug 23, 2019
Merged
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
Next Next commit
fix: enable preload on MFS commands that accept IPFS paths
fixes #2335

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
  • Loading branch information
Alan Shaw committed Aug 14, 2019
commit 8fa7cb7f25d0fdd220b95fed78949aef3f204812
60 changes: 42 additions & 18 deletions src/core/components/files-mfs.js
Original file line number Diff line number Diff line change
@@ -10,13 +10,15 @@ const callbackify = require('callbackify')
const PassThrough = require('stream').PassThrough
const pull = require('pull-stream/pull')
const map = require('pull-stream/throughs/map')
const isIpfs = require('is-ipfs')
const { cidToString } = require('../../utils/cid')

const mapLsFile = (options = {}) => {
const long = options.long || options.l

return (file) => {
return {
hash: long ? file.cid.toBaseEncodedString(options.cidBase) : '',
hash: long ? cidToString(file.cid, { base: options.cidBase }) : '',
name: file.name,
type: long ? file.type : 0,
size: long ? file.size || 0 : 0
@@ -32,15 +34,37 @@ module.exports = self => {
repoOwner: self._options.repoOwner
})

const withPreload = fn => (...args) => {
const cids = args.reduce((cids, p) => {
if (isIpfs.ipfsPath(p)) {
Copy link
Member

@lidel lidel Aug 15, 2019

Choose a reason for hiding this comment

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

IIRC /ipns/ paths are not supported by files.stat.

That is why HTTP gateway resolves /ipns//ipfs/ before passing it to stat:

// The resolver from ipfs-http-response supports only immutable /ipfs/ for now,
// so we convert /ipns/ to /ipfs/ before passing it to the resolver ¯\_(ツ)_/¯
// This could be removed if a solution proposed in
// https://github.com/ipfs/js-ipfs-http-response/issues/22 lands upstream
let ipfsPath = decodeURI(path.startsWith('/ipns/')
? await ipfs.name.resolve(path, { recursive: true })
: path)

That being said, is it possible /ipns/ paths will be supported in the future, as suggested in ipfs/js-ipfs-http-response#22? If so, a future-proof check would be:

Suggested change
if (isIpfs.ipfsPath(p)) {
if (isIpfs.path(p)) {

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we'd also have to add the resolve step to this PR if we wanted to "future-proof" the check...right now it would end up sending Peer IDs to the preload nodes...

Copy link
Member

@lidel lidel Aug 21, 2019

Choose a reason for hiding this comment

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

No, preload call for /ipns/ path will work fine. Check with:

$ curl "https://node0.preload.ipfs.io/api/v0/refs?arg=/ipns/docs.ipfs.io&r=true"

Above is DNSLink, however I think preload of real IPNS (with peerid) will be actually beneficial by making the key available in more places.

IPNSsupport does not need to land/block this PR. Let's merge is asap.

cids.push(p.split('/')[2])
} else if (isIpfs.cid(p)) {
cids.push(p)
} else if (isIpfs.cidPath(p)) {
cids.push(p.split('/')[0])
}
return cids
}, [])

if (cids.length) {
const options = args[args.length - 1]
if (options.preload !== false) {
cids.forEach(cid => self._preload(cid))
}
}

return fn(...args)
}

return {
cp: callbackify.variadic(methods.cp),
cp: callbackify.variadic(withPreload(methods.cp)),
flush: callbackify.variadic(methods.flush),
ls: callbackify.variadic(async (path, options = {}) => {
ls: callbackify.variadic(withPreload(async (path, options = {}) => {
const files = await all(methods.ls(path, options))

return files.map(mapLsFile(options))
}),
lsReadableStream: (path, options = {}) => {
})),
lsReadableStream: withPreload((path, options = {}) => {
const stream = toReadableStream.obj(methods.ls(path, options))
const through = new PassThrough({
objectMode: true
@@ -60,33 +84,33 @@ module.exports = self => {
})

return through
},
lsPullStream: (path, options = {}) => {
}),
lsPullStream: withPreload((path, options = {}) => {
return pull(
toPullStream.source(methods.ls(path, options)),
map(mapLsFile(options))
)
},
}),
mkdir: callbackify.variadic(methods.mkdir),
mv: callbackify.variadic(methods.mv),
read: callbackify(async (path, options = {}) => {
mv: callbackify.variadic(withPreload(methods.mv)),
read: callbackify(withPreload(async (path, options = {}) => {
return Buffer.concat(await all(methods.read(path, options)))
}),
readPullStream: (path, options = {}) => {
})),
readPullStream: withPreload((path, options = {}) => {
return toPullStream.source(methods.read(path, options))
},
readReadableStream: (path, options = {}) => {
}),
readReadableStream: withPreload((path, options = {}) => {
return toReadableStream(methods.read(path, options))
},
}),
rm: callbackify.variadic(methods.rm),
stat: callbackify(async (path, options = {}) => {
stat: callbackify(withPreload(async (path, options = {}) => {
const stats = await methods.stat(path, options)

stats.hash = stats.cid.toBaseEncodedString(options && options.cidBase)
stats.hash = cidToString(stats.cid, { base: options.cidBase })
delete stats.cid

return stats
}),
})),
write: callbackify.variadic(async (path, content, options = {}) => {
if (isPullStream.isSource(content)) {
content = pullStreamToAsyncIterator(content)