From 15ae157e61d8856a469e86bf8455c531b583ac90 Mon Sep 17 00:00:00 2001
From: Bulat-Ziganshin <bulat.ziganshin@gmail.com>
Date: Tue, 28 Jun 2022 19:10:05 +0300
Subject: [PATCH] BlockStore: delBlock got new type (#123)

Now every store traces only its own operations and failures (currently implemented only for delBlock)
---
 codex/stores/blockstore.nim           |  4 ++--
 codex/stores/cachestore.nim           | 19 ++++++++-----------
 codex/stores/fsstore.nim              | 25 ++++++++++++-------------
 codex/stores/networkstore.nim         |  7 ++++---
 tests/codex/stores/testcachestore.nim |  7 ++++---
 tests/codex/stores/testfsstore.nim    |  2 +-
 tests/codex/testerasure.nim           | 12 ++++++------
 7 files changed, 37 insertions(+), 39 deletions(-)

diff --git a/codex/stores/blockstore.nim b/codex/stores/blockstore.nim
index 6ae774be..a7439f8d 100644
--- a/codex/stores/blockstore.nim
+++ b/codex/stores/blockstore.nim
@@ -41,8 +41,8 @@ method putBlock*(
 
 method delBlock*(
   s: BlockStore,
-  cid: Cid): Future[bool] {.base.} =
-  ## Delete a block/s from the block store
+  cid: Cid): Future[?!void] {.base.} =
+  ## Delete a block from the blockstore
   ##
 
   raiseAssert("Not implemented!")
diff --git a/codex/stores/cachestore.nim b/codex/stores/cachestore.nim
index 0864c55d..e92ec50b 100644
--- a/codex/stores/cachestore.nim
+++ b/codex/stores/cachestore.nim
@@ -109,23 +109,20 @@ method putBlock*(
 
 method delBlock*(
   self: CacheStore,
-  cid: Cid): Future[bool] {.async.} =
-  ## delete a block/s from the block store
+  cid: Cid): Future[?!void] {.async.} =
+  ## Delete a block from the blockstore
   ##
 
   trace "Deleting block from cache", cid
   if cid.isEmpty:
     trace "Empty block, ignoring"
-    return true
+    return success()
 
-  try:
-    let removed = self.cache.del(cid)
-    if removed.isSome:
-      self.currentSize -= removed.get.data.len
-      return true
-    return false
-  except EmptyLruCacheError:
-    return false
+  let removed = self.cache.del(cid)
+  if removed.isSome:
+    self.currentSize -= removed.get.data.len
+
+  return success()
 
 func new*(
     _: type CacheStore,
diff --git a/codex/stores/fsstore.nim b/codex/stores/fsstore.nim
index de846e50..5ec1bead 100644
--- a/codex/stores/fsstore.nim
+++ b/codex/stores/fsstore.nim
@@ -97,26 +97,25 @@ method putBlock*(
 
 method delBlock*(
   self: FSStore,
-  cid: Cid): Future[bool] {.async.} =
-  ## Delete a block/s from the block store
+  cid: Cid): Future[?!void] {.async.} =
+  ## Delete a block from the blockstore
   ##
 
+  trace "Deleting block from filestore", cid
   if cid.isEmpty:
     trace "Empty block, ignoring"
-    return true
+    return success()
 
-  let path = self.blockPath(cid)
-  if (
-    let res = io2.removeFile(path);
-    res.isErr):
-    let error = io2.ioErrorMsg(res.error)
-    trace "Unable to delete block", path, cid, error
-    return false
+  let
+    path = self.blockPath(cid)
+    res = io2.removeFile(path)
 
-  if not (await self.cache.delBlock(cid)):
-    trace "Unable to delete block from cache", cid
+  if res.isErr:
+    let errmsg = io2.ioErrorMsg(res.error)
+    trace "Unable to delete block", path, cid, errmsg
+    return errmsg.failure
 
-  return true
+  return await self.cache.delBlock(cid)
 
 method hasBlock*(self: FSStore, cid: Cid): bool =
   ## Check if the block exists in the blockstore
diff --git a/codex/stores/networkstore.nim b/codex/stores/networkstore.nim
index 951d853f..5ce90d36 100644
--- a/codex/stores/networkstore.nim
+++ b/codex/stores/networkstore.nim
@@ -66,11 +66,12 @@ method putBlock*(
 
 method delBlock*(
   self: NetworkStore,
-  cid: Cid): Future[bool] =
-  ## Delete a block/s from the block store
+  cid: Cid): Future[?!void] =
+  ## Delete a block from the blockstore
   ##
 
-  self.localStore.delBlock(cid)
+  trace "Deleting block from networkstore", cid
+  return self.localStore.delBlock(cid)
 
 {.pop.}
 
diff --git a/tests/codex/stores/testcachestore.nim b/tests/codex/stores/testcachestore.nim
index 2fcdf736..0a0d2fc6 100644
--- a/tests/codex/stores/testcachestore.nim
+++ b/tests/codex/stores/testcachestore.nim
@@ -93,17 +93,18 @@ suite "Cache Store tests":
 
   test "delBlock":
     # empty cache
-    check not await store.delBlock(newBlock1.cid)
+    (await store.delBlock(newBlock1.cid)).tryGet()
 
     # successfully deleted
     discard await store.putBlock(newBlock1)
-    check await store.delBlock(newBlock1.cid)
+    (await store.delBlock(newBlock1.cid)).tryGet()
 
     # deletes item should decrement size
     store = CacheStore.new(@[newBlock1, newBlock2, newBlock3])
     check:
       store.currentSize == 300
-      await store.delBlock(newBlock2.cid)
+    (await store.delBlock(newBlock2.cid)).tryGet()
+    check:
       store.currentSize == 200
       newBlock2.cid notin store
 
diff --git a/tests/codex/stores/testfsstore.nim b/tests/codex/stores/testfsstore.nim
index 070a602c..4089623e 100644
--- a/tests/codex/stores/testfsstore.nim
+++ b/tests/codex/stores/testfsstore.nim
@@ -68,5 +68,5 @@ suite "FS Store":
     createDir(store.blockPath(newBlock.cid).parentDir)
     writeFile(store.blockPath(newBlock.cid), newBlock.data)
 
-    check await store.delBlock(newBlock.cid)
+    (await store.delBlock(newBlock.cid)).tryGet()
     check not fileExists(store.blockPath(newBlock.cid))
diff --git a/tests/codex/testerasure.nim b/tests/codex/testerasure.nim
index 23eab4e2..26e22475 100644
--- a/tests/codex/testerasure.nim
+++ b/tests/codex/testerasure.nim
@@ -66,7 +66,7 @@ suite "Erasure encode/decode":
 
     for _ in 0..<encoded.M:
       dropped.add(encoded[column])
-      check (await store.delBlock(encoded[column]))
+      (await store.delBlock(encoded[column])).tryGet()
       column.inc(encoded.steps)
 
     var
@@ -93,7 +93,7 @@ suite "Erasure encode/decode":
 
     for _ in 0..<encoded.M + 1:
       dropped.add(encoded[column])
-      check (await store.delBlock(encoded[column]))
+      (await store.delBlock(encoded[column])).tryGet()
       column.inc(encoded.steps)
 
     var
@@ -125,7 +125,7 @@ suite "Erasure encode/decode":
       offset.inc
 
     for idx in blocks:
-      check (await store.delBlock(encoded[idx]))
+      (await store.delBlock(encoded[idx])).tryGet()
 
     discard (await erasure.decode(encoded)).tryGet()
 
@@ -158,7 +158,7 @@ suite "Erasure encode/decode":
       offset.inc
 
     for idx in blocks:
-      check (await store.delBlock(encoded[idx]))
+      (await store.delBlock(encoded[idx])).tryGet()
 
     var
       decoded: Manifest
@@ -174,7 +174,7 @@ suite "Erasure encode/decode":
     let encoded = await encode(buffers, parity)
 
     for b in encoded.blocks[0..<encoded.steps * encoded.M]:
-      check (await store.delBlock(b))
+      (await store.delBlock(b)).tryGet()
 
     discard (await erasure.decode(encoded)).tryGet()
 
@@ -189,7 +189,7 @@ suite "Erasure encode/decode":
     let encoded = await encode(buffers, parity)
 
     for b in encoded.blocks[^(encoded.steps * encoded.M)..^1]:
-      check (await store.delBlock(b))
+      (await store.delBlock(b)).tryGet()
 
     discard (await erasure.decode(encoded)).tryGet()