Several fixes and missing features (#38)

* shorten lines

* only return data when `query.value == true`

* test `query.value = false`

* close mounted ds

* allow passing dispose to query iter constructor

* fix fs querying

* use currentSourcePath

* remove `dsobj` extensions from directories

* don't return error on missing key delete

* return `DatastoreKeyNotFound` on empty `get`

* return `DatastoreKeyNotFound` on missing sql key

datastore/fsds.nim

@@ -29,6 +29,7 @@ proc isRootSubdir*(self: FSDatastore, path: string): bool =
 proc path*(self: FSDatastore, key: Key): ?!string =
   ## Return filename corresponding to the key
   ## or failure if the key doesn't correspond to a valid filename
+  ##
 
   if not self.validDepth(key):
     return failure "Path has invalid depth!"
@@ -38,21 +39,25 @@ proc path*(self: FSDatastore, key: Key): ?!string =
 
   for ns in key:
     let basename = ns.value.extractFilename
-    if basename=="" or not basename.isValidFilename:
+    if basename == "" or not basename.isValidFilename:
       return failure "Filename contains invalid chars!"
 
     if ns.field == "":
       segments.add(ns.value)
     else:
       let basename = ns.field.extractFilename
-      if basename=="" or not basename.isValidFilename:
+      if basename == "" or not basename.isValidFilename:
         return failure "Filename contains invalid chars!"
 
       # `:` are replaced with `/`
       segments.add(ns.field / ns.value)
 
   let
-    fullname = (self.root / segments.joinPath()).absolutePath().catch().get().addFileExt(FileExt)
+    fullname = (self.root / segments.joinPath())
+      .absolutePath()
+      .catch()
+      .get()
+      .addFileExt(FileExt)
 
   if not self.isRootSubdir(fullname):
     return failure "Path is outside of `root` directory!"
@@ -67,14 +72,15 @@ method delete*(self: FSDatastore, key: Key): Future[?!void] {.async.} =
     return failure error
 
   if not path.fileExists():
-    return failure newException(DatastoreKeyNotFound, "Key not found!")
+    return success()
 
   try:
     removeFile(path)
-    return success()
   except OSError as e:
     return failure e
 
+  return success()
+
 method delete*(self: FSDatastore, keys: seq[Key]): Future[?!void] {.async.} =
   for key in keys:
     if err =? (await self.delete(key)).errorOption:
@@ -117,14 +123,15 @@ method get*(self: FSDatastore, key: Key): Future[?!seq[byte]] {.async.} =
     return failure error
 
   if not path.fileExists():
-    return failure(newException(DatastoreKeyNotFound, "Key doesn't exist"))
+    return failure(
+      newException(DatastoreKeyNotFound, "Key doesn't exist"))
 
   return self.readFile(path)
 
 method put*(
   self: FSDatastore,
   key: Key,
-  data: seq[byte]): Future[?!void] {.async, locks: "unknown".} =
+  data: seq[byte]): Future[?!void] {.async.} =
 
   without path =? self.path(key), error:
     return failure error
@@ -139,7 +146,7 @@ method put*(
 
 method put*(
   self: FSDatastore,
-  batch: seq[BatchEntry]): Future[?!void] {.async, locks: "unknown".} =
+  batch: seq[BatchEntry]): Future[?!void] {.async.} =
 
   for entry in batch:
     if err =? (await self.put(entry.key, entry.data)).errorOption:
@@ -155,12 +162,26 @@ proc dirWalker(path: string): iterator: string {.gcsafe.} =
     except CatchableError as exc:
       raise newException(Defect, exc.msg)
 
+method close*(self: FSDatastore): Future[?!void] {.async.} =
+  return success()
+
 method query*(
   self: FSDatastore,
   query: Query): Future[?!QueryIter] {.async.} =
 
-  without basePath =? self.path(query.key).?parentDir, error:
+  without path =? self.path(query.key), error:
     return failure error
+
+  let basePath =
+    # it there is a file in the directory
+    # with the same name then list the contents
+    # of the directory, otherwise recurse
+    # into subdirectories
+    if path.fileExists:
+      path.parentDir
+    else:
+      path.changeFileExt("")
+
   let
     walker = dirWalker(basePath)
 
