fix: add backwards compatibility and more tests (#138)

* fix: backwards compatibility

Author: dignifiedquire

package.json

@@ -4,6 +4,9 @@
   "description": "IPFS Repo implementation",
   "main": "src/index.js",
   "browser": {
+    "rimraf": false,
+    "datastore-fs": "datastore-level",
+    "leveldown": "level-js",
     "./src/lock.js": "./src/lock-memory.js",
     "./src/default-options.js": "./src/default-options-browser.js"
   },
@@ -40,8 +43,8 @@
   },
   "devDependencies": {
     "aegir": "^11.0.2",
-    "chai": "^4.0.1",
-    "dirty-chai": "^1.2.2",
+    "chai": "^4.0.2",
+    "dirty-chai": "^2.0.0",
     "lodash": "^4.17.4",
     "memdown": "^1.2.4",
     "multihashes": "~0.4.5",
@@ -50,7 +53,7 @@
     "rimraf": "^2.6.1"
   },
   "dependencies": {
-    "async": "^2.4.1",
+    "async": "^2.5.0",
     "base32.js": "^0.1.0",
     "cids": "^0.5.0",
     "datastore-core": "^0.2.0",
@@ -60,10 +63,10 @@
     "interface-datastore": "^0.2.2",
     "ipfs-block": "~0.6.0",
     "level-js": "timkuijsten/level.js#idbunwrapper",
-    "leveldown": "^1.7.1",
+    "leveldown": "^1.7.2",
     "lock-me": "^1.0.2",
     "multiaddr": "^2.3.0",
-    "safe-buffer": "^5.1.0"
+    "safe-buffer": "^5.1.1"
   },
   "license": "MIT",
   "contributors": [
@@ -73,7 +76,6 @@
     "Francisco Baio Dias <[email protected]>",
     "Friedel Ziegelmayer <[email protected]>",
     "Greenkeeper <[email protected]>",
-    "Justin Chase <[email protected]>",
     "Lars-Magnus Skog <[email protected]>",
     "Pau Ramon Revilla <[email protected]>",
     "Richard Littauer <[email protected]>",
@@ -82,4 +84,4 @@
     "nginnever <[email protected]>",
     "npmcdn-to-unpkg-bot <[email protected]>"
   ]
-}
+}

src/default-options-browser.js

@@ -2,9 +2,11 @@
 
 // Default configuration for a repo in the browser
 module.exports = {
-  blockStore: require('datastore-level'),
-  blockStoreOptions: { db: require('level-js') },
-  dataStore: require('datastore-level'),
-  dataStoreOptions: { db: require('level-js') },
-  sharding: false
+  fs: require('datastore-level'),
+  sharding: false,
+  lock: 'memory',
+  fsOptions: {
+    db: require('level-js')
+  },
+  level: require('level-js')
 }

src/default-options.js

@@ -2,7 +2,8 @@
 
 // Default configuration for a repo in node.js
 module.exports = {
-  blockStore: require('datastore-fs'),
-  dataStore: require('datastore-level'),
-  dataStoreOptions: { db: require('leveldown') }
+  sharding: true,
+  lock: 'fs',
+  fs: require('datastore-fs'),
+  level: require('leveldown')
 }

src/index.js

@@ -5,6 +5,7 @@ const MountStore = core.MountDatastore
 const ShardingStore = core.ShardingDatastore
 
 const Key = require('interface-datastore').Key
+const LevelStore = require('datastore-level')
 const waterfall = require('async/waterfall')
 const series = require('async/series')
 const parallel = require('async/parallel')
@@ -17,12 +18,13 @@ const debug = require('debug')
 const version = require('./version')
 const config = require('./config')
 const blockstore = require('./blockstore')
+const defaultOptions = require('./default-options')
 
 const log = debug('repo')
 
 const apiFile = new Key('api')
-const blockStoreDirectory = 'blocks'
-const dataStoreDirectory = 'datastore'
+const flatfsDirectory = 'blocks'
+const levelDirectory = 'datastore'
 const repoVersion = 5
 
 /**
@@ -33,27 +35,27 @@ class IpfsRepo {
   /**
    * @param {string} repoPath - path where the repo is stored
    * @param {object} options - Configuration
-   * @param {datastore} options.blockStore
-   * @param {datastore} options.dataStore
-   * @param {object} [options.blockStoreOptions={}]
-   * @param {object} [options.dataStoreOptions={}]
+   * @param {Datastore} options.fs
+   * @param {Leveldown} options.level
+   * @param {object} [options.fsOptions={}]
    * @param {bool} [options.sharding=true] - Enable sharding (flatfs on disk), not needed in the browser.
    * @param {string} [options.lock='fs'] - Either `fs` or `memory`.
    */
   constructor (repoPath, options) {
     assert.equal(typeof repoPath, 'string', 'missing repoPath')
 
+    this.options = Object.assign({}, defaultOptions, options)
+
     this.closed = true
     this.path = repoPath
-    this.options = Object.assign({ lock: 'memory', sharding: true },
-                                 options, require('./default-options'))
-
-    const BlockStore = this.options.blockStore
-    this._blockStore = new BlockStore(this.path,
-      Object.assign(this.options.blockStoreOptions || {}, { extension: '' }))
+    const FsStore = this.options.fs
+    const fsOptions = Object.assign({}, this.options.fsOptions, {
+      extension: ''
+    })
+    this._fsStore = new FsStore(this.path, fsOptions)
 
-    this.version = version(this._blockStore)
-    this.config = config(this._blockStore)
+    this.version = version(this._fsStore)
+    this.config = config(this._fsStore)
 
     if (this.options.lock === 'memory') {
       this._locker = require('./lock-memory')
@@ -75,7 +77,7 @@ class IpfsRepo {
     log('initializing at: %s', this.path)
 
     series([
-      (cb) => this._blockStore.open((err) => {
+      (cb) => this._fsStore.open((err) => {
         if (err && err.message === 'Already open') {
           return cb()
         }
@@ -101,7 +103,7 @@ class IpfsRepo {
 
     // check if the repo is already initialized
     waterfall([
-      (cb) => this._blockStore.open((err) => {
+      (cb) => this._fsStore.open((err) => {
         if (err && err.message === 'Already open') {
           return cb()
         }
@@ -114,8 +116,8 @@ class IpfsRepo {
         this.lockfile = lck
 
         log('creating flatfs')
-        const BlockStore = this.options.blockStore
-        const s = new BlockStore(path.join(this.path, blockStoreDirectory), this.options.blockStoreOptions)
+        const FsStore = this.options.fs
+        const s = new FsStore(path.join(this.path, flatfsDirectory), Object.assign({}, this.options.fsOptions))
 
         if (this.options.sharding) {
           const shard = new core.shard.NextToLast(2)
@@ -124,21 +126,17 @@ class IpfsRepo {
           cb(null, s)
         }
       },
-      (blockStore, cb) => {
+      (flatfs, cb) => {
         log('Flatfs store opened')
-        const DataStore = this.options.dataStore
-        const dataStore = new DataStore(path.join(this.path, dataStoreDirectory), this.options.dataStoreOptions)
-        log(dataStore)
-        this.store = new MountStore([
-          {
-            prefix: new Key(blockStoreDirectory),
-            datastore: blockStore
-          },
-          {
-            prefix: new Key('/'),
-            datastore: dataStore
-          }
-        ])
+        this.store = new MountStore([{
+          prefix: new Key(flatfsDirectory),
+          datastore: flatfs
+        }, {
+          prefix: new Key('/'),
+          datastore: new LevelStore(path.join(this.path, levelDirectory), {
+            db: this.options.level
+          })
+        }])
 
         this.blockstore = blockstore(this)
         this.closed = false
@@ -194,14 +192,14 @@ class IpfsRepo {
 
     log('closing at: %s', this.path)
     series([
-      (cb) => this._blockStore.delete(apiFile, (err) => {
+      (cb) => this._fsStore.delete(apiFile, (err) => {
         if (err && err.message.startsWith('ENOENT')) {
           return cb()
         }
         cb(err)
       }),
       (cb) => this.store.close(cb),
-      (cb) => this._blockStore.close(cb),
+      (cb) => this._fsStore.close(cb),
       (cb) => {
         log('unlocking')
         this.closed = true
@@ -232,7 +230,7 @@ class IpfsRepo {
    * @returns {void}
    */
   setApiAddress (addr, callback) {
-    this._blockStore.put(apiFile, Buffer.from(addr.toString()), callback)
+    this._fsStore.put(apiFile, Buffer.from(addr.toString()), callback)
   }
 
   /**
@@ -242,7 +240,7 @@ class IpfsRepo {
    * @returns {void}
    */
   apiAddress (callback) {
-    this._blockStore.get(apiFile, (err, rawAddr) => {
+    this._fsStore.get(apiFile, (err, rawAddr) => {
       if (err) {
         return callback(err)
       }

test/browser.js

@@ -7,6 +7,7 @@ const series = require('async/series')
 const IPFSRepo = require('../src')
 
 describe('IPFS Repo Tests on the Browser', () => {
+  require('./options-test')
   const repo = new IPFSRepo('myrepo')
 
   before((done) => {

test/node.js

@@ -13,28 +13,37 @@ const expect = chai.expect
 const IPFSRepo = require('../src')
 
 describe('IPFS Repo Tests on on Node.js', () => {
-  const repos = [
-    {
-      name: 'default',
-      opts: undefined
+  require('./options-test')
+
+  const repos = [{
+    name: 'default',
+    opts: undefined,
+    init: false
+  }, {
+    name: 'memory',
+    opts: {
+      fs: require('interface-datastore').MemoryDatastore,
+      level: require('memdown'),
+      lock: 'memory'
     },
-    {
-      name: 'memory',
-      opts: {
-        blockStore: require('interface-datastore').MemoryDatastore,
-        dataStore: require('interface-datastore').MemoryDatastore
-      }
-    }
-  ]
+    init: true
+  }]
   repos.forEach((r) => describe(r.name, () => {
     const testRepoPath = path.join(__dirname, 'test-repo')
     const date = Date.now().toString()
     const repoPath = testRepoPath + '-for-' + date
+
     const repo = new IPFSRepo(repoPath, r.opts)
+
     before((done) => {
       series([
-        (cb) => ncp(testRepoPath, repoPath, cb),
-        (cb) => repo.init({}, cb),
+        (cb) => {
+          if (r.init) {
+            repo.init({}, cb)
+          } else {
+            ncp(testRepoPath, repoPath, cb)
+          }
+        },
         (cb) => repo.open(cb)
       ], done)
     })
@@ -66,7 +75,7 @@ describe('IPFS Repo Tests on on Node.js', () => {
     require('./repo-test')(repo)
     require('./blockstore-test')(repo)
     require('./datastore-test')(repo)
-    if (r.name === 'default') {
+    if (!r.init) {
       require('./interop-test')(repo)
     }
   }))

test/options-test.js

@@ -0,0 +1,52 @@
+/* eslint-env mocha */
+'use strict'
+
+const chai = require('chai')
+chai.use(require('dirty-chai'))
+const expect = chai.expect
+const path = require('path')
+const rimraf = require('rimraf')
+if (!rimraf.sync) {
+  // browser
+  rimraf.sync = noop
+}
+const Repo = require('../')
+
+describe('IPFS Repo options Tests', () => {
+  const repoPath = path.join(__dirname, 'slash', 'path')
+  after(() => {
+    rimraf.sync(repoPath)
+  })
+
+  it('missing repoPath', () => {
+    expect(
+      () => new Repo()
+    ).to.throw('missing repoPath')
+  })
+
+  it('default options', () => {
+    const repo = new Repo(repoPath)
+    expect(repo.options).to.deep.equal(expectedRepoOptions())
+  })
+})
+
+function noop () {}
+
+function expectedRepoOptions () {
+  const options = {
+    // packages are exchanged to browser-compatible
+    // equivalents via package.browser.
+    fs: require('datastore-fs'),
+    level: require('leveldown'),
+    lock: process.browser ? 'memory' : 'fs',
+    sharding: !process.browser
+  }
+
+  if (process.browser) {
+    options.fsOptions = {
+      db: require('leveldown')
+    }
+  }
+
+  return options
+}

test/repo-test.js

@@ -7,18 +7,8 @@ const expect = chai.expect
 const series = require('async/series')
 const waterfall = require('async/waterfall')
 
-const Repo = require('../src')
-
 module.exports = (repo) => {
   describe('IPFS Repo Tests', () => {
-    describe('new', () => {
-      it('missing arguments', () => {
-        expect(
-          () => new Repo()
-        ).to.throw(Error)
-      })
-    })
-
     it('check if Repo exists', (done) => {
       repo.exists((err, exists) => {
         expect(err).to.not.exist()
@@ -83,7 +73,6 @@ module.exports = (repo) => {
           (cb) => repo.open(cb),
           (cb) => repo.version.get(cb),
           (version, cb) => {
-            console.log(version)
             expect(version).to.exist()
             cb()
           }