Make opening & closing idempotent and add events

Very similar to `levelup` but more precise. If `open()` and
`close()` are called repeatedly (while the previous call has not
yet completed) the last call dictates the final status. Callbacks
are not called until any pending state changes are done, meaning
that the status is not 'opening' or 'closing'. Same for events.

For example, in a sequence of calls like `open(); close(); open()`
the final status will be 'open', only the second call will error,
only an 'open' event is emitted, and all callbacks will see that
status is 'open'. The callbacks are called in the order that the
`open()` or `close()` calls were made.

In addition, unlike on `levelup`, it is safe to call `open()` while
status is 'closing'. It will wait for closing to complete and then
reopen.

We should now have complete safety, including in `leveldown` because
the native code there delays `close()` if any operations are in
flight. In other words, the JavaScript side in `abstract-leveldown`
prevents new operations before opening the db, and the C++ side in
`leveldown` prevents closing the db before operations completed.

Ref Level/leveldown#8
Ref Level/community#58

UPGRADING.md

@@ -43,7 +43,7 @@ This document describes breaking changes and how to upgrade. For a complete list
 
 All methods that take a callback now also support promises. They return a promise if no callback is provided, the same as `levelup`.
 
-The prototype of `require('abstract-leveldown').AbstractLevelDOWN` has changed. It now inherits from `require('events').EventEmitter`.
+The prototype of `require('abstract-leveldown').AbstractLevelDOWN` has changed. It now inherits from `require('events').EventEmitter`. Opening and closing is idempotent and safe, and emits the same events as `levelup` would (with the exception of the 'ready' alias that `levelup` has for the 'open' event - `abstract-leveldown` only emits 'open').
 
 On any operation, `abstract-leveldown` now checks if it's open. If not, it will either throw an error (if the relevant API is synchronous) or asynchronously yield an error. For example:
 

abstract-leveldown.js

@@ -11,6 +11,7 @@ const { getCallback, getOptions } = require('./lib/common')
 const hasOwnProperty = Object.prototype.hasOwnProperty
 const rangeOptions = ['lt', 'lte', 'gt', 'gte']
 const kPromise = Symbol('promise')
+const kLanded = Symbol('landed')
 
 function AbstractLevelDOWN (manifest) {
   EventEmitter.call(this)
@@ -34,22 +35,42 @@ AbstractLevelDOWN.prototype.open = function (options, callback) {
   options.createIfMissing = options.createIfMissing !== false
   options.errorIfExists = !!options.errorIfExists
 
+  const maybeOpened = () => {
+    if (this.status === 'closing' || this.status === 'opening') {
+      // Wait until pending state changes are done
+      this.once(kLanded, maybeOpened)
+    } else if (this.status !== 'open') {
+      callback(new Error('Database is not open'))
+    } else {
+      callback()
+    }
+  }
+
   if (this.status === 'new' || this.status === 'closed') {
     const oldStatus = this.status
+
     this.status = 'opening'
+    this.emit('opening')
+
     this._open(options, (err) => {
       if (err) {
         this.status = oldStatus
+        this.emit(kLanded)
         return callback(err)
       }
+
       this.status = 'open'
-      callback()
+      this.emit(kLanded)
+
+      // Only emit public event if pending state changes are done
+      if (this.status === 'open') this.emit('open')
+
+      maybeOpened()
     })
   } else if (this.status === 'open') {
-    this._nextTick(callback)
+    this._nextTick(maybeOpened)
   } else {
-    // Cannot handle yet for lack of events
-    this._nextTick(callback, new Error('Database is not open'))
+    this.once(kLanded, () => this.open(options, callback))
   }
 
   return callback[kPromise]
@@ -62,21 +83,40 @@ AbstractLevelDOWN.prototype._open = function (options, callback) {
 AbstractLevelDOWN.prototype.close = function (callback) {
   callback = fromCallback(callback, kPromise)
 
+  const maybeClosed = () => {
+    if (this.status === 'opening' || this.status === 'closing') {
+      // Wait until pending state changes are done
+      this.once(kLanded, maybeClosed)
+    } else if (this.status !== 'closed' && this.status !== 'new') {
+      callback(new Error('Database is not closed'))
+    } else {
+      callback()
+    }
+  }
+
   if (this.status === 'open') {
     this.status = 'closing'
+    this.emit('closing')
+
     this._close((err) => {
       if (err) {
         this.status = 'open'
+        this.emit(kLanded)
         return callback(err)
       }
+
       this.status = 'closed'
-      callback()
+      this.emit(kLanded)
+
+      // Only emit public event if pending state changes are done
+      if (this.status === 'closed') this.emit('closed')
+
+      maybeClosed()
     })
   } else if (this.status === 'closed' || this.status === 'new') {
-    this._nextTick(callback)
+    this._nextTick(maybeClosed)
   } else {
-    // Cannot handle yet for lack of events
-    this._nextTick(callback, new Error('Database is not open'))
+    this.once(kLanded, () => this.close(callback))
   }
 
   return callback[kPromise]

test/open-test.js

@@ -1,5 +1,7 @@
 'use strict'
 
+const { assertAsync } = require('./util')
+
 exports.open = function (test, testCommon) {
   test('test database open, no options', function (t) {
     const db = testCommon.factory()
@@ -74,6 +76,164 @@ exports.open = function (test, testCommon) {
       })
     }).catch(t.fail.bind(t))
   })
+
+  test('test database open and close in same tick', assertAsync.ctx(function (t) {
+    t.plan(10)
+
+    const db = testCommon.factory()
+    const order = []
+
+    db.open(assertAsync(function (err) {
+      order.push('A')
+      t.is(err && err.message, 'Database is not open', 'got open() error')
+      t.is(db.status, 'closed', 'is closed')
+    }))
+
+    t.is(db.status, 'opening', 'is opening')
+
+    // This wins from the open() call
+    db.close(assertAsync(function (err) {
+      order.push('B')
+      t.same(order, ['A', 'closed event', 'B'], 'order is correct')
+      t.ifError(err, 'no close() error')
+      t.is(db.status, 'closed', 'is closed')
+    }))
+
+    // But open() is still in control
+    t.is(db.status, 'opening', 'is still opening')
+
+    // Should not emit 'open', because close() wins
+    db.on('open', t.fail.bind(t))
+    db.on('closed', assertAsync(() => { order.push('closed event') }))
+  }))
+
+  test('test database open, close and open in same tick', assertAsync.ctx(function (t) {
+    t.plan(14)
+
+    const db = testCommon.factory()
+    const order = []
+
+    db.open(assertAsync(function (err) {
+      order.push('A')
+      t.ifError(err, 'no open() error (1)')
+      t.is(db.status, 'open', 'is open')
+    }))
+
+    t.is(db.status, 'opening', 'is opening')
+
+    // This wins from the open() call
+    db.close(assertAsync(function (err) {
+      order.push('B')
+      t.is(err && err.message, 'Database is not closed')
+      t.is(db.status, 'open', 'is open')
+    }))
+
+    t.is(db.status, 'opening', 'is still opening')
+
+    // This wins from the close() call
+    db.open(assertAsync(function (err) {
+      order.push('C')
+      t.same(order, ['A', 'B', 'open event', 'C'], 'callback order is the same as call order')
+      t.ifError(err, 'no open() error (2)')
+      t.is(db.status, 'open', 'is open')
+    }))
+
+    // Should not emit 'closed', because open() wins
+    db.on('closed', t.fail.bind(t))
+    db.on('open', assertAsync(() => { order.push('open event') }))
+
+    t.is(db.status, 'opening', 'is still opening')
+  }))
+
+  test('test database open if already open', function (t) {
+    t.plan(7)
+
+    const db = testCommon.factory()
+
+    db.open(assertAsync(function (err) {
+      t.ifError(err, 'no open() error (1)')
+      t.is(db.status, 'open', 'is open')
+
+      db.open(assertAsync(function (err) {
+        t.ifError(err, 'no open() error (2)')
+        t.is(db.status, 'open', 'is open')
+      }))
+
+      t.is(db.status, 'open', 'is open', 'not reopening')
+      db.on('open', t.fail.bind(t))
+      assertAsync.end(t)
+    }))
+
+    assertAsync.end(t)
+  })
+
+  test('test database close if already closed', function (t) {
+    t.plan(8)
+
+    const db = testCommon.factory()
+
+    db.open(function (err) {
+      t.ifError(err, 'no open() error')
+
+      db.close(assertAsync(function (err) {
+        t.ifError(err, 'no close() error (1)')
+        t.is(db.status, 'closed', 'is closed')
+
+        db.close(assertAsync(function (err) {
+          t.ifError(err, 'no close() error (2)')
+          t.is(db.status, 'closed', 'is closed')
+        }))
+
+        t.is(db.status, 'closed', 'is closed', 'not reclosing')
+        db.on('closed', t.fail.bind(t))
+        assertAsync.end(t)
+      }))
+
+      assertAsync.end(t)
+    })
+  })
+
+  test('test database close if new', assertAsync.ctx(function (t) {
+    t.plan(5)
+
+    const db = testCommon.factory()
+    const expectedStatus = testCommon.deferredOpen ? 'opening' : 'new'
+
+    t.is(db.status, expectedStatus, 'status ok')
+
+    db.close(assertAsync(function (err) {
+      t.ifError(err, 'no close() error')
+      t.is(db.status, expectedStatus, 'status unchanged')
+    }))
+
+    t.is(db.status, expectedStatus, 'status unchanged')
+    db.on('closed', t.fail.bind(t))
+  }))
+
+  test('test database close on open event', function (t) {
+    t.plan(5)
+
+    const db = testCommon.factory()
+    const order = []
+
+    db.open(function (err) {
+      order.push('A')
+      t.is(err && err.message, 'Database is not open', 'got open() error')
+      t.is(db.status, 'closed', 'is closed')
+    })
+
+    db.on('open', function () {
+      // This wins from the (still in progress) open() call
+      db.close(function (err) {
+        order.push('B')
+        t.same(order, ['A', 'closed event', 'B'], 'order is correct')
+        t.ifError(err, 'no close() error')
+        t.is(db.status, 'closed', 'is closed')
+      })
+    })
+
+    db.on('closed', () => { order.push('closed event') })
+  })
 }
 
 exports.all = function (test, testCommon) {