From 9fdefaa4e09f40f66c03c40ea5ef6a607f328ed0 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 18 Sep 2017 20:42:07 +0100 Subject: [PATCH] Remove BlockStoreWithRandomKeys --- src/blockstore/CMakeLists.txt | 3 +- .../ParallelAccessBlockStore.cpp | 4 +- .../testfake/FakeBlockStore.cpp | 4 + .../implementations/testfake/FakeBlockStore.h | 5 +- src/blockstore/interface/BlockStore2.h | 6 +- .../helpers/BlockStoreWithRandomKeys.cpp | 1 - .../helpers/BlockStoreWithRandomKeys.h | 24 --- test/blockstore/CMakeLists.txt | 2 +- .../inmemory/InMemoryBlockStoreTest.cpp | 12 -- .../IntegrityBlockStoreTest_Generic.cpp | 3 - .../mock/MockBlockStoreTest.cpp | 1 - .../ondisk/OnDiskBlockStoreTest_Generic.cpp | 17 -- .../testfake/TestFakeBlockStoreTest.cpp | 11 -- test/blockstore/interface/BlockStore2Test.cpp | 141 +++++++++++++++ test/blockstore/interface/BlockStoreTest.cpp | 164 +++++++++++++++++- .../helpers/BlockStoreWithRandomKeysTest.cpp | 151 ---------------- test/blockstore/testutils/BlockStore2Test.h | 44 ++++- test/blockstore/testutils/BlockStoreTest.h | 60 ++++++- .../testutils/BlockStoreWithRandomKeysTest.h | 88 ---------- 19 files changed, 420 insertions(+), 321 deletions(-) delete mode 100644 src/blockstore/interface/helpers/BlockStoreWithRandomKeys.cpp delete mode 100644 src/blockstore/interface/helpers/BlockStoreWithRandomKeys.h create mode 100644 test/blockstore/interface/BlockStore2Test.cpp delete mode 100644 test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp delete mode 100644 test/blockstore/testutils/BlockStoreWithRandomKeysTest.h diff --git a/src/blockstore/CMakeLists.txt b/src/blockstore/CMakeLists.txt index 652b566f..e20c64b8 100644 --- a/src/blockstore/CMakeLists.txt +++ b/src/blockstore/CMakeLists.txt @@ -5,7 +5,6 @@ set(SOURCES utils/IdWrapper.cpp utils/BlockStoreUtils.cpp utils/FileDoesntExistException.cpp - interface/helpers/BlockStoreWithRandomKeys.cpp implementations/testfake/FakeBlockStore.cpp implementations/testfake/FakeBlock.cpp implementations/inmemory/InMemoryBlockStore2.cpp @@ -27,7 +26,7 @@ set(SOURCES implementations/low2highlevel/LowToHighLevelBlockStore.cpp implementations/integrity/IntegrityBlockStore2.cpp implementations/integrity/KnownBlockVersions.cpp - implementations/integrity/ClientIdAndBlockId.cpp + implementations/integrity/ClientIdAndBlockId.cpp implementations/integrity/IntegrityViolationError.cpp implementations/mock/MockBlockStore.cpp implementations/mock/MockBlock.cpp diff --git a/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp b/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp index 76019375..1005cf57 100644 --- a/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp +++ b/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp @@ -28,7 +28,9 @@ BlockId ParallelAccessBlockStore::createBlockId() { } optional> ParallelAccessBlockStore::tryCreate(const BlockId &blockId, Data data) { - ASSERT(!_parallelAccessStore.isOpened(blockId), ("BlockId "+blockId.ToString()+"already exists").c_str()); + if (_parallelAccessStore.isOpened(blockId)) { + return none; // block already exists + } auto block = _baseBlockStore->tryCreate(blockId, std::move(data)); if (block == none) { //TODO Test this code branch diff --git a/src/blockstore/implementations/testfake/FakeBlockStore.cpp b/src/blockstore/implementations/testfake/FakeBlockStore.cpp index 9cb9cb03..fbfec4cb 100644 --- a/src/blockstore/implementations/testfake/FakeBlockStore.cpp +++ b/src/blockstore/implementations/testfake/FakeBlockStore.cpp @@ -19,6 +19,10 @@ namespace testfake { FakeBlockStore::FakeBlockStore() : _blocks(), _used_dataregions_for_blocks(), _mutex() {} +BlockId FakeBlockStore::createBlockId() { + return BlockId::Random(); +} + optional> FakeBlockStore::tryCreate(const BlockId &blockId, Data data) { std::unique_lock lock(_mutex); auto insert_result = _blocks.emplace(blockId, std::move(data)); diff --git a/src/blockstore/implementations/testfake/FakeBlockStore.h b/src/blockstore/implementations/testfake/FakeBlockStore.h index d5dea019..f3eb837b 100644 --- a/src/blockstore/implementations/testfake/FakeBlockStore.h +++ b/src/blockstore/implementations/testfake/FakeBlockStore.h @@ -2,7 +2,7 @@ #ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_TESTFAKE_FAKEBLOCKSTORE_H_ #define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_TESTFAKE_FAKEBLOCKSTORE_H_ -#include "../../interface/helpers/BlockStoreWithRandomKeys.h" +#include "../../interface/BlockStore.h" #include #include @@ -27,10 +27,11 @@ class FakeBlock; * the data (instead of a direct pointer as InMemoryBlockStore does) and flushing will copy the data back to the * background. This way, tests are more likely to fail if they use the blockstore wrongly. */ -class FakeBlockStore final: public BlockStoreWithRandomKeys { +class FakeBlockStore final: public BlockStore { public: FakeBlockStore(); + BlockId createBlockId() override; boost::optional> tryCreate(const BlockId &blockId, cpputils::Data data) override; cpputils::unique_ref overwrite(const blockstore::BlockId &blockId, cpputils::Data data) override; boost::optional> load(const BlockId &blockId) override; diff --git a/src/blockstore/interface/BlockStore2.h b/src/blockstore/interface/BlockStore2.h index 48c54866..36032e68 100644 --- a/src/blockstore/interface/BlockStore2.h +++ b/src/blockstore/interface/BlockStore2.h @@ -15,6 +15,10 @@ class BlockStore2 { public: virtual ~BlockStore2() {} + virtual BlockId createBlockId() const { + return BlockId::Random(); + } + __attribute__((warn_unused_result)) virtual bool tryCreate(const BlockId &blockId, const cpputils::Data &data) = 0; __attribute__((warn_unused_result)) @@ -27,7 +31,7 @@ public: virtual void store(const BlockId &blockId, const cpputils::Data &data) = 0; BlockId create(const cpputils::Data& data) { - BlockId blockId = BlockId::Random(); + BlockId blockId = createBlockId(); bool success = tryCreate(blockId, data); if (success) { return blockId; diff --git a/src/blockstore/interface/helpers/BlockStoreWithRandomKeys.cpp b/src/blockstore/interface/helpers/BlockStoreWithRandomKeys.cpp deleted file mode 100644 index f9d7c120..00000000 --- a/src/blockstore/interface/helpers/BlockStoreWithRandomKeys.cpp +++ /dev/null @@ -1 +0,0 @@ -#include "BlockStoreWithRandomKeys.h" diff --git a/src/blockstore/interface/helpers/BlockStoreWithRandomKeys.h b/src/blockstore/interface/helpers/BlockStoreWithRandomKeys.h deleted file mode 100644 index e2ebb0ca..00000000 --- a/src/blockstore/interface/helpers/BlockStoreWithRandomKeys.h +++ /dev/null @@ -1,24 +0,0 @@ -#pragma once -#ifndef MESSMER_BLOCKSTORE_INTERFACE_HELPERS_BLOCKSTOREWITHRANDOMKEYS_H_ -#define MESSMER_BLOCKSTORE_INTERFACE_HELPERS_BLOCKSTOREWITHRANDOMKEYS_H_ - -#include "../BlockStore.h" -#include "../Block.h" -#include - -namespace blockstore { - -// This is an implementation helpers for BlockStores that use random block ids. -// You should never give this static type to the client. The client should always -// work with the BlockStore interface instead. -// TODO Delete this class -class BlockStoreWithRandomKeys: public BlockStore { -public: - BlockId createBlockId() final { - return BlockId::Random(); - } -}; - -} - -#endif diff --git a/test/blockstore/CMakeLists.txt b/test/blockstore/CMakeLists.txt index eef4f4f8..54167aef 100644 --- a/test/blockstore/CMakeLists.txt +++ b/test/blockstore/CMakeLists.txt @@ -2,8 +2,8 @@ project (blockstore-test) set(SOURCES utils/BlockStoreUtilsTest.cpp - interface/helpers/BlockStoreWithRandomKeysTest.cpp interface/BlockStoreTest.cpp + interface/BlockStore2Test.cpp interface/BlockTest.cpp implementations/testfake/TestFakeBlockStoreTest.cpp implementations/mock/MockBlockStoreTest.cpp diff --git a/test/blockstore/implementations/inmemory/InMemoryBlockStoreTest.cpp b/test/blockstore/implementations/inmemory/InMemoryBlockStoreTest.cpp index 85789440..0a636ba1 100644 --- a/test/blockstore/implementations/inmemory/InMemoryBlockStoreTest.cpp +++ b/test/blockstore/implementations/inmemory/InMemoryBlockStoreTest.cpp @@ -2,7 +2,6 @@ #include "blockstore/implementations/inmemory/InMemoryBlockStore2.h" #include "../../testutils/BlockStoreTest.h" #include "../../testutils/BlockStore2Test.h" -#include "../../testutils/BlockStoreWithRandomKeysTest.h" #include #include #include @@ -26,17 +25,6 @@ public: INSTANTIATE_TYPED_TEST_CASE_P(InMemory, BlockStoreTest, InMemoryBlockStoreTestFixture); -class InMemoryBlockStoreWithRandomKeysTestFixture: public BlockStoreWithRandomKeysTestFixture { -public: - unique_ref createBlockStore() override { - return make_unique_ref( - make_unique_ref() - ); - } -}; - -INSTANTIATE_TYPED_TEST_CASE_P(InMemory, BlockStoreWithRandomKeysTest, InMemoryBlockStoreWithRandomKeysTestFixture); - class InMemoryBlockStore2TestFixture: public BlockStore2TestFixture { public: unique_ref createBlockStore() override { diff --git a/test/blockstore/implementations/integrity/IntegrityBlockStoreTest_Generic.cpp b/test/blockstore/implementations/integrity/IntegrityBlockStoreTest_Generic.cpp index f88973b1..e5be4e33 100644 --- a/test/blockstore/implementations/integrity/IntegrityBlockStoreTest_Generic.cpp +++ b/test/blockstore/implementations/integrity/IntegrityBlockStoreTest_Generic.cpp @@ -39,9 +39,6 @@ using IntegrityBlockStoreTestFixture_singleclient = IntegrityBlockStoreTestFixtu using IntegrityBlockStoreTestFixture_multiclient_noIntegrityChecks = IntegrityBlockStoreTestFixture; using IntegrityBlockStoreTestFixture_singleclient_noIntegrityChecks = IntegrityBlockStoreTestFixture; - -// TODO Why is here no IntegrityBlockStoreWithRandomKeysTest? - INSTANTIATE_TYPED_TEST_CASE_P(Integrity_multiclient, BlockStoreTest, IntegrityBlockStoreTestFixture_multiclient); INSTANTIATE_TYPED_TEST_CASE_P(Integrity_singleclient, BlockStoreTest, IntegrityBlockStoreTestFixture_singleclient); INSTANTIATE_TYPED_TEST_CASE_P(Integrity_multiclient_noIntegrityChecks, BlockStoreTest, IntegrityBlockStoreTestFixture_multiclient_noIntegrityChecks); diff --git a/test/blockstore/implementations/mock/MockBlockStoreTest.cpp b/test/blockstore/implementations/mock/MockBlockStoreTest.cpp index 209a9e5d..33648382 100644 --- a/test/blockstore/implementations/mock/MockBlockStoreTest.cpp +++ b/test/blockstore/implementations/mock/MockBlockStoreTest.cpp @@ -5,7 +5,6 @@ #include using blockstore::BlockStore; -using blockstore::BlockStoreWithRandomKeys; using blockstore::mock::MockBlockStore; using blockstore::testfake::FakeBlockStore; using cpputils::unique_ref; diff --git a/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Generic.cpp b/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Generic.cpp index 9c58f9e0..476deb91 100644 --- a/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Generic.cpp +++ b/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Generic.cpp @@ -2,7 +2,6 @@ #include "blockstore/implementations/ondisk/OnDiskBlockStore2.h" #include "../../testutils/BlockStoreTest.h" #include "../../testutils/BlockStore2Test.h" -#include "../../testutils/BlockStoreWithRandomKeysTest.h" #include #include @@ -32,22 +31,6 @@ private: INSTANTIATE_TYPED_TEST_CASE_P(OnDisk, BlockStoreTest, OnDiskBlockStoreTestFixture); -// TODO Add BlockStoreWithRandomKeysTest to BlockStoreTest and BlockStore2Test -class OnDiskBlockStoreWithRandomKeysTestFixture: public BlockStoreWithRandomKeysTestFixture { -public: - OnDiskBlockStoreWithRandomKeysTestFixture(): tempdir() {} - - unique_ref createBlockStore() override { - return make_unique_ref( - make_unique_ref(tempdir.path()) - ); - } -private: - TempDir tempdir; -}; - -INSTANTIATE_TYPED_TEST_CASE_P(OnDisk, BlockStoreWithRandomKeysTest, OnDiskBlockStoreWithRandomKeysTestFixture); - class OnDiskBlockStore2TestFixture: public BlockStore2TestFixture { public: OnDiskBlockStore2TestFixture(): tempdir() {} diff --git a/test/blockstore/implementations/testfake/TestFakeBlockStoreTest.cpp b/test/blockstore/implementations/testfake/TestFakeBlockStoreTest.cpp index 5f4a9a5a..5c20f639 100644 --- a/test/blockstore/implementations/testfake/TestFakeBlockStoreTest.cpp +++ b/test/blockstore/implementations/testfake/TestFakeBlockStoreTest.cpp @@ -1,12 +1,10 @@ #include "blockstore/implementations/testfake/FakeBlock.h" #include "blockstore/implementations/testfake/FakeBlockStore.h" #include "../../testutils/BlockStoreTest.h" -#include "../../testutils/BlockStoreWithRandomKeysTest.h" #include #include using blockstore::BlockStore; -using blockstore::BlockStoreWithRandomKeys; using blockstore::testfake::FakeBlockStore; using cpputils::unique_ref; using cpputils::make_unique_ref; @@ -19,12 +17,3 @@ public: }; INSTANTIATE_TYPED_TEST_CASE_P(TestFake, BlockStoreTest, FakeBlockStoreTestFixture); - -class FakeBlockStoreWithRandomKeysTestFixture: public BlockStoreWithRandomKeysTestFixture { -public: - unique_ref createBlockStore() override { - return make_unique_ref(); - } -}; - -INSTANTIATE_TYPED_TEST_CASE_P(TestFake, BlockStoreWithRandomKeysTest, FakeBlockStoreWithRandomKeysTestFixture); diff --git a/test/blockstore/interface/BlockStore2Test.cpp b/test/blockstore/interface/BlockStore2Test.cpp new file mode 100644 index 00000000..9d8bf171 --- /dev/null +++ b/test/blockstore/interface/BlockStore2Test.cpp @@ -0,0 +1,141 @@ +#include "blockstore/interface/BlockStore2.h" +#include +#include +#include + +using ::testing::Test; +using ::testing::_; +using ::testing::Return; +using ::testing::Invoke; +using ::testing::Eq; +using ::testing::ByRef; + +using std::string; +using cpputils::Data; +using cpputils::DataFixture; +using cpputils::unique_ref; +using boost::optional; + +using namespace blockstore; + +class BlockStore2Mock: public BlockStore2 { +public: + MOCK_CONST_METHOD0(createBlockId, BlockId()); + MOCK_METHOD2(tryCreate, bool(const BlockId &blockId, const cpputils::Data &data)); + MOCK_METHOD2(store, void(const BlockId &, const Data &data)); + MOCK_CONST_METHOD1(load, optional(const BlockId &)); + MOCK_METHOD1(remove, bool(const BlockId &)); + MOCK_CONST_METHOD0(numBlocks, uint64_t()); + MOCK_CONST_METHOD0(estimateNumFreeBytes, uint64_t()); + MOCK_CONST_METHOD1(blockSizeFromPhysicalBlockSize, uint64_t(uint64_t)); + MOCK_CONST_METHOD1(forEachBlock, void(std::function)); +}; + +class BlockStore2Test: public Test { +public: + BlockStore2Test() :blockStoreMock(), blockStore(blockStoreMock), + blockId1(BlockId::FromString("1491BB4932A389EE14BC7090AC772972")), + blockId2(BlockId::FromString("AC772971491BB4932A389EE14BC7090A")), + blockId3(BlockId::FromString("1BB4932A38AC77C7090A2971499EE14B")) {} + + BlockStore2Mock blockStoreMock; + BlockStore2 &blockStore; + const BlockId blockId1; + const BlockId blockId2; + const BlockId blockId3; + + Data createDataWithSize(size_t size) { + Data fixture(DataFixture::generate(size)); + Data data(size); + std::memcpy(data.data(), fixture.data(), size); + return data; + } +}; + +TEST_F(BlockStore2Test, DataIsPassedThrough0) { + Data data = createDataWithSize(0); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, tryCreate(_, Eq(ByRef(data)))).WillOnce(Return(true)); + EXPECT_EQ(blockId1, blockStore.create(data)); +} + +TEST_F(BlockStore2Test, DataIsPassedThrough1) { + Data data = createDataWithSize(1); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, tryCreate(_, Eq(ByRef(data)))).WillOnce(Return(true)); + EXPECT_EQ(blockId1, blockStore.create(data)); +} + +TEST_F(BlockStore2Test, DataIsPassedThrough1024) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, tryCreate(_, Eq(ByRef(data)))).WillOnce(Return(true)); + EXPECT_EQ(blockId1, blockStore.create(data)); +} + +TEST_F(BlockStore2Test, BlockIdIsCorrect) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, tryCreate(blockId1, _)).WillOnce(Return(true)); + EXPECT_EQ(blockId1, blockStore.create(data)); +} + +TEST_F(BlockStore2Test, TwoBlocksGetDifferentIds) { + EXPECT_CALL(blockStoreMock, createBlockId()) + .WillOnce(Return(blockId1)) + .WillOnce(Return(blockId2)); + EXPECT_CALL(blockStoreMock, tryCreate(_, _)) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId1, blockId); + return true; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId2, blockId); + return true; + })); + + Data data = createDataWithSize(1024); + EXPECT_EQ(blockId1, blockStore.create(data)); + EXPECT_EQ(blockId2, blockStore.create(data)); +} + +TEST_F(BlockStore2Test, WillTryADifferentIdIfKeyAlreadyExists) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()) + .WillOnce(Return(blockId1)) + .WillOnce(Return(blockId2)); + EXPECT_CALL(blockStoreMock, tryCreate(_, Eq(ByRef(data)))) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId1, blockId); + return false; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId2, blockId); + return true; + })); + + EXPECT_EQ(blockId2, blockStore.create(data)); +} + +TEST_F(BlockStore2Test, WillTryADifferentIdIfIdAlreadyExistsTwoTimes) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()) + .WillOnce(Return(blockId1)) + .WillOnce(Return(blockId2)) + .WillOnce(Return(blockId3)); + EXPECT_CALL(blockStoreMock, tryCreate(_, Eq(ByRef(data)))) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId1, blockId); + return false; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId2, blockId); + return false; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId3, blockId); + return true; + })); + + EXPECT_EQ(blockId3, blockStore.create(data)); +} diff --git a/test/blockstore/interface/BlockStoreTest.cpp b/test/blockstore/interface/BlockStoreTest.cpp index 6b2b29c9..97dad4f2 100644 --- a/test/blockstore/interface/BlockStoreTest.cpp +++ b/test/blockstore/interface/BlockStoreTest.cpp @@ -1,4 +1,162 @@ -/* - * Tests that the header can be included without needing additional header includes as dependencies. - */ #include "blockstore/interface/BlockStore.h" +#include +#include +#include + +using ::testing::Test; +using ::testing::_; +using ::testing::Return; +using ::testing::Invoke; +using ::testing::Eq; +using ::testing::ByRef; + +using std::string; +using cpputils::Data; +using cpputils::DataFixture; +using cpputils::unique_ref; +using boost::optional; + +using namespace blockstore; + +class BlockStoreMock: public BlockStore { +public: + MOCK_METHOD0(createBlockId, BlockId()); + optional> tryCreate(const BlockId &blockId, Data data) { + return cpputils::nullcheck(std::unique_ptr(do_create(blockId, data))); + } + MOCK_METHOD2(do_create, Block*(const BlockId &, const Data &data)); + unique_ref overwrite(const BlockId &blockId, Data data) { + return cpputils::nullcheck(std::unique_ptr(do_overwrite(blockId, data))).value(); + } + MOCK_METHOD2(do_overwrite, Block*(const BlockId &, const Data &data)); + optional> load(const BlockId &blockId) { + return cpputils::nullcheck(std::unique_ptr(do_load(blockId))); + } + MOCK_METHOD1(do_load, Block*(const BlockId &)); + void remove(unique_ref block) {UNUSED(block);} + MOCK_METHOD1(remove, void(const BlockId &)); + MOCK_CONST_METHOD0(numBlocks, uint64_t()); + MOCK_CONST_METHOD0(estimateNumFreeBytes, uint64_t()); + MOCK_CONST_METHOD1(blockSizeFromPhysicalBlockSize, uint64_t(uint64_t)); + MOCK_CONST_METHOD1(forEachBlock, void(std::function)); +}; + +class BlockMock: public Block { +public: + BlockMock(): Block(BlockId::Random()) {} + MOCK_CONST_METHOD0(data, const void*()); + MOCK_METHOD3(write, void(const void*, uint64_t, uint64_t)); + MOCK_METHOD0(flush, void()); + MOCK_CONST_METHOD0(size, size_t()); + MOCK_METHOD1(resize, void(size_t)); + MOCK_CONST_METHOD0(blockId, const BlockId&()); +}; + +class BlockStoreTest: public Test { +public: + BlockStoreTest() :blockStoreMock(), blockStore(blockStoreMock), + blockId1(BlockId::FromString("1491BB4932A389EE14BC7090AC772972")), + blockId2(BlockId::FromString("AC772971491BB4932A389EE14BC7090A")), + blockId3(BlockId::FromString("1BB4932A38AC77C7090A2971499EE14B")) {} + + BlockStoreMock blockStoreMock; + BlockStore &blockStore; + const BlockId blockId1; + const BlockId blockId2; + const BlockId blockId3; + + Data createDataWithSize(size_t size) { + Data fixture(DataFixture::generate(size)); + Data data(size); + std::memcpy(data.data(), fixture.data(), size); + return data; + } +}; + +TEST_F(BlockStoreTest, DataIsPassedThrough0) { + Data data = createDataWithSize(0); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))).WillOnce(Return(new BlockMock)); + blockStore.create(data); +} + +TEST_F(BlockStoreTest, DataIsPassedThrough1) { + Data data = createDataWithSize(1); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))).WillOnce(Return(new BlockMock)); + blockStore.create(data); +} + +TEST_F(BlockStoreTest, DataIsPassedThrough1024) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))).WillOnce(Return(new BlockMock)); + blockStore.create(data); +} + +TEST_F(BlockStoreTest, BlockIdIsCorrect) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()).WillOnce(Return(blockId1)); + EXPECT_CALL(blockStoreMock, do_create(blockId1, _)).WillOnce(Return(new BlockMock)); + blockStore.create(data); +} + +TEST_F(BlockStoreTest, TwoBlocksGetDifferentIds) { + EXPECT_CALL(blockStoreMock, createBlockId()) + .WillOnce(Return(blockId1)) + .WillOnce(Return(blockId2)); + EXPECT_CALL(blockStoreMock, do_create(_, _)) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId1, blockId); + return new BlockMock; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId2, blockId); + return new BlockMock; + })); + + Data data = createDataWithSize(1024); + blockStore.create(data); + blockStore.create(data); +} + +TEST_F(BlockStoreTest, WillTryADifferentIdIfKeyAlreadyExists) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()) + .WillOnce(Return(blockId1)) + .WillOnce(Return(blockId2)); + EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId1, blockId); + return nullptr; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId2, blockId); + return new BlockMock; + })); + + blockStore.create(data); +} + +TEST_F(BlockStoreTest, WillTryADifferentIdIfIdAlreadyExistsTwoTimes) { + Data data = createDataWithSize(1024); + EXPECT_CALL(blockStoreMock, createBlockId()) + .WillOnce(Return(blockId1)) + .WillOnce(Return(blockId2)) + .WillOnce(Return(blockId3)); + EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId1, blockId); + return nullptr; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId2, blockId); + return nullptr; + })) + .WillOnce(Invoke([this](const BlockId &blockId, const Data &) { + EXPECT_EQ(blockId3, blockId); + return new BlockMock; + })); + + blockStore.create(data); +} diff --git a/test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp b/test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp deleted file mode 100644 index 33b77e98..00000000 --- a/test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp +++ /dev/null @@ -1,151 +0,0 @@ -#include "blockstore/interface/helpers/BlockStoreWithRandomKeys.h" -#include -#include -#include - -using ::testing::Test; -using ::testing::_; -using ::testing::Return; -using ::testing::Invoke; -using ::testing::Eq; -using ::testing::ByRef; - -using std::string; -using cpputils::Data; -using cpputils::DataFixture; -using cpputils::unique_ref; -using boost::optional; - -using namespace blockstore; - -class BlockStoreWithRandomKeysMock: public BlockStoreWithRandomKeys { -public: - optional> tryCreate(const BlockId &blockId, Data data) { - return cpputils::nullcheck(std::unique_ptr(do_create(blockId, data))); - } - MOCK_METHOD2(do_create, Block*(const BlockId &, const Data &data)); - unique_ref overwrite(const BlockId &blockId, Data data) { - return cpputils::nullcheck(std::unique_ptr(do_overwrite(blockId, data))).value(); - } - MOCK_METHOD2(do_overwrite, Block*(const BlockId &, const Data &data)); - optional> load(const BlockId &blockId) { - return cpputils::nullcheck(std::unique_ptr(do_load(blockId))); - } - MOCK_METHOD1(do_load, Block*(const BlockId &)); - void remove(unique_ref block) {UNUSED(block);} - MOCK_METHOD1(remove, void(const BlockId &)); - MOCK_CONST_METHOD0(numBlocks, uint64_t()); - MOCK_CONST_METHOD0(estimateNumFreeBytes, uint64_t()); - MOCK_CONST_METHOD1(blockSizeFromPhysicalBlockSize, uint64_t(uint64_t)); - MOCK_CONST_METHOD1(forEachBlock, void(std::function)); -}; - -class BlockMock: public Block { -public: - BlockMock(): Block(BlockId::Random()) {} - MOCK_CONST_METHOD0(data, const void*()); - MOCK_METHOD3(write, void(const void*, uint64_t, uint64_t)); - MOCK_METHOD0(flush, void()); - MOCK_CONST_METHOD0(size, size_t()); - MOCK_METHOD1(resize, void(size_t)); - MOCK_CONST_METHOD0(blockId, const BlockId&()); -}; - -class BlockStoreWithRandomKeysTest: public Test { -public: - BlockStoreWithRandomKeysTest() :blockStoreMock(), blockStore(blockStoreMock), - blockId(BlockId::FromString("1491BB4932A389EE14BC7090AC772972")) {} - - BlockStoreWithRandomKeysMock blockStoreMock; - BlockStore &blockStore; - const blockstore::BlockId blockId; - - Data createDataWithSize(size_t size) { - Data fixture(DataFixture::generate(size)); - Data data(size); - std::memcpy(data.data(), fixture.data(), size); - return data; - } -}; - -TEST_F(BlockStoreWithRandomKeysTest, DataIsPassedThrough0) { - Data data = createDataWithSize(0); - EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))).WillOnce(Return(new BlockMock)); - blockStore.create(data); -} - -TEST_F(BlockStoreWithRandomKeysTest, DataIsPassedThrough1) { - Data data = createDataWithSize(1); - EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))).WillOnce(Return(new BlockMock)); - blockStore.create(data); -} - -TEST_F(BlockStoreWithRandomKeysTest, DataIsPassedThrough1024) { - Data data = createDataWithSize(1024); - EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))).WillOnce(Return(new BlockMock)); - blockStore.create(data); -} - -TEST_F(BlockStoreWithRandomKeysTest, KeyHasCorrectSize) { - EXPECT_CALL(blockStoreMock, do_create(_, _)).WillOnce(Invoke([](const BlockId &blockId, const Data &) { - EXPECT_EQ(BlockId::STRING_LENGTH, blockId.ToString().size()); - return new BlockMock; - })); - - blockStore.create(createDataWithSize(1024)); -} - -TEST_F(BlockStoreWithRandomKeysTest, TwoBlocksGetDifferentKeys) { - BlockId first_blockId = blockId; - EXPECT_CALL(blockStoreMock, do_create(_, _)) - .WillOnce(Invoke([&first_blockId](const BlockId &blockId, const Data &) { - first_blockId = blockId; - return new BlockMock; - })) - .WillOnce(Invoke([&first_blockId](const BlockId &blockId, const Data &) { - EXPECT_NE(first_blockId, blockId); - return new BlockMock; - })); - - Data data = createDataWithSize(1024); - blockStore.create(data); - blockStore.create(data); -} - -TEST_F(BlockStoreWithRandomKeysTest, WillTryADifferentKeyIfKeyAlreadyExists) { - BlockId first_blockId = blockId; - Data data = createDataWithSize(1024); - EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))) - .WillOnce(Invoke([&first_blockId](const BlockId &blockId, const Data &) { - first_blockId = blockId; - return nullptr; - })) - //TODO Check that this test case fails when the second do_create call gets different data - .WillOnce(Invoke([&first_blockId](const BlockId &blockId, const Data &) { - EXPECT_NE(first_blockId, blockId); - return new BlockMock; - })); - - blockStore.create(data); -} - -TEST_F(BlockStoreWithRandomKeysTest, WillTryADifferentKeyIfKeyAlreadyExistsTwoTimes) { - BlockId first_blockId = blockId; - Data data = createDataWithSize(1024); - EXPECT_CALL(blockStoreMock, do_create(_, Eq(ByRef(data)))) - .WillOnce(Invoke([&first_blockId](const BlockId &blockId, const Data &) { - first_blockId = blockId; - return nullptr; - })) - //TODO Check that this test case fails when the second/third do_create calls get different data - .WillOnce(Invoke([&first_blockId](const BlockId &blockId, const Data &) { - first_blockId = blockId; - return nullptr; - })) - .WillOnce(Invoke([&first_blockId](const BlockId &blockId, const Data &) { - EXPECT_NE(first_blockId, blockId); - return new BlockMock; - })); - - blockStore.create(data); -} diff --git a/test/blockstore/testutils/BlockStore2Test.h b/test/blockstore/testutils/BlockStore2Test.h index a0aad969..7f3ee0df 100644 --- a/test/blockstore/testutils/BlockStore2Test.h +++ b/test/blockstore/testutils/BlockStore2Test.h @@ -383,6 +383,41 @@ TYPED_TEST_P(BlockStore2Test, ForEachBlock_doesntListRemovedBlocks_twoblocks) { this->EXPECT_UNORDERED_EQ({blockId2}, mockForEachBlockCallback.called_with); } +TYPED_TEST_P(BlockStore2Test, TryCreateTwoBlocksWithSameBlockIdAndSameSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + EXPECT_TRUE(blockStore->tryCreate(blockId, cpputils::Data(1024))); + EXPECT_FALSE(blockStore->tryCreate(blockId, cpputils::Data(1024))); +} + +TYPED_TEST_P(BlockStore2Test, TryCreateTwoBlocksWithSameBlockIdAndDifferentSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + EXPECT_TRUE(blockStore->tryCreate(blockId, cpputils::Data(1024))); + EXPECT_FALSE(blockStore->tryCreate(blockId, cpputils::Data(4096))); +} + +TYPED_TEST_P(BlockStore2Test, TryCreateTwoBlocksWithSameBlockIdAndFirstNullSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + EXPECT_TRUE(blockStore->tryCreate(blockId, cpputils::Data(0))); + EXPECT_FALSE(blockStore->tryCreate(blockId, cpputils::Data(1024))); +} +/* +TYPED_TEST_P(BlockStore2Test, TryCreateTwoBlocksWithSameBlockIdAndSecondNullSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + EXPECT_TRUE(blockStore->tryCreate(blockId, cpputils::Data(1024))); + EXPECT_FALSE(blockStore->tryCreate(blockId, cpputils::Data(0))); +} + +TYPED_TEST_P(BlockStore2Test, TryCreateTwoBlocksWithSameBlockIdAndBothNullSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + EXPECT_TRUE(blockStore->tryCreate(blockId, cpputils::Data(0))); + EXPECT_FALSE(blockStore->tryCreate(blockId, cpputils::Data(0))); +}*/ + REGISTER_TYPED_TEST_CASE_P(BlockStore2Test, givenNonEmptyBlockStore_whenCallingTryCreateOnExistingBlock_thenFails, givenEmptyBlockStore_whenCallingTryCreateOnNonExistingBlock_thenSucceeds, @@ -430,7 +465,14 @@ REGISTER_TYPED_TEST_CASE_P(BlockStore2Test, ForEachBlock_twoblocks, ForEachBlock_threeblocks, ForEachBlock_doesntListRemovedBlocks_oneblock, - ForEachBlock_doesntListRemovedBlocks_twoblocks + ForEachBlock_doesntListRemovedBlocks_twoblocks, + TryCreateTwoBlocksWithSameBlockIdAndSameSize, + TryCreateTwoBlocksWithSameBlockIdAndDifferentSize, + TryCreateTwoBlocksWithSameBlockIdAndFirstNullSize + //TODO Just disabled because gtest doesn't allow more template parameters. Fix and reenable! + // see https://github.com/google/googletest/issues/1267 + //TryCreateTwoBlocksWithSameBlockIdAndSecondNullSize, + //TryCreateTwoBlocksWithSameBlockIdAndBothNullSize ); diff --git a/test/blockstore/testutils/BlockStoreTest.h b/test/blockstore/testutils/BlockStoreTest.h index 1d54a42c..4dcd8276 100644 --- a/test/blockstore/testutils/BlockStoreTest.h +++ b/test/blockstore/testutils/BlockStoreTest.h @@ -286,6 +286,56 @@ TYPED_TEST_P(BlockStoreTest, Resize_Smaller_ToZero_BlockIsStillUsable) { this->TestBlockIsUsable(std::move(block), blockStore.get()); } +TYPED_TEST_P(BlockStoreTest, TryCreateTwoBlocksWithSameBlockIdAndSameSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + auto block = blockStore->tryCreate(blockId, cpputils::Data(1024)); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. + auto block2 = blockStore->tryCreate(blockId, cpputils::Data(1024)); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); +} +/* +TYPED_TEST_P(BlockStoreTest, TryCreateTwoBlocksWithSameBlockIdAndDifferentSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + auto block = blockStore->tryCreate(blockId, cpputils::Data(1024)); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. + auto block2 = blockStore->tryCreate(blockId, cpputils::Data(4096)); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); +} + +TYPED_TEST_P(BlockStoreTest, TryCreateTwoBlocksWithSameBlockIdAndFirstNullSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + auto block = blockStore->tryCreate(blockId, cpputils::Data(0)); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. + auto block2 = blockStore->tryCreate(blockId, cpputils::Data(1024)); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); +} + +TYPED_TEST_P(BlockStoreTest, TryCreateTwoBlocksWithSameBlockIdAndSecondNullSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + auto block = blockStore->tryCreate(blockId, cpputils::Data(1024)); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. + auto block2 = blockStore->tryCreate(blockId, cpputils::Data(0)); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); +} + +TYPED_TEST_P(BlockStoreTest, TryCreateTwoBlocksWithSameBlockIdAndBothNullSize) { + auto blockStore = this->fixture.createBlockStore(); + blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); + auto block = blockStore->tryCreate(blockId, cpputils::Data(0)); + (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. + auto block2 = blockStore->tryCreate(blockId, cpputils::Data(0)); + EXPECT_NE(boost::none, block); + EXPECT_EQ(boost::none, block2); +}*/ + #include "BlockStoreTest_Size.h" #include "BlockStoreTest_Data.h" @@ -333,7 +383,6 @@ REGISTER_TYPED_TEST_CASE_P(BlockStoreTest, ForEachBlock_twoblocks, ForEachBlock_threeblocks, ForEachBlock_doesntListRemovedBlocks_oneblock, - ForEachBlock_doesntListRemovedBlocks_twoblocks, Resize_Larger_FromZero, Resize_Larger_FromZero_BlockIsStillUsable, Resize_Larger, @@ -341,7 +390,14 @@ REGISTER_TYPED_TEST_CASE_P(BlockStoreTest, Resize_Smaller, Resize_Smaller_BlockIsStillUsable, Resize_Smaller_ToZero, - Resize_Smaller_ToZero_BlockIsStillUsable + Resize_Smaller_ToZero_BlockIsStillUsable, + TryCreateTwoBlocksWithSameBlockIdAndSameSize + //TODO Just disabled because gtest doesn't allow more template parameters. Fix and reenable! + // see https://github.com/google/googletest/issues/1267 + //TryCreateTwoBlocksWithSameBlockIdAndDifferentSize, + //TryCreateTwoBlocksWithSameBlockIdAndFirstNullSize, + //TryCreateTwoBlocksWithSameBlockIdAndSecondNullSize, + //TryCreateTwoBlocksWithSameBlockIdAndBothNullSize, ); diff --git a/test/blockstore/testutils/BlockStoreWithRandomKeysTest.h b/test/blockstore/testutils/BlockStoreWithRandomKeysTest.h deleted file mode 100644 index f08fcad3..00000000 --- a/test/blockstore/testutils/BlockStoreWithRandomKeysTest.h +++ /dev/null @@ -1,88 +0,0 @@ -#pragma once -#ifndef MESSMER_BLOCKSTORE_TEST_TESTUTILS_BLOCKSTOREWITHRANDOMKEYSTEST_H_ -#define MESSMER_BLOCKSTORE_TEST_TESTUTILS_BLOCKSTOREWITHRANDOMKEYSTEST_H_ - -#include - -#include "blockstore/interface/BlockStore.h" - -class BlockStoreWithRandomKeysTestFixture { -public: - virtual ~BlockStoreWithRandomKeysTestFixture() {} - virtual cpputils::unique_ref createBlockStore() = 0; -}; - -template -class BlockStoreWithRandomKeysTest: public ::testing::Test { -public: - BlockStoreWithRandomKeysTest(): fixture() {} - - BOOST_STATIC_ASSERT_MSG( - (std::is_base_of::value), - "Given test fixture for instantiating the (type parameterized) BlockStoreWithRandomKeysTest must inherit from BlockStoreWithRandomKeysTestFixture" - ); - - blockstore::BlockId blockId = blockstore::BlockId::FromString("1491BB4932A389EE14BC7090AC772972"); - - const std::vector SIZES = {0, 1, 1024, 4096, 10*1024*1024}; - - ConcreteBlockStoreWithRandomKeysTestFixture fixture; -}; - -TYPED_TEST_CASE_P(BlockStoreWithRandomKeysTest); - -TYPED_TEST_P(BlockStoreWithRandomKeysTest, CreateTwoBlocksWithSameKeyAndSameSize) { - auto blockStore = this->fixture.createBlockStore(); - auto block = blockStore->tryCreate(this->blockId, cpputils::Data(1024)); - (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. - auto block2 = blockStore->tryCreate(this->blockId, cpputils::Data(1024)); - 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->blockId, cpputils::Data(1024)); - (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. - auto block2 = blockStore->tryCreate(this->blockId, cpputils::Data(4096)); - 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->blockId, cpputils::Data(0)); - (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. - auto block2 = blockStore->tryCreate(this->blockId, cpputils::Data(1024)); - 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->blockId, cpputils::Data(1024)); - (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. - auto block2 = blockStore->tryCreate(this->blockId, cpputils::Data(0)); - 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->blockId, cpputils::Data(0)); - (*block)->flush(); //TODO Ideally, flush shouldn't be necessary here. - auto block2 = blockStore->tryCreate(this->blockId, cpputils::Data(0)); - EXPECT_NE(boost::none, block); - EXPECT_EQ(boost::none, block2); -} - -REGISTER_TYPED_TEST_CASE_P(BlockStoreWithRandomKeysTest, - CreateTwoBlocksWithSameKeyAndSameSize, - CreateTwoBlocksWithSameKeyAndDifferentSize, - CreateTwoBlocksWithSameKeyAndFirstNullSize, - CreateTwoBlocksWithSameKeyAndSecondNullSize, - CreateTwoBlocksWithSameKeyAndBothNullSize -); - - -#endif