@@ -175,9 +196,6 @@ method query*(
       iter.finished = true
       return success (Key.none, EmptyBytes)
 
-    without data =? self.readFile((basePath / path).absolutePath), err:
-      return failure err
-
     var
       keyPath = basePath
 
@@ -187,6 +205,12 @@ method query*(
 
     let
       key = Key.init(keyPath).expect("should not fail")
+      data =
+        if query.value:
+          self.readFile((basePath / path).absolutePath)
+            .expect("Should read file")
+        else:
+          @[]
 
     return success (key.some, data)
 

datastore/mountedds.nim

@@ -119,6 +119,13 @@ method put*(
 
   return success()
 
+method close*(self: MountedDatastore): Future[?!void] {.async.} =
+  for s in self.stores.values:
+    discard await s.store.close()
+
+  # TODO: how to handle failed close?
+  return success()
+
 func new*(
   T: type MountedDatastore,
   stores: Table[Key, Datastore] = initTable[Key, Datastore]()): ?!T =

datastore/query.nim

@@ -12,11 +12,11 @@ type
     Descending
 
   Query* = object
-    key*: Key
-    value*: bool
-    limit*: int
-    offset*: int
-    sort*: SortOrder
+    key*: Key         # Key to be queried
+    value*: bool      # Flag to indicate if data should be returned
+    limit*: int       # Max items to return - not available in all backends
+    offset*: int      # Offset from which to start querying - not available in all backends
+    sort*: SortOrder  # Sort order - not available in all backends
 
   QueryResponse* = tuple[key: ?Key, data: seq[byte]]
   QueryEndedError* = object of DatastoreError
@@ -35,13 +35,13 @@ iterator items*(q: QueryIter): Future[?!QueryResponse] =
 proc defaultDispose(): Future[?!void] {.upraises: [], gcsafe, async.} =
   return success()
 
-proc new*(T: type QueryIter): T =
-  QueryIter(dispose: defaultDispose)
+proc new*(T: type QueryIter, dispose = defaultDispose): T =
+  QueryIter(dispose: dispose)
 
 proc init*(
   T: type Query,
   key: Key,
-  value = false,
+  value = true,
   sort = SortOrder.Assending,
   offset = 0,
   limit = -1): T =

datastore/sql/sqliteds.nim

@@ -1,4 +1,5 @@
 import std/times
+import std/options
 
 import pkg/chronos
 import pkg/questionable
@@ -47,7 +48,7 @@ method delete*(self: SQLiteDatastore, key: Key): Future[?!void] {.async.} =
 
 method delete*(self: SQLiteDatastore, keys: seq[Key]): Future[?!void] {.async.} =
   if err =? self.db.beginStmt.exec().errorOption:
-    return failure err.msg
+    return failure(err)
 
   for key in keys:
     if err =? self.db.deleteStmt.exec((key.id)).errorOption:
@@ -72,10 +73,12 @@ method get*(self: SQLiteDatastore, key: Key): Future[?!seq[byte]] {.async.} =
   proc onData(s: RawStmtPtr) =
     bytes = self.db.getDataCol()
 
-  if (
-    let res = self.db.getStmt.query((key.id), onData);
-    res.isErr):
-    return failure res.error.msg
+  if err =? self.db.getStmt.query((key.id), onData).errorOption:
+    return failure(err)
+
+  if bytes.len <= 0:
+    return failure(
+      newException(DatastoreKeyNotFound, "Key doesn't exist"))
 
   return success bytes
 
@@ -109,7 +112,10 @@ method query*(
 
   var
     iter = QueryIter()
-    queryStr = QueryStmtStr
+    queryStr = if query.value:
+        QueryStmtDataIdStr
+      else:
+        QueryStmtIdStr
 
   if query.sort == SortOrder.Descending:
     queryStr &= QueryStmtOrderDescending
@@ -158,10 +164,14 @@ method query*(
     of SQLITE_ROW:
       let
         key = Key.init(
-          $sqlite3_column_text_not_null(s,QueryStmtIdCol))
+          $sqlite3_column_text_not_null(s, QueryStmtIdCol))
           .expect("should not fail")
 
-        blob = sqlite3_column_blob(s, QueryStmtDataCol)
+        blob: ?pointer =
+          if query.value:
+              sqlite3_column_blob(s, QueryStmtDataCol).some
+            else:
+              pointer.none
 
       # detect out-of-memory error
       # see the conversion table and final paragraph of:
@@ -171,7 +181,7 @@ method query*(
       # the "data" column can be NULL so in order to detect an out-of-memory
       # error it is necessary to check that the result is a null pointer and
       # that the result code is an error code
-      if blob.isNil:
+      if blob.isSome and blob.get().isNil:
         let
           v = sqlite3_errcode(sqlite3_db_handle(s))
 
@@ -181,8 +191,13 @@ method query*(
 
       let
         dataLen = sqlite3_column_bytes(s, QueryStmtDataCol)
-        dataBytes = cast[ptr UncheckedArray[byte]](blob)
-        data = @(toOpenArray(dataBytes, 0, dataLen - 1))
+        data = if blob.isSome:
+            @(
+              toOpenArray(cast[ptr UncheckedArray[byte]](blob.get),
+              0,
+              dataLen - 1))
+          else:
+            @[]
 
       return success (key.some, data)
     of SQLITE_DONE:

datastore/sql/sqlitedsdb.nim

@@ -93,7 +93,12 @@ const
     ) VALUES (?, ?, ?)
   """
 
-  QueryStmtStr* = """
+  QueryStmtIdStr* = """
+    SELECT """ & IdColName & """ FROM """ & TableName &
+        """ WHERE """ & IdColName & """ GLOB ?
+  """
+
+  QueryStmtDataIdStr* = """
     SELECT """ & IdColName & """, """ & DataColName & """ FROM """ & TableName &
         """ WHERE """ & IdColName & """ GLOB ?
   """

tests/datastore/querycommontests.nim

@@ -10,15 +10,24 @@ import pkg/stew/byteutils
 import pkg/datastore
 
 template queryTests*(ds: Datastore, extended = true) {.dirty.} =
-  test "Key should query all key and all it's children":
-    let
-      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
+  var
+    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
+
+  test "Key should query all keys and all it's children":
+    let
       q = Query.init(key1)
 
     (await ds.put(key1, val1)).tryGet
@@ -44,15 +53,35 @@ template queryTests*(ds: Datastore, extended = true) {.dirty.} =
 
     (await iter.dispose()).tryGet
 
+  test "Key should query all keys without values":
+    let
+      q = Query.init(key1, value = false)
+
+    (await ds.put(key1, val1)).tryGet
+    (await ds.put(key2, val2)).tryGet
+    (await ds.put(key3, val3)).tryGet
+
+    let
+      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.len == 0
+
+      res[1].key.get == key2
+      res[1].data.len == 0
+
+      res[2].key.get == key3
+      res[2].data.len == 0
+
+    (await iter.dispose()).tryGet
+
   test "Key should not query parent":
     let
-      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
-
       q = Query.init(key2)
 
     (await ds.put(key1, val1)).tryGet
@@ -78,13 +107,6 @@ template queryTests*(ds: Datastore, extended = true) {.dirty.} =
   test "Key should all list all keys at the same level":
     let
       queryKey = Key.init("/a").tryGet
-      key1 = Key.init("/a/1").tryGet
-      key2 = Key.init("/a/2").tryGet
-      key3 = Key.init("/a/3").tryGet
-      val1 = "value for 1".toBytes
-      val2 = "value for 2".toBytes
-      val3 = "value for 3".toBytes
-
       q = Query.init(queryKey)
 
     (await ds.put(key1, val1)).tryGet
@@ -117,13 +139,16 @@ template queryTests*(ds: Datastore, extended = true) {.dirty.} =
 
   if extended:
     test "Should apply limit":
-
       let
         key = Key.init("/a").tryGet
         q = Query.init(key, limit = 10)
 
       for i in 0..<100:
-        (await ds.put(Key.init(key, Key.init("/" & $i).tryGet).tryGet, ("val " & $i).toBytes)).tryGet
+        let
+          key = Key.init(key, Key.init("/" & $i).tryGet).tryGet
+          val = ("val " & $i).toBytes
+
+        (await ds.put(key, val)).tryGet
 
       let
         iter = (await ds.query(q)).tryGet
@@ -142,7 +167,11 @@ template queryTests*(ds: Datastore, extended = true) {.dirty.} =
         q = Query.init(key, offset = 90)
 
       for i in 0..<100:
-        (await ds.put(Key.init(key, Key.init("/" & $i).tryGet).tryGet, ("val " & $i).toBytes)).tryGet
+        let
+          key = Key.init(key, Key.init("/" & $i).tryGet).tryGet
+          val = ("val " & $i).toBytes
+
+        (await ds.put(key, val)).tryGet
 
       let
         iter = (await ds.query(q)).tryGet
@@ -161,7 +190,11 @@ template queryTests*(ds: Datastore, extended = true) {.dirty.} =
         q = Query.init(key, offset = 95, limit = 5)
 
       for i in 0..<100:
-        (await ds.put(Key.init(key, Key.init("/" & $i).tryGet).tryGet, ("val " & $i).toBytes)).tryGet
+        let
+          key = Key.init(key, Key.init("/" & $i).tryGet).tryGet
+          val = ("val " & $i).toBytes
+
+        (await ds.put(key, val)).tryGet
 
       let
         iter = (await ds.query(q)).tryGet

tests/datastore/sql/testsqliteds.nim

@@ -27,7 +27,7 @@ suite "Test Basic SQLiteDatastore":
 
 suite "Test Read Only SQLiteDatastore":
   let
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     basePath = "tests_data"
     basePathAbs = path.parentDir / basePath
     filename = "test_store" & DbExt

tests/datastore/sql/testsqlitedsdb.nim

@@ -11,7 +11,7 @@ import pkg/datastore/sql/sqliteds
 
 suite "Test Open SQLite Datastore DB":
   let
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     basePath = "tests_data"
     basePathAbs = path.parentDir / basePath
     filename = "test_store" & DbExt
@@ -70,7 +70,7 @@ suite "Test Open SQLite Datastore DB":
 
 suite "Test SQLite Datastore DB operations":
   let
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     basePath = "tests_data"
     basePathAbs = path.parentDir / basePath
     filename = "test_store" & DbExt

tests/datastore/testfsds.nim

@@ -15,7 +15,7 @@ import ./querycommontests
 
 suite "Test Basic FSDatastore":
   let
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     basePath = "tests_data"
     basePathAbs = path.parentDir / basePath
     key = Key.init("/a/b").tryGet()
@@ -40,7 +40,7 @@ suite "Test Basic FSDatastore":
 
 suite "Test Misc FSDatastore":
   let
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     basePath = "tests_data"
     basePathAbs = path.parentDir / basePath
     bytes = "some bytes".toBytes
@@ -116,7 +116,7 @@ suite "Test Misc FSDatastore":
 
 suite "Test Query":
   let
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     basePath = "tests_data"
     basePathAbs = path.parentDir / basePath
 

tests/datastore/testkey.nim

@@ -145,10 +145,16 @@ suite "Key":
 
   test "should equal":
     check:
-      Key.init(Namespace.init("a:b").tryGet(), Namespace.init("c").tryGet()).tryGet() == Key.init("a:b/c").tryGet()
+      Key.init(
+        Namespace.init("a:b").tryGet(),
+        Namespace.init("c").tryGet()).tryGet() == Key.init("a:b/c").tryGet()
+
       Key.init("a:b", "c").tryGet() == Key.init("a:b/c").tryGet()
       Key.init("a:b/c").tryGet() == Key.init("a:b/c").tryGet()
-      Key.init(Namespace.init("a:b").tryGet(), Namespace.init("c").tryGet()).tryGet() != Key.init("c:b/a").tryGet()
+      Key.init(
+        Namespace.init("a:b").tryGet(),
+        Namespace.init("c").tryGet()).tryGet() != Key.init("c:b/a").tryGet()
+
       Key.init("a:b/c").tryGet() == Key.init("/a:b/c/").tryGet()
       Key.init("a:b/c").tryGet() == Key.init("///a:b///c///").tryGet()
       Key.init("a:X/b/c").tryGet() == Key.init("a:X/b/c").tryGet()

tests/datastore/testmountedds.nim

@@ -16,7 +16,7 @@ import ./dscommontests
 suite "Test Basic Mounted Datastore":
   let
     root = "tests" / "test_data"
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     rootAbs = path.parentDir / root
 
     key = Key.init("a:b/c/d:e").get

tests/datastore/testtieredds.nim

@@ -18,7 +18,7 @@ suite "Test Basic Tired Datastore":
     otherBytes = "some other bytes".toBytes
     key = Key.init("a:b/c/d:e").get
     root = "tests" / "test_data"
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     rootAbs = path.parentDir / root
 
   var
@@ -47,7 +47,7 @@ suite "TieredDatastore":
     bytes = @[1.byte, 2.byte, 3.byte]
     key = Key.init("a:b/c/d:e").get
     root = "tests" / "test_data"
-    (path, _, _) = instantiationInfo(-1, fullPaths = true) # get this file's name
+    path = currentSourcePath() # get this file's name
     rootAbs = path.parentDir / root
 
   var
@@ -105,8 +105,8 @@ suite "TieredDatastore":
     (await ds.put(key, bytes)).tryGet
     (await ds.delete(key)).tryGet
 
-    check:
-      (await ds1.get(key)).tryGet.len == 0
+    expect DatastoreKeyNotFound:
+      discard (await ds1.get(key)).tryGet
 
     expect DatastoreKeyNotFound:
       discard (await ds2.get(key)).tryGet
@@ -153,7 +153,3 @@ suite "TieredDatastore":
       (await ds2.get(key)).tryGet == bytes
       (await ds.get(key)).tryGet == bytes
       (await ds1.get(key)).tryGet == bytes
-
-  # # test "query":
-  # #   check:
-  # #     true