Skip to content

Commit

Permalink
feat(lock): allow for custom lock
Browse files Browse the repository at this point in the history
fix: failing tests

test: add a lock test suite

doc: update repo docs for custom locking
  • Loading branch information
jacobheun authored and daviddias committed Apr 23, 2018
1 parent d187e7d commit c97db6c
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 7 deletions.
35 changes: 33 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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.

Expand All @@ -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)
Expand Down
64 changes: 59 additions & 5 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -121,7 +120,7 @@ class IpfsRepo {
}
], (err) => {
if (err && this.lockfile) {
this.lockfile.close((err2) => {
this._closeLock((err2) => {
if (!err2) {
this.lockfile = null
} else {
Expand All @@ -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.
*
Expand Down Expand Up @@ -186,7 +240,7 @@ class IpfsRepo {
(cb) => {
log('unlocking')
this.closed = true
this.lockfile.close(cb)
this._closeLock(cb)
},
(cb) => {
this.lockfile = null
Expand Down
63 changes: 63 additions & 0 deletions test/lock-test.js
Original file line number Diff line number Diff line change
@@ -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()
})
})
})
}
37 changes: 37 additions & 0 deletions test/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down
13 changes: 13 additions & 0 deletions test/options-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {}
Expand Down

0 comments on commit c97db6c

Please sign in to comment.