From 2769ce1de21e595e712fd1df7a195c34cd5d18de Mon Sep 17 00:00:00 2001 From: "Michael Bradley, Jr" Date: Wed, 3 Aug 2022 20:54:50 -0500 Subject: [PATCH] refactor Datastore impls so root/basePath creation is user's responsibility --- datastore/filesystem_datastore.nim | 11 ++-- datastore/sqlite_datastore.nim | 15 ++--- tests/datastore/test_filesystem_datastore.nim | 36 ++++++------ tests/datastore/test_sqlite_datastore.nim | 56 +++++++++---------- tests/datastore/test_tiered_datastore.nim | 7 ++- 5 files changed, 59 insertions(+), 66 deletions(-) diff --git a/datastore/filesystem_datastore.nim b/datastore/filesystem_datastore.nim index 3e9ad16..51c50a0 100644 --- a/datastore/filesystem_datastore.nim +++ b/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 diff --git a/datastore/sqlite_datastore.nim b/datastore/sqlite_datastore.nim index c188ee1..0a4d321 100644 --- a/datastore/sqlite_datastore.nim +++ b/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 diff --git a/tests/datastore/test_filesystem_datastore.nim b/tests/datastore/test_filesystem_datastore.nim index 9e037ae..3c93244 100644 --- a/tests/datastore/test_filesystem_datastore.nim +++ b/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 diff --git a/tests/datastore/test_sqlite_datastore.nim b/tests/datastore/test_sqlite_datastore.nim index 2a902a7..e09767a 100644 --- a/tests/datastore/test_sqlite_datastore.nim +++ b/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 diff --git a/tests/datastore/test_tiered_datastore.nim b/tests/datastore/test_tiered_datastore.nim index 85bd7a5..5dfa68c 100644 --- a/tests/datastore/test_tiered_datastore.nim +++ b/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