diff --git a/datastore/datastore.nim b/datastore/datastore.nim index 47f339b1..7dfa9924 100644 --- a/datastore/datastore.nim +++ b/datastore/datastore.nim @@ -71,6 +71,9 @@ method modifyGet*(self: Datastore, key: Key, fn: ModifyGet): Future[?!seq[byte]] ## | some(u) | none | delete u | ## | some(u) | some(v) | replace u with v (if u != v) | ## + ## If `fn` raises an error, the value associated with `key` remains unchanged and raised error is wrapped + ## into a failure and returned as a result. + ## ## Note that `fn` can be called multiple times (when concurrent modification was detected). In such case ## only the last auxillary value is returned. ## diff --git a/datastore/defaultimpl.nim b/datastore/defaultimpl.nim index be7178ae..475d6272 100644 --- a/datastore/defaultimpl.nim +++ b/datastore/defaultimpl.nim @@ -37,6 +37,10 @@ proc defaultModifyGetImpl*( except CatchableError as err: return failure(err) + if maybeCurrentData == maybeNewData: + # no need to change currently stored value (if any) + return aux.success + if newData =? maybeNewData: if err =? (await self.put(key, newData)).errorOption: return failure(err) diff --git a/datastore/sql/sqliteds.nim b/datastore/sql/sqliteds.nim index 8f16f07d..70bc06fa 100644 --- a/datastore/sql/sqliteds.nim +++ b/datastore/sql/sqliteds.nim @@ -66,7 +66,7 @@ method modifyGet*(self: SQLiteDatastore, key: Key, fn: ModifyGet): Future[?!seq[ return failure(err) if maybeCurrentData == maybeNewData: - # no need to change currently stored value + # no need to change currently stored value (if any) break if err =? self.db.beginStmt.exec().errorOption: diff --git a/tests/datastore/testdefaultimpl.nim b/tests/datastore/testdefaultimpl.nim new file mode 100644 index 00000000..1c381cb2 --- /dev/null +++ b/tests/datastore/testdefaultimpl.nim @@ -0,0 +1,95 @@ +import std/options +import std/sequtils +import std/tables + +import pkg/asynctest/chronos/unittest2 +import pkg/chronos +import pkg/questionable +import pkg/questionable/results + +import pkg/datastore +import pkg/datastore/defaultimpl + +type + MockDatastore* = ref object of Datastore + values: TableRef[Key, seq[byte]] + lock: AsyncLock + calls: seq[MethodCall] + + Method = enum + Put, Get, Delete, Has + + MethodCall = object + key: Key + case kind: Method + of Put: + data: seq[byte] + of Get, Delete, Has: + discard + +method put*(self: MockDatastore, key: Key, data: seq[byte]): Future[?!void] {.async.} = + self.calls.add(MethodCall(kind: Put, key: key, data: data)) + self.values[key] = data + success() + +method get*(self: MockDatastore, key: Key): Future[?!seq[byte]] {.async.} = + self.calls.add(MethodCall(kind: Get, key: key)) + if key notin self.values: + failure(newException(DatastoreKeyNotFound, "Key doesn't exist")) + else: + success(self.values[key]) + +method has*(self: MockDatastore, key: Key): Future[?!bool] {.async.} = + self.calls.add(MethodCall(kind: Has, key: key)) + success(key in self.values) + +method delete*(self: MockDatastore, key: Key): Future[?!void] {.async.} = + self.calls.add(MethodCall(kind: Delete, key: key)) + self.values.del(key) + success() + +method modifyGet*(self: MockDatastore, key: Key, fn: ModifyGet): Future[?!seq[byte]] {.async.} = + await defaultModifyGetImpl(self, self.lock, key, fn) + +method modify*(self: MockDatastore, key: Key, fn: Modify): Future[?!void] {.async.} = + await defaultModifyImpl(self, self.lock, key, fn) + +proc new*( + T: type MockDatastore): T = + T( + values: newTable[Key, seq[byte]](), + lock: newAsyncLock(), + calls: newSeq[MethodCall]() + ) + +suite "Test defaultimpl": + var + ds: MockDatastore + key: Key + + setup: + ds = MockDatastore.new() + key = Key.init("/a/b").tryGet() + + test "should put a value that is different than the current value": + + (await ds.put(key, @[byte 1, 2, 3])).tryGet() + + proc modifyFn(maybeCurr: ?seq[byte]): Future[?seq[byte]] {.async.} = + some(@[byte 3, 2, 1]) + + (await ds.modify(key, modifyFn)).tryGet() + + check: + ds.calls.filterIt(it.kind == Put).len == 2 + + test "should not attempt to put a value that is equal to the current value": + (await ds.put(key, @[byte 1, 2, 3])).tryGet() + + proc modifyFn(maybeCurr: ?seq[byte]): Future[?seq[byte]] {.async.} = + some(@[byte 1, 2, 3]) + + (await ds.modify(key, modifyFn)).tryGet() + + check: + ds.calls.filterIt(it.kind == Put).len == 1 diff --git a/tests/testall.nim b/tests/testall.nim index 3db5a319..e2ca5689 100644 --- a/tests/testall.nim +++ b/tests/testall.nim @@ -5,6 +5,7 @@ import ./datastore/testsql, ./datastore/testleveldb, ./datastore/testtieredds, - ./datastore/testmountedds + ./datastore/testmountedds, + ./datastore/testdefaultimpl {.warning[UnusedImport]: off.}