Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Query Responses Factory #1724

Merged
merged 19 commits into from
Sep 26, 2018
Merged

Query Responses Factory #1724

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7978025
Started implementing the factory
Akvinikym Sep 13, 2018
8a31c52
Completed factory
Akvinikym Sep 14, 2018
4bf2ba9
Started tests
Akvinikym Sep 14, 2018
6960c7d
Fixed some things with factory
Akvinikym Sep 15, 2018
7e76a93
Finished tests
Akvinikym Sep 15, 2018
f1a1d3f
All tests are up
Akvinikym Sep 17, 2018
7c9bc9d
Brushing before the PR
Akvinikym Sep 17, 2018
bb78344
Merged with dev
Akvinikym Sep 17, 2018
ce1f5ca
Added error responses
Akvinikym Sep 17, 2018
a713d9e
Passing arguments as values or unique pointers
Akvinikym Sep 19, 2018
791a82b
More review issues fixed
Akvinikym Sep 19, 2018
6bf4193
Added query hashes
Akvinikym Sep 19, 2018
16bdc4d
Review issues
Akvinikym Sep 24, 2018
58708e2
Merge branch 'dev' into feature/qry-resp-factory
Akvinikym Sep 24, 2018
685f127
Fixed memory leak
Akvinikym Sep 24, 2018
2b23679
Fixed memory leak(2)
Akvinikym Sep 24, 2018
722d149
Many review issues and refactoring
Akvinikym Sep 25, 2018
935ce77
Fix build
Akvinikym Sep 25, 2018
bc1b8ee
Got rid of SpecifiedVisitor
Akvinikym Sep 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 62 additions & 42 deletions shared_model/backend/protobuf/impl/proto_query_response_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@

#include "backend/protobuf/proto_query_response_factory.hpp"
#include "backend/protobuf/permissions.hpp"
#include "backend/protobuf/query_responses/proto_block_query_response.hpp"
#include "backend/protobuf/query_responses/proto_query_response.hpp"

std::unique_ptr<shared_model::interface::AccountAssetResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createAccountAssetResponse(
const std::vector<std::shared_ptr<shared_model::interface::AccountAsset>>
&assets) {
std::vector<std::shared_ptr<shared_model::interface::AccountAsset>> assets,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not vector<unique_ptr>, so that it is possible to move the assets to query response? Same in createTransactionsResponse

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unique_ptr seems not that handy to use, since most of the methods in iroha works with shared_ptr

const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a common method for this, so these lines are not duplicated in all the methods.

iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());
...
return std::make_unique<shared_model::proto::QueryResponse>(
      std::move(protocol_query_response));

Maybe create a method, so that it is possible to pass something in place of ...?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo it's better leave as is, otherwise code will be harder to read and modify


iroha::protocol::AccountAssetResponse *protocol_specific_response =
protocol_query_response.mutable_account_assets_response();
Expand All @@ -20,46 +23,53 @@ shared_model::proto::ProtoQueryResponseFactory::createAccountAssetResponse(
->getTransport());
}

