Serialize compact range options

README.md

@@ -174,10 +174,16 @@ The `start` and `end` parameters may be strings or Buffers representing keys in
 The `callback` function will be called with a single `error` if the operation failed for any reason. If successful the first argument will be `null` and the second argument will be the approximate size as a Number.
 
 <a name="leveldown_compactRange"></a>
-### `db.compactRange(start, end, callback)`
+### `db.compactRange([start][, end], callback)`
 <code>compactRange()</code> is an instance method on an existing database object. Used to manually trigger a database compaction in the range `[start..end)`.
 
-The `start` and `end` parameters may be strings or Buffers representing keys in the LevelDB store.
+The `start` and `end` parameters may be strings or Buffers representing keys in the LevelDB store. If `start` is a zero-length string, a zero-length Buffer or omitted, it is treated as a key *before* all keys in the database. Likewise, if `end` meets those criteria, it is treated as a key *after* all keys in the database. Therefore the following call will compact the entire database:
+
+```js
+db.compactRange(callback)
+```
+
+Any other type of `start` or `end` will be converted to a string.
 
 The `callback` function will be called with no arguments if the operation is successful or with a single `error` argument if the operation failed for any reason.
 

leveldown.js

@@ -76,6 +76,21 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) {
 }
 
 LevelDOWN.prototype.compactRange = function (start, end, callback) {
+  // TODO: consider an options object instead.
+  if (typeof start === 'function') {
+    callback = start
+    start = ''
+    end = ''
+  } else if (typeof end === 'function') {
+    callback = end
+    end = ''
+  } else if (typeof callback !== 'function') {
+    throw new Error('compactRange() requires a callback argument')
+  }
+
+  start = this._serializeKey(start)
+  end = this._serializeKey(end)
+
   this.binding.compactRange(start, end, callback)
 }
 

src/database_async.cc

@@ -267,7 +267,11 @@ CompactRangeWorker::CompactRangeWorker(Database *database,
 };
 
 void CompactRangeWorker::Execute() {
-  database->CompactRangeFromDatabase(&rangeStart, &rangeEnd);
+  // TODO: use null as "not defined" signal in JS-land too.
+  database->CompactRangeFromDatabase(
+    rangeStart.empty() ? NULL : &rangeStart,
+    rangeEnd.empty() ? NULL : &rangeEnd
+  );
 }
 
 void CompactRangeWorker::WorkComplete() {

test/compact-range-test.js

@@ -1,34 +1,86 @@
 const test = require('tape')
 const testCommon = require('./common')
 
-let db
+const key1 = '000000'
+const key2 = '000001'
+const val1 = Buffer.allocUnsafe(64).fill(1)
+const val2 = Buffer.allocUnsafe(64).fill(1)
 
-test('setUp common', testCommon.setUp)
+test('setUp', testCommon.setUp)
 
-test('setUp db', function (t) {
-  db = testCommon.factory()
-  db.open(t.end.bind(t))
+make('test compactRange() frees disk space after key deletion', function (db, t, done) {
+  db.batch().put(key1, val1).put(key2, val2).write(function (err) {
+    t.ifError(err, 'no batch put error')
+
+    db.compactRange(key1, key2, function (err) {
+      t.ifError(err, 'no compactRange1 error')
+
+      db.approximateSize('0', 'z', function (err, sizeAfterPuts) {
+        t.error(err, 'no approximateSize1 error')
+
+        db.batch().del(key1).del(key2).write(function (err) {
+          t.ifError(err, 'no batch del error')
+
+          db.compactRange(key1, key2, function (err) {
+            t.ifError(err, 'no compactRange2 error')
+
+            db.approximateSize('0', 'z', function (err, sizeAfterCompact) {
+              t.error(err, 'no approximateSize2 error')
+              t.ok(sizeAfterCompact < sizeAfterPuts)
+              done()
+            })
+          })
+        })
+      })
+    })
+  })
+})
+
+make('test compactRange() serializes start and end', function (db, t, done) {
+  var count = 0
+
+  db._serializeKey = function (key) {
+    t.is(key, count++)
+    return String(key)
+  }
+
+  db.compactRange(0, 1, function (err) {
+    t.ifError(err, 'no compactRange error')
+    t.is(count, 2, '_serializeKey called twice')
+
+    done()
+  })
 })
 
-test('test compactRange() frees disk space after key deletion', function (t) {
-  var key1 = '000000'
-  var key2 = '000001'
-  var val1 = Buffer.allocUnsafe(64).fill(1)
-  var val2 = Buffer.allocUnsafe(64).fill(1)
-  db.put(key1, val1, function () {
-    db.put(key2, val2, function () {
-      db.compactRange(key1, key2, function () {
-        db.approximateSize('0', 'z', function (err, sizeAfterPuts) {
-          t.error(err, 'no error')
-          db.del(key1, function () {
-            db.del(key2, function () {
-              db.compactRange(key1, key2, function () {
-                db.approximateSize('0', 'z', function (err, sizeAfterCompact) {
-                  t.error(err, 'no error')
-                  t.ok(sizeAfterCompact < sizeAfterPuts)
-                  t.end()
-                })
-              })
+make('test compactRange() throws if callback is missing', function (db, t, done) {
+  try {
+    db.compactRange()
+  } catch (err) {
+    t.is(err.message, 'compactRange() requires a callback argument')
+    done()
+  }
+})
+
+make('test compactRange() without end', function (db, t, done) {
+  db.batch().put(key1, val1).put(key2, val2).write(function (err) {
+    t.ifError(err, 'no batch put error')
+
+    db.compactRange(key1, function (err) {
+      t.ifError(err, 'no compactRange1 error')
+
+      db.approximateSize('0', 'z', function (err, sizeAfterPuts) {
+        t.error(err, 'no approximateSize1 error')
+
+        db.batch().del(key1).del(key2).write(function (err) {
+          t.ifError(err, 'no batch del error')
+
+          db.compactRange(key1, function (err) {
+            t.ifError(err, 'no compactRange2 error')
+
+            db.approximateSize('0', 'z', function (err, sizeAfterCompact) {
+              t.error(err, 'no approximateSize2 error')
+              t.ok(sizeAfterCompact < sizeAfterPuts)
+              done()
             })
           })
         })
@@ -37,6 +89,79 @@ test('test compactRange() frees disk space after key deletion', function (t) {
   })
 })
 
-test('tearDown', function (t) {
-  db.close(testCommon.tearDown.bind(null, t))
+make('test compactRange() without start and end', function (db, t, done) {
+  db.batch().put(key1, val1).put(key2, val2).write(function (err) {
+    t.ifError(err, 'no batch put error')
+
+    db.compactRange(function (err) {
+      t.ifError(err, 'no compactRange1 error')
+
+      db.approximateSize('0', 'z', function (err, sizeAfterPuts) {
+        t.error(err, 'no approximateSize1 error')
+
+        db.batch().del(key1).del(key2).write(function (err) {
+          t.ifError(err, 'no batch del error')
+
+          db.compactRange(function (err) {
+            t.ifError(err, 'no compactRange2 error')
+
+            db.approximateSize('0', 'z', function (err, sizeAfterCompact) {
+              t.error(err, 'no approximateSize2 error')
+              t.ok(sizeAfterCompact < sizeAfterPuts)
+              done()
+            })
+          })
+        })
+      })
+    })
+  })
 })
+
+make('test compactRange() outside of key space', function (db, t, done) {
+  db.batch().put(key1, val1).put(key2, val2).write(function (err) {
+    t.ifError(err, 'no batch put error')
+
+    db.compactRange(function (err) {
+      t.ifError(err, 'no compactRange1 error')
+
+      db.approximateSize('0', 'z', function (err, sizeAfterPuts) {
+        t.error(err, 'no approximateSize1 error')
+
+        db.batch().del(key1).del(key2).write(function (err) {
+          t.ifError(err, 'no batch del error')
+
+          db.compactRange('z', function (err) {
+            t.ifError(err, 'no compactRange2 error')
+
+            db.approximateSize('0', 'z', function (err, sizeAfterCompact) {
+              t.error(err, 'no approximateSize2 error')
+              t.ok(sizeAfterCompact >= sizeAfterPuts, 'compactRange did nothing')
+              done()
+            })
+          })
+        })
+      })
+    })
+  })
+})
+
+test('tearDown', testCommon.tearDown)
+
+function make (name, testFn) {
+  test(name, function (t) {
+    var db = testCommon.factory()
+    var done = function (err) {
+      t.ifError(err, 'no done error')
+
+      db.close(function (err) {
+        t.ifError(err, 'no error from close()')
+        t.end()
+      })
+    }
+
+    db.open(function (err) {
+      t.ifError(err, 'no error from open()')
+      testFn(db, t, done)
+    })
+  })
+}