From f4989fcce5d74a648e7e2598a72a7b21948f4a85 Mon Sep 17 00:00:00 2001
From: Ben Bierens <39762930+benbierens@users.noreply.github.com>
Date: Thu, 23 May 2024 18:31:49 +0200
Subject: [PATCH] LevelDB review (#65)

* Replaces stew-results with results package

* Applies leveldb batch-put

* links in dispose call to leveldb wrapper

* Handles trailing wildcards in leveldb query.

* Fixes tests for leveldb typed-ds.

* Adjusts exception handling in leveldbds to match what can be raised by leveldb iterator callbacks.

* Pulls in leveldbstatic 0.1.4

* Replaces replace with substring in leveldbds query

* Adds cmake to windows CI install
---
 .github/workflows/tests.yml               |  1 +
 datastore.nimble                          |  2 +-
 datastore/leveldb/leveldbds.nim           | 30 +++++---
 tests/datastore/leveldb/testleveldbds.nim | 86 +++++++++++++++++++++--
 4 files changed, 105 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index 73df27f7..8e79e0d8 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -62,6 +62,7 @@ jobs:
             base-devel
             git
             mingw-w64-ucrt-x86_64-toolchain
+            mingw-w64-ucrt-x86_64-cmake
 
       - name: Checkout sources from GitHub
         uses: actions/checkout@v2
diff --git a/datastore.nimble b/datastore.nimble
index b0a61ff5..c13bfdaa 100644
--- a/datastore.nimble
+++ b/datastore.nimble
@@ -11,7 +11,7 @@ requires "nim >= 1.2.0",
          "chronos#c41599a", # FIXME change to Chronos >= 4.0.0 once it's out
          "questionable >= 0.10.15 & < 0.11.0",
          "sqlite3_abi",
-         "leveldbstatic >= 0.1.2",
+         "leveldbstatic >= 0.1.4",
          "stew",
          "unittest2"
 
diff --git a/datastore/leveldb/leveldbds.nim b/datastore/leveldb/leveldbds.nim
index 29089ecb..30e30f67 100644
--- a/datastore/leveldb/leveldbds.nim
+++ b/datastore/leveldb/leveldbds.nim
@@ -4,6 +4,7 @@ import std/options
 import std/tables
 import std/os
 import std/strformat
+import std/strutils
 
 import pkg/leveldbstatic
 import pkg/chronos
@@ -58,10 +59,14 @@ method put*(self: LevelDbDatastore, key: Key, data: seq[byte]): Future[?!void] {
     return failure("LevelDbDatastore.put exception: " & $e.msg)
 
 method put*(self: LevelDbDatastore, batch: seq[BatchEntry]): Future[?!void] {.async.} =
-  for entry in batch:
-    if err =? (await self.put(entry.key, entry.data)).errorOption:
-      return failure(err.msg)
-  return success()
+  try:
+    let b = newBatch()
+    for entry in batch:
+      b.put($(entry.key), string.fromBytes(entry.data))
+    self.db.write(b)
+    return success()
+  except LevelDbException as e:
+    return failure("LevelDbDatastore.put (batch) exception: " & $e.msg)
 
 method close*(self: LevelDbDatastore): Future[?!void] {.async.} =
   try:
@@ -70,6 +75,13 @@ method close*(self: LevelDbDatastore): Future[?!void] {.async.} =
   except LevelDbException as e:
     return failure("LevelDbDatastore.close exception: " & $e.msg)
 
+proc getQueryString(query: Query): string =
+  result = $(query.key)
+  let toTrim = ["/*", "\\*"]
+  for trim in toTrim:
+    if result.endswith(trim):
+      result = result[0 ..< ^(trim.len)]
+
 method query*(
   self: LevelDbDatastore,
   query: Query): Future[?!QueryIter] {.async, gcsafe.} =
@@ -80,7 +92,7 @@ method query*(
   var
     iter = QueryIter()
     dbIter = self.db.queryIter(
-      prefix = $(query.key),
+      prefix = getQueryString(query),
       keysOnly = not query.value,
       skip = query.offset,
       limit = query.limit
@@ -101,13 +113,13 @@ method query*(
         return success (key.some, valueStr.toBytes())
     except LevelDbException as e:
       return failure("LevelDbDatastore.query -> next exception: " & $e.msg)
-    except Exception as e:
-      return failure("Unknown exception in LevelDbDatastore.query -> next: " & $e.msg)
 
-  iter.next = next
-  iter.dispose = proc(): Future[?!void] {.async.} =
+  proc dispose(): Future[?!void] {.async.} =
+    dbIter.dispose()
     return success()
 
+  iter.next = next
+  iter.dispose = dispose
   return success iter
 
 method modifyGet*(
diff --git a/tests/datastore/leveldb/testleveldbds.nim b/tests/datastore/leveldb/testleveldbds.nim
index b0598802..16ae5ae9 100644
--- a/tests/datastore/leveldb/testleveldbds.nim
+++ b/tests/datastore/leveldb/testleveldbds.nim
@@ -53,10 +53,88 @@ suite "Test LevelDB Query":
   )
 
 suite "Test LevelDB Typed Query":
-  let
-    ds = SQLiteDatastore.new(Memory).tryGet()
+  let tempDir = getTempDir() / "testleveldbds"
+  var ds: LevelDbDatastore
 
-  teardownAll:
+  setup:
+    createdir(tempDir)
+    ds = LevelDbDatastore.new(tempDir).tryGet()
+
+  teardown:
+    (await ds.close()).tryGet
+    removeDir(tempDir)
+
+  test "Typed Queries":
+    typedDsQueryTests(ds)
+
+suite "LevelDB Query: keys should disregard trailing wildcards":
+  let tempDir = getTempDir() / "testleveldbds"
+  var
+    ds: LevelDbDatastore
+    key1: Key
+    key2: Key
+    key3: Key
+    val1: seq[byte]
+    val2: seq[byte]
+    val3: seq[byte]
+
+  setupAll:
+    key1 = Key.init("/a").tryGet
+    key2 = Key.init("/a/b").tryGet
+    key3 = Key.init("/a/b/c").tryGet
+    val1 = "value for 1".toBytes
+    val2 = "value for 2".toBytes
+    val3 = "value for 3".toBytes
+
+  setup:
+    createdir(tempDir)
+    ds = LevelDbDatastore.new(tempDir).tryGet()
+    (await ds.put(key1, val1)).tryGet
+    (await ds.put(key2, val2)).tryGet
+    (await ds.put(key3, val3)).tryGet
+
+  teardown:
     (await ds.close()).tryGet
+    removeDir(tempDir)
+
+  test "Forward":
+    let
+      q = Query.init(Key.init("/a/*").tryGet)
+      iter = (await ds.query(q)).tryGet
+      res = (await allFinished(toSeq(iter)))
+        .mapIt( it.read.tryGet )
+        .filterIt( it.key.isSome )
+
+    check:
+      res.len == 3
+      res[0].key.get == key1
+      res[0].data == val1
+
+      res[1].key.get == key2
+      res[1].data == val2
+
+      res[2].key.get == key3
+      res[2].data == val3
+
+    (await iter.dispose()).tryGet
+
+  test "Backwards":
+    let
+      q = Query.init(Key.init("/a\\*").tryGet)
+      iter = (await ds.query(q)).tryGet
+      res = (await allFinished(toSeq(iter)))
+        .mapIt( it.read.tryGet )
+        .filterIt( it.key.isSome )
+
+    check:
+      res.len == 3
+      res[0].key.get == key1
+      res[0].data == val1
+
+      res[1].key.get == key2
+      res[1].data == val2
+
+      res[2].key.get == key3
+      res[2].data == val3
 
-  typedDsQueryTests(ds)
+    (await iter.dispose()).tryGet