From a945e4f0fcb06a3c10e559364be2b43312539df5 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 20 Jul 2015 18:57:48 +0200 Subject: [PATCH] Changed Blockstore::tryCreate() to return optional> instead of unique_ptr --- implementations/caching/CachingBlockStore.cpp | 13 +++++--- implementations/caching/CachingBlockStore.h | 4 +-- implementations/caching/NewBlock.cpp | 6 ++-- implementations/encrypted/EncryptedBlock.h | 11 +++---- .../encrypted/EncryptedBlockStore.h | 13 +++++--- .../inmemory/InMemoryBlockStore.cpp | 10 +++++-- implementations/inmemory/InMemoryBlockStore.h | 2 +- implementations/ondisk/OnDiskBlock.cpp | 12 +++++--- implementations/ondisk/OnDiskBlock.h | 7 ++--- implementations/ondisk/OnDiskBlockStore.cpp | 15 ++++++---- implementations/ondisk/OnDiskBlockStore.h | 2 +- .../ParallelAccessBlockStore.cpp | 12 ++++---- .../parallelaccess/ParallelAccessBlockStore.h | 2 +- implementations/testfake/FakeBlockStore.cpp | 11 +++++-- implementations/testfake/FakeBlockStore.h | 2 +- interface/BlockStore.h | 20 +++++++------ .../inmemory/InMemoryBlockStoreTest.cpp | 1 + .../OnDiskBlockTest/OnDiskBlockCreateTest.cpp | 5 ++-- .../OnDiskBlockTest/OnDiskBlockFlushTest.cpp | 16 +++++----- .../helpers/BlockStoreWithRandomKeysTest.cpp | 6 ++-- test/testutils/BlockStoreTest.h | 6 ++-- test/testutils/BlockStoreTest_Size.h | 2 +- test/testutils/BlockStoreWithRandomKeysTest.h | 30 +++++++++---------- utils/BlockStoreUtils.cpp | 5 ++-- utils/BlockStoreUtils.h | 4 +-- 25 files changed, 128 insertions(+), 89 deletions(-) diff --git a/implementations/caching/CachingBlockStore.cpp b/implementations/caching/CachingBlockStore.cpp index 100d36be..4b6e40cc 100644 --- a/implementations/caching/CachingBlockStore.cpp +++ b/implementations/caching/CachingBlockStore.cpp @@ -10,6 +10,10 @@ using std::unique_ptr; using std::make_unique; using cpputils::dynamic_pointer_move; using cpputils::Data; +using boost::optional; +using cpputils::unique_ref; +using cpputils::make_unique_ref; +using boost::none; namespace blockstore { namespace caching { @@ -22,9 +26,10 @@ Key CachingBlockStore::createKey() { return _baseBlockStore->createKey(); } -unique_ptr CachingBlockStore::tryCreate(const Key &key, Data data) { +optional> CachingBlockStore::tryCreate(const Key &key, Data data) { + //TODO Shouldn't we return boost::none if the key already exists? ++_numNewBlocks; - return make_unique(make_unique(key, std::move(data), this), this); + return unique_ref(make_unique_ref(make_unique(key, std::move(data), this), this)); } unique_ptr CachingBlockStore::load(const Key &key) { @@ -64,9 +69,9 @@ void CachingBlockStore::release(unique_ptr block) { _cache.push(key, std::move(block)); } -std::unique_ptr CachingBlockStore::tryCreateInBaseStore(const Key &key, Data data) { +optional> CachingBlockStore::tryCreateInBaseStore(const Key &key, Data data) { auto block = _baseBlockStore->tryCreate(key, std::move(data)); - if (block.get() != nullptr) { + if (block != none) { --_numNewBlocks; } return block; diff --git a/implementations/caching/CachingBlockStore.h b/implementations/caching/CachingBlockStore.h index 073ecd97..bb364bd7 100644 --- a/implementations/caching/CachingBlockStore.h +++ b/implementations/caching/CachingBlockStore.h @@ -14,14 +14,14 @@ public: explicit CachingBlockStore(std::unique_ptr baseBlockStore); Key createKey() override; - std::unique_ptr tryCreate(const Key &key, cpputils::Data data) override; + boost::optional> tryCreate(const Key &key, cpputils::Data data) override; std::unique_ptr load(const Key &key) override; void remove(std::unique_ptr block) override; uint64_t numBlocks() const override; void release(std::unique_ptr block); - std::unique_ptr tryCreateInBaseStore(const Key &key, cpputils::Data data); + boost::optional> tryCreateInBaseStore(const Key &key, cpputils::Data data); void removeFromBaseStore(std::unique_ptr block); private: diff --git a/implementations/caching/NewBlock.cpp b/implementations/caching/NewBlock.cpp index 3587f6fb..3cee333f 100644 --- a/implementations/caching/NewBlock.cpp +++ b/implementations/caching/NewBlock.cpp @@ -32,9 +32,11 @@ void NewBlock::write(const void *source, uint64_t offset, uint64_t size) { void NewBlock::writeToBaseBlockIfChanged() { if (_dataChanged) { if (_baseBlock.get() == nullptr) { - //TODO What if tryCreate fails due to a duplicate key? We should ensure we don't use duplicate keys. //TODO _data.copy() necessary? - _baseBlock = _blockStore->tryCreateInBaseStore(key(), _data.copy()); + 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 = cpputils::to_unique_ptr(std::move(*newBase)); } else { _baseBlock->write(_data.data(), 0, _data.size()); } diff --git a/implementations/encrypted/EncryptedBlock.h b/implementations/encrypted/EncryptedBlock.h index f7b96f5b..35adbf2d 100644 --- a/implementations/encrypted/EncryptedBlock.h +++ b/implementations/encrypted/EncryptedBlock.h @@ -22,7 +22,7 @@ template class EncryptedBlock: public Block { public: BOOST_CONCEPT_ASSERT((CipherConcept)); - static std::unique_ptr TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, const typename Cipher::EncryptionKey &encKey); + static boost::optional> TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, const typename Cipher::EncryptionKey &encKey); static std::unique_ptr TryDecrypt(std::unique_ptr baseBlock, const typename Cipher::EncryptionKey &key); //TODO Storing key twice (in parent class and in object pointed to). Once would be enough. @@ -57,16 +57,17 @@ constexpr unsigned int EncryptedBlock::HEADER_LENGTH; template -std::unique_ptr> EncryptedBlock::TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, const typename Cipher::EncryptionKey &encKey) { +boost::optional>> EncryptedBlock::TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, const typename Cipher::EncryptionKey &encKey) { cpputils::Data plaintextWithHeader = _prependKeyHeaderToData(key, std::move(data)); cpputils::Data encrypted = Cipher::encrypt((byte*)plaintextWithHeader.data(), plaintextWithHeader.size(), encKey); auto baseBlock = baseBlockStore->tryCreate(key, std::move(encrypted)); - if (baseBlock.get() == nullptr) { + if (baseBlock == boost::none) { //TODO Test this code branch - return nullptr; + return boost::none; } - return std::make_unique(std::move(baseBlock), encKey, std::move(plaintextWithHeader)); + //TODO Don't use to_unique_ptr + return cpputils::make_unique_ref(cpputils::to_unique_ptr(std::move(*baseBlock)), encKey, std::move(plaintextWithHeader)); } template diff --git a/implementations/encrypted/EncryptedBlockStore.h b/implementations/encrypted/EncryptedBlockStore.h index e677869a..67429dc9 100644 --- a/implementations/encrypted/EncryptedBlockStore.h +++ b/implementations/encrypted/EncryptedBlockStore.h @@ -18,7 +18,7 @@ public: //TODO Are createKey() tests included in generic BlockStoreTest? If not, add it! Key createKey() override; - std::unique_ptr tryCreate(const Key &key, cpputils::Data data) override; + boost::optional> tryCreate(const Key &key, cpputils::Data data) override; std::unique_ptr load(const Key &key) override; void remove(std::unique_ptr block) override; uint64_t numBlocks() const override; @@ -46,9 +46,14 @@ Key EncryptedBlockStore::createKey() { } template -std::unique_ptr EncryptedBlockStore::tryCreate(const Key &key, cpputils::Data data) { - //TODO Test that this returns nullptr when base blockstore returns nullptr (for all pass-through-blockstores) - return EncryptedBlock::TryCreateNew(_baseBlockStore.get(), key, std::move(data), _encKey); +boost::optional> EncryptedBlockStore::tryCreate(const Key &key, cpputils::Data data) { + //TODO Test that this returns boost::none when base blockstore returns nullptr (for all pass-through-blockstores) + //TODO Easier implementation? This is only so complicated because of the case EncryptedBlock -> Block + auto result = EncryptedBlock::TryCreateNew(_baseBlockStore.get(), key, std::move(data), _encKey); + if (result == boost::none) { + return boost::none; + } + return cpputils::unique_ref(std::move(*result)); } template diff --git a/implementations/inmemory/InMemoryBlockStore.cpp b/implementations/inmemory/InMemoryBlockStore.cpp index b4446349..04be4bc2 100644 --- a/implementations/inmemory/InMemoryBlockStore.cpp +++ b/implementations/inmemory/InMemoryBlockStore.cpp @@ -10,6 +10,10 @@ using std::lock_guard; using std::piecewise_construct; using std::make_tuple; using cpputils::Data; +using cpputils::unique_ref; +using cpputils::make_unique_ref; +using boost::optional; +using boost::none; namespace blockstore { namespace inmemory { @@ -17,15 +21,15 @@ namespace inmemory { InMemoryBlockStore::InMemoryBlockStore() : _blocks() {} -unique_ptr InMemoryBlockStore::tryCreate(const Key &key, Data data) { +optional> InMemoryBlockStore::tryCreate(const Key &key, Data data) { auto insert_result = _blocks.emplace(piecewise_construct, make_tuple(key.ToString()), make_tuple(key, std::move(data))); if (!insert_result.second) { - return nullptr; + return none; } //Return a pointer to the stored InMemoryBlock - return make_unique(insert_result.first->second); + return optional>(make_unique_ref(insert_result.first->second)); } unique_ptr InMemoryBlockStore::load(const Key &key) { diff --git a/implementations/inmemory/InMemoryBlockStore.h b/implementations/inmemory/InMemoryBlockStore.h index b9ec5d25..15dd0edc 100644 --- a/implementations/inmemory/InMemoryBlockStore.h +++ b/implementations/inmemory/InMemoryBlockStore.h @@ -16,7 +16,7 @@ class InMemoryBlockStore: public BlockStoreWithRandomKeys { public: InMemoryBlockStore(); - std::unique_ptr tryCreate(const Key &key, cpputils::Data data) override; + boost::optional> tryCreate(const Key &key, cpputils::Data data) override; std::unique_ptr load(const Key &key) override; void remove(std::unique_ptr block) override; uint64_t numBlocks() const override; diff --git a/implementations/ondisk/OnDiskBlock.cpp b/implementations/ondisk/OnDiskBlock.cpp index 4139a999..55ad1ac1 100644 --- a/implementations/ondisk/OnDiskBlock.cpp +++ b/implementations/ondisk/OnDiskBlock.cpp @@ -13,6 +13,10 @@ using std::ifstream; using std::ofstream; using std::ios; using cpputils::Data; +using cpputils::make_unique_ref; +using cpputils::unique_ref; +using boost::optional; +using boost::none; namespace bf = boost::filesystem; @@ -61,15 +65,15 @@ unique_ptr OnDiskBlock::LoadFromDisk(const bf::path &rootdir, const } } -unique_ptr OnDiskBlock::CreateOnDisk(const bf::path &rootdir, const Key &key, Data data) { +optional> OnDiskBlock::CreateOnDisk(const bf::path &rootdir, const Key &key, Data data) { auto filepath = rootdir / key.ToString(); if (bf::exists(filepath)) { - return nullptr; + return none; } - auto block = unique_ptr(new OnDiskBlock(key, filepath, std::move(data))); + auto block = make_unique_ref(key, filepath, std::move(data)); block->_storeToDisk(); - return block; + return std::move(block); } void OnDiskBlock::RemoveFromDisk(const bf::path &rootdir, const Key &key) { diff --git a/implementations/ondisk/OnDiskBlock.h b/implementations/ondisk/OnDiskBlock.h index 3acea481..8ea85334 100644 --- a/implementations/ondisk/OnDiskBlock.h +++ b/implementations/ondisk/OnDiskBlock.h @@ -7,7 +7,7 @@ #include #include -#include "messmer/cpp-utils/macros.h" +#include namespace blockstore { namespace ondisk { @@ -15,10 +15,11 @@ class OnDiskBlockStore; class OnDiskBlock: public Block { public: + OnDiskBlock(const Key &key, const boost::filesystem::path &filepath, cpputils::Data data); virtual ~OnDiskBlock(); static std::unique_ptr LoadFromDisk(const boost::filesystem::path &rootdir, const Key &key); - static std::unique_ptr CreateOnDisk(const boost::filesystem::path &rootdir, const Key &key, cpputils::Data data); + static boost::optional> CreateOnDisk(const boost::filesystem::path &rootdir, const Key &key, cpputils::Data data); static void RemoveFromDisk(const boost::filesystem::path &rootdir, const Key &key); const void *data() const override; @@ -33,8 +34,6 @@ private: cpputils::Data _data; bool _dataChanged; - OnDiskBlock(const Key &key, const boost::filesystem::path &filepath, cpputils::Data data); - void _fillDataWithZeroes(); void _storeToDisk() const; diff --git a/implementations/ondisk/OnDiskBlockStore.cpp b/implementations/ondisk/OnDiskBlockStore.cpp index 3335a047..9be60699 100644 --- a/implementations/ondisk/OnDiskBlockStore.cpp +++ b/implementations/ondisk/OnDiskBlockStore.cpp @@ -5,6 +5,9 @@ using std::unique_ptr; using std::make_unique; using std::string; using cpputils::Data; +using cpputils::unique_ref; +using boost::optional; +using boost::none; namespace bf = boost::filesystem; @@ -14,13 +17,13 @@ namespace ondisk { OnDiskBlockStore::OnDiskBlockStore(const boost::filesystem::path &rootdir) : _rootdir(rootdir) {} -unique_ptr OnDiskBlockStore::tryCreate(const Key &key, Data data) { - auto block = OnDiskBlock::CreateOnDisk(_rootdir, key, std::move(data)); - - if (!block) { - return nullptr; +optional> OnDiskBlockStore::tryCreate(const Key &key, Data data) { + //TODO Easier implementation? This is only so complicated because of the cast OnDiskBlock -> Block + auto result = std::move(OnDiskBlock::CreateOnDisk(_rootdir, key, std::move(data))); + if (result == boost::none) { + return boost::none; } - return std::move(block); + return unique_ref(std::move(*result)); } unique_ptr OnDiskBlockStore::load(const Key &key) { diff --git a/implementations/ondisk/OnDiskBlockStore.h b/implementations/ondisk/OnDiskBlockStore.h index af7f72fc..a75429c2 100644 --- a/implementations/ondisk/OnDiskBlockStore.h +++ b/implementations/ondisk/OnDiskBlockStore.h @@ -14,7 +14,7 @@ class OnDiskBlockStore: public BlockStoreWithRandomKeys { public: explicit OnDiskBlockStore(const boost::filesystem::path &rootdir); - std::unique_ptr tryCreate(const Key &key, cpputils::Data data) override; + boost::optional> tryCreate(const Key &key, cpputils::Data data) override; std::unique_ptr load(const Key &key) override; void remove(std::unique_ptr block) override; uint64_t numBlocks() const override; diff --git a/implementations/parallelaccess/ParallelAccessBlockStore.cpp b/implementations/parallelaccess/ParallelAccessBlockStore.cpp index 9bd59064..58d257da 100644 --- a/implementations/parallelaccess/ParallelAccessBlockStore.cpp +++ b/implementations/parallelaccess/ParallelAccessBlockStore.cpp @@ -15,6 +15,9 @@ using cpputils::dynamic_pointer_move; using cpputils::make_unique_ref; using boost::none; using cpputils::unique_ref; +using cpputils::make_unique_ref; +using boost::optional; +using boost::none; namespace blockstore { namespace parallelaccess { @@ -27,14 +30,13 @@ Key ParallelAccessBlockStore::createKey() { return _baseBlockStore->createKey(); } -unique_ptr ParallelAccessBlockStore::tryCreate(const Key &key, cpputils::Data data) { - //TODO Don't use nullcheck/to_unique_ptr but make blockstore use unique_ref - auto block = cpputils::nullcheck(_baseBlockStore->tryCreate(key, std::move(data))); +optional> ParallelAccessBlockStore::tryCreate(const Key &key, cpputils::Data data) { + auto block = _baseBlockStore->tryCreate(key, std::move(data)); if (block == none) { //TODO Test this code branch - return nullptr; + return none; } - return cpputils::to_unique_ptr(_parallelAccessStore.add(key, std::move(*block))); + return unique_ref(_parallelAccessStore.add(key, std::move(*block))); } unique_ptr ParallelAccessBlockStore::load(const Key &key) { diff --git a/implementations/parallelaccess/ParallelAccessBlockStore.h b/implementations/parallelaccess/ParallelAccessBlockStore.h index fd570542..9ec4f5a4 100644 --- a/implementations/parallelaccess/ParallelAccessBlockStore.h +++ b/implementations/parallelaccess/ParallelAccessBlockStore.h @@ -16,7 +16,7 @@ public: explicit ParallelAccessBlockStore(cpputils::unique_ref baseBlockStore); Key createKey() override; - std::unique_ptr tryCreate(const Key &key, cpputils::Data data) override; + boost::optional> tryCreate(const Key &key, cpputils::Data data) override; std::unique_ptr load(const Key &key) override; void remove(std::unique_ptr block) override; uint64_t numBlocks() const override; diff --git a/implementations/testfake/FakeBlockStore.cpp b/implementations/testfake/FakeBlockStore.cpp index eb1e09f4..7857c776 100644 --- a/implementations/testfake/FakeBlockStore.cpp +++ b/implementations/testfake/FakeBlockStore.cpp @@ -8,6 +8,10 @@ using std::string; using std::mutex; using std::lock_guard; using cpputils::Data; +using cpputils::unique_ref; +using cpputils::make_unique_ref; +using boost::optional; +using boost::none; namespace blockstore { namespace testfake { @@ -15,15 +19,16 @@ namespace testfake { FakeBlockStore::FakeBlockStore() : _blocks(), _used_dataregions_for_blocks() {} -unique_ptr FakeBlockStore::tryCreate(const Key &key, Data data) { +optional> FakeBlockStore::tryCreate(const Key &key, Data data) { auto insert_result = _blocks.emplace(key.ToString(), std::move(data)); if (!insert_result.second) { - return nullptr; + return none; } //Return a copy of the stored data - return load(key); + //TODO Don't use nullcheck but make load() use unique_ref + return cpputils::nullcheck(load(key)); } unique_ptr FakeBlockStore::load(const Key &key) { diff --git a/implementations/testfake/FakeBlockStore.h b/implementations/testfake/FakeBlockStore.h index 9d291cec..2d198e3f 100644 --- a/implementations/testfake/FakeBlockStore.h +++ b/implementations/testfake/FakeBlockStore.h @@ -31,7 +31,7 @@ class FakeBlockStore: public BlockStoreWithRandomKeys { public: FakeBlockStore(); - std::unique_ptr tryCreate(const Key &key, cpputils::Data data) override; + boost::optional> tryCreate(const Key &key, cpputils::Data data) override; std::unique_ptr load(const Key &key) override; void remove(std::unique_ptr block) override; uint64_t numBlocks() const override; diff --git a/interface/BlockStore.h b/interface/BlockStore.h index 9bb9b14e..4a1facfe 100644 --- a/interface/BlockStore.h +++ b/interface/BlockStore.h @@ -4,7 +4,8 @@ #include "Block.h" #include -#include +#include +#include #include namespace blockstore { @@ -14,21 +15,22 @@ public: virtual ~BlockStore() {} virtual Key createKey() = 0; - //Returns nullptr if key already exists - virtual std::unique_ptr tryCreate(const Key &key, cpputils::Data data) = 0; + //Returns boost::none if key already exists + virtual boost::optional> 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 load(const Key &key) = 0; virtual void remove(std::unique_ptr block) = 0; virtual uint64_t numBlocks() const = 0; - std::unique_ptr create(const cpputils::Data &data) { - std::unique_ptr block(nullptr); - while(block.get() == nullptr) { - //TODO Copy necessary? - block = tryCreate(createKey(), data.copy()); + cpputils::unique_ref create(const cpputils::Data &data) { + while(true) { + //TODO Copy (data.copy()) necessary? + auto block = tryCreate(createKey(), data.copy()); + if (block != boost::none) { + return std::move(*block); + } } - return block; } }; diff --git a/test/implementations/inmemory/InMemoryBlockStoreTest.cpp b/test/implementations/inmemory/InMemoryBlockStoreTest.cpp index 8cf12258..b8dfa8e8 100644 --- a/test/implementations/inmemory/InMemoryBlockStoreTest.cpp +++ b/test/implementations/inmemory/InMemoryBlockStoreTest.cpp @@ -3,6 +3,7 @@ #include "../../testutils/BlockStoreTest.h" #include "../../testutils/BlockStoreWithRandomKeysTest.h" #include "google/gtest/gtest.h" +#include using blockstore::BlockStore; diff --git a/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp b/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp index 89fe0bfa..2635f88e 100644 --- a/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp +++ b/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp @@ -12,6 +12,7 @@ using std::unique_ptr; using cpputils::Data; using cpputils::TempFile; using cpputils::TempDir; +using cpputils::unique_ref; using namespace blockstore; using namespace blockstore::ondisk; @@ -49,11 +50,11 @@ TEST_F(OnDiskBlockCreateTest, CreatingExistingBlockReturnsNull) { class OnDiskBlockCreateSizeTest: public OnDiskBlockCreateTest, public WithParamInterface { public: - unique_ptr block; + unique_ref block; Data ZEROES; OnDiskBlockCreateSizeTest(): - block(OnDiskBlock::CreateOnDisk(dir.path(), key, std::move(Data(GetParam()).FillWithZeroes()))), + block(OnDiskBlock::CreateOnDisk(dir.path(), key, std::move(Data(GetParam()).FillWithZeroes())).value()), ZEROES(block->size()) { ZEROES.FillWithZeroes(); diff --git a/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp b/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp index 8d723b92..d90950e6 100644 --- a/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp +++ b/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp @@ -14,6 +14,7 @@ using cpputils::Data; using cpputils::DataFixture; using cpputils::TempFile; using cpputils::TempDir; +using cpputils::unique_ref; using namespace blockstore; using namespace blockstore::ondisk; @@ -35,22 +36,23 @@ public: Data randomData; - unique_ptr CreateBlockAndLoadItFromDisk() { + unique_ref CreateBlockAndLoadItFromDisk() { { - OnDiskBlock::CreateOnDisk(dir.path(), key, randomData.copy()); + OnDiskBlock::CreateOnDisk(dir.path(), key, randomData.copy()).value(); } - return OnDiskBlock::LoadFromDisk(dir.path(), key); + //TODO Don't use nullcheck + return cpputils::nullcheck(OnDiskBlock::LoadFromDisk(dir.path(), key)).value(); } - unique_ptr CreateBlock() { - return OnDiskBlock::CreateOnDisk(dir.path(), key, randomData.copy()); + unique_ref CreateBlock() { + return OnDiskBlock::CreateOnDisk(dir.path(), key, randomData.copy()).value(); } - void WriteDataToBlock(const unique_ptr &block) { + void WriteDataToBlock(const unique_ref &block) { block->write(randomData.data(), 0, randomData.size()); } - void EXPECT_BLOCK_DATA_CORRECT(const unique_ptr &block) { + void EXPECT_BLOCK_DATA_CORRECT(const unique_ref &block) { EXPECT_EQ(randomData.size(), block->size()); EXPECT_EQ(0, std::memcmp(randomData.data(), block->data(), randomData.size())); } diff --git a/test/interface/helpers/BlockStoreWithRandomKeysTest.cpp b/test/interface/helpers/BlockStoreWithRandomKeysTest.cpp index 4d5c834d..0f59fbc2 100644 --- a/test/interface/helpers/BlockStoreWithRandomKeysTest.cpp +++ b/test/interface/helpers/BlockStoreWithRandomKeysTest.cpp @@ -15,13 +15,15 @@ using std::unique_ptr; using std::make_unique; using cpputils::Data; using cpputils::DataFixture; +using cpputils::unique_ref; +using boost::optional; using namespace blockstore; class BlockStoreWithRandomKeysMock: public BlockStoreWithRandomKeys { public: - unique_ptr tryCreate(const Key &key, Data data) { - return unique_ptr(do_create(key, data)); + optional> tryCreate(const Key &key, Data data) { + return cpputils::nullcheck(unique_ptr(do_create(key, data))); } MOCK_METHOD2(do_create, Block*(const Key &, const Data &data)); unique_ptr load(const Key &key) { diff --git a/test/testutils/BlockStoreTest.h b/test/testutils/BlockStoreTest.h index f97eb4d5..122beb95 100644 --- a/test/testutils/BlockStoreTest.h +++ b/test/testutils/BlockStoreTest.h @@ -60,7 +60,8 @@ TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectAfterAddingOneBlock_AfterClosingB TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectAfterRemovingTheLastBlock) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->create(cpputils::Data(1)); - blockStore->remove(std::move(block)); + //TODO Don't use to_unique_ptr + blockStore->remove(cpputils::to_unique_ptr(std::move(block))); EXPECT_EQ(0, blockStore->numBlocks()); } @@ -96,7 +97,8 @@ TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectAfterRemovingABlock) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->create(cpputils::Data(1)); blockStore->create(cpputils::Data(1)); - blockStore->remove(std::move(block)); + // TODO Don't use to_unique_ptr + blockStore->remove(cpputils::to_unique_ptr(std::move(block))); EXPECT_EQ(1, blockStore->numBlocks()); } diff --git a/test/testutils/BlockStoreTest_Size.h b/test/testutils/BlockStoreTest_Size.h index 20ee3fbb..c81ca388 100644 --- a/test/testutils/BlockStoreTest_Size.h +++ b/test/testutils/BlockStoreTest_Size.h @@ -124,7 +124,7 @@ private: return blockStore->load(key); } - std::unique_ptr CreateBlock() { + cpputils::unique_ref CreateBlock() { return blockStore->create(cpputils::Data(size)); } diff --git a/test/testutils/BlockStoreWithRandomKeysTest.h b/test/testutils/BlockStoreWithRandomKeysTest.h index 3f2db103..8ea57da6 100644 --- a/test/testutils/BlockStoreWithRandomKeysTest.h +++ b/test/testutils/BlockStoreWithRandomKeysTest.h @@ -31,46 +31,46 @@ TYPED_TEST_CASE_P(BlockStoreWithRandomKeysTest); TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndSameSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->tryCreate(this->key, cpputils::Data(1024)); - block->flush(); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. auto block2 = blockStore->tryCreate(this->key, cpputils::Data(1024)); - EXPECT_TRUE((bool)block); - EXPECT_FALSE((bool)block2); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); } TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndDifferentSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->tryCreate(this->key, cpputils::Data(1024)); - block->flush(); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. auto block2 = blockStore->tryCreate(this->key, cpputils::Data(4096)); - EXPECT_TRUE((bool)block); - EXPECT_FALSE((bool)block2); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); } TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndFirstNullSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->tryCreate(this->key, cpputils::Data(0)); - block->flush(); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. auto block2 = blockStore->tryCreate(this->key, cpputils::Data(1024)); - EXPECT_TRUE((bool)block); - EXPECT_FALSE((bool)block2); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); } TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndSecondNullSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->tryCreate(this->key, cpputils::Data(1024)); - block->flush(); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. auto block2 = blockStore->tryCreate(this->key, cpputils::Data(0)); - EXPECT_TRUE((bool)block); - EXPECT_FALSE((bool)block2); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); } TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndBothNullSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->tryCreate(this->key, cpputils::Data(0)); - block->flush(); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. auto block2 = blockStore->tryCreate(this->key, cpputils::Data(0)); - EXPECT_TRUE((bool)block); - EXPECT_FALSE((bool)block2); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); } REGISTER_TYPED_TEST_CASE_P(BlockStoreWithRandomKeysTest, diff --git a/utils/BlockStoreUtils.cpp b/utils/BlockStoreUtils.cpp index 2b20cdef..2c358344 100644 --- a/utils/BlockStoreUtils.cpp +++ b/utils/BlockStoreUtils.cpp @@ -1,16 +1,15 @@ #include "../interface/BlockStore.h" #include "BlockStoreUtils.h" #include -#include #include -using std::unique_ptr; using cpputils::Data; +using cpputils::unique_ref; namespace blockstore { namespace utils { -unique_ptr copyToNewBlock(BlockStore *blockStore, const Block &block) { +unique_ref copyToNewBlock(BlockStore *blockStore, const Block &block) { Data data(block.size()); std::memcpy(data.data(), block.data(), block.size()); return blockStore->create(data); diff --git a/utils/BlockStoreUtils.h b/utils/BlockStoreUtils.h index 8e620a27..9dfe8536 100644 --- a/utils/BlockStoreUtils.h +++ b/utils/BlockStoreUtils.h @@ -2,14 +2,14 @@ #ifndef BLOCKSTORE_UTILS_BLOCKSTOREUTILS_H_ #define BLOCKSTORE_UTILS_BLOCKSTOREUTILS_H_ -#include +#include namespace blockstore { class BlockStore; class Block; namespace utils { -std::unique_ptr copyToNewBlock(BlockStore *blockStore, const Block &block); +cpputils::unique_ref copyToNewBlock(BlockStore *blockStore, const Block &block); void copyTo(Block *target, const Block &source); void fillWithZeroes(Block *target);