Made del return a bool, and added clear. (#33)

* Made del return a bool, and added clear.

What I've done here feels very awkward. Maybe I'm missing something,
but it looks to me like RocksDB doesn't support these operations in
any natural way.

These changes were made in order to get our implementation of
KvStoreRef for RocksDB working again after these changes to the
KvStoreRef interface:

status-im/nim-eth@8f0ae55

I don't really recommend merging this; I think I'd prefer to just stop
trying to use this common KvStoreRef interface.

Still, if we do want to keep the common interface, I think this commit
will work well enough.

* Updated the tests to expect del to return a bool too.

* Leave the new `clear` operation unimplemented.

rocksdb.nim

@@ -11,6 +11,8 @@
 
 import cpuinfo, options, stew/[byteutils, results]
 
+from system/ansi_c import c_free
+
 export results
 
 const useCApi = true
@@ -37,6 +39,7 @@ type
     options*: rocksdb_options_t
     readOptions*: rocksdb_readoptions_t
     writeOptions: rocksdb_writeoptions_t
+    dbPath: string  # needed for clear()
 
   DataProc* = proc(val: openArray[byte]) {.gcsafe, raises: [Defect].}
 
@@ -57,6 +60,7 @@ proc init*(rocks: var RocksDBInstance,
   rocks.options = rocksdb_options_create()
   rocks.readOptions = rocksdb_readoptions_create()
   rocks.writeOptions = rocksdb_writeoptions_create()
+  rocks.dbPath = dbPath
 
   # Optimize RocksDB. This is the easiest way to get RocksDB to perform well:
   rocksdb_options_increase_parallelism(rocks.options, cpus.int32)
@@ -164,17 +168,6 @@ proc put*(db: RocksDBInstance, key, val: openArray[byte]): RocksDBResult[void] =
   bailOnErrors()
   ok()
 
-proc del*(db: RocksDBInstance, key: openArray[byte]): RocksDBResult[void] =
-  if key.len <= 0:
-    return err("rocksdb: key cannot be empty on del")
-
-  var errors: cstring
-  rocksdb_delete(db.db, db.writeOptions,
-                  cast[cstring](unsafeAddr key[0]), csize_t(key.len),
-                  errors.addr)
-  bailOnErrors()
-  ok()
-
 proc contains*(db: RocksDBInstance, key: openArray[byte]): RocksDBResult[bool] =
   if key.len <= 0:
     return err("rocksdb: key cannot be empty on contains")
@@ -192,6 +185,25 @@ proc contains*(db: RocksDBInstance, key: openArray[byte]): RocksDBResult[bool] =
   else:
     ok(false)
 
+proc del*(db: RocksDBInstance, key: openArray[byte]): RocksDBResult[bool] =
+  if key.len <= 0:
+    return err("rocksdb: key cannot be empty on del")
+
+  # This seems like a bad idea, but right now I don't want to
+  # get sidetracked by this. --Adam
+  if not db.contains(key).get:
+    return ok(false)
+
+  var errors: cstring
+  rocksdb_delete(db.db, db.writeOptions,
+                  cast[cstring](unsafeAddr key[0]), csize_t(key.len),
+                  errors.addr)
+  bailOnErrors()
+  ok(true)
+
+proc clear*(db: var RocksDBInstance): RocksDBResult[bool] =
+  raiseAssert "unimplemented"
+
 proc backup*(db: RocksDBInstance): RocksDBResult[void] =
   var errors: cstring
   rocksdb_backup_engine_create_new_backup(db.backupEngine, db.db, errors.addr)

tests/test_rocksdb.nim

@@ -68,8 +68,8 @@ suite "Nim API tests":
       var e2 = db.rocksdb.contains(otherKey)
       check e2.isok and e2.value == false
 
-      s = db.rocksdb.del(key)
-      check s.isok
+      var d = db.rocksdb.del(key)
+      check d.isok and d.value == true
 
       e1 = db.rocksdb.contains(key)
       check e1.isok and e1.value == false