Merge pull request #130 from Level/key-types

Fix and test key types

Author: vweevers

index.js

@@ -5,7 +5,6 @@
 module.exports = Level
 
 var AbstractLevelDOWN = require('abstract-leveldown').AbstractLevelDOWN
-var isDate = require('is-date-object')
 var util = require('util')
 var Iterator = require('./iterator')
 var mixedToBuffer = require('./util/mixed-to-buffer')
@@ -117,20 +116,22 @@ Level.prototype._put = function (key, value, options, callback) {
 // - Number, except NaN. Includes Infinity and -Infinity
 // - Date, except invalid (NaN)
 // - String
-// - ArrayBuffer or a view thereof (typed arrays). In level-js we only support
-//   Buffer (which is an Uint8Array).
+// - ArrayBuffer or a view thereof (typed arrays). In level-js we also support
+//   Buffer (which is an Uint8Array) (and the primary binary type of Level).
 // - Array, except cyclical and empty (e.g. Array(10)). Elements must be valid
 //   types themselves.
 Level.prototype._serializeKey = function (key) {
   if (Buffer.isBuffer(key)) {
     return Level.binaryKeys ? key : key.toString()
   } else if (Array.isArray(key)) {
     return Level.arrayKeys ? key.map(this._serializeKey, this) : String(key)
-  } else if ((typeof key === 'number' || isDate(key)) && !isNaN(key)) {
+  } else if (typeof key === 'boolean' || (typeof key === 'number' && isNaN(key))) {
+    // These types are invalid per the IndexedDB spec and ideally we'd treat
+    // them that way, but they're valid per the current abstract test suite.
+    return String(key)
+  } else {
     return key
   }
-
-  return String(key)
 }
 
 Level.prototype._serializeValue = function (value) {

package.json

@@ -37,7 +37,6 @@
   "dependencies": {
     "abstract-leveldown": "~5.0.0",
     "immediate": "~3.2.3",
-    "is-date-object": "~1.0.1",
     "ltgt": "^2.1.2",
     "typedarray-to-buffer": "~3.1.5"
   },

test/custom-test.js

@@ -105,6 +105,38 @@ module.exports = function (leveljs, test, testCommon) {
     })
   })
 
