From 75d1ef11fe2324f95a1bf8fe32f968f15ffa2cda Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Fri, 6 Mar 2015 02:21:20 +0100 Subject: [PATCH] Fixed loading nonexisting blobs and wrote more test cases --- implementations/onblocks/BlobOnBlocks.cpp | 4 + implementations/onblocks/BlobOnBlocks.h | 2 + .../onblocks/BlobStoreOnBlocks.cpp | 13 +++- implementations/onblocks/BlobStoreOnBlocks.h | 2 + interface/BlobStore.h | 3 +- .../onblocks/BlobResizeTest.cpp | 58 --------------- .../implementations/onblocks/BlobSizeTest.cpp | 74 +++++++++++++++++++ .../onblocks/BlobStoreTest.cpp | 32 ++++++++ 8 files changed, 127 insertions(+), 61 deletions(-) delete mode 100644 test/implementations/onblocks/BlobResizeTest.cpp create mode 100644 test/implementations/onblocks/BlobSizeTest.cpp create mode 100644 test/implementations/onblocks/BlobStoreTest.cpp diff --git a/implementations/onblocks/BlobOnBlocks.cpp b/implementations/onblocks/BlobOnBlocks.cpp index 505fc227..26ecffe1 100644 --- a/implementations/onblocks/BlobOnBlocks.cpp +++ b/implementations/onblocks/BlobOnBlocks.cpp @@ -68,5 +68,9 @@ Key BlobOnBlocks::key() const { return _datatree->key(); } +unique_ptr BlobOnBlocks::releaseTree() { + return std::move(_datatree); +} + } } diff --git a/implementations/onblocks/BlobOnBlocks.h b/implementations/onblocks/BlobOnBlocks.h index 4cb5ef3d..67f008e5 100644 --- a/implementations/onblocks/BlobOnBlocks.h +++ b/implementations/onblocks/BlobOnBlocks.h @@ -28,6 +28,8 @@ public: void read(void *target, uint64_t offset, uint64_t size) const override; void write(const void *source, uint64_t offset, uint64_t size) override; + std::unique_ptr releaseTree(); + private: void traverseLeaves(uint64_t offsetBytes, uint64_t sizeBytes, std::function) const; diff --git a/implementations/onblocks/BlobStoreOnBlocks.cpp b/implementations/onblocks/BlobStoreOnBlocks.cpp index 5293a606..4048314d 100644 --- a/implementations/onblocks/BlobStoreOnBlocks.cpp +++ b/implementations/onblocks/BlobStoreOnBlocks.cpp @@ -5,12 +5,14 @@ #include "BlobStoreOnBlocks.h" #include "BlobOnBlocks.h" +#include using std::unique_ptr; using std::make_unique; using blockstore::BlockStore; using blockstore::Key; +using cpputils::dynamic_pointer_move; namespace blobstore { namespace onblocks { @@ -30,7 +32,16 @@ unique_ptr BlobStoreOnBlocks::create() { } unique_ptr BlobStoreOnBlocks::load(const Key &key) { - return make_unique(_dataTreeStore->load(key)); + auto tree = _dataTreeStore->load(key); + if (tree == nullptr) { + return nullptr; + } + return make_unique(std::move(tree)); +} + +void BlobStoreOnBlocks::remove(unique_ptr blob) { + auto _blob = dynamic_pointer_move(blob); + _dataTreeStore->remove(_blob->releaseTree()); } } diff --git a/implementations/onblocks/BlobStoreOnBlocks.h b/implementations/onblocks/BlobStoreOnBlocks.h index d391c7ad..6ea4721e 100644 --- a/implementations/onblocks/BlobStoreOnBlocks.h +++ b/implementations/onblocks/BlobStoreOnBlocks.h @@ -19,6 +19,8 @@ public: std::unique_ptr create() override; std::unique_ptr load(const blockstore::Key &key) override; + void remove(std::unique_ptr blob) override; + private: std::unique_ptr _dataTreeStore; }; diff --git a/interface/BlobStore.h b/interface/BlobStore.h index db476954..4941f137 100644 --- a/interface/BlobStore.h +++ b/interface/BlobStore.h @@ -18,8 +18,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 load(const blockstore::Key &key) = 0; - //TODO Needed for performance? Or is deleting loaded blocks enough? - //virtual void remove(const std::string &key) = 0; + virtual void remove(std::unique_ptr blob) = 0; }; } diff --git a/test/implementations/onblocks/BlobResizeTest.cpp b/test/implementations/onblocks/BlobResizeTest.cpp deleted file mode 100644 index 6a10fdbb..00000000 --- a/test/implementations/onblocks/BlobResizeTest.cpp +++ /dev/null @@ -1,58 +0,0 @@ -#include "testutils/BlobStoreTest.h" - -using std::unique_ptr; - -using namespace blobstore; - -class BlobResizeTest: public BlobStoreTest { -public: - BlobResizeTest(): blob(blobStore->create()) {} - - static constexpr uint32_t LARGE_SIZE = 10 * 1024 * 1024; - - unique_ptr blob; -}; -constexpr uint32_t BlobResizeTest::LARGE_SIZE; - -TEST_F(BlobResizeTest, CreatedBlobIsEmpty) { - EXPECT_EQ(0, blob->size()); -} - -TEST_F(BlobResizeTest, Growing_1Byte) { - blob->resize(1); - EXPECT_EQ(1, blob->size()); -} - -TEST_F(BlobResizeTest, Growing_Large) { - blob->resize(LARGE_SIZE); - EXPECT_EQ(LARGE_SIZE, blob->size()); -} - -TEST_F(BlobResizeTest, Shrinking_Empty) { - blob->resize(LARGE_SIZE); - blob->resize(0); - EXPECT_EQ(0, blob->size()); -} - -TEST_F(BlobResizeTest, Shrinking_1Byte) { - blob->resize(LARGE_SIZE); - blob->resize(1); - EXPECT_EQ(1, blob->size()); -} - -TEST_F(BlobResizeTest, ResizingToItself_Empty) { - blob->resize(0); - EXPECT_EQ(0, blob->size()); -} - -TEST_F(BlobResizeTest, ResizingToItself_1Byte) { - blob->resize(1); - blob->resize(1); - EXPECT_EQ(1, blob->size()); -} - -TEST_F(BlobResizeTest, ResizingToItself_Large) { - blob->resize(LARGE_SIZE); - blob->resize(LARGE_SIZE); - EXPECT_EQ(LARGE_SIZE, blob->size()); -} diff --git a/test/implementations/onblocks/BlobSizeTest.cpp b/test/implementations/onblocks/BlobSizeTest.cpp new file mode 100644 index 00000000..7f8a20bf --- /dev/null +++ b/test/implementations/onblocks/BlobSizeTest.cpp @@ -0,0 +1,74 @@ +#include "testutils/BlobStoreTest.h" + +using std::unique_ptr; + +using namespace blobstore; +using blockstore::Key; + +class BlobSizeTest: public BlobStoreTest { +public: + BlobSizeTest(): blob(blobStore->create()) {} + + static constexpr uint32_t LARGE_SIZE = 10 * 1024 * 1024; + + unique_ptr blob; +}; +constexpr uint32_t BlobSizeTest::LARGE_SIZE; + +TEST_F(BlobSizeTest, CreatedBlobIsEmpty) { + EXPECT_EQ(0, blob->size()); +} + +TEST_F(BlobSizeTest, Growing_1Byte) { + blob->resize(1); + EXPECT_EQ(1, blob->size()); +} + +TEST_F(BlobSizeTest, Growing_Large) { + blob->resize(LARGE_SIZE); + EXPECT_EQ(LARGE_SIZE, blob->size()); +} + +TEST_F(BlobSizeTest, Shrinking_Empty) { + blob->resize(LARGE_SIZE); + blob->resize(0); + EXPECT_EQ(0, blob->size()); +} + +TEST_F(BlobSizeTest, Shrinking_1Byte) { + blob->resize(LARGE_SIZE); + blob->resize(1); + EXPECT_EQ(1, blob->size()); +} + +TEST_F(BlobSizeTest, ResizingToItself_Empty) { + blob->resize(0); + EXPECT_EQ(0, blob->size()); +} + +TEST_F(BlobSizeTest, ResizingToItself_1Byte) { + blob->resize(1); + blob->resize(1); + EXPECT_EQ(1, blob->size()); +} + +TEST_F(BlobSizeTest, ResizingToItself_Large) { + blob->resize(LARGE_SIZE); + blob->resize(LARGE_SIZE); + EXPECT_EQ(LARGE_SIZE, blob->size()); +} + +TEST_F(BlobSizeTest, EmptyBlobStaysEmptyWhenLoading) { + Key key = blob->key(); + blob.reset(); + auto loaded = blobStore->load(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); + EXPECT_EQ(LARGE_SIZE, loaded->size()); +} diff --git a/test/implementations/onblocks/BlobStoreTest.cpp b/test/implementations/onblocks/BlobStoreTest.cpp new file mode 100644 index 00000000..4bd60021 --- /dev/null +++ b/test/implementations/onblocks/BlobStoreTest.cpp @@ -0,0 +1,32 @@ +#include "testutils/BlobStoreTest.h" + +using blockstore::Key; + +TEST_F(BlobStoreTest, TwoCreatedBlobsHaveDifferentKeys) { + auto blob1 = blobStore->create(); + auto blob2 = blobStore->create(); + EXPECT_NE(blob1->key(), blob2->key()); +} + +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()); +} + +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()); +} + +//TODO Test read/write +//TODO Test read/write with loading inbetween + +//Taken from BlockStoreTest.h: +//TODO Created blob is zeroed out +//TODO Created blob is zeroed out after loading +//TODO Try loading nonexisting blob