Skip to content

Commit

Permalink
fixup! Network transaction endpoints: remove code duplication and mak…
Browse files Browse the repository at this point in the history
…e more strict validation
  • Loading branch information
lebdron committed Oct 19, 2019
1 parent d1f578f commit 6607dcf
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,23 @@ grpc::Status MstTransportGrpc::SendState(

auto transactions = shared_model::proto::deserializeTransactions(
*transaction_factory_, request->transactions());
if (auto e = boost::get<expected::ErrorOf<decltype(transactions)>>(
&transactions)) {
log_->warn("Transaction deserialization failed: hash {}, {}",
e->error.hash,
e->error.error);
if (auto e = expected::resultToOptionalError(transactions)) {
log_->warn(
"Transaction deserialization failed: hash {}, {}", e->hash, e->error);
return ::grpc::Status::OK;
}

auto batches = shared_model::interface::parseAndCreateBatches(
*batch_parser_,
*batch_factory_,
boost::get<expected::ValueOf<decltype(transactions)>>(
std::move(transactions))
.value);
if (auto e = boost::get<expected::ErrorOf<decltype(batches)>>(&batches)) {
log_->warn("Batch deserialization failed: {}", e->error);
*expected::resultToOptionalValue(std::move(transactions)));
if (auto e = expected::resultToOptionalError(batches)) {
log_->warn("Batch deserialization failed: {}", *e);
return ::grpc::Status::OK;
}
MstState new_state = MstState::empty(mst_state_logger_, mst_completer_);
for (auto &&batch :
boost::get<expected::ValueOf<decltype(batches)>>(std::move(batches))
.value) {
auto opt_batches = expected::resultToOptionalValue(std::move(batches));
for (auto &batch : *opt_batches) {
auto cache_presence = tx_presence_cache_->check(*batch);
if (not cache_presence) {
// TODO andrei 30.11.18 IR-51 Handle database error
Expand Down
19 changes: 7 additions & 12 deletions irohad/ordering/impl/on_demand_os_server_grpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,23 @@ grpc::Status OnDemandOsServerGrpc::SendBatches(
::google::protobuf::Empty *response) {
auto transactions = shared_model::proto::deserializeTransactions(
*transaction_factory_, request->transactions());
if (auto e = boost::get<expected::ErrorOf<decltype(transactions)>>(
&transactions)) {
log_->warn("Transaction deserialization failed: hash {}, {}",
e->error.hash,
e->error.error);
if (auto e = expected::resultToOptionalError(transactions)) {
log_->warn(
"Transaction deserialization failed: hash {}, {}", e->hash, e->error);
return ::grpc::Status::OK;
}

auto batches = shared_model::interface::parseAndCreateBatches(
*batch_parser_,
*batch_factory_,
boost::get<expected::ValueOf<decltype(transactions)>>(
std::move(transactions))
.value);
if (auto e = boost::get<expected::ErrorOf<decltype(batches)>>(&batches)) {
log_->warn("Batch deserialization failed: {}", e->error);
*expected::resultToOptionalValue(std::move(transactions)));
if (auto e = expected::resultToOptionalError(batches)) {
log_->warn("Batch deserialization failed: {}", *e);
return ::grpc::Status::OK;
}

ordering_service_->onBatches(
boost::get<expected::ValueOf<decltype(batches)>>(std::move(batches))
.value);
*expected::resultToOptionalValue(std::move(batches)));

return ::grpc::Status::OK;
}
Expand Down
20 changes: 8 additions & 12 deletions irohad/torii/impl/command_service_transport_grpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,28 +87,24 @@ namespace iroha {

auto transactions = shared_model::proto::deserializeTransactions(
*transaction_factory_, request->transactions());
if (auto e = boost::get<expected::ErrorOf<decltype(transactions)>>(
&transactions)) {
if (auto e = expected::resultToOptionalError(transactions)) {
return publish_stateless_fail(
fmt::format("Transaction deserialization failed: hash {}, {}",
e->error.hash,
e->error.error));
e->hash,
e->error));
}

auto batches = shared_model::interface::parseAndCreateBatches(
*batch_parser_,
*batch_factory_,
boost::get<expected::ValueOf<decltype(transactions)>>(
std::move(transactions))
.value);
if (auto e = boost::get<expected::ErrorOf<decltype(batches)>>(&batches)) {
*expected::resultToOptionalValue(std::move(transactions)));
if (auto e = expected::resultToOptionalError(batches)) {
return publish_stateless_fail(
fmt::format("Batch deserialization failed: {}", e->error));
fmt::format("Batch deserialization failed: {}", *e));
}

for (auto &&batch :
boost::get<expected::ValueOf<decltype(batches)>>(std::move(batches))
.value) {
auto opt_batches = expected::resultToOptionalValue(std::move(batches));
for (auto &batch : *opt_batches) {
this->command_service_->handleTransactionBatch(std::move(batch));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@ shared_model::proto::deserializeTransactions(
interface::types::SharedTxsCollectionType tx_collection;
for (const auto &tx : transactions) {
auto model_tx = transaction_factory.build(tx);
if (auto e = boost::get<iroha::expected::ErrorOf<decltype(model_tx)>>(
&model_tx)) {
if (auto e = iroha::expected::resultToOptionalError(model_tx)) {
return *e;
}
tx_collection.emplace_back(
boost::get<iroha::expected::ValueOf<decltype(model_tx)>>(
std::move(model_tx))
.value);
*iroha::expected::resultToOptionalValue(std::move(model_tx)));
}
return tx_collection;
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ shared_model::interface::parseAndCreateBatches(
types::BatchesCollectionType batches;
for (auto &cand : batch_candidates) {
auto batch = batch_factory.createTransactionBatch(cand);
if (auto e =
boost::get<iroha::expected::ErrorOf<decltype(batch)>>(&batch)) {
if (auto e = iroha::expected::resultToOptionalError(batch)) {
return *e;
}
batches.push_back(
boost::get<iroha::expected::ValueOf<decltype(batch)>>(std::move(batch))
.value);
*iroha::expected::resultToOptionalValue(std::move(batch)));
}
return batches;
}
4 changes: 2 additions & 2 deletions test/module/irohad/torii/torii_transport_command_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,10 @@ TEST_F(CommandServiceTransportGrpcTest, ListToriiPartialInvalid) {

size_t counter = 0;
EXPECT_CALL(*proto_tx_validator, validate(_))
.Times(AtLeast(1))
.Times(kTimes)
.WillRepeatedly(Return(shared_model::validation::Answer{}));
EXPECT_CALL(*tx_validator, validate(_))
.Times(AtLeast(1))
.Times(kTimes)
.WillRepeatedly(Invoke([this, &counter, kError](const auto &) mutable {
shared_model::validation::Answer res;
if (counter++ == kTimes - 1) {
Expand Down

0 comments on commit 6607dcf

Please sign in to comment.