feat: jsipfs pin improvements (#1249)

* initial sweep through to understand how pin works. Did make some changes but mostly minor.

* refactor pb schema to it's own file

* fix: don't pin files during files.add if opts.pin === false

* feat: add some http qs parsing, http route/resources cleanup, cleanup core/utils.parseIpfsPath

* feat: expand pin tests. \nFirst draft. still needs some further work.

* feat: Add logging for entry/exit of pins: add/rm/flush/load. Clean some documentation.

* feat: add --pin to files.add, fix: improper pin option parsing in core.

* feat: Use ipfs.files.add to add init-docs instead of directly using the unix-fs importer.

* feat(tests): Add tests for cli --pin option. I know this should be more of an integration test. Should be written in /core. Maybe talk with Victor about testing different layers

* feat: use isIPFS to valiate a multihash.

Author: JonKrone

package.json

@@ -107,10 +107,8 @@
     "hapi": "^16.6.2",
     "hapi-set-header": "^1.0.2",
     "hoek": "^5.0.3",
-    "interface-datastore": "^0.4.1",
-    "ipfs-api": "^18.0.0",
-    "ipfs-bitswap": "~0.19.0",
     "human-to-milliseconds": "^1.0.0",
+    "interface-datastore": "^0.4.1",
     "ipfs-api": "^18.1.1",
     "ipfs-bitswap": "~0.19.0",
     "ipfs-block": "~0.6.1",

src/cli/commands/files/add.js

@@ -172,6 +172,11 @@ module.exports = {
       type: 'boolean',
       default: false,
       describe: 'Write no output'
+    },
+    pin: {
+      type: 'boolean',
+      default: true,
+      describe: 'Pin this object when adding'
     }
   },
 
@@ -182,7 +187,8 @@ module.exports = {
       strategy: argv.trickle ? 'trickle' : 'balanced',
       shardSplitThreshold: argv.enableShardingExperiment ? argv.shardSplitThreshold : Infinity,
       'cid-version': argv['cid-version'],
-      'raw-leaves': argv['raw-leaves']
+      'raw-leaves': argv['raw-leaves'],
+      pin: argv.pin
     }
 
     // Temporary restriction on raw-leaves:

src/cli/commands/pin/add.js

@@ -1,5 +1,7 @@
 'use strict'
 
