From c2a814e1b96f4e84cd9288207242ec07995030c8 Mon Sep 17 00:00:00 2001 From: Mikhail Boldyrev Date: Fri, 28 Jun 2019 10:08:55 +0300 Subject: [PATCH] BlockQuery::GetBlockError Signed-off-by: Mikhail Boldyrev --- irohad/ametsuchi/block_query.hpp | 11 ++++++- .../ametsuchi/impl/postgres_block_query.cpp | 10 ++++-- irohad/ametsuchi/impl/storage_impl.cpp | 19 +++++++---- irohad/ametsuchi/impl/wsv_restorer_impl.cpp | 24 +++++++------- irohad/main/application.cpp | 12 +++---- irohad/network/impl/block_loader_service.cpp | 31 +++++++++++++----- .../irohad/ametsuchi/block_query_test.cpp | 32 ++++++++++++------- .../irohad/network/block_loader_test.cpp | 4 ++- 8 files changed, 95 insertions(+), 48 deletions(-) diff --git a/irohad/ametsuchi/block_query.hpp b/irohad/ametsuchi/block_query.hpp index 137ca9490d4..20d93c4f701 100644 --- a/irohad/ametsuchi/block_query.hpp +++ b/irohad/ametsuchi/block_query.hpp @@ -19,9 +19,18 @@ namespace iroha { */ class BlockQuery { public: + struct GetBlockError { + enum class Code { + kNoBlock, + kInternalError, + }; + Code code; + std::string message; + }; + using BlockResult = expected::Result, - std::string>; + GetBlockError>; virtual ~BlockQuery() = default; diff --git a/irohad/ametsuchi/impl/postgres_block_query.cpp b/irohad/ametsuchi/impl/postgres_block_query.cpp index 72bb305f39c..31f649bdc34 100644 --- a/irohad/ametsuchi/impl/postgres_block_query.cpp +++ b/irohad/ametsuchi/impl/postgres_block_query.cpp @@ -41,9 +41,15 @@ namespace iroha { if (not serialized_block) { auto error = boost::format("Failed to retrieve block with height %d") % height; - return expected::makeError(error.str()); + return expected::makeError( + GetBlockError{GetBlockError::Code::kNoBlock, error.str()}); } - return converter_->deserialize(bytesToString(*serialized_block)); + return converter_->deserialize(bytesToString(*serialized_block)) + .match([](auto &&val) -> BlockResult { return std::move(val.value); }, + [](auto &&err) -> BlockResult { + return GetBlockError{GetBlockError::Code::kInternalError, + std::move(err.error)}; + }); } shared_model::interface::types::HeightType diff --git a/irohad/ametsuchi/impl/storage_impl.cpp b/irohad/ametsuchi/impl/storage_impl.cpp index ba8aa777ad1..de1cfe0d64f 100644 --- a/irohad/ametsuchi/impl/storage_impl.cpp +++ b/irohad/ametsuchi/impl/storage_impl.cpp @@ -285,19 +285,24 @@ namespace iroha { auto opt_ledger_state = [&] { soci::session sql{*pool_wrapper.connection_pool_}; - auto get_top_block_info = - [&]() -> expected::Result { + using BlockInfoResult = + expected::Result; + auto get_top_block_info = [&]() -> BlockInfoResult { PostgresBlockQuery block_query( sql, *ctx.block_store, converter, log_manager->getChild("PostgresBlockQuery")->getLogger()); const auto ledger_height = block_query.getTopBlockHeight(); - return block_query.getBlock(ledger_height) | - [&ledger_height](const auto &block) { - return expected::makeValue( - iroha::TopBlockInfo{ledger_height, block->hash()}); - }; + return block_query.getBlock(ledger_height) + .match( + [&ledger_height](const auto &block) -> BlockInfoResult { + return expected::makeValue(iroha::TopBlockInfo{ + ledger_height, block.value->hash()}); + }, + [](auto &&err) -> BlockInfoResult { + return std::move(err).error.message; + }); }; auto get_ledger_peers = diff --git a/irohad/ametsuchi/impl/wsv_restorer_impl.cpp b/irohad/ametsuchi/impl/wsv_restorer_impl.cpp index 69195d5b0b5..478c69a4290 100644 --- a/irohad/ametsuchi/impl/wsv_restorer_impl.cpp +++ b/irohad/ametsuchi/impl/wsv_restorer_impl.cpp @@ -74,18 +74,20 @@ namespace { // apply all blocks starting from the genesis auto top_height = block_query->getTopBlockHeight(); for (decltype(top_height) i = 1; i <= top_height; ++i) { - auto block_result = block_query->getBlock(i); - auto result = std::move(block_result) | [&mutable_storage](auto &&block) - -> iroha::expected::Result { - if (not mutable_storage->apply(std::move(block))) { - return iroha::expected::makeError("Cannot apply " - + block->toString()); - } - return iroha::expected::Value(); - }; + auto result = block_query->getBlock(i).match( + [&mutable_storage]( + auto &&block) -> iroha::expected::Result { + if (not mutable_storage->apply(std::move(block).value)) { + return iroha::expected::makeError("Cannot apply block!"); + } + return iroha::expected::Value(); + }, + [](auto &&err) -> iroha::expected::Result { + return std::move(err).error.message; + }); - if (auto e = boost::get>(&result)) { - return *e; + if (auto e = iroha::expected::resultToOptionalError(result)) { + return std::move(e).value(); } } diff --git a/irohad/main/application.cpp b/irohad/main/application.cpp index a7b017d9b2d..42137946893 100644 --- a/irohad/main/application.cpp +++ b/irohad/main/application.cpp @@ -423,8 +423,8 @@ Irohad::RunResult Irohad::initOrderingGate() { ++i) { auto block_result = (*block_query)->getBlock(i); - if (auto e = boost::get>(&block_result)) { - return iroha::expected::makeError(std::move(e->error)); + if (auto e = expected::resultToOptionalError(block_result)) { + return iroha::expected::makeError(std::move(e->message)); } auto &block = @@ -545,9 +545,9 @@ Irohad::RunResult Irohad::initConsensusGate() { } auto block_var = (*block_query)->getBlock((*block_query)->getTopBlockHeight()); - if (auto e = boost::get>(&block_var)) { + if (auto e = expected::resultToOptionalError(block_var)) { return iroha::expected::makeError( - "Failed to get the top block: " + e->error); + "Failed to get the top block: " + e->message); } auto &block = @@ -807,8 +807,8 @@ Irohad::RunResult Irohad::run() { } auto block_var = (*block_query)->getBlock((*block_query)->getTopBlockHeight()); - if (auto e = boost::get>(&block_var)) { - return expected::makeError("Failed to get the top block: " + e->error); + if (auto e = expected::resultToOptionalError(block_var)) { + return expected::makeError("Failed to get the top block: " + e->message); } auto &block = diff --git a/irohad/network/impl/block_loader_service.cpp b/irohad/network/impl/block_loader_service.cpp index 3c5211a8664..e5e094b85ad 100644 --- a/irohad/network/impl/block_loader_service.cpp +++ b/irohad/network/impl/block_loader_service.cpp @@ -13,6 +13,25 @@ using namespace iroha; using namespace iroha::ametsuchi; using namespace iroha::network; +static grpc::Status handleGetBlockError(const BlockQuery::GetBlockError &error, + const logger::LoggerPtr &log) { + switch (error.code) { + case BlockQuery::GetBlockError::Code::kNoBlock: + log->error("Could not retrieve a block from block storage: {}", + error.message); + return grpc::Status(grpc::StatusCode::NOT_FOUND, "No such block."); + default: + log->error("Unexpected GetBlockError code!"); + assert(false); + case BlockQuery::GetBlockError::Code::kInternalError: + log->error("Could not retrieve a block from block storage: {}", + error.message); + return grpc::Status(grpc::StatusCode::INTERNAL, + std::string{"Internal error while retrieving block."} + + error.message); + } +} + BlockLoaderService::BlockLoaderService( std::shared_ptr block_query_factory, std::shared_ptr @@ -36,11 +55,8 @@ grpc::Status BlockLoaderService::retrieveBlocks( for (decltype(top_height) i = request->height(); i <= top_height; ++i) { auto block_result = (*block_query)->getBlock(i); - if (auto e = boost::get>(&block_result)) { - log_->error("Could not retrieve a block from block storage: {}", - e->error); - return grpc::Status(grpc::StatusCode::INTERNAL, - "internal error happened"); + if (auto e = expected::resultToOptionalError(block_result)) { + return handleGetBlockError(e.value(), log_); } auto &block = @@ -97,9 +113,8 @@ grpc::Status BlockLoaderService::retrieveBlock( auto block_result = (*block_query)->getBlock(height); - if (auto e = boost::get>(&block_result)) { - log_->error("Could not retrieve a block from block storage: {}", e->error); - return grpc::Status(grpc::StatusCode::INTERNAL, "internal error happened"); + if (auto e = expected::resultToOptionalError(block_result)) { + return handleGetBlockError(e.value(), log_); } auto &block = diff --git a/test/module/irohad/ametsuchi/block_query_test.cpp b/test/module/irohad/ametsuchi/block_query_test.cpp index 499aa26610f..b346f93db54 100644 --- a/test/module/irohad/ametsuchi/block_query_test.cpp +++ b/test/module/irohad/ametsuchi/block_query_test.cpp @@ -127,7 +127,9 @@ TEST_F(BlockQueryTest, GetNonExistentBlock) { FAIL() << "Nonexistent block request matched value " << v.value->toString(); }, - [](auto &&e) { SUCCEED(); }); + [](auto &&e) { + EXPECT_EQ(e.error.code, BlockQuery::GetBlockError::Code::kNoBlock); + }); } /** @@ -140,8 +142,8 @@ TEST_F(BlockQueryTest, GetExactlyOneBlock) { auto stored_block = blocks->getBlock(1); stored_block.match([](auto &&v) { SUCCEED(); }, [](auto &&e) { - FAIL() << "Existing block request matched error " - << e.error; + FAIL() << "Existing block request failed: " + << e.error.message; }); } @@ -158,7 +160,9 @@ TEST_F(BlockQueryTest, GetZeroBlock) { FAIL() << "Nonexistent block request matched value " << v.value->toString(); }, - [](auto &&e) { SUCCEED(); }); + [](auto &&e) { + EXPECT_EQ(e.error.code, BlockQuery::GetBlockError::Code::kNoBlock); + }); } /** @@ -185,7 +189,10 @@ TEST_F(BlockQueryTest, GetBlockButItIsNotJSON) { FAIL() << "Nonexistent block request matched value " << v.value->toString(); }, - [](auto &&e) { SUCCEED(); }); + [](auto &&e) { + EXPECT_EQ(e.error.code, + BlockQuery::GetBlockError::Code::kInternalError); + }); } /** @@ -215,7 +222,10 @@ TEST_F(BlockQueryTest, GetBlockButItIsInvalidBlock) { FAIL() << "Nonexistent block request matched value " << v.value->toString(); }, - [](auto &&e) { SUCCEED(); }); + [](auto &&e) { + EXPECT_EQ(e.error.code, + BlockQuery::GetBlockError::Code::kInternalError); + }); } /** @@ -277,18 +287,16 @@ TEST_F(BlockQueryTest, GetTopBlockSuccess) { /** * @given empty block store * @when getTopBlock is invoked on this block store - * @then result must be a string error, because no block was fetched + * @then result must be a kNoBlock error, because no block was fetched */ TEST_F(BlockQueryTest, GetTopBlockFail) { EXPECT_CALL(*mock_file, last_id()).WillRepeatedly(Return(0)); EXPECT_CALL(*mock_file, get(mock_file->last_id())) .WillOnce(Return(boost::none)); - auto top_block_error = framework::expected::err( + auto top_block_error = iroha::expected::resultToOptionalError( empty_blocks->getBlock(empty_blocks->getTopBlockHeight())); ASSERT_TRUE(top_block_error); - auto expected_error = - boost::format("Failed to retrieve block with height %d"); - ASSERT_EQ(top_block_error.value().error, - (expected_error % mock_file->last_id()).str()); + ASSERT_EQ(top_block_error.value().code, + BlockQuery::GetBlockError::Code::kNoBlock); } diff --git a/test/module/irohad/network/block_loader_test.cpp b/test/module/irohad/network/block_loader_test.cpp index 5b5971f2081..6096c1d24ef 100644 --- a/test/module/irohad/network/block_loader_test.cpp +++ b/test/module/irohad/network/block_loader_test.cpp @@ -308,7 +308,9 @@ TEST_F(BlockLoaderTest, NoBlocksInStorage) { EXPECT_CALL(*peer_query, getLedgerPeers()) .WillOnce(Return(std::vector{peer})); EXPECT_CALL(*storage, getBlock(1)) - .WillOnce(Return(ByMove(iroha::expected::makeError("no block")))); + .WillOnce( + Return(ByMove(iroha::expected::makeError(BlockQuery::GetBlockError{ + BlockQuery::GetBlockError::Code::kNoBlock, "no block"})))); auto block = loader->retrieveBlock(peer_key, 1); ASSERT_FALSE(block);