refactor Datastore impls so root/basePath creation is user's responsibility

datastore/filesystem_datastore.nim

@@ -21,18 +21,17 @@ const
 
 proc new*(
   T: type FileSystemDatastore,
-  root = "data"): ?!T =
+  root: string): ?!T =
 
   try:
     let
       root = if root.isAbsolute: root
              else: getCurrentDir() / root
 
-    createDir(root)
-    success T(root: root)
-
-  except IOError as e:
-    failure e
+    if not dirExists(root):
+      failure "directory does not exist: " & root
+    else:
+      success T(root: root)
 
   except OSError as e:
     failure e

datastore/sqlite_datastore.nim

@@ -58,6 +58,8 @@ const
   dataColType = "BLOB"
   timestampColType = "INTEGER"
 
+  memory* = ":memory:"
+
   # https://stackoverflow.com/a/9756276
   # EXISTS returns a boolean value represented by an integer:
   # https://sqlite.org/datatype3.html#boolean_datatype
@@ -172,10 +174,9 @@ proc timestampCol*(
 
 proc new*(
   T: type SQLiteDatastore,
-  basePath = "data",
+  basePath: string,
   filename = "store" & dbExt,
-  readOnly = false,
-  inMemory = false): ?!T =
+  readOnly = false): ?!T =
 
   # make it optional to enable WAL with it enabled being the default?
 
