Change way of deleting blocks to be more foolproof (when deleting a block, the user is forced to give up its reference to the block)

implementations/inmemory/InMemoryBlockStore.cpp

@@ -36,7 +36,9 @@ unique_ptr<Block> InMemoryBlockStore::load(const Key &key) {
   }
 }
 
-void InMemoryBlockStore::remove(const Key &key) {
+void InMemoryBlockStore::remove(unique_ptr<Block> block) {
+  Key key = block->key();
+  block.reset();
   int numRemoved = _blocks.erase(key.ToString());
   assert(1==numRemoved);
 }

implementations/inmemory/InMemoryBlockStore.h

@@ -18,7 +18,7 @@ public:
 
   std::unique_ptr<Block> create(const Key &key, size_t size) override;
   std::unique_ptr<Block> load(const Key &key) override;
-  void remove(const Key &key) override;
+  void remove(std::unique_ptr<Block> block) override;
 
 private:
   std::map<std::string, InMemoryBlock> _blocks;

implementations/ondisk/OnDiskBlockStore.cpp

@@ -28,7 +28,9 @@ unique_ptr<Block> OnDiskBlockStore::load(const Key &key) {
   return OnDiskBlock::LoadFromDisk(_rootdir, key);
 }
 
-void OnDiskBlockStore::remove(const Key &key) {
+void OnDiskBlockStore::remove(unique_ptr<Block> block) {
+  Key key = block->key();
+  block.reset();
   OnDiskBlock::RemoveFromDisk(_rootdir, key);
 }
 

implementations/ondisk/OnDiskBlockStore.h

@@ -18,7 +18,7 @@ public:
 
   std::unique_ptr<Block> create(const Key &key, size_t size) override;
   std::unique_ptr<Block> load(const Key &key) override;
-  void remove(const Key &key) override;
+  void remove(std::unique_ptr<Block> block) override;
 
 private:
   const boost::filesystem::path _rootdir;

implementations/testfake/FakeBlockStore.cpp

@@ -36,7 +36,9 @@ unique_ptr<Block> FakeBlockStore::load(const Key &key) {
   }
 }
 
-void FakeBlockStore::remove(const Key &key) {
+void FakeBlockStore::remove(unique_ptr<Block> block) {
+  Key key = block->key();
+  block.reset();
   int numRemoved = _blocks.erase(key.ToString());
   assert(numRemoved == 1);
 }

implementations/testfake/FakeBlockStore.h

@@ -33,7 +33,7 @@ public:
 
   std::unique_ptr<Block> create(const Key &key, size_t size) override;
   std::unique_ptr<Block> load(const Key &key) override;
-  void remove(const Key &key) override;
+  void remove(std::unique_ptr<Block> block) override;
 
   void updateData(const Key &key, const Data &data);
 

interface/BlockStore.h

@@ -19,7 +19,7 @@ public:
   //TODO Use boost::optional (if key doesn't exist)
   // Return nullptr if block with this key doesn't exists
   virtual std::unique_ptr<Block> load(const Key &key) = 0;
-  virtual void remove(const Key &key) = 0;
+  virtual void remove(std::unique_ptr<Block> block) = 0;
 };
 
 }

test/interface/helpers/BlockStoreWithRandomKeysTest.cpp

@@ -24,7 +24,7 @@ public:
     return unique_ptr<Block>(do_load(key));
   }
   MOCK_METHOD1(do_load, Block*(const Key &));
-  MOCK_METHOD1(remove, void(const Key &));
+  void remove(unique_ptr<Block> block) {}
 };
 
 class BlockMock: public Block {

test/testutils/BlockStoreTest.h

@@ -203,18 +203,12 @@ TYPED_TEST_P(BlockStoreTest, TwoCreatedBlocksHaveDifferentKeys) {
 TYPED_TEST_P(BlockStoreTest, BlockIsNotLoadableAfterDeleting) {
   auto blockStore = this->fixture.createBlockStore();
   auto blockkey = blockStore->create(1024)->key();
-  EXPECT_NE(nullptr, blockStore->load(blockkey));
-  blockStore->remove(blockkey);
+  auto block = blockStore->load(blockkey);
+  EXPECT_NE(nullptr, block);
+  blockStore->remove(std::move(block));
   EXPECT_EQ(nullptr, blockStore->load(blockkey));
 }
 
-TYPED_TEST_P(BlockStoreTest, CrashesWhenTryingToDeleteNonexistingBlock) {
-  auto blockStore = this->fixture.createBlockStore();
-  auto blockkey = blockStore->create(1024)->key();
-  blockStore->remove(blockkey);
-  EXPECT_DEATH(blockStore->remove(blockkey), "");
-}
-
 REGISTER_TYPED_TEST_CASE_P(BlockStoreTest,
     CreatedBlockHasCorrectSize,
     LoadingUnchangedBlockHasCorrectSize,
@@ -229,8 +223,7 @@ REGISTER_TYPED_TEST_CASE_P(BlockStoreTest,
     LoadNonExistingBlock,
     LoadNonExistingBlockWithEmptyKey,
     TwoCreatedBlocksHaveDifferentKeys,
-    BlockIsNotLoadableAfterDeleting,
-    CrashesWhenTryingToDeleteNonexistingBlock
+    BlockIsNotLoadableAfterDeleting
 );