+  // This should be covered by abstract-leveldown tests, but that's
+  // prevented by process.browser checks (Level/abstract-leveldown#121).
+  leveljs.binaryKeys && test('iterator yields buffer keys', function (t) {
+    var db = leveljs(testCommon.location())
+
+    db.open(function (err) {
+      t.ifError(err, 'no open error')
+
+      db.batch([
+        { type: 'put', key: Buffer.from([0]), value: 0 },
+        { type: 'put', key: Buffer.from([1]), value: 1 }
+      ], function (err) {
+        t.ifError(err, 'no batch error')
+
+        var it = db.iterator({ valueAsBuffer: false })
+        testCommon.collectEntries(it, function (err, entries) {
+          t.ifError(err, 'no iterator error')
+
+          t.same(entries, [
+            { key: Buffer.from([0]), value: 0 },
+            { key: Buffer.from([1]), value: 1 }
+          ], 'keys are Buffers')
+
+          db.close(function (err) {
+            t.ifError(err, 'no close error')
+            t.end()
+          })
+        })
+      })
+    })
+  })
+
   // Adapted from a memdown test.
   test('iterator stringifies buffer input', function (t) {
     t.plan(6)

test/index.js

@@ -31,4 +31,5 @@ require('abstract-leveldown/abstract/iterator-range-test').all(leveljs, test, te
 // Additional tests for this implementation
 require('./custom-test')(leveljs, test, testCommon)
 require('./structured-clone-test')(leveljs, test, testCommon)
+require('./key-type-test')(leveljs, test, testCommon)
 require('./levelup-test')(leveljs, test, testCommon)

test/key-type-test.js

@@ -0,0 +1,130 @@
+/* global indexedDB */
+
+'use strict'
+
+var ta = require('./util/create-typed-array')
+var support = require('../util/support')
+
+var types = [
+  { type: 'number', value: -20 },
+  { type: '+Infinity', value: Infinity },
+  { type: '-Infinity', value: -Infinity },
+  { type: 'string', value: 'test' },
+  { type: 'Date', ctor: true, value: new Date() },
+  { type: 'Array', ctor: true, allowFailure: true, value: [0, '1'] },
+  { type: 'ArrayBuffer', ctor: true, allowFailure: true, value: ta(Buffer).buffer },
+  { type: 'Int8Array', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Uint8Array', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Uint8ClampedArray', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Int16Array', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Uint16Array', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Int32Array', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Uint32Array', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Float32Array', ctor: true, allowFailure: true, createValue: ta, view: true },
+  { type: 'Float64Array', ctor: true, allowFailure: true, createValue: ta, view: true }
+]
+
+// TODO: test types that are not supported by IndexedDB Second Edition
+// - Date NaN (should be rejected by IndexedDB)
+// - empty array (should be rejected by abstract-leveldown)
+// - array containing null (should be rejected by IndexedDB)
+// - cyclical array (not sure)
+// - Array(10) (not sure)
+// var illegalTypes = []
+
+// TODO: test types that are not supported by IndexedDB Second Edition, but get
+// stringified for abstract-leveldown compatibility.
+// - NaN
+// - boolean
+// var stringifiedTypes = []
+
+module.exports = function (leveljs, test, testCommon) {
+  var db
+
+  test('setUp', testCommon.setUp)
+  test('open', function (t) {
+    db = leveljs(testCommon.location())
+    db.open(t.end.bind(t))
+  })
+
+  types.forEach(function (item) {
+    var testName = item.name || item.type
+
+    test('key type: ' + testName, function (t) {
+      var Constructor = item.ctor ? global[item.type] : null
+      var skip = item.allowFailure ? 'pass' : 'fail'
+      var input = item.value
+
+      if (item.ctor && !Constructor) {
+        t[skip]('constructor is undefined in this environment')
+        return t.end()
+      }
+
+      if (item.createValue) {
+        try {
+          input = item.createValue(Constructor)
+        } catch (err) {
+          t[skip]('constructor is not spec-compliant in this environment')
+          return t.end()
+        }
+      }
+
+      if (!support.test(input)(indexedDB)) {
+        t[skip]('type is not supported in this environment')
+        return t.end()
+      }
+
+      db.put(input, testName, function (err) {
+        t.ifError(err, 'no put error')
+
+        db.get(input, { asBuffer: false }, function (err, value) {
+          t.ifError(err, 'no get error')
+          t.same(value, testName, 'correct value')
+
+          var it = db.iterator({ keyAsBuffer: false, valueAsBuffer: false })
+
+          testCommon.collectEntries(it, function (err, entries) {
+            t.ifError(err, 'no iterator error')
+            t.is(entries.length, 1, '1 entry')
+
+            var key = entries[0].key
+            var value = entries[0].value
+
+            if (Constructor) {
+              var type = item.view ? 'ArrayBuffer' : item.type
+              var expected = '[object ' + type + ']'
+              var actual = Object.prototype.toString.call(key)
+
+              if (actual === expected) {
+                t.is(actual, expected, 'prototype')
+              } else {
+                t[skip]('(de)serializing is not supported by this environment')
+                return t.end()
+              }
+
+              if (item.view) {
+                t.ok(key instanceof ArrayBuffer, 'key is instanceof ArrayBuffer')
+                t.same(Buffer.from(new Constructor(key)), ta(Buffer), 'correct octets')
+              } else {
+                t.ok(key instanceof Constructor, 'key is instanceof ' + type)
+                t.same(key, input, 'correct key')
+              }
+            } else {
+              t.is(key, input, 'correct key')
+            }
+
+            t.same(value, testName, 'correct value')
+
+            db.del(input, function (err) {
+              t.ifError(err, 'no del error')
+              t.end()
+            })
+          })
+        })
+      })
+    })
+  })
+
+  test('close', function (t) { db.close(t.end.bind(t)) })
+  test('tearDown', testCommon.tearDown)
+}

test/structured-clone-test.js

@@ -1,14 +1,7 @@
 'use strict'
 
 var isDataCloneError = require('../util/is-data-clone-error')
-var bytes = [0, 127]
-
-// Replacement for TypedArray.from(bytes)
-function ta (TypedArray) {
-  var arr = new TypedArray(bytes.length)
-  for (var i = 0; i < bytes.length; i++) arr[i] = bytes[i]
-  return arr
-}
+var ta = require('./util/create-typed-array')
 
 // level-js supports all types of the structured clone algorithm
 // except for null and undefined (unless nested in another type).

test/util/create-typed-array.js

@@ -0,0 +1,10 @@
+'use strict'
+
+var bytes = [0, 127]
+
+// Replacement for TypedArray.from(bytes)
+module.exports = function (TypedArray) {
+  var arr = new TypedArray(bytes.length)
+  for (var i = 0; i < bytes.length; i++) arr[i] = bytes[i]
+  return arr
+}

util/mixed-to-buffer.js

@@ -4,5 +4,6 @@ var toBuffer = require('typedarray-to-buffer')
 
 module.exports = function (value) {
   if (value instanceof Uint8Array) return toBuffer(value)
+  else if (value instanceof ArrayBuffer) return Buffer.from(value) // For keys.
   else return Buffer.from(String(value))
 }

util/support.js

@@ -1,19 +1,15 @@
 'use strict'
 
-exports.binaryKeys = function (impl) {
-  try {
-    impl.cmp(new Uint8Array(0), 0)
-    return true
-  } catch (err) {
-    return false
+exports.test = function (key) {
+  return function test (impl) {
+    try {
+      impl.cmp(key, 0)
+      return true
+    } catch (err) {
+      return false
+    }
   }
 }
 
-exports.arrayKeys = function (impl) {
-  try {
-    impl.cmp([1], 0)
-    return true
-  } catch (err) {
-    return false
-  }
-}
+exports.binaryKeys = exports.test(new Uint8Array(0))
+exports.arrayKeys = exports.test([1])