return std::make_unique<shared_model::proto::AccountAssetResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::AccountDetailResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createAccountDetailResponse(
const shared_model::interface::types::DetailType &account_detail) {
shared_model::interface::types::DetailType account_detail,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::AccountDetailResponse *protocol_specific_response =
protocol_query_response.mutable_account_detail_response();
protocol_specific_response->set_detail(account_detail);

return std::make_unique<shared_model::proto::AccountDetailResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::AccountResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createAccountResponse(
const shared_model::interface::Account &account,
const std::vector<std::string> &roles) {
std::unique_ptr<shared_model::interface::Account> account,
std::vector<std::string> roles,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::AccountResponse *protocol_specific_response =
protocol_query_response.mutable_account_response();
protocol_specific_response->mutable_account()->CopyFrom(
static_cast<const shared_model::proto::Account &>(account)
.getTransport());
*protocol_specific_response->mutable_account() =
static_cast<shared_model::proto::Account *>(account.release())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a memleak

->getTransport();
for (const auto &role : roles) {
protocol_specific_response->add_account_roles(role);
}

return std::make_unique<shared_model::proto::AccountResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::ErrorQueryResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createErrorQueryResponse(
ErrorQueryType error_type) {
ErrorQueryType error_type,
std::string error_msg,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::ErrorResponse_Reason reason;
switch (error_type) {
Expand Down Expand Up @@ -94,16 +104,18 @@ shared_model::proto::ProtoQueryResponseFactory::createErrorQueryResponse(
iroha::protocol::ErrorResponse *protocol_specific_response =
protocol_query_response.mutable_error_response();
protocol_specific_response->set_reason(reason);
protocol_specific_response->set_message(std::move(error_msg));

return std::make_unique<shared_model::proto::ErrorQueryResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about setting the message?

}

std::unique_ptr<shared_model::interface::SignatoriesResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createSignatoriesResponse(
const std::vector<shared_model::interface::types::PubkeyType>
&signatories) {
std::vector<shared_model::interface::types::PubkeyType> signatories,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::SignatoriesResponse *protocol_specific_response =
protocol_query_response.mutable_signatories_response();
Expand All @@ -112,15 +124,17 @@ shared_model::proto::ProtoQueryResponseFactory::createSignatoriesResponse(
protocol_specific_response->add_keys(blob.data(), blob.size());
}

return std::make_unique<shared_model::proto::SignatoriesResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::TransactionsResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createTransactionsResponse(
const std::vector<std::shared_ptr<shared_model::interface::Transaction>>
&transactions) {
std::vector<std::shared_ptr<shared_model::interface::Transaction>>
transactions,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::TransactionsResponse *protocol_specific_response =
protocol_query_response.mutable_transactions_response();
Expand All @@ -130,43 +144,50 @@ shared_model::proto::ProtoQueryResponseFactory::createTransactionsResponse(
->getTransport());
}

return std::make_unique<shared_model::proto::TransactionsResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::AssetResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createAssetResponse(
const shared_model::interface::Asset &asset) {
std::unique_ptr<shared_model::interface::Asset> asset,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::AssetResponse *protocol_specific_response =
protocol_query_response.mutable_asset_response();
protocol_specific_response->mutable_asset()->CopyFrom(
static_cast<const shared_model::proto::Asset &>(asset).getTransport());
*protocol_specific_response->mutable_asset() =
static_cast<shared_model::proto::Asset *>(asset.release())
->getTransport();

return std::make_unique<shared_model::proto::AssetResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::RolesResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createRolesResponse(
const std::vector<shared_model::interface::types::RoleIdType> &roles) {
std::vector<shared_model::interface::types::RoleIdType> roles,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::RolesResponse *protocol_specific_response =
protocol_query_response.mutable_roles_response();
for (const auto &role : roles) {
protocol_specific_response->add_roles(role);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be add_roles(std::move(role)) if const is removed above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also possible to use auto &&role, btw

}

return std::make_unique<shared_model::proto::RolesResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::RolePermissionsResponse>
std::unique_ptr<shared_model::interface::QueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createRolePermissionsResponse(
const shared_model::interface::RolePermissionSet &role_permissions) {
shared_model::interface::RolePermissionSet role_permissions,
const crypto::Hash &query_hash) {
iroha::protocol::QueryResponse protocol_query_response;
protocol_query_response.set_query_hash(query_hash.hex());

iroha::protocol::RolePermissionsResponse *protocol_specific_response =
protocol_query_response.mutable_role_permissions_response();
Expand All @@ -178,29 +199,28 @@ shared_model::proto::ProtoQueryResponseFactory::createRolePermissionsResponse(
}
}

return std::make_unique<shared_model::proto::RolePermissionsResponse>(
return std::make_unique<shared_model::proto::QueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::BlockQueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createBlockQueryResponse(
const shared_model::interface::Block &block) {
std::unique_ptr<shared_model::interface::Block> block) {
iroha::protocol::BlockQueryResponse protocol_query_response;

iroha::protocol::BlockResponse *protocol_specific_response =
protocol_query_response.mutable_block_response();
const auto &proto_block =
static_cast<const shared_model::proto::Block &>(block);
protocol_specific_response->set_allocated_block(
new iroha::protocol::Block(proto_block.getTransport()));
*protocol_specific_response->mutable_block() =
static_cast<shared_model::proto::Block *>(block.release())
->getTransport();

return std::make_unique<shared_model::proto::BlockQueryResponse>(
std::move(protocol_query_response));
}

std::unique_ptr<shared_model::interface::BlockQueryResponse>
shared_model::proto::ProtoQueryResponseFactory::createBlockQueryResponse(
const std::string &error_message) {
std::string error_message) {
iroha::protocol::BlockQueryResponse protocol_query_response;

iroha::protocol::BlockErrorResponse *protocol_specific_response =
Expand Down
81 changes: 41 additions & 40 deletions shared_model/backend/protobuf/proto_query_response_factory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,57 +6,58 @@
#ifndef IROHA_PROTO_QUERY_RESPONSE_FACTORY_HPP
#define IROHA_PROTO_QUERY_RESPONSE_FACTORY_HPP

#include "backend/protobuf/query_responses/proto_block_query_response.hpp"
#include "backend/protobuf/query_responses/proto_query_response.hpp"
#include "interfaces/iroha_internal/query_response_factory.hpp"

namespace shared_model {
namespace proto {

class ProtoQueryResponseFactory : public interface::QueryResponseFactory {
public:
std::unique_ptr<interface::AccountAssetResponse>
createAccountAssetResponse(
const std::vector<
std::shared_ptr<shared_model::interface::AccountAsset>> &assets)
override;

std::unique_ptr<interface::AccountDetailResponse>
createAccountDetailResponse(
const interface::types::DetailType &account_detail) override;

std::unique_ptr<interface::AccountResponse> createAccountResponse(
const interface::Account &account,
const std::vector<std::string> &roles) override;

std::unique_ptr<interface::ErrorQueryResponse> createErrorQueryResponse(
ErrorQueryType error_type) override;

std::unique_ptr<interface::SignatoriesResponse> createSignatoriesResponse(
const std::vector<interface::types::PubkeyType> &signatories)
override;

std::unique_ptr<interface::TransactionsResponse>
createTransactionsResponse(
const std::vector<
std::shared_ptr<shared_model::interface::Transaction>>
&transactions) override;

std::unique_ptr<interface::AssetResponse> createAssetResponse(
const shared_model::interface::Asset &asset) override;

std::unique_ptr<interface::RolesResponse> createRolesResponse(
const std::vector<interface::types::RoleIdType> &roles) override;

std::unique_ptr<interface::RolePermissionsResponse>
createRolePermissionsResponse(
const interface::RolePermissionSet &role_permissions) override;
std::unique_ptr<interface::QueryResponse> createAccountAssetResponse(
std::vector<std::shared_ptr<shared_model::interface::AccountAsset>>
assets,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createAccountDetailResponse(
interface::types::DetailType account_detail,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createAccountResponse(
std::unique_ptr<interface::Account> account,
std::vector<std::string> roles,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createErrorQueryResponse(
ErrorQueryType error_type,
std::string error_msg,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createSignatoriesResponse(
std::vector<interface::types::PubkeyType> signatories,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createTransactionsResponse(
std::vector<std::shared_ptr<shared_model::interface::Transaction>>
transactions,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createAssetResponse(
std::unique_ptr<shared_model::interface::Asset> asset,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createRolesResponse(
std::vector<interface::types::RoleIdType> roles,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::QueryResponse> createRolePermissionsResponse(
interface::RolePermissionSet role_permissions,
const crypto::Hash &query_hash) override;

std::unique_ptr<interface::BlockQueryResponse> createBlockQueryResponse(
const interface::Block &block) override;
std::unique_ptr<interface::Block> block) override;

std::unique_ptr<interface::BlockQueryResponse> createBlockQueryResponse(
const std::string &error_message) override;
std::string error_message) override;
};

} // namespace proto
Expand Down
Loading