From cc30dcde104ac18b125b6054417585c676e0c576 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Wed, 15 Apr 2015 14:51:41 +0200 Subject: [PATCH] Fix FakeBlockStore --- implementations/ondisk/OnDiskBlock.cpp | 1 + implementations/testfake/FakeBlock.cpp | 4 +-- implementations/testfake/FakeBlock.h | 2 +- implementations/testfake/FakeBlockStore.cpp | 25 +++++++++++-------- implementations/testfake/FakeBlockStore.h | 2 +- interface/Block.h | 3 +++ .../OnDiskBlockTest/OnDiskBlockCreateTest.cpp | 3 +++ test/testutils/BlockStoreWithRandomKeysTest.h | 5 ++++ 8 files changed, 30 insertions(+), 15 deletions(-) diff --git a/implementations/ondisk/OnDiskBlock.cpp b/implementations/ondisk/OnDiskBlock.cpp index 4325c561..6bcf580e 100644 --- a/implementations/ondisk/OnDiskBlock.cpp +++ b/implementations/ondisk/OnDiskBlock.cpp @@ -80,6 +80,7 @@ void OnDiskBlock::RemoveFromDisk(const bf::path &rootdir, const Key &key) { void OnDiskBlock::_fillDataWithZeroes() { _data.FillWithZeroes(); + _dataChanged = true; } void OnDiskBlock::_storeToDisk() const { diff --git a/implementations/testfake/FakeBlock.cpp b/implementations/testfake/FakeBlock.cpp index 5f1285e8..adf8bcdf 100644 --- a/implementations/testfake/FakeBlock.cpp +++ b/implementations/testfake/FakeBlock.cpp @@ -14,8 +14,8 @@ using std::string; namespace blockstore { namespace testfake { -FakeBlock::FakeBlock(FakeBlockStore *store, const Key &key, shared_ptr data) - : Block(key), _store(store), _data(data), _dataChanged(false) { +FakeBlock::FakeBlock(FakeBlockStore *store, const Key &key, shared_ptr data, bool dirty) + : Block(key), _store(store), _data(data), _dataChanged(dirty) { } FakeBlock::~FakeBlock() { diff --git a/implementations/testfake/FakeBlock.h b/implementations/testfake/FakeBlock.h index c667b5a6..7f49f224 100644 --- a/implementations/testfake/FakeBlock.h +++ b/implementations/testfake/FakeBlock.h @@ -13,7 +13,7 @@ class FakeBlockStore; class FakeBlock: public Block { public: - FakeBlock(FakeBlockStore *store, const Key &key, std::shared_ptr data); + FakeBlock(FakeBlockStore *store, const Key &key, std::shared_ptr data, bool dirty); virtual ~FakeBlock(); const void *data() const override; diff --git a/implementations/testfake/FakeBlockStore.cpp b/implementations/testfake/FakeBlockStore.cpp index d10be72f..6d71ee69 100644 --- a/implementations/testfake/FakeBlockStore.cpp +++ b/implementations/testfake/FakeBlockStore.cpp @@ -15,22 +15,19 @@ FakeBlockStore::FakeBlockStore() : _blocks(), _used_dataregions_for_blocks() {} unique_ptr FakeBlockStore::create(const Key &key, size_t size) { - auto insert_result = _blocks.emplace(key.ToString(), size); - insert_result.first->second.FillWithZeroes(); - - if (!insert_result.second) { + if (_blocks.find(key.ToString()) != _blocks.end()) { return nullptr; } - - //Return a copy of the stored data - return load(key); + Data data(size); + data.FillWithZeroes(); + return makeFakeBlockFromData(key, data, true); } unique_ptr 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)); + return makeFakeBlockFromData(key, _blocks.at(key_string), false); } catch (const std::out_of_range &e) { return nullptr; } @@ -43,14 +40,20 @@ void FakeBlockStore::remove(unique_ptr block) { assert(numRemoved == 1); } -unique_ptr FakeBlockStore::makeFakeBlockFromData(const Key &key, const Data &data) { +unique_ptr FakeBlockStore::makeFakeBlockFromData(const Key &key, const Data &data, bool dirty) { auto newdata = make_shared(data.copy()); _used_dataregions_for_blocks.push_back(newdata); - return make_unique(this, key, newdata); + return make_unique(this, key, newdata, dirty); } void FakeBlockStore::updateData(const Key &key, const Data &data) { - Data &stored_data = _blocks.at(key.ToString()); + auto found = _blocks.find(key.ToString()); + if (found == _blocks.end()) { + auto insertResult = _blocks.emplace(key.ToString(), data.size()); + assert(true == insertResult.second); + found = insertResult.first; + } + Data &stored_data = found->second; assert(data.size() == stored_data.size()); std::memcpy(stored_data.data(), data.data(), data.size()); } diff --git a/implementations/testfake/FakeBlockStore.h b/implementations/testfake/FakeBlockStore.h index bef47ed9..670d047b 100644 --- a/implementations/testfake/FakeBlockStore.h +++ b/implementations/testfake/FakeBlockStore.h @@ -48,7 +48,7 @@ private: //We want to avoid this for the reasons mentioned above (overflow data). std::vector> _used_dataregions_for_blocks; - std::unique_ptr makeFakeBlockFromData(const Key &key, const Data &data); + std::unique_ptr makeFakeBlockFromData(const Key &key, const Data &data, bool dirty); DISALLOW_COPY_AND_ASSIGN(FakeBlockStore); }; diff --git a/interface/Block.h b/interface/Block.h index 031d7a67..e532db77 100644 --- a/interface/Block.h +++ b/interface/Block.h @@ -7,6 +7,9 @@ namespace blockstore { +//TODO Make Block non-virtual class that stores ptr to its blockstore and writes itself back to the blockstore who is offering a corresponding function. +// Then ondisk blockstore can be actually create the file on disk in blockstore::create() and cachingblockstore will delay that call to its base block store. + class Block { public: virtual ~Block() {} diff --git a/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp b/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp index 5ff681a6..e47c7279 100644 --- a/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp +++ b/test/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp @@ -36,6 +36,7 @@ TEST_F(OnDiskBlockCreateTest, CreatingBlockCreatesFile) { EXPECT_FALSE(bf::exists(file.path())); auto block = OnDiskBlock::CreateOnDisk(dir.path(), key, 0); + block->flush(); EXPECT_TRUE(bf::exists(file.path())); EXPECT_TRUE(bf::is_regular_file(file.path())); @@ -43,6 +44,7 @@ TEST_F(OnDiskBlockCreateTest, CreatingBlockCreatesFile) { TEST_F(OnDiskBlockCreateTest, CreatingExistingBlockReturnsNull) { auto block1 = OnDiskBlock::CreateOnDisk(dir.path(), key, 0); + block1->flush(); auto block2 = OnDiskBlock::CreateOnDisk(dir.path(), key, 0); EXPECT_TRUE((bool)block1); EXPECT_FALSE((bool)block2); @@ -57,6 +59,7 @@ public: block(OnDiskBlock::CreateOnDisk(dir.path(), key, GetParam())), ZEROES(block->size()) { + block->flush(); ZEROES.FillWithZeroes(); } }; diff --git a/test/testutils/BlockStoreWithRandomKeysTest.h b/test/testutils/BlockStoreWithRandomKeysTest.h index a69c8a74..1ed93e06 100644 --- a/test/testutils/BlockStoreWithRandomKeysTest.h +++ b/test/testutils/BlockStoreWithRandomKeysTest.h @@ -32,6 +32,7 @@ TYPED_TEST_CASE_P(BlockStoreWithRandomKeysTest); TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndSameSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->create(this->key, 1024); + block->flush(); auto block2 = blockStore->create(this->key, 1024); EXPECT_TRUE((bool)block); EXPECT_FALSE((bool)block2); @@ -40,6 +41,7 @@ TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndSameSize TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndDifferentSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->create(this->key, 1024); + block->flush(); auto block2 = blockStore->create(this->key, 4096); EXPECT_TRUE((bool)block); EXPECT_FALSE((bool)block2); @@ -48,6 +50,7 @@ TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndDifferen TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndFirstNullSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->create(this->key, 0); + block->flush(); auto block2 = blockStore->create(this->key, 1024); EXPECT_TRUE((bool)block); EXPECT_FALSE((bool)block2); @@ -56,6 +59,7 @@ TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndFirstNul TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndSecondNullSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->create(this->key, 1024); + block->flush(); auto block2 = blockStore->create(this->key, 0); EXPECT_TRUE((bool)block); EXPECT_FALSE((bool)block2); @@ -64,6 +68,7 @@ TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndSecondNu TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndBothNullSize) { auto blockStore = this->fixture.createBlockStore(); auto block = blockStore->create(this->key, 0); + block->flush(); auto block2 = blockStore->create(this->key, 0); EXPECT_TRUE((bool)block); EXPECT_FALSE((bool)block2);