diff --git a/codex/stores/memorystore.nim b/codex/stores/memorystore.nim index f28a3b45..e42aa7ad 100644 --- a/codex/stores/memorystore.nim +++ b/codex/stores/memorystore.nim @@ -55,11 +55,7 @@ method getBlock*(self: MemoryStore, cid: Cid): Future[?!Block] {.async.} = if cid notin self.table: return failure (ref BlockNotFoundError)(msg: "Block not in memory store") - try: - return success self.table[cid].value.val - except CatchableError as exc: - trace "Error getting block from memory store", cid, error = exc.msg - return failure exc + return success self.table[cid].value.val method hasBlock*(self: MemoryStore, cid: Cid): Future[?!bool] {.async.} = trace "Checking MemoryStore for block presence", cid @@ -76,6 +72,21 @@ func cids(self: MemoryStore): (iterator: Cid {.gcsafe.}) = yield it.value.key it = it.next +proc isOfBlockType(cid: Cid, blockType: BlockType): ?!bool = + without isManifest =? cid.isManifest, err: + trace "Error checking if cid is a manifest", err = err.msg + return failure("Unable to determine if CID is a manifest") + + case blockType: + of BlockType.Manifest: + return success(isManifest) + of BlockType.Block: + return success(not isManifest) + of BlockType.Both: + return success(true) + + return failure("Unknown block type: " & $blockType) + method listBlocks*(self: MemoryStore, blockType = BlockType.Manifest): Future[?!BlocksIter] {.async.} = var iter = BlocksIter() @@ -97,27 +108,15 @@ method listBlocks*(self: MemoryStore, blockType = BlockType.Manifest): Future[?! iter.finished = true return Cid.none - without isManifest =? cid.isManifest, err: - trace "Error checking if cid is a manifest", err = err.msg + without isCorrectBlockType =? isOfBlockType(cid, blockType), err: + warn "Error checking if cid of blocktype", err = err.msg return Cid.none - case blockType: - of BlockType.Manifest: - if not isManifest: - trace "Cid is not manifest, skipping", cid - continue + if not isCorrectBlockType: + trace "Cid does not match blocktype, skipping", cid, blockType + continue - break - of BlockType.Block: - if isManifest: - trace "Cid is a manifest, skipping", cid - continue - - break - of BlockType.Both: - break - - return cid.some + return cid.some iter.next = next @@ -156,7 +155,7 @@ method delBlock*(self: MemoryStore, cid: Cid): Future[?!void] {.async.} = return success() if cid notin self.table: - return failure (ref BlockNotFoundError)(msg: "Block not in memory store") + return success() let nodeToRemove = self.table[cid] diff --git a/tests/codex/stores/commonstoretests.nim b/tests/codex/stores/commonstoretests.nim index 157464b2..ab8fb070 100644 --- a/tests/codex/stores/commonstoretests.nim +++ b/tests/codex/stores/commonstoretests.nim @@ -79,6 +79,11 @@ proc commonBlockStoreTests*( check not (await store.hasBlock(newBlock1.cid)).tryGet() + test "delBlock silently ignores missing block": + check: + not (await store.hasBlock(newBlock1.cid)).tryGet() + not (await store.delBlock(newBlock1.cid)).isErr + test "listBlocks Blocks": let blocks = @[newBlock1, newBlock2, newBlock3] diff --git a/tests/codex/stores/testcachestore.nim b/tests/codex/stores/testcachestore.nim index 3c780ab1..478fad6d 100644 --- a/tests/codex/stores/testcachestore.nim +++ b/tests/codex/stores/testcachestore.nim @@ -39,6 +39,15 @@ suite "Cache Store": newBlock == received2 backingStore.numberOfGetCalls == 1 + test "getBlock should return empty block immediately": + let expectedEmptyBlock = newBlock.cid.emptyBlock + + let received = (await store.getBlock(expectedEmptyBlock.cid)).tryGet() + + check: + expectedEmptyBlock == received + backingStore.numberOfGetCalls == 0 + commonBlockStoreTests( "Cache", proc: BlockStore = BlockStore(CacheStore.new(MemoryStore.new()))) diff --git a/tests/codex/stores/testmemorystore.nim b/tests/codex/stores/testmemorystore.nim index cfb9f209..cbeca7d0 100644 --- a/tests/codex/stores/testmemorystore.nim +++ b/tests/codex/stores/testmemorystore.nim @@ -13,16 +13,89 @@ import ../helpers import ./commonstoretests suite "MemoryStore": - test "Should store initial blocks": - let - capacity = 100 - blk = createTestBlock(10) + let + capacity = 100 + blk = createTestBlock(10) + emptyBlk = blk.cid.emptyBlock + test "Should store initial blocks": let store = MemoryStore.new([blk], capacity) - let receivedBlk = await store.getBlock(blk.cid) + let receivedBlk = (await store.getBlock(blk.cid)).tryGet() - check receivedBlk.tryGet() == blk + check receivedBlk == blk + + test "getBlock should return empty block": + let store = MemoryStore.new([], capacity) + + let received = (await store.getBlock(emptyBlk.cid)).tryGet() + + check: + emptyBlk == received + + test "hasBlock should return true for empty block": + let store = MemoryStore.new([], capacity) + + let hasBlock = (await store.hasBlock(emptyBlk.cid)).tryGet() + + check hasBlock + + test "getBlock should return failure when getting an unknown block": + let + store = MemoryStore.new([blk], capacity) + unknownBlock = createTestBlock(11) + + let received = (await store.getBlock(unknownBlock.cid)) + + check received.isErr + + test "putBlock should increase bytes used": + let store = MemoryStore.new([], capacity) + + check: + store.capacity == capacity + store.bytesUsed == 0 + + (await store.putBlock(blk)).tryGet() + + check: + store.capacity == capacity + store.bytesUsed == blk.data.len + + test "putBlock should fail when memorystore is full": + let + largeBlk = createTestBlock(capacity) + store = MemoryStore.new([largeBlk], capacity) + + check: + store.capacity == capacity + store.bytesUsed == capacity + + let response = await store.putBlock(blk) + + check response.isErr + + test "putBlock should ignore empty blocks": + let store = MemoryStore.new([], capacity) + + (await store.putBlock(emptyBlk)).tryGet() + (await store.putBlock(emptyBlk)).tryGet() + (await store.putBlock(emptyBlk)).tryGet() + + check: + store.capacity == capacity + store.bytesUsed == 0 + + test "delBlock should ignore empty blocks": + let store = MemoryStore.new([], capacity) + + (await store.delBlock(emptyBlk.cid)).tryGet() + (await store.delBlock(emptyBlk.cid)).tryGet() + (await store.delBlock(emptyBlk.cid)).tryGet() + + check: + store.capacity == capacity + store.bytesUsed == 0 commonBlockStoreTests( "MemoryStore", proc: BlockStore = diff --git a/tests/codex/teststores.nim b/tests/codex/teststores.nim index 834565ec..a7c6237d 100644 --- a/tests/codex/teststores.nim +++ b/tests/codex/teststores.nim @@ -2,5 +2,6 @@ import ./stores/testcachestore import ./stores/testrepostore import ./stores/testmaintenance import ./stores/testmemorystore +import ./stores/testkeyutils {.warning[UnusedImport]: off.}