+const print = require('../../utils').print
+
 module.exports = {
   command: 'add <ipfs-path>',
 
@@ -21,7 +23,7 @@ module.exports = {
     argv.ipfs.pin.add(paths[0], { recursive: recursive }, (err, results) => {
       if (err) { throw err }
       results.forEach((res) => {
-        console.log(`pinned ${res.hash} ${type}ly`)
+        print(`pinned ${res.hash} ${type}ly`)
       })
     })
   }

src/cli/commands/pin/ls.js

@@ -1,21 +1,20 @@
 'use strict'
 
+const print = require('../../utils').print
+
 module.exports = {
-  command: 'ls',
+  // bracket syntax with '...' tells yargs to optionally accept a list
+  command: 'ls [ipfs-path...]',
 
   describe: 'List objects pinned to local storage.',
 
   builder: {
-    path: {
-      type: 'string',
-      describe: 'List pinned state of specific <ipfs-path>.'
-    },
     type: {
       type: 'string',
       alias: 't',
       default: 'all',
-      describe: ('The type of pinned keys to list. ' +
-                 'Can be "direct", "indirect", "recursive", or "all".')
+      choices: ['direct', 'indirect', 'recursive', 'all'],
+      describe: 'The type of pinned keys to list.'
     },
     quiet: {
       type: 'boolean',
@@ -26,7 +25,7 @@ module.exports = {
   },
 
   handler: (argv) => {
-    const paths = argv.path && argv.path.split(' ')
+    const paths = argv.ipfsPath || ''
     const type = argv.type
     const quiet = argv.quiet
     argv.ipfs.pin.ls(paths, { type: type }, (err, results) => {
@@ -36,7 +35,7 @@ module.exports = {
         if (!quiet) {
           line += ` ${res.type}`
         }
-        console.log(line)
+        print(line)
       })
     })
   }

src/cli/commands/pin/rm.js

@@ -1,5 +1,7 @@
 'use strict'
 
+const print = require('../../utils').print
+
 module.exports = {
   command: 'rm <ipfs-path>',
 
@@ -20,7 +22,7 @@ module.exports = {
     argv.ipfs.pin.rm(paths, { recursive: recursive }, (err, results) => {
       if (err) { throw err }
       results.forEach((res) => {
-        console.log(`unpinned ${res.hash}`)
+        print(`unpinned ${res.hash}`)
       })
     })
   }

src/core/components/files.js

@@ -18,6 +18,7 @@ const CID = require('cids')
 const toB58String = require('multihashes').toB58String
 
 function noop () {}
+function identity (x) { return x }
 
 function prepareFile (self, opts, file, callback) {
   opts = opts || {}
@@ -125,7 +126,8 @@ module.exports = function files (self) {
     }, options)
 
     let total = 0
-    let prog = opts.progress || (() => {})
+    const shouldPin = 'pin' in opts ? opts.pin : true
+    const prog = opts.progress || noop
     const progress = (bytes) => {
       total += bytes
       prog(total)
@@ -137,7 +139,7 @@ module.exports = function files (self) {
       pull.flatten(),
       importer(self._ipld, opts),
       pull.asyncMap(prepareFile.bind(null, self, opts)),
-      pull.asyncMap(pinFile.bind(null, self))
+      shouldPin ? pull.asyncMap(pinFile.bind(null, self)) : identity
     )
   }
 

src/core/components/init-assets.js

@@ -15,23 +15,20 @@ module.exports = function addDefaultAssets (self, log, callback) {
 
   pull(
     pull.values([initDocsPath]),
-    pull.asyncMap((val, cb) => glob(path.join(val, '/**/*'), cb)),
+    pull.asyncMap((val, cb) =>
+      glob(path.join(val, '/**/*'), { nodir: true }, cb)
+    ),
     pull.flatten(),
-    pull.map((element) => {
+    pull.map(element => {
       const addPath = element.substring(index + 1)
-
-      if (fs.statSync(element).isDirectory()) { return }
-
       return { path: addPath, content: file(element) }
     }),
-    // Filter out directories, which are undefined from above
-    pull.filter(Boolean),
-    importer(self._ipld),
-    pull.through((el) => {
-      if (el.path === 'init-docs') {
-        const cid = new CID(el.multihash)
+    self.files.addPullStream(),
+    pull.through(file => {
+      if (file.path === 'init-docs') {
+        const cid = new CID(file.hash)
         log('to get started, enter:\n')
-        log(`\t jsipfs files cat /ipfs/${cid.toBaseEncodedString()}/readme\n`)
+        log(`\tjsipfs files cat /ipfs/${cid.toBaseEncodedString()}/readme\n`)
       }
     }),
     pull.collect((err) => {

src/core/components/init.js

@@ -85,16 +85,11 @@ module.exports = function init (self) {
         }
 
         self.log('adding assets')
-        const tasks = [
+        parallel([
           // add empty unixfs dir object (go-ipfs assumes this exists)
-          (cb) => self.object.new('unixfs-dir', cb)
-        ]
-
-        if (typeof addDefaultAssets === 'function') {
-          tasks.push((cb) => addDefaultAssets(self, opts.log, cb))
-        }
-
-        parallel(tasks, (err) => {
+          (cb) => self.object.new('unixfs-dir', cb),
+          (cb) => addDefaultAssets(self, opts.log, cb)
+        ], (err) => {
           if (err) {
             cb(err)
           } else {

src/core/components/pin-set.js

@@ -11,28 +11,14 @@ const DAGLink = dagPB.DAGLink
 const varint = require('varint')
 const once = require('once')
 
+const pbSchema = require('./pin.proto')
+
 const emptyKeyHash = 'QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n'
 const emptyKey = multihashes.fromB58String(emptyKeyHash)
 const defaultFanout = 256
 const maxItems = 8192
-
-// Protobuf interface
-// from go-ipfs/pin/internal/pb/header.proto
-const pbSchema = `
-  syntax = "proto2";
-
-  package ipfs.pin;
-
-  option go_package = "pb";
-
-  message Set {
-    optional uint32 version = 1;
-    optional uint32 fanout = 2;
-    optional fixed32 seed = 3;
-  }
-`
-
 const pb = protobuf(pbSchema)
+
 function readHeader (rootNode) {
   // rootNode.data should be a buffer of the format:
   // < varint(headerLength) | header | itemData... >
@@ -63,7 +49,6 @@ exports = module.exports = function (dag) {
   const pinSet = {
     // should this be part of `object` API?
     hasChild: (root, childhash, callback, _links, _checked, _seen) => {
-      // callback (err, has)
       callback = once(callback)
       if (typeof childhash === 'object') {
         childhash = toB58String(childhash)
@@ -81,11 +66,13 @@ exports = module.exports = function (dag) {
         if (bs58link === childhash) {
           return callback(null, true)
         }
+
+        // don't check the same links twice
+        if (bs58link in _seen) { return }
+        _seen[bs58link] = true
+
         dag.get(new CID(link.multihash), (err, res) => {
           if (err) { return callback(err) }
-          // don't check the same links twice
-          if (bs58link in _seen) { return }
-          _seen[bs58link] = true
 
           _checked++
           _links += res.value.links.length
@@ -95,7 +82,6 @@ exports = module.exports = function (dag) {
     },
 
     storeSet: (keys, logInternalKey, callback) => {
-      // callback (err, rootNode)
       callback = once(callback)
       const items = keys.map((key) => {
         return {
@@ -115,10 +101,8 @@ exports = module.exports = function (dag) {
     },
 
     storeItems: (items, logInternalKey, callback, _depth, _subcalls, _done) => {
-      // callback (err, rootNode)
       callback = once(callback)
-      // const seed = crypto.randomBytes(4).readUInt32LE(0, true)  // old nondeterministic behavior
-      const seed = _depth // new deterministic behavior
+      const seed = _depth
       const pbHeader = pb.Set.encode({
         version: 1,
         fanout: defaultFanout,
@@ -210,9 +194,8 @@ exports = module.exports = function (dag) {
     },
 
     loadSet: (rootNode, name, logInternalKey, callback) => {
-      // callback (err, keys)
       callback = once(callback)
-      const link = rootNode.links.filter(l => l.name === name).pop()
+      const link = rootNode.links.find(l => l.name === name)
       if (!link) { return callback(new Error('No link found with name ' + name)) }
       logInternalKey(link.multihash)
       dag.get(new CID(link.multihash), (err, res) => {
@@ -229,7 +212,6 @@ exports = module.exports = function (dag) {
     },
 
     walkItems: (node, walkerFn, logInternalKey, callback) => {
-      // callback (err)
       callback = once(callback)
       const h = readHeader(node)
       if (h.err) { return callback(h.err) }