@@ -191,11 +192,11 @@ proc new*(
   var
     basep, fname, dbPath: string
 
-  if inMemory:
+  if basePath == memory:
     if readOnly:
       return failure "SQLiteDatastore cannot be read-only and in-memory"
     else:
-      dbPath = ":memory:"
+      dbPath = memory
   else:
     try:
       basep = normalizePathEnd(
@@ -207,8 +208,8 @@ proc new*(
 
       if readOnly and not fileExists(dbPath):
         return failure "read-only database does not exist: " & dbPath
-      else:
-        createDir(basep)
+      elif not dirExists(basep):
+        return failure "directory does not exist: " & basep
 
     except IOError as e:
       return failure e

tests/datastore/test_filesystem_datastore.nim

@@ -3,8 +3,10 @@ import std/os
 
 import pkg/asynctest/unittest2
 import pkg/chronos
+import pkg/questionable
+import pkg/questionable/results
 import pkg/stew/byteutils
-import pkg/stew/results
+from pkg/stew/results as stewResults import get, isOk
 
 import ../../datastore/filesystem_datastore
 import ./templates
@@ -18,6 +20,7 @@ suite "FileSystemDatastore":
   setup:
     removeDir(rootAbs)
     require(not dirExists(rootAbs))
+    createDir(rootAbs)
 
   teardown:
     removeDir(rootAbs)
@@ -25,25 +28,19 @@ suite "FileSystemDatastore":
 
   asyncTest "new":
     var
-      dsRes: Result[FileSystemDatastore, ref CatchableError]
-      ds: FileSystemDatastore
+      dsRes: ?!FileSystemDatastore
+
+    dsRes = FileSystemDatastore.new(rootAbs / "missing")
+
+    check: dsRes.isErr
 
     dsRes = FileSystemDatastore.new(rootAbs)
 
-    assert dsRes.isOk
-    ds = dsRes.get
-
-    check: dirExists(rootAbs)
-
-    removeDir(rootAbs)
-    assert not dirExists(rootAbs)
+    check: dsRes.isOk
 
     dsRes = FileSystemDatastore.new(root)
 
-    assert dsRes.isOk
-    ds = dsRes.get
-
-    check: dirExists(rootAbs)
+    check: dsRes.isOk
 
   asyncTest "accessors":
     let
@@ -142,18 +139,19 @@ suite "FileSystemDatastore":
     var
       containsRes = await ds.contains(key)
 
-    assert containsRes.isOk
-
-    check: containsRes.get == true
+    check:
+      containsRes.isOk
+      containsRes.get == true
 
     key = Key.init("X/Y/Z").get
     path = ds.path(key)
     assert not fileExists(path)
 
     containsRes = await ds.contains(key)
-    assert containsRes.isOk
 
-    check: containsRes.get == false
+    check:
+      containsRes.isOk
+      containsRes.get == false
 
   asyncTest "get":
     let

tests/datastore/test_sqlite_datastore.nim

@@ -23,6 +23,7 @@ suite "SQLiteDatastore":
   setup:
     removeDir(basePathAbs)
     require(not dirExists(basePathAbs))
+    createDir(basePathAbs)
 
   teardown:
     if not ds.isNil: ds.close
@@ -37,29 +38,28 @@ suite "SQLiteDatastore":
     # for `readOnly = true` to succeed the database file must already exist
     check: dsRes.isErr
 
+    dsRes = SQLiteDatastore.new(basePathAbs / "missing", filename)
+
+    check: dsRes.isErr
+
     dsRes = SQLiteDatastore.new(basePathAbs, filename)
 
-    assert dsRes.isOk
-    ds = dsRes.get
-
     check:
-      dirExists(basePathAbs)
+      dsRes.isOk
       fileExists(dbPathAbs)
 
-    ds.close
+    dsRes.get.close
     removeDir(basePathAbs)
     assert not dirExists(basePathAbs)
+    createDir(basePathAbs)
 
     dsRes = SQLiteDatastore.new(basePath, filename)
 
-    assert dsRes.isOk
-    ds = dsRes.get
-
     check:
-      dirExists(basePathAbs)
+      dsRes.isOk
       fileExists(dbPathAbs)
 
-    ds.close
+    dsRes.get.close
 
     # for `readOnly = true` to succeed the database file must already exist, so
     # the existing file (per previous step) is not deleted prior to the next
@@ -67,29 +67,20 @@ suite "SQLiteDatastore":
 
     dsRes = SQLiteDatastore.new(basePath, filename, readOnly = true)
 
-    assert dsRes.isOk
-    ds = dsRes.get
+    check: dsRes.isOk
 
-    check:
-      dirExists(basePathAbs)
-      fileExists(dbPathAbs)
-
-    ds.close
+    dsRes.get.close
     removeDir(basePathAbs)
     assert not dirExists(basePathAbs)
+    createDir(basePathAbs)
 
-    dsRes = SQLiteDatastore.new(inMemory = true)
+    dsRes = SQLiteDatastore.new(memory)
 
-    assert dsRes.isOk
-    ds = dsRes.get
+    check: dsRes.isOk
 
-    check:
-      not dirExists(basePathAbs)
-      not fileExists(dbPathAbs)
+    dsRes.get.close
 
-    ds.close
-
-    dsRes = SQLiteDatastore.new(readOnly = true, inMemory = true)
+    dsRes = SQLiteDatastore.new(memory, readOnly = true)
 
     check: dsRes.isErr
 
@@ -128,6 +119,7 @@ suite "SQLiteDatastore":
     ds.close
     removeDir(basePathAbs)
     assert not dirExists(basePathAbs)
+    createDir(basePathAbs)
 
     ds = SQLiteDatastore.new(basePathAbs, filename).get
 
@@ -229,6 +221,7 @@ suite "SQLiteDatastore":
     ds.close
     removeDir(basePathAbs)
     assert not dirExists(basePathAbs)
+    createDir(basePathAbs)
 
     ds = SQLiteDatastore.new(basePathAbs, filename).get
 
@@ -286,16 +279,17 @@ suite "SQLiteDatastore":
     var
       containsRes = await ds.contains(key)
 
-    assert containsRes.isOk
-
-    check: containsRes.get == true
+    check:
+      containsRes.isOk
+      containsRes.get == true
 
     key = Key.init("X/Y/Z").get
 
     containsRes = await ds.contains(key)
-    assert containsRes.isOk
 
-    check: containsRes.get == false
+    check:
+      containsRes.isOk
+      containsRes.get == false
 
   asyncTest "get":
     ds = SQLiteDatastore.new(basePathAbs, filename).get

tests/datastore/test_tiered_datastore.nim

@@ -23,10 +23,11 @@ suite "TieredDatastore":
     ds2: FileSystemDatastore
 
   setup:
-    ds1 = SQLiteDatastore.new(inMemory = true).get
-    ds2 = FileSystemDatastore.new(rootAbs).get
     removeDir(rootAbs)
     require(not dirExists(rootAbs))
+    createDir(rootAbs)
+    ds1 = SQLiteDatastore.new(memory).get
+    ds2 = FileSystemDatastore.new(rootAbs).get
 
   teardown:
     if not ds1.isNil: ds1.close
@@ -132,7 +133,7 @@ suite "TieredDatastore":
       (await ds2.get(key)).get.get == bytes
 
     ds1.close
-    ds1 = SQLiteDatastore.new(inMemory = true).get
+    ds1 = SQLiteDatastore.new(memory).get
     ds = TieredDatastore.new(ds1, ds2).get
 
     assert (await ds1.get(key)).get.isNone