Switched whole blockstore interface to unique_ref instead of unique_ptr

implementations/caching/CachedBlock.cpp

@@ -2,11 +2,12 @@
 #include "CachingBlockStore.h"
 
 using std::unique_ptr;
+using cpputils::unique_ref;
 
 namespace blockstore {
 namespace caching {
 
-CachedBlock::CachedBlock(std::unique_ptr<Block> baseBlock, CachingBlockStore *blockStore)
+CachedBlock::CachedBlock(unique_ref<Block> baseBlock, CachingBlockStore *blockStore)
     :Block(baseBlock->key()),
      _blockStore(blockStore),
      _baseBlock(std::move(baseBlock)) {
@@ -34,7 +35,7 @@ size_t CachedBlock::size() const {
   return _baseBlock->size();
 }
 
-unique_ptr<Block> CachedBlock::releaseBlock() {
+unique_ref<Block> CachedBlock::releaseBlock() {
   return std::move(_baseBlock);
 }
 

implementations/caching/CachedBlock.h

@@ -4,8 +4,7 @@
 
 #include "../../interface/Block.h"
 
-#include "messmer/cpp-utils/macros.h"
+#include <messmer/cpp-utils/pointer/unique_ref.h>
-#include <memory>
 
 namespace blockstore {
 namespace caching {
@@ -14,7 +13,7 @@ class CachingBlockStore;
 class CachedBlock: public Block {
 public:
   //TODO Storing key twice (in parent class and in object pointed to). Once would be enough.
-  CachedBlock(std::unique_ptr<Block> baseBlock, CachingBlockStore *blockStore);
+  CachedBlock(cpputils::unique_ref<Block> baseBlock, CachingBlockStore *blockStore);
   virtual ~CachedBlock();
 
   const void *data() const override;
@@ -23,11 +22,11 @@ public:
 
   size_t size() const override;
 
-  std::unique_ptr<Block> releaseBlock();
+  cpputils::unique_ref<Block> releaseBlock();
 
 private:
   CachingBlockStore *_blockStore;
-  std::unique_ptr<Block> _baseBlock;
+  cpputils::unique_ref<Block> _baseBlock;
 
   DISALLOW_COPY_AND_ASSIGN(CachedBlock);
 };

implementations/caching/CachingBlockStore.cpp

@@ -29,32 +29,34 @@ Key CachingBlockStore::createKey() {
 optional<unique_ref<Block>> CachingBlockStore::tryCreate(const Key &key, Data data) {
   //TODO Shouldn't we return boost::none if the key already exists?
   ++_numNewBlocks;
-  return unique_ref<Block>(make_unique_ref<CachedBlock>(make_unique<NewBlock>(key, std::move(data), this), this));
+  return unique_ref<Block>(make_unique_ref<CachedBlock>(make_unique_ref<NewBlock>(key, std::move(data), this), this));
 }
 
-unique_ptr<Block> CachingBlockStore::load(const Key &key) {
+optional<unique_ref<Block>> CachingBlockStore::load(const Key &key) {
-  boost::optional<unique_ptr<Block>> optBlock = _cache.pop(key);
+  optional<unique_ref<Block>> optBlock = _cache.pop(key);
-  unique_ptr<Block> block;
   //TODO an optional<> class with .getOrElse() would make this code simpler. boost::optional<>::value_or_eval didn't seem to work with unique_ptr members.
-  if (optBlock) {
+  if (optBlock != none) {
-    block = std::move(*optBlock);
+    return optional<unique_ref<Block>>(make_unique_ref<CachedBlock>(std::move(*optBlock), this));
   } else {
-    block = _baseBlockStore->load(key);
+    auto block = _baseBlockStore->load(key);
-    if (block.get() == nullptr) {
+    if (block == none) {
-      return nullptr;
+      return none;
+    } else {
+      return optional<unique_ref<Block>>(make_unique_ref<CachedBlock>(std::move(*block), this));
     }
   }
-  return make_unique<CachedBlock>(std::move(block), this);
 }
 
-void CachingBlockStore::remove(std::unique_ptr<Block> block) {
+void CachingBlockStore::remove(cpputils::unique_ref<Block> block) {
-  auto baseBlock = dynamic_pointer_move<CachedBlock>(block)->releaseBlock();
+  auto cached_block = dynamic_pointer_move<CachedBlock>(block);
+  assert(cached_block != none);
+  auto baseBlock = (*cached_block)->releaseBlock();
   auto baseNewBlock = dynamic_pointer_move<NewBlock>(baseBlock);
-  if (baseNewBlock.get() != nullptr) {
+  if (baseNewBlock != none) {
-	if(!baseNewBlock->alreadyExistsInBaseStore()) {
+	if(!(*baseNewBlock)->alreadyExistsInBaseStore()) {
 	  --_numNewBlocks;
 	}
-	baseNewBlock->remove();
+    (*baseNewBlock)->remove();
   } else {
     _baseBlockStore->remove(std::move(baseBlock));
   }
@@ -64,7 +66,7 @@ uint64_t CachingBlockStore::numBlocks() const {
   return _baseBlockStore->numBlocks() + _numNewBlocks;
 }
 
-void CachingBlockStore::release(unique_ptr<Block> block) {
+void CachingBlockStore::release(unique_ref<Block> block) {
   Key key = block->key();
   _cache.push(key, std::move(block));
 }
@@ -77,7 +79,7 @@ optional<unique_ref<Block>> CachingBlockStore::tryCreateInBaseStore(const Key &k
   return block;
 }
 
-void CachingBlockStore::removeFromBaseStore(std::unique_ptr<Block> block) {
+void CachingBlockStore::removeFromBaseStore(cpputils::unique_ref<Block> block) {
   _baseBlockStore->remove(std::move(block));
 }
 

implementations/caching/CachingBlockStore.h

@@ -15,18 +15,18 @@ public:
 
   Key createKey() override;
   boost::optional<cpputils::unique_ref<Block>> tryCreate(const Key &key, cpputils::Data data) override;
-  std::unique_ptr<Block> load(const Key &key) override;
+  boost::optional<cpputils::unique_ref<Block>> load(const Key &key) override;
-  void remove(std::unique_ptr<Block> block) override;
+  void remove(cpputils::unique_ref<Block> block) override;
   uint64_t numBlocks() const override;
 
-  void release(std::unique_ptr<Block> block);
+  void release(cpputils::unique_ref<Block> block);
 
   boost::optional<cpputils::unique_ref<Block>> tryCreateInBaseStore(const Key &key, cpputils::Data data);
-  void removeFromBaseStore(std::unique_ptr<Block> block);
+  void removeFromBaseStore(cpputils::unique_ref<Block> block);
 
 private:
   std::unique_ptr<BlockStore> _baseBlockStore;
-  Cache<Key, std::unique_ptr<Block>> _cache;
+  Cache<Key, cpputils::unique_ref<Block>> _cache;
   uint32_t _numNewBlocks;
 
   DISALLOW_COPY_AND_ASSIGN(CachingBlockStore);

implementations/caching/NewBlock.cpp

@@ -3,6 +3,7 @@
 
 using std::unique_ptr;
 using cpputils::Data;
+using boost::none;
 
 namespace blockstore {
 namespace caching {
@@ -11,7 +12,7 @@ NewBlock::NewBlock(const Key &key, Data data, CachingBlockStore *blockStore)
     :Block(key),
      _blockStore(blockStore),
      _data(std::move(data)),
-     _baseBlock(nullptr),
+     _baseBlock(none),
      _dataChanged(true) {
 }
 
@@ -31,29 +32,29 @@ void NewBlock::write(const void *source, uint64_t offset, uint64_t size) {
 
 void NewBlock::writeToBaseBlockIfChanged() {
   if (_dataChanged) {
-    if (_baseBlock.get() == nullptr) {
+    if (_baseBlock == none) {
       //TODO _data.copy() necessary?
       auto newBase = _blockStore->tryCreateInBaseStore(key(), _data.copy());
       assert(newBase != boost::none); //TODO What if tryCreate fails due to a duplicate key? We should ensure we don't use duplicate keys.
-      //TODO Don't use to_unique_ptr but make _baseBlock a unique_ref
+      _baseBlock = std::move(*newBase);
-      _baseBlock = cpputils::to_unique_ptr(std::move(*newBase));
     } else {
-	  _baseBlock->write(_data.data(), 0, _data.size());
+        (*_baseBlock)->write(_data.data(), 0, _data.size());
     }
 	_dataChanged = false;
   }
 }
 
 void NewBlock::remove() {
-  if (_baseBlock.get() != nullptr) {
+  if (_baseBlock != none) {
-	_blockStore->removeFromBaseStore(std::move(_baseBlock));
+	_blockStore->removeFromBaseStore(std::move(*_baseBlock));
   }
   _dataChanged = false;
 }
 
 void NewBlock::flush() {
   writeToBaseBlockIfChanged();
-  _baseBlock->flush();
+  assert(_baseBlock != none);
+  (*_baseBlock)->flush();
 }
 
 size_t NewBlock::size() const {
@@ -61,7 +62,7 @@ size_t NewBlock::size() const {
 }
 
 bool NewBlock::alreadyExistsInBaseStore() const {
-  return _baseBlock.get() != nullptr;
+  return _baseBlock != none;
 }
 
 }

implementations/caching/NewBlock.h

@@ -36,7 +36,7 @@ public:
 private:
   CachingBlockStore *_blockStore;
   cpputils::Data _data;
-  std::unique_ptr<Block> _baseBlock;
+  boost::optional<cpputils::unique_ref<Block>> _baseBlock;
   bool _dataChanged;
 
   void writeToBaseBlockIfChanged();

implementations/encrypted/EncryptedBlock.h

@@ -23,10 +23,10 @@ class EncryptedBlock: public Block {
 public:
   BOOST_CONCEPT_ASSERT((CipherConcept<Cipher>));
   static boost::optional<cpputils::unique_ref<EncryptedBlock>> TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, const typename Cipher::EncryptionKey &encKey);
-  static std::unique_ptr<EncryptedBlock> TryDecrypt(std::unique_ptr<Block> baseBlock, const typename Cipher::EncryptionKey &key);
+  static boost::optional<cpputils::unique_ref<EncryptedBlock>> TryDecrypt(cpputils::unique_ref<Block> baseBlock, const typename Cipher::EncryptionKey &key);
 
   //TODO Storing key twice (in parent class and in object pointed to). Once would be enough.
-  EncryptedBlock(std::unique_ptr<Block> baseBlock, const typename Cipher::EncryptionKey &key, cpputils::Data plaintextWithHeader);
+  EncryptedBlock(cpputils::unique_ref<Block> baseBlock, const typename Cipher::EncryptionKey &key, cpputils::Data plaintextWithHeader);
   virtual ~EncryptedBlock();
 
   const void *data() const override;
@@ -35,10 +35,10 @@ public:
 
   size_t size() const override;
 
-  std::unique_ptr<Block> releaseBlock();
+  cpputils::unique_ref<Block> releaseBlock();
 
 private:
-  std::unique_ptr<Block> _baseBlock;
+  cpputils::unique_ref<Block> _baseBlock;
   cpputils::Data _plaintextWithHeader;
   typename Cipher::EncryptionKey _encKey;
   bool _dataChanged;
@@ -66,27 +66,26 @@ boost::optional<cpputils::unique_ref<EncryptedBlock<Cipher>>> EncryptedBlock<Cip
     return boost::none;
   }
 
-  //TODO Don't use to_unique_ptr
+  return cpputils::make_unique_ref<EncryptedBlock>(std::move(*baseBlock), encKey, std::move(plaintextWithHeader));
-  return cpputils::make_unique_ref<EncryptedBlock>(cpputils::to_unique_ptr(std::move(*baseBlock)), encKey, std::move(plaintextWithHeader));
 }
 
 template<class Cipher>
-std::unique_ptr<EncryptedBlock<Cipher>> EncryptedBlock<Cipher>::TryDecrypt(std::unique_ptr<Block> baseBlock, const typename Cipher::EncryptionKey &encKey) {
+boost::optional<cpputils::unique_ref<EncryptedBlock<Cipher>>> EncryptedBlock<Cipher>::TryDecrypt(cpputils::unique_ref<Block> baseBlock, const typename Cipher::EncryptionKey &encKey) {
   //TODO Change BlockStore so we can read their "class Data" objects instead of "void *data()", and then we can change the Cipher interface to take Data objects instead of "byte *" + size
   boost::optional<cpputils::Data> plaintextWithHeader = Cipher::decrypt((byte*)baseBlock->data(), baseBlock->size(), encKey);
-  if(!plaintextWithHeader) {
+  if(plaintextWithHeader == boost::none) {
     //Decryption failed (e.g. an authenticated cipher detected modifications to the ciphertext)
     //TODO Think about logging
     std::cerr << "Decrypting block " << baseBlock->key().ToString() << " failed. Was the block modified by an attacker?" << std::endl;
-    return nullptr;
+    return boost::none;
   }
   if(!_keyHeaderIsCorrect(baseBlock->key(), *plaintextWithHeader)) {
     //The stored key in the block data is incorrect - an attacker might have exchanged the contents with the encrypted data from a different block
     //TODO Think about logging
     std::cerr << "Decrypting block " << baseBlock->key().ToString() << " failed due to invalid block key. Was the block modified by an attacker?" << std::endl;
-    return nullptr;
+    return boost::none;
   }
-  return std::make_unique<EncryptedBlock<Cipher>>(std::move(baseBlock), encKey, std::move(*plaintextWithHeader));
+  return cpputils::make_unique_ref<EncryptedBlock<Cipher>>(std::move(baseBlock), encKey, std::move(*plaintextWithHeader));
 }
 
 template<class Cipher>
@@ -104,7 +103,7 @@ bool EncryptedBlock<Cipher>::_keyHeaderIsCorrect(const Key &key, const cpputils:
 }
 
 template<class Cipher>
-EncryptedBlock<Cipher>::EncryptedBlock(std::unique_ptr<Block> baseBlock, const typename Cipher::EncryptionKey &encKey, cpputils::Data plaintextWithHeader)
+EncryptedBlock<Cipher>::EncryptedBlock(cpputils::unique_ref<Block> baseBlock, const typename Cipher::EncryptionKey &encKey, cpputils::Data plaintextWithHeader)
     :Block(baseBlock->key()),
    _baseBlock(std::move(baseBlock)),
    _plaintextWithHeader(std::move(plaintextWithHeader)),
@@ -150,7 +149,7 @@ void EncryptedBlock<Cipher>::_encryptToBaseBlock() {
 }
 
 template<class Cipher>
-std::unique_ptr<Block> EncryptedBlock<Cipher>::releaseBlock() {
+cpputils::unique_ref<Block> EncryptedBlock<Cipher>::releaseBlock() {
   return std::move(_baseBlock);
 }
 

implementations/encrypted/EncryptedBlockStore.h

@@ -19,8 +19,8 @@ public:
   //TODO Are createKey() tests included in generic BlockStoreTest? If not, add it!
   Key createKey() override;
   boost::optional<cpputils::unique_ref<Block>> tryCreate(const Key &key, cpputils::Data data) override;
-  std::unique_ptr<Block> load(const Key &key) override;
+  boost::optional<cpputils::unique_ref<Block>> load(const Key &key) override;
-  void remove(std::unique_ptr<Block> block) override;
+  void remove(cpputils::unique_ref<Block> block) override;
   uint64_t numBlocks() const override;
 
   //This function should only be used by test cases
@@ -57,18 +57,20 @@ boost::optional<cpputils::unique_ref<Block>> EncryptedBlockStore<Cipher>::tryCre
 }
 
 template<class Cipher>
-std::unique_ptr<Block> EncryptedBlockStore<Cipher>::load(const Key &key) {
+boost::optional<cpputils::unique_ref<Block>> EncryptedBlockStore<Cipher>::load(const Key &key) {
   auto block = _baseBlockStore->load(key);
-  if (block.get() == nullptr) {
+  if (block == boost::none) {
     //TODO Test this path (for all pass-through-blockstores)
-    return nullptr;
+    return boost::none;
   }
-  return EncryptedBlock<Cipher>::TryDecrypt(std::move(block), _encKey);
+  return boost::optional<cpputils::unique_ref<Block>>(EncryptedBlock<Cipher>::TryDecrypt(std::move(*block), _encKey));
 }
 
 template<class Cipher>
-void EncryptedBlockStore<Cipher>::remove(std::unique_ptr<Block> block) {
+void EncryptedBlockStore<Cipher>::remove(cpputils::unique_ref<Block> block) {
-  auto baseBlock = cpputils::dynamic_pointer_move<EncryptedBlock<Cipher>>(block)->releaseBlock();
+  auto encryptedBlock = cpputils::dynamic_pointer_move<EncryptedBlock<Cipher>>(block);
+  assert(encryptedBlock != boost::none);
+  auto baseBlock = (*encryptedBlock)->releaseBlock();
   return _baseBlockStore->remove(std::move(baseBlock));
 }
 

implementations/inmemory/InMemoryBlockStore.cpp

@@ -32,18 +32,19 @@ optional<unique_ref<Block>> InMemoryBlockStore::tryCreate(const Key &key, Data d
   return optional<unique_ref<Block>>(make_unique_ref<InMemoryBlock>(insert_result.first->second));
 }
 
-unique_ptr<Block> InMemoryBlockStore::load(const Key &key) {
+optional<unique_ref<Block>> InMemoryBlockStore::load(const Key &key) {
   //Return a pointer to the stored InMemoryBlock
   try {
-    return make_unique<InMemoryBlock>(_blocks.at(key.ToString()));
+    return optional<unique_ref<Block>>(make_unique_ref<InMemoryBlock>(_blocks.at(key.ToString())));
   } catch (const std::out_of_range &e) {
-    return nullptr;
+    return none;
   }
 }
 
-void InMemoryBlockStore::remove(unique_ptr<Block> block) {
+void InMemoryBlockStore::remove(unique_ref<Block> block) {
   Key key = block->key();
-  block.reset();
+  //TODO Better way to destruct?
+  cpputils::to_unique_ptr(std::move(block)); // Destruct
   int numRemoved = _blocks.erase(key.ToString());
   assert(1==numRemoved);
 }

implementations/inmemory/InMemoryBlockStore.h

@@ -17,8 +17,8 @@ public:
   InMemoryBlockStore();
 
   boost::optional<cpputils::unique_ref<Block>> tryCreate(const Key &key, cpputils::Data data) override;
-  std::unique_ptr<Block> load(const Key &key) override;
+  boost::optional<cpputils::unique_ref<Block>> load(const Key &key) override;
-  void remove(std::unique_ptr<Block> block) override;
+  void remove(cpputils::unique_ref<Block> block) override;
   uint64_t numBlocks() const override;
 
 private:

implementations/ondisk/OnDiskBlock.cpp

@@ -45,7 +45,7 @@ size_t OnDiskBlock::size() const {
   return _data.size();
 }
 
-unique_ptr<OnDiskBlock> OnDiskBlock::LoadFromDisk(const bf::path &rootdir, const Key &key) {
+optional<unique_ref<OnDiskBlock>> OnDiskBlock::LoadFromDisk(const bf::path &rootdir, const Key &key) {
   auto filepath = rootdir / key.ToString();
   try {
     //If it isn't a file, Data::LoadFromFile() would usually also crash. We still need this extra check
@@ -53,15 +53,15 @@ unique_ptr<OnDiskBlock> OnDiskBlock::LoadFromDisk(const bf::path &rootdir, const
     //instead the path of a file.
     //TODO Data::LoadFromFile now returns boost::optional. Do we then still need this?
     if(!bf::is_regular_file(filepath)) {
-      return nullptr;
+      return none;
     }
     boost::optional<Data> data = Data::LoadFromFile(filepath);
     if (!data) {
-      return nullptr;
+      return none;
     }
-    return unique_ptr<OnDiskBlock>(new OnDiskBlock(key, filepath, std::move(*data)));
+    return make_unique_ref<OnDiskBlock>(key, filepath, std::move(*data));
   } catch (const FileDoesntExistException &e) {
-    return nullptr;
+    return none;
   }
 }
 

implementations/ondisk/OnDiskBlock.h

@@ -18,7 +18,7 @@ public:
   OnDiskBlock(const Key &key, const boost::filesystem::path &filepath, cpputils::Data data);
   virtual ~OnDiskBlock();
 
-  static std::unique_ptr<OnDiskBlock> LoadFromDisk(const boost::filesystem::path &rootdir, const Key &key);
+  static boost::optional<cpputils::unique_ref<OnDiskBlock>> LoadFromDisk(const boost::filesystem::path &rootdir, const Key &key);
   static boost::optional<cpputils::unique_ref<OnDiskBlock>> CreateOnDisk(const boost::filesystem::path &rootdir, const Key &key, cpputils::Data data);
   static void RemoveFromDisk(const boost::filesystem::path &rootdir, const Key &key);
 

implementations/ondisk/OnDiskBlockStore.cpp

@@ -26,13 +26,14 @@ optional<unique_ref<Block>> OnDiskBlockStore::tryCreate(const Key &key, Data dat
   return unique_ref<Block>(std::move(*result));
 }
 
-unique_ptr<Block> OnDiskBlockStore::load(const Key &key) {
+optional<unique_ref<Block>> OnDiskBlockStore::load(const Key &key) {
-  return OnDiskBlock::LoadFromDisk(_rootdir, key);
+  return optional<unique_ref<Block>>(OnDiskBlock::LoadFromDisk(_rootdir, key));
 }
 
-void OnDiskBlockStore::remove(unique_ptr<Block> block) {
+void OnDiskBlockStore::remove(unique_ref<Block> block) {
   Key key = block->key();
-  block.reset();
+  //TODO Better way to destruct?
+  cpputils::to_unique_ptr(std::move(block)); // Destruct
   OnDiskBlock::RemoveFromDisk(_rootdir, key);
 }
 

implementations/ondisk/OnDiskBlockStore.h

@@ -15,8 +15,8 @@ public:
   explicit OnDiskBlockStore(const boost::filesystem::path &rootdir);
 
   boost::optional<cpputils::unique_ref<Block>> tryCreate(const Key &key, cpputils::Data data) override;
-  std::unique_ptr<Block> load(const Key &key) override;
+  boost::optional<cpputils::unique_ref<Block>> load(const Key &key) override;
-  void remove(std::unique_ptr<Block> block) override;
+  void remove(cpputils::unique_ref<Block> block) override;
   uint64_t numBlocks() const override;
 
 private:

implementations/parallelaccess/ParallelAccessBlockStore.cpp

@@ -39,21 +39,20 @@ optional<unique_ref<Block>> ParallelAccessBlockStore::tryCreate(const Key &key,
   return unique_ref<Block>(_parallelAccessStore.add(key, std::move(*block)));
 }
 
-unique_ptr<Block> ParallelAccessBlockStore::load(const Key &key) {
+optional<unique_ref<Block>> ParallelAccessBlockStore::load(const Key &key) {
   auto block = _parallelAccessStore.load(key);
   if (block == none) {
-    return nullptr;
+    return none;
   }
-  //TODO Don't use to_unique_ptr but make blockstore use unique_ref
+  return unique_ref<Block>(std::move(*block));
-  return cpputils::to_unique_ptr(std::move(*block));
 }
 
 
-void ParallelAccessBlockStore::remove(unique_ptr<Block> block) {
+void ParallelAccessBlockStore::remove(unique_ref<Block> block) {
   Key key = block->key();
-  //TODO Don't use nullcheck but make blockstore use unique_ref
+  auto block_ref = dynamic_pointer_move<BlockRef>(block);
-  auto block_ref = cpputils::nullcheck(std::move(block)).value();
+  assert(block_ref != none);
-  return _parallelAccessStore.remove(key, dynamic_pointer_move<BlockRef>(block_ref).value());
+  return _parallelAccessStore.remove(key, std::move(*block_ref));
 }
 
 uint64_t ParallelAccessBlockStore::numBlocks() const {

implementations/parallelaccess/ParallelAccessBlockStore.h

@@ -17,8 +17,8 @@ public:
 
   Key createKey() override;
   boost::optional<cpputils::unique_ref<Block>> tryCreate(const Key &key, cpputils::Data data) override;
-  std::unique_ptr<Block> load(const Key &key) override;
+  boost::optional<cpputils::unique_ref<Block>> load(const Key &key) override;
-  void remove(std::unique_ptr<Block> block) override;
+  void remove(cpputils::unique_ref<Block> block) override;
   uint64_t numBlocks() const override;
 
 private:

implementations/parallelaccess/ParallelAccessBlockStoreAdapter.h

@@ -15,13 +15,11 @@ public:
   }
 
   boost::optional<cpputils::unique_ref<Block>> loadFromBaseStore(const Key &key) override {
-    //TODO Don't use nullcheck but make blockstore use unique_ref
+	return _baseBlockStore->load(key);
-	return cpputils::nullcheck(_baseBlockStore->load(key));
   }
 
   void removeFromBaseStore(cpputils::unique_ref<Block> block) override {
-    //TODO Don't use to_unique_ptr but make blockstore use unique_ref
+	return _baseBlockStore->remove(std::move(block));
-	return _baseBlockStore->remove(cpputils::to_unique_ptr(std::move(block)));
   }
 
 private:

implementations/testfake/FakeBlockStore.cpp

@@ -27,31 +27,31 @@ optional<unique_ref<Block>> FakeBlockStore::tryCreate(const Key &key, Data data)
   }
 
   //Return a copy of the stored data
-  //TODO Don't use nullcheck but make load() use unique_ref
+  return load(key);
-  return cpputils::nullcheck(load(key));
 }
 
-unique_ptr<Block> FakeBlockStore::load(const Key &key) {
+optional<unique_ref<Block>> FakeBlockStore::load(const Key &key) {
   //Return a copy of the stored data
   string key_string = key.ToString();
   try {
     return makeFakeBlockFromData(key, _blocks.at(key_string), false);
   } catch (const std::out_of_range &e) {
-    return nullptr;
+    return none;
   }
 }
 
-void FakeBlockStore::remove(unique_ptr<Block> block) {
+void FakeBlockStore::remove(unique_ref<Block> block) {
   Key key = block->key();
-  block.reset();
+  //TODO Better way to destruct
+  cpputils::to_unique_ptr(std::move(block)); // Destruct
   int numRemoved = _blocks.erase(key.ToString());
   assert(numRemoved == 1);
 }
 
-unique_ptr<Block> FakeBlockStore::makeFakeBlockFromData(const Key &key, const Data &data, bool dirty) {
+unique_ref<Block> FakeBlockStore::makeFakeBlockFromData(const Key &key, const Data &data, bool dirty) {
   auto newdata = make_shared<Data>(data.copy());
   _used_dataregions_for_blocks.push_back(newdata);
-  return make_unique<FakeBlock>(this, key, newdata, dirty);
+  return make_unique_ref<FakeBlock>(this, key, newdata, dirty);
 }
 
 void FakeBlockStore::updateData(const Key &key, const Data &data) {

implementations/testfake/FakeBlockStore.h

@@ -32,8 +32,8 @@ public:
   FakeBlockStore();
 
   boost::optional<cpputils::unique_ref<Block>> tryCreate(const Key &key, cpputils::Data data) override;
-  std::unique_ptr<Block> load(const Key &key) override;
+  boost::optional<cpputils::unique_ref<Block>> load(const Key &key) override;
-  void remove(std::unique_ptr<Block> block) override;
+  void remove(cpputils::unique_ref<Block> block) override;
   uint64_t numBlocks() const override;
 
   void updateData(const Key &key, const cpputils::Data &data);
@@ -48,7 +48,7 @@ private:
   //We want to avoid this for the reasons mentioned above (overflow data).
   std::vector<std::shared_ptr<cpputils::Data>> _used_dataregions_for_blocks;
 
-  std::unique_ptr<Block> makeFakeBlockFromData(const Key &key, const cpputils::Data &data, bool dirty);
+  cpputils::unique_ref<Block> makeFakeBlockFromData(const Key &key, const cpputils::Data &data, bool dirty);
 
   DISALLOW_COPY_AND_ASSIGN(FakeBlockStore);
 };

interface/BlockStore.h

@@ -19,8 +19,8 @@ public:
   virtual boost::optional<cpputils::unique_ref<Block>> tryCreate(const Key &key, cpputils::Data data) = 0;
   //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 boost::optional<cpputils::unique_ref<Block>> load(const Key &key) = 0;
-  virtual void remove(std::unique_ptr<Block> block) = 0;
+  virtual void remove(cpputils::unique_ref<Block> block) = 0;
   virtual uint64_t numBlocks() const = 0;
 
   cpputils::unique_ref<Block> create(const cpputils::Data &data) {

test/implementations/encrypted/EncryptedBlockStoreTest_Specific.cpp

@@ -42,14 +42,14 @@ public:
   }
 
   void ModifyBaseBlock(const blockstore::Key &key) {
-    auto block = baseBlockStore->load(key);
+    auto block = baseBlockStore->load(key).value();
     uint8_t middle_byte = ((byte*)block->data())[10];
     uint8_t new_middle_byte = middle_byte + 1;
     block->write(&new_middle_byte, 10, 1);
   }
 
   blockstore::Key CopyBaseBlock(const blockstore::Key &key) {
-    auto source = baseBlockStore->load(key);
+    auto source = baseBlockStore->load(key).value();
     return blockstore::utils::copyToNewBlock(baseBlockStore, *source)->key();
   }
 };
@@ -57,57 +57,57 @@ public:
 TEST_F(EncryptedBlockStoreTest, LoadingWithSameKeyWorks_WriteOnCreate) {
   auto key = CreateBlockDirectlyWithFixtureAndReturnKey();
   auto loaded = blockStore->load(key);
-  EXPECT_NE(nullptr, loaded.get());
+  EXPECT_NE(boost::none, loaded);
-  EXPECT_EQ(data.size(), loaded->size());
+  EXPECT_EQ(data.size(), (*loaded)->size());
-  EXPECT_EQ(0, std::memcmp(data.data(), loaded->data(), data.size()));
+  EXPECT_EQ(0, std::memcmp(data.data(), (*loaded)->data(), data.size()));
 }
 
 TEST_F(EncryptedBlockStoreTest, LoadingWithSameKeyWorks_WriteSeparately) {
   auto key = CreateBlockWriteFixtureToItAndReturnKey();
   auto loaded = blockStore->load(key);
-  EXPECT_NE(nullptr, loaded.get());
+  EXPECT_NE(boost::none, loaded);
-  EXPECT_EQ(data.size(), loaded->size());
+  EXPECT_EQ(data.size(), (*loaded)->size());
-  EXPECT_EQ(0, std::memcmp(data.data(), loaded->data(), data.size()));
+  EXPECT_EQ(0, std::memcmp(data.data(), (*loaded)->data(), data.size()));
 }
 
 TEST_F(EncryptedBlockStoreTest, LoadingWithDifferentKeyDoesntWork_WriteOnCreate) {
   auto key = CreateBlockDirectlyWithFixtureAndReturnKey();
   blockStore->__setKey(FakeAuthenticatedCipher::Key2());
   auto loaded = blockStore->load(key);
-  EXPECT_EQ(nullptr, loaded.get());
+  EXPECT_EQ(boost::none, loaded);
 }
 
 TEST_F(EncryptedBlockStoreTest, LoadingWithDifferentKeyDoesntWork_WriteSeparately) {
   auto key = CreateBlockWriteFixtureToItAndReturnKey();
   blockStore->__setKey(FakeAuthenticatedCipher::Key2());
   auto loaded = blockStore->load(key);
-  EXPECT_EQ(nullptr, loaded.get());
+  EXPECT_EQ(boost::none, loaded);
 }
 
 TEST_F(EncryptedBlockStoreTest, LoadingModifiedBlockFails_WriteOnCreate) {
   auto key = CreateBlockDirectlyWithFixtureAndReturnKey();
   ModifyBaseBlock(key);
   auto loaded = blockStore->load(key);
-  EXPECT_EQ(nullptr, loaded.get());
+  EXPECT_EQ(boost::none, loaded);
 }
 
 TEST_F(EncryptedBlockStoreTest, LoadingModifiedBlockFails_WriteSeparately) {
   auto key = CreateBlockWriteFixtureToItAndReturnKey();
   ModifyBaseBlock(key);
   auto loaded = blockStore->load(key);
-  EXPECT_EQ(nullptr, loaded.get());
+  EXPECT_EQ(boost::none, loaded);
 }
 
 TEST_F(EncryptedBlockStoreTest, LoadingWithDifferentBlockIdFails_WriteOnCreate) {
   auto key = CreateBlockDirectlyWithFixtureAndReturnKey();
   auto key2 = CopyBaseBlock(key);
   auto loaded = blockStore->load(key2);
-  EXPECT_EQ(nullptr, loaded.get());
+  EXPECT_EQ(boost::none, loaded);
 }
 
 TEST_F(EncryptedBlockStoreTest, LoadingWithDifferentBlockIdFails_WriteSeparately) {
   auto key = CreateBlockWriteFixtureToItAndReturnKey();
   auto key2 = CopyBaseBlock(key);
   auto loaded = blockStore->load(key2);
-  EXPECT_EQ(nullptr, loaded.get());
+  EXPECT_EQ(boost::none, loaded);
 }

test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp

@@ -40,8 +40,7 @@ public:
     {
       OnDiskBlock::CreateOnDisk(dir.path(), key, randomData.copy()).value();
     }
-    //TODO Don't use nullcheck
+    return OnDiskBlock::LoadFromDisk(dir.path(), key).value();
-    return cpputils::nullcheck(OnDiskBlock::LoadFromDisk(dir.path(), key)).value();
   }
 
   unique_ref<OnDiskBlock> CreateBlock() {

test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockLoadTest.cpp

@@ -6,6 +6,7 @@
 #include <messmer/cpp-utils/data/Data.h>
 #include <messmer/cpp-utils/tempfile/TempFile.h>
 #include <messmer/cpp-utils/tempfile/TempDir.h>
+#include <messmer/cpp-utils/pointer/unique_ref_boost_optional_gtest_workaround.h>
 #include <fstream>
 
 using ::testing::Test;
@@ -19,6 +20,7 @@ using cpputils::Data;
 using cpputils::DataFixture;
 using cpputils::TempFile;
 using cpputils::TempDir;
+using cpputils::unique_ref;
 
 using namespace blockstore;
 using namespace blockstore::ondisk;
@@ -45,8 +47,8 @@ public:
     data.StoreToFile(file.path());
   }
 
-  unique_ptr<OnDiskBlock> LoadBlock() {
+  unique_ref<OnDiskBlock> LoadBlock() {
-    return OnDiskBlock::LoadFromDisk(dir.path(), key);
+    return OnDiskBlock::LoadFromDisk(dir.path(), key).value();
   }
 
   void EXPECT_BLOCK_DATA_EQ(const Data &expected, const OnDiskBlock &actual) {
@@ -75,7 +77,5 @@ TEST_P(OnDiskBlockLoadTest, LoadedDataIsCorrect) {
 
 TEST_F(OnDiskBlockLoadTest, LoadNotExistingBlock) {
   Key key2 = Key::FromString("272EE5517627CFA147A971A8E6E747E0");
-  EXPECT_FALSE(
+  EXPECT_EQ(boost::none, OnDiskBlock::LoadFromDisk(dir.path(), key2));
-      (bool)OnDiskBlock::LoadFromDisk(dir.path(), key2)
-  );
 }

test/interface/helpers/BlockStoreWithRandomKeysTest.cpp

@@ -26,11 +26,11 @@ public:
     return cpputils::nullcheck(unique_ptr<Block>(do_create(key, data)));
   }
   MOCK_METHOD2(do_create, Block*(const Key &, const Data &data));
-  unique_ptr<Block> load(const Key &key) {
+  optional<unique_ref<Block>> load(const Key &key) {
-    return unique_ptr<Block>(do_load(key));
+    return cpputils::nullcheck(unique_ptr<Block>(do_load(key)));
   }
   MOCK_METHOD1(do_load, Block*(const Key &));
-  void remove(unique_ptr<Block> block) {UNUSED(block);}
+  void remove(unique_ref<Block> block) {UNUSED(block);}
   MOCK_CONST_METHOD0(numBlocks, uint64_t());
 };
 

test/testutils/BlockStoreTest.h

@@ -35,9 +35,9 @@ TYPED_TEST_P(BlockStoreTest, BlockIsNotLoadableAfterDeleting) {
   auto blockStore = this->fixture.createBlockStore();
   auto blockkey = blockStore->create(cpputils::Data(1024))->key();
   auto block = blockStore->load(blockkey);
-  EXPECT_NE(nullptr, block.get());
+  EXPECT_NE(boost::none, block);
-  blockStore->remove(std::move(block));
+  blockStore->remove(std::move(*block));
-  EXPECT_EQ(nullptr, blockStore->load(blockkey).get());
+  EXPECT_EQ(boost::none, blockStore->load(blockkey));
 }
 
 TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectOnEmptyBlockstore) {
@@ -60,8 +60,7 @@ TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectAfterAddingOneBlock_AfterClosingB
 TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectAfterRemovingTheLastBlock) {
   auto blockStore = this->fixture.createBlockStore();
   auto block = blockStore->create(cpputils::Data(1));
-  //TODO Don't use to_unique_ptr
+  blockStore->remove(std::move(block));
-  blockStore->remove(cpputils::to_unique_ptr(std::move(block)));
   EXPECT_EQ(0, blockStore->numBlocks());
 }
 
@@ -97,8 +96,7 @@ TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectAfterRemovingABlock) {
   auto blockStore = this->fixture.createBlockStore();
   auto block = blockStore->create(cpputils::Data(1));
   blockStore->create(cpputils::Data(1));
-  // TODO Don't use to_unique_ptr
+  blockStore->remove(std::move(block));
-  blockStore->remove(cpputils::to_unique_ptr(std::move(block)));
   EXPECT_EQ(1, blockStore->numBlocks());
 }
 

test/testutils/BlockStoreTest_Data.h

@@ -26,7 +26,7 @@ public:
   void TestWriteAndReadAfterLoading() {
     blockstore::Key key = CreateBlockWriteToItAndReturnKey(foregroundData);
 
-    auto loaded_block = blockStore->load(key);
+    auto loaded_block = blockStore->load(key).value();
     EXPECT_DATA_READS_AS(foregroundData, *loaded_block, testData.offset, testData.count);
     EXPECT_DATA_IS_ZEROES_OUTSIDE_OF(*loaded_block, testData.offset, testData.count);
   }

test/testutils/BlockStoreTest_Size.h

@@ -14,7 +14,7 @@ public:
 
   void TestLoadingUnchangedBlockHasCorrectSize() {
     blockstore::Key key = CreateBlock()->key();
-    auto loaded_block = blockStore->load(key);
+    auto loaded_block = blockStore->load(key).value();
     EXPECT_EQ(size, loaded_block->size());
   }
 
@@ -27,7 +27,7 @@ public:
   void TestLoadingUnchangedBlockData() {
     cpputils::Data data = cpputils::DataFixture::generate(size);
     blockstore::Key key = blockStore->create(data)->key();
-    auto loaded_block = blockStore->load(key);
+    auto loaded_block = blockStore->load(key).value();
     EXPECT_EQ(0, std::memcmp(data.data(), loaded_block->data(), size));
   }
 
@@ -71,7 +71,7 @@ public:
       key = block->key();
       WriteDataToBlock(block.get(), randomData);
     }
-    auto loaded_block = blockStore->load(key);
+    auto loaded_block = blockStore->load(key).value();
     EXPECT_BLOCK_DATA_CORRECT(*loaded_block, randomData);
   }
 
@@ -80,17 +80,15 @@ public:
     blockstore::Key key = key;
     {
       key = CreateBlock()->key();
-      auto block = blockStore->load(key);
+      auto block = blockStore->load(key).value();
       WriteDataToBlock(block.get(), randomData);
     }
-    auto loaded_block = blockStore->load(key);
+    auto loaded_block = blockStore->load(key).value();
     EXPECT_BLOCK_DATA_CORRECT(*loaded_block, randomData);
   }
 
   void TestLoadNonExistingBlock() {
-    EXPECT_FALSE(
+    EXPECT_EQ(boost::none, blockStore->load(key));
-        (bool)blockStore->load(key)
-    );
   }
 
 private:
@@ -104,24 +102,24 @@ private:
     return ZEROES;
   }
 
-  std::unique_ptr<blockstore::Block> StoreDataToBlockAndLoadIt(const cpputils::Data &data) {
+  cpputils::unique_ref<blockstore::Block> StoreDataToBlockAndLoadIt(const cpputils::Data &data) {
     blockstore::Key key = StoreDataToBlockAndGetKey(data);
-    return blockStore->load(key);
+    return blockStore->load(key).value();
   }
 
   blockstore::Key StoreDataToBlockAndGetKey(const cpputils::Data &data) {
     return blockStore->create(data)->key();
   }
 
-  std::unique_ptr<blockstore::Block> StoreDataToBlockAndLoadItDirectlyAfterFlushing(const cpputils::Data &data) {
+  cpputils::unique_ref<blockstore::Block> StoreDataToBlockAndLoadItDirectlyAfterFlushing(const cpputils::Data &data) {
     auto block = blockStore->create(data);
     block->flush();
-    return blockStore->load(block->key());
+    return blockStore->load(block->key()).value();
   }
 
-  std::unique_ptr<blockstore::Block> CreateBlockAndLoadIt() {
+  cpputils::unique_ref<blockstore::Block> CreateBlockAndLoadIt() {
     blockstore::Key key = CreateBlock()->key();
-    return blockStore->load(key);
+    return blockStore->load(key).value();
   }
 
   cpputils::unique_ref<blockstore::Block> CreateBlock() {