feat(lock): allow for custom lock

fix: failing tests

test: add a lock test suite

doc: update repo docs for custom locking

Author: jacobheun

README.md

@@ -142,7 +142,7 @@ Arguments:
 
 * `path` (string, mandatory): the path for this repo
 * `options` (object, optional): may contain the following values
-  * `lock` (string, defaults to `"fs"` in Node.js, `"memory"` in the browser): what type of lock to use. Lock has to be acquired when opening.
+  * `lock` (string *Deprecated* or [Lock](#lock)), string can be `"fs"` or `"memory"`: what type of lock to use. Lock has to be acquired when opening.
   * `storageBackends` (object, optional): may contain the following values, which should each be a class implementing the [datastore interface](https://github.com/ipfs/interface-datastore#readme):
     * `root` (defaults to [`datastore-fs`](https://github.com/ipfs/js-datastore-fs#readme) in Node.js and [`datastore-level`](https://github.com/ipfs/js-datastore-level#readme) in the browser). Defines the back-end type used for gets and puts of values at the root (`repo.set()`, `repo.get()`)
     * `blocks` (defaults to [`datastore-fs`](https://github.com/ipfs/js-datastore-fs#readme) in Node.js and [`datastore-level`](https://github.com/ipfs/js-datastore-level#readme) in the browser). Defines the back-end type used for gets and puts of values at `repo.blocks`.
@@ -284,7 +284,7 @@ Sets the API address.
 
 ### `repo.stat ([options], callback)`
 
-Gets the repo status. 
+Gets the repo status.
 
 `options` is an object which might contain the key `human`, which is a boolean indicating whether or not the `repoSize` should be displayed in MiB or not.
 
@@ -296,6 +296,37 @@ Gets the repo status.
 - `version`
 - `storageMax`
 
+### Lock
+
+IPFS Repo comes with two built in locks: memory and fs. These can be imported via the following:
+
+```js
+const fsLock = require('ipfs-repo/lock')  // Default in Node.js
+const memLock = require('ipfs-repo/lock-memory')  // Default in browser
+```
+
+#### `lock.open (dir, callback)`
+
+Sets the lock if one does not already exist.
+
+`dir` is a string to the directory the lock should be created at. The repo typically creates the lock at its root.
+
+`callback` is a function with the signature `function (err, closer)`, where `closer` has a `close` method for removing the lock.
+
+##### `closer.close (callback)`
+
+Closes the lock created by `lock.open`
+
+`callback` is a function with the signature `function (err)`. If no error was returned, the lock was successfully removed.
+
+#### `lock.locked (dir, callback)`
+
+Checks the existence of the lock.
+
+`dir` is a string to the directory to check for the lock. The repo typically checks for the lock at its root.
+
+`callback` is a function with the signature `function (err, boolean)`, where `boolean` indicates the existence of the lock.
+
 ## Notes
 
 - [Explanation of how repo is structured](https://github.com/ipfs/js-ipfs-repo/pull/111#issuecomment-279948247)

src/index.js

@@ -44,8 +44,7 @@ class IpfsRepo {
     this.closed = true
     this.path = repoPath
 
-    this._locker = lockers[this.options.lock]
-    assert(this._locker, 'Unknown lock type: ' + this.options.lock)
+    this._locker = this._getLocker()
 
     this.root = backends.create('root', this.path, this.options)
     this.version = version(this.root)
@@ -88,7 +87,7 @@ class IpfsRepo {
     waterfall([
       (cb) => this.root.open(ignoringAlreadyOpened(cb)),
       (cb) => this._isInitialized(cb),
-      (cb) => this._locker.lock(this.path, cb),
+      (cb) => this._openLock(this.path, cb),
       (lck, cb) => {
         log('aquired repo.lock')
         this.lockfile = lck
@@ -121,7 +120,7 @@ class IpfsRepo {
       }
     ], (err) => {
       if (err && this.lockfile) {
-        this.lockfile.close((err2) => {
+        this._closeLock((err2) => {
           if (!err2) {
             this.lockfile = null
           } else {
@@ -135,6 +134,61 @@ class IpfsRepo {
     })
   }
 
+  /**
+   * Returns the repo locker to be used. Null will be returned if no locker is requested
+   *
+   * @private
+   * @returns {Locker}
+   */
+  _getLocker () {
+    if (typeof this.options.lock === 'string') {
+      assert(lockers[this.options.lock], 'Unknown lock type: ' + this.options.lock)
+      return lockers[this.options.lock]
+    }
+
+    assert(this.options.lock, 'No lock provided')
+    return this.options.lock
+  }
+
+  /**
+   * Creates a lock on the repo if a locker is specified. The lockfile object will
+   * be returned in the callback if one has been created.
+   *
+   * @param {string} path
+   * @param {function(Error, lockfile)} callback
+   * @returns {void}
+   */
+  _openLock (path, callback) {
+    this._locker.lock(path, callback)
+  }
+
+  /**
+   * Closes the lock on the repo
+   *
+   * @param {function(Error)} callback
+   * @returns {void}
+   */
+  _closeLock (callback) {
+    if (this.lockfile) {
+      return this.lockfile.close(callback)
+    }
+    callback()
+  }
+
+  /**
+   * Gets the status of the lock on the repo
+   *
+   * @param {string} path
+   * @param {function(Error, boolean)} callback
+   * @returns {void}
+   */
+  _isLocked (path, callback) {
+    if (this._locker) {
+      return this._locker.locked(path, callback)
+    }
+    callback(null, false)
+  }
+
   /**
    * Check if the repo is already initialized.
    *
@@ -186,7 +240,7 @@ class IpfsRepo {
       (cb) => {
         log('unlocking')
         this.closed = true
-        this.lockfile.close(cb)
+        this._closeLock(cb)
       },
       (cb) => {
         this.lockfile = null

test/lock-test.js

@@ -0,0 +1,63 @@
+/* eslint-env mocha */
+'use strict'
+
+const chai = require('chai')
+chai.use(require('dirty-chai'))
+const expect = chai.expect
+const series = require('async/series')
+const IPFSRepo = require('../')
+
+module.exports = (repo) => {
+  describe('Repo lock tests', () => {
+    it('should handle locking for a repo lifecycle', (done) => {
+      expect(repo.lockfile).to.not.equal(null)
+      series([
+        (cb) => {
+          repo.close(cb)
+        },
+        (cb) => {
+          expect(repo.lockfile).to.equal(null)
+          cb()
+        },
+        (cb) => {
+          repo.open(cb)
+        }
+      ], done)
+    })
+
+    it('should prevent multiple repos from using the same path', (done) => {
+      const repoClone = new IPFSRepo(repo.path, repo.options)
+
+      // Levelup throws an uncaughtException when a lock already exists, catch it
+      const mochaExceptionHandler = process.listeners('uncaughtException').pop()
+      process.removeListener('uncaughtException', mochaExceptionHandler)
+      process.once('uncaughtException', function (err) {
+        expect(err.message).to.match(/already held/)
+      })
+
+      series([
+        (cb) => {
+          try {
+            repoClone.init({}, cb)
+          } catch (err) {
+            cb(err)
+          }
+        },
+        (cb) => {
+          repoClone.open(cb)
+        }
+      ], function (err) {
+        // There will be no listeners if the uncaughtException was triggered
+        if (process.listeners('uncaughtException').length > 0) {
+          expect(err.message).to.match(/already locked|already held|ENOENT/)
+        }
+
+        // Reset listeners to maintain test integrity
+        process.removeAllListeners('uncaughtException')
+        process.addListener('uncaughtException', mochaExceptionHandler)
+
+        done()
+      })
+    })
+  })
+}

test/node.js

@@ -3,6 +3,7 @@
 
 const ncp = require('ncp').ncp
 const rimraf = require('rimraf')
+const fs = require('fs')
 const path = require('path')
 const series = require('async/series')
 const chai = require('chai')
@@ -13,6 +14,35 @@ const IPFSRepo = require('../src')
 describe('IPFS Repo Tests onNode.js', () => {
   require('./options-test')
 
+  const customLock = {
+    lockName: 'test.lock',
+    lock: (dir, callback) => {
+      customLock.locked(dir, (err, isLocked) => {
+        if (err || isLocked) {
+          return callback(new Error('already locked'))
+        }
+
+        let lockPath = path.join(dir, customLock.lockName)
+        fs.writeFileSync(lockPath, '')
+
+        callback(null, {
+          close: (cb) => {
+            rimraf(lockPath, cb)
+          }
+        })
+      })
+    },
+    locked: (dir, callback) => {
+      fs.stat(path.join(dir, customLock.lockName), (err, stats) => {
+        if (err) {
+          callback(null, false)
+        } else {
+          callback(null, true)
+        }
+      })
+    }
+  }
+
   const repos = [{
     name: 'default inited',
     opts: undefined,
@@ -25,6 +55,12 @@ describe('IPFS Repo Tests onNode.js', () => {
       lock: 'memory'
     },
     init: true
+  }, {
+    name: 'custom locker',
+    opts: {
+      lock: customLock
+    },
+    init: true
   }, {
     name: 'default existing',
     opts: undefined,
@@ -62,6 +98,7 @@ describe('IPFS Repo Tests onNode.js', () => {
     require('./datastore-test')(repo)
     require('./keystore-test')(repo)
     require('./stat-test')(repo)
+    require('./lock-test')(repo)
     if (!r.init) {
       require('./interop-test')(repo)
     }

test/options-test.js

@@ -28,6 +28,19 @@ describe('custom options tests', () => {
     const repo = new Repo(repoPath)
     expect(repo.options).to.deep.equal(expectedRepoOptions())
   })
+
+  it('allows for a custom locker', () => {
+    const lock = {
+      lock: (path, callback) => { },
+      locked: (path, callback) => { }
+    }
+
+    const repo = new Repo(repoPath, {
+      lock
+    })
+
+    expect(repo._getLocker()).to.deep.equal(lock)
+  })
 })
 
 function noop () {}