From 5039205cd26cf39c2006ba2a150cfad7113d2940 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Me=C3=9Fmer?= Date: Thu, 18 Jun 2015 12:45:37 +0200 Subject: [PATCH] BlobStore handles unique_ref instead of unique_ptr --- .../onblocks/BlobStoreOnBlocks.cpp | 17 ++++++++----- implementations/onblocks/BlobStoreOnBlocks.h | 6 ++--- interface/BlobStore.h | 9 +++---- .../onblocks/BlobReadWriteTest.cpp | 10 ++++---- .../implementations/onblocks/BlobSizeTest.cpp | 17 ++++++------- .../onblocks/BlobStoreTest.cpp | 25 ++++++++++++++----- .../onblocks/testutils/BlobStoreTest.h | 11 ++++++++ 7 files changed, 61 insertions(+), 34 deletions(-) diff --git a/implementations/onblocks/BlobStoreOnBlocks.cpp b/implementations/onblocks/BlobStoreOnBlocks.cpp index 5ba84194..68780823 100644 --- a/implementations/onblocks/BlobStoreOnBlocks.cpp +++ b/implementations/onblocks/BlobStoreOnBlocks.cpp @@ -12,10 +12,15 @@ using std::unique_ptr; using std::make_unique; +using cpputils::unique_ref; +using cpputils::make_unique_ref; + using blockstore::BlockStore; using blockstore::parallelaccess::ParallelAccessBlockStore; using blockstore::Key; using cpputils::dynamic_pointer_move; +using boost::optional; +using boost::none; namespace blobstore { namespace onblocks { @@ -31,19 +36,19 @@ BlobStoreOnBlocks::BlobStoreOnBlocks(unique_ptr blockStore, uint32_t BlobStoreOnBlocks::~BlobStoreOnBlocks() { } -unique_ptr BlobStoreOnBlocks::create() { - return make_unique(_dataTreeStore->createNewTree()); +unique_ref BlobStoreOnBlocks::create() { + return make_unique_ref(_dataTreeStore->createNewTree()); } -unique_ptr BlobStoreOnBlocks::load(const Key &key) { +optional> BlobStoreOnBlocks::load(const Key &key) { auto tree = _dataTreeStore->load(key); if (tree == nullptr) { - return nullptr; + return none; } - return make_unique(std::move(tree)); + return optional>(make_unique_ref(std::move(tree))); } -void BlobStoreOnBlocks::remove(unique_ptr blob) { +void BlobStoreOnBlocks::remove(unique_ref blob) { auto _blob = dynamic_pointer_move(blob); _dataTreeStore->remove(_blob->releaseTree()); } diff --git a/implementations/onblocks/BlobStoreOnBlocks.h b/implementations/onblocks/BlobStoreOnBlocks.h index 0a2cd2ea..a01922f5 100644 --- a/implementations/onblocks/BlobStoreOnBlocks.h +++ b/implementations/onblocks/BlobStoreOnBlocks.h @@ -18,10 +18,10 @@ public: BlobStoreOnBlocks(std::unique_ptr blockStore, uint32_t blocksizeBytes); virtual ~BlobStoreOnBlocks(); - std::unique_ptr create() override; - std::unique_ptr load(const blockstore::Key &key) override; + cpputils::unique_ref create() override; + boost::optional> load(const blockstore::Key &key) override; - void remove(std::unique_ptr blob) override; + void remove(cpputils::unique_ref blob) override; private: std::unique_ptr _dataTreeStore; diff --git a/interface/BlobStore.h b/interface/BlobStore.h index 4941f137..d9871495 100644 --- a/interface/BlobStore.h +++ b/interface/BlobStore.h @@ -7,6 +7,7 @@ #include #include "messmer/blockstore/utils/Key.h" +#include namespace blobstore { @@ -14,11 +15,9 @@ class BlobStore { public: virtual ~BlobStore() {} - virtual std::unique_ptr create() = 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 blockstore::Key &key) = 0; - virtual void remove(std::unique_ptr blob) = 0; + virtual cpputils::unique_ref create() = 0; + virtual boost::optional> load(const blockstore::Key &key) = 0; + virtual void remove(cpputils::unique_ref blob) = 0; }; } diff --git a/test/implementations/onblocks/BlobReadWriteTest.cpp b/test/implementations/onblocks/BlobReadWriteTest.cpp index 0ff562a8..9f63a479 100644 --- a/test/implementations/onblocks/BlobReadWriteTest.cpp +++ b/test/implementations/onblocks/BlobReadWriteTest.cpp @@ -3,7 +3,7 @@ #include #include "../../../implementations/onblocks/datanodestore/DataNodeView.h" -using std::unique_ptr; +using cpputils::unique_ref; using ::testing::WithParamInterface; using ::testing::Values; @@ -37,7 +37,7 @@ public: } Data randomData; - unique_ptr blob; + unique_ref blob; }; constexpr uint32_t BlobReadWriteTest::LARGE_SIZE; constexpr DataNodeLayout BlobReadWriteTest::LAYOUT; @@ -45,14 +45,14 @@ constexpr DataNodeLayout BlobReadWriteTest::LAYOUT; TEST_F(BlobReadWriteTest, WritingImmediatelyFlushes_SmallSize) { blob->resize(5); blob->write(randomData.data(), 0, 5); - auto loaded = blobStore->load(blob->key()); + auto loaded = loadBlob(blob->key()); EXPECT_DATA_READS_AS(randomData, *loaded, 0, 5); } TEST_F(BlobReadWriteTest, WritingImmediatelyFlushes_LargeSize) { blob->resize(LARGE_SIZE); blob->write(randomData.data(), 0, LARGE_SIZE); - auto loaded = blobStore->load(blob->key()); + auto loaded = loadBlob(blob->key()); EXPECT_DATA_READS_AS(randomData, *loaded, 0, LARGE_SIZE); } @@ -127,7 +127,7 @@ TEST_P(BlobReadWriteDataTest, WriteAndReadImmediately) { TEST_P(BlobReadWriteDataTest, WriteAndReadAfterLoading) { blob->resize(GetParam().blobsize); blob->write(this->foregroundData.data(), GetParam().offset, GetParam().count); - auto loaded = blobStore->load(blob->key()); + auto loaded = loadBlob(blob->key()); EXPECT_DATA_READS_AS(this->foregroundData, *loaded, GetParam().offset, GetParam().count); EXPECT_DATA_IS_ZEROES_OUTSIDE_OF(*loaded, GetParam().offset, GetParam().count); diff --git a/test/implementations/onblocks/BlobSizeTest.cpp b/test/implementations/onblocks/BlobSizeTest.cpp index 16e5077c..a972f551 100644 --- a/test/implementations/onblocks/BlobSizeTest.cpp +++ b/test/implementations/onblocks/BlobSizeTest.cpp @@ -2,12 +2,11 @@ #include #include -using std::unique_ptr; - using namespace blobstore; using blockstore::Key; using cpputils::Data; using cpputils::DataFixture; +using cpputils::unique_ref; class BlobSizeTest: public BlobStoreTest { public: @@ -16,7 +15,7 @@ public: static constexpr uint32_t MEDIUM_SIZE = 5 * 1024 * 1024; static constexpr uint32_t LARGE_SIZE = 10 * 1024 * 1024; - unique_ptr blob; + unique_ref blob; }; constexpr uint32_t BlobSizeTest::MEDIUM_SIZE; constexpr uint32_t BlobSizeTest::LARGE_SIZE; @@ -66,16 +65,16 @@ TEST_F(BlobSizeTest, ResizingToItself_Large) { TEST_F(BlobSizeTest, EmptyBlobStaysEmptyWhenLoading) { Key key = blob->key(); - blob.reset(); - auto loaded = blobStore->load(key); + reset(std::move(blob)); + auto loaded = loadBlob(key); EXPECT_EQ(0, loaded->size()); } TEST_F(BlobSizeTest, BlobSizeStaysIntactWhenLoading) { blob->resize(LARGE_SIZE); Key key = blob->key(); - blob.reset(); - auto loaded = blobStore->load(key); + reset(std::move(blob)); + auto loaded = loadBlob(key); EXPECT_EQ(LARGE_SIZE, loaded->size()); } @@ -114,7 +113,7 @@ TEST_F(BlobSizeTest, WritingAfterEndOfBlobGrowsBlob_NonEmpty) { TEST_F(BlobSizeTest, ChangingSizeImmediatelyFlushes) { blob->resize(LARGE_SIZE); - auto loaded = blobStore->load(blob->key()); + auto loaded = loadBlob(blob->key()); EXPECT_EQ(LARGE_SIZE, loaded->size()); } @@ -143,7 +142,7 @@ TEST_F(BlobSizeDataTest, BlobIsZeroedOutAfterGrowing) { TEST_F(BlobSizeDataTest, BlobIsZeroedOutAfterGrowingAndLoading) { blob->resize(LARGE_SIZE); - auto loaded = blobStore->load(blob->key()); + auto loaded = loadBlob(blob->key()); EXPECT_EQ(0, std::memcmp(readBlob(*loaded).data(), ZEROES.data(), LARGE_SIZE)); } diff --git a/test/implementations/onblocks/BlobStoreTest.cpp b/test/implementations/onblocks/BlobStoreTest.cpp index cff3eb6e..7481fdaf 100644 --- a/test/implementations/onblocks/BlobStoreTest.cpp +++ b/test/implementations/onblocks/BlobStoreTest.cpp @@ -1,16 +1,29 @@ #include "testutils/BlobStoreTest.h" +#include using blockstore::Key; +using cpputils::unique_ref; +using blobstore::Blob; +using boost::none; + + +//gtest/boost::optional workaround for working with optional> +namespace boost { + std::ostream& operator<<(std::ostream& out, const unique_ref &ref) { + out << "[" << ref->key().ToString() << "]" << ref.get(); + return out; + } +} TEST_F(BlobStoreTest, LoadNonexistingKeyOnEmptyBlobstore) { const blockstore::Key key = blockstore::Key::FromString("1491BB4932A389EE14BC7090AC772972"); - EXPECT_EQ(nullptr, blobStore->load(key)); + EXPECT_EQ(none, blobStore->load(key)); } TEST_F(BlobStoreTest, LoadNonexistingKeyOnNonEmptyBlobstore) { blobStore->create(); const blockstore::Key key = blockstore::Key::FromString("1491BB4932A389EE14BC7090AC772972"); - EXPECT_EQ(nullptr, blobStore->load(key)); + EXPECT_EQ(none, blobStore->load(key)); } TEST_F(BlobStoreTest, TwoCreatedBlobsHaveDifferentKeys) { @@ -23,13 +36,13 @@ TEST_F(BlobStoreTest, BlobIsNotLoadableAfterDeletion_DeleteDirectly) { auto blob = blobStore->create(); Key key = blob->key(); blobStore->remove(std::move(blob)); - EXPECT_EQ(nullptr, blobStore->load(key).get()); + EXPECT_FALSE((bool)blobStore->load(key)); } TEST_F(BlobStoreTest, BlobIsNotLoadableAfterDeletion_DeleteAfterLoading) { auto blob = blobStore->create(); Key key = blob->key(); - blob.reset(); - blobStore->remove(blobStore->load(key)); - EXPECT_EQ(nullptr, blobStore->load(key).get()); + reset(std::move(blob)); + blobStore->remove(loadBlob(key)); + EXPECT_FALSE((bool)blobStore->load(key)); } diff --git a/test/implementations/onblocks/testutils/BlobStoreTest.h b/test/implementations/onblocks/testutils/BlobStoreTest.h index d81a9369..2c664646 100644 --- a/test/implementations/onblocks/testutils/BlobStoreTest.h +++ b/test/implementations/onblocks/testutils/BlobStoreTest.h @@ -9,4 +9,15 @@ public: static constexpr uint32_t BLOCKSIZE_BYTES = 4096; std::unique_ptr blobStore; + + cpputils::unique_ref loadBlob(const blockstore::Key &key) { + auto loaded = blobStore->load(key); + EXPECT_TRUE((bool)loaded); + return std::move(*loaded); + } + + void reset(cpputils::unique_ref ref) { + UNUSED(ref); + //ref is moved into here and then destructed + } };