Add own status

Ref #726

lib/levelup.js

@@ -13,6 +13,7 @@ const getOptions = require('./common').getOptions
 
 // TODO: after we drop node 10, also use queueMicrotask() in node
 const nextTick = require('./next-tick')
+const kStatus = Symbol('status')
 
 const WriteError = errors.WriteError
 const ReadError = errors.ReadError
@@ -50,6 +51,7 @@ function LevelUP (db, options, callback) {
   }
 
   this.options = getOptions(options)
+  this[kStatus] = 'new'
   this._db = db
   this.db = null
   this.open(callback || ((err) => {
@@ -80,17 +82,15 @@ LevelUP.prototype.emit = EventEmitter.prototype.emit
 LevelUP.prototype.once = EventEmitter.prototype.once
 inherits(LevelUP, EventEmitter)
 
-// TODO: tests
 Object.defineProperty(LevelUP.prototype, 'status', {
   enumerable: true,
   get () {
-    return this.db.status
+    return this[kStatus]
   }
 })
 
-// TODO: tests
 LevelUP.prototype.isOperational = function () {
-  return this.db.status === 'open' || this.db.status === 'opening'
+  return this[kStatus] === 'open' || this[kStatus] === 'opening'
 }
 
 LevelUP.prototype.open = function (opts, callback) {
@@ -105,29 +105,35 @@ LevelUP.prototype.open = function (opts, callback) {
     opts = this.options
   }
 
-  // 1) Don't check db.status until levelup has opened,
-  // in order for levelup events to be consistent
-  if (this.db && this.isOpen()) {
+  if (this[kStatus] === 'open') {
     nextTick(callback, null, this)
     return callback.promise
   }
 
-  if (this.db && this._isOpening()) {
+  if (this[kStatus] === 'opening') {
     this.once('open', () => { callback(null, this) })
     return callback.promise
   }
 
-  // 2) Instead let deferred-leveldown handle already-open cases.
-  // TODO: ideally though, levelup would have its own status
+  const oldStatus = this[kStatus]
+  this[kStatus] = 'opening'
+
+  // Let deferred-leveldown handle already-open cases
   this.db = new DeferredLevelDOWN(this._db)
   this.emit('opening')
 
   this.db.open(opts, (err) => {
     if (err) {
+      this[kStatus] = oldStatus
       return callback(new OpenError(err))
     }
+
+    this[kStatus] = 'open'
     this.db = this._db
+
+    // TODO (next major): call after events, so we can check if still open
     callback(null, this)
+
     this.emit('open')
     this.emit('ready')
   })
@@ -137,18 +143,28 @@ LevelUP.prototype.open = function (opts, callback) {
 
 LevelUP.prototype.close = function (callback) {
   callback = catering.fromCallback(callback)
+  const oldStatus = this[kStatus]
+
+  if (this[kStatus] === 'open') {
+    this[kStatus] = 'closing'
 
-  if (this.isOpen()) {
     this.db.close((err, ...rest) => {
+      if (err) {
+        this[kStatus] = oldStatus
+        return callback(err)
+      }
+      this[kStatus] = 'closed'
       this.emit('closed')
-      callback(err, ...rest)
+      callback(null, ...rest)
     })
+
+    // TODO (next major): emit before close() for consistency with 'opening'
     this.emit('closing')
-  } else if (this.isClosed()) {
+  } else if (this[kStatus] === 'closed' || this[kStatus] === 'new') {
     nextTick(callback)
-  } else if (this.db.status === 'closing') {
+  } else if (this[kStatus] === 'closing') {
     this.once('closed', callback)
-  } else if (this._isOpening()) {
+  } else if (this[kStatus] === 'opening') {
     this.once('open', () => {
       this.close(callback)
     })
@@ -159,17 +175,18 @@ LevelUP.prototype.close = function (callback) {
 
 // TODO: remove in future major
 LevelUP.prototype.isOpen = function () {
-  return this.db.status === 'open'
+  return this[kStatus] === 'open'
 }
 
 // TODO: remove in future major
 LevelUP.prototype._isOpening = function () {
-  return this.db.status === 'opening'
+  return this[kStatus] === 'opening'
 }
 
 // TODO: remove in future major
+// TODO: matches 'closing'?
 LevelUP.prototype.isClosed = function () {
-  return (/^clos|new/).test(this.db.status)
+  return (/^clos|new/).test(this[kStatus])
 }
 
 LevelUP.prototype.get = function (key, options, callback) {

test/init-test.js

@@ -20,13 +20,14 @@ module.exports = function (test, testCommon) {
   test('Init & open(): open and close statuses', function (t) {
     levelup(memdown(), function (err, db) {
       t.ifError(err, 'no error')
-      t.is(db.isOpen(), true)
+      t.is(db.status, 'open')
+      t.is(db.isOperational(), true)
 
       db.close(function (err) {
         t.ifError(err)
 
-        t.is(db.isOpen(), false)
-        t.is(db.isClosed(), true)
+        t.is(db.status, 'closed')
+        t.is(db.isOperational(), false)
 
         levelup(memdown(), function (err, db) {
           t.ifError(err)
@@ -35,50 +36,62 @@ module.exports = function (test, testCommon) {
           db.close(t.end.bind(t))
         })
       })
+
+      t.is(db.isOperational(), false)
     })
   })
 
   test('Init & open(): without callback', function (t) {
     const db = levelup(memdown())
-    t.ok(typeof db === 'object' && db !== null)
+
+    t.is(db.isOperational(), true)
+
     db.on('ready', function () {
-      t.is(db.isOpen(), true)
+      t.is(db.status, 'open')
+      t.is(db.isOperational(), true)
+
       db.close(t.end.bind(t))
     })
   })
 
   test('Init & open(): error with callback', function (t) {
-    t.plan(1)
+    t.plan(3)
 
     const mem = memdown()
     mem._open = function (opts, cb) {
       nextTick(cb, new Error('from underlying store'))
     }
 
-    levelup(mem, function (err) {
+    const db = levelup(mem, function (err) {
       t.is(err.message, 'from underlying store')
+      t.is(db.isOperational(), false)
     }).on('open', function () {
       t.fail('should not finish opening')
     }).on('error', function () {
       t.fail('should not emit error')
     })
+
+    t.is(db.isOperational(), true)
   })
 
   test('Init & open(): error without callback', function (t) {
-    t.plan(1)
+    t.plan(3)
 
     const mem = memdown()
     mem._open = function (opts, cb) {
       nextTick(cb, new Error('from underlying store'))
     }
 
-    levelup(mem)
+    const db = levelup(mem)
       .on('open', function () {
         t.fail('should not finish opening')
       })
       .on('error', function (err) {
         t.is(err.message, 'from underlying store')
+        t.is(db.isOperational(), false)
       })
+
+    t.is(db.isOperational(), true)
   })
 
   test('Init & open(): validate abstract-leveldown', function (t) {

test/maybe-error-test.js

@@ -6,12 +6,11 @@ module.exports = function (test, testCommon) {
   test('maybeError() should be called async: put()', function (t) {
     discardable(t, testCommon, function (db, done) {
       db.close(function () {
-        t.is(db.isClosed(), true, 'db is closed')
+        t.is(db.status, 'closed')
         let sync = false
         db.put('key', 'value', {}, function (err) {
           sync = true
-          t.ok(err)
-          t.is(err.message, 'Database is not open')
+          t.is(err && err.message, 'Database is not open')
         })
         t.is(sync, false, '.put cb called asynchronously')
         done() // TODO: called too soon, we still have 2 pending assertions
@@ -24,12 +23,11 @@ module.exports = function (test, testCommon) {
       db.put('key', 'value', {}, function (err) {
         t.ifError(err)
         db.close(function () {
-          t.is(db.isClosed(), true, 'db is closed')
+          t.is(db.status, 'closed')
           let sync = false
           db.get('key', {}, function (err, value) {
             sync = true
-            t.ok(err)
-            t.is(err.message, 'Database is not open')
+            t.is(err && err.message, 'Database is not open')
           })
           t.is(sync, false, '.get cb called asynchronously')
           done()
@@ -43,12 +41,11 @@ module.exports = function (test, testCommon) {
       db.put('key', 'value', {}, function (err) {
         t.ifError(err)
         db.close(function () {
-          t.is(db.isClosed(), true, 'db is closed')
+          t.is(db.status, 'closed')
           let sync = false
           db.del('key', {}, function (err) {
             sync = true
-            t.ok(err)
-            t.is(err.message, 'Database is not open')
+            t.is(err && err.message, 'Database is not open')
           })
           t.is(sync, false, '.del cb called asynchronously')
           done()
@@ -60,12 +57,11 @@ module.exports = function (test, testCommon) {
   test('maybeError() should be called async: batch()', function (t) {
     discardable(t, testCommon, function (db, done) {
       db.close(function () {
-        t.is(db.isClosed(), true, 'db is closed')
+        t.is(db.status, 'closed')
         let sync = false
-        db.batch([{ type: 'put', key: 'key' }], {}, function (err) {
+        db.batch([{ type: 'put', key: 'key', value: 'v' }], {}, function (err) {
           sync = true
-          t.ok(err)
-          t.is(err.message, 'Database is not open')
+          t.is(err && err.message, 'Database is not open')
         })
         t.is(sync, false, '.batch cb called asynchronously')
         done()

test/open-patchsafe-test.js

@@ -57,10 +57,7 @@ module.exports = function (test, testCommon) {
         db.close(t.end.bind(t))
       })
 
-      // Expected state is 'opening'
-      t.is(db._isOpening(), true)
-      t.is(db.isOpen(), false)
-      t.is(db.isClosed(), false)
+      t.is(db.status, 'opening')
     }
   }
 }

test/read-stream-test.js

@@ -420,9 +420,7 @@ module.exports = function (test, testCommon) {
           t.ifError(err, 'no open error')
         })
 
-        // is in limbo
-        t.is(db.isOpen(), false)
-        t.is(db.isClosed(), false)
+        t.is(db.status, 'opening')
 
         const rs = createReadStream(db)
         rs.on('data', ctx.dataSpy)

test/self.js

@@ -34,6 +34,8 @@ suite({
 // TODO to make this pass:
 // - Have abstract-leveldown use level-errors
 // - Perform type checks in same order (e.g. check key before callback)
+// - Add status checks to abstract-leveldown
+// - Add chainedBatch.length to abstract-leveldown, or remove from levelup
 suite({
   test: noop,
   factory: function (options) {