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

Common objects factory #1556

Merged
merged 14 commits into from
Jul 17, 2018
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

The whole file is implantation inside an .hpp, won't is cause troubles with recompiling etc? Isn't it better to split it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will only be used in setup of storage, so its fine.

* Copyright Soramitsu Co., Ltd. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef IROHA_PROTO_COMMON_OBJECTS_FACTORY_HPP
#define IROHA_PROTO_COMMON_OBJECTS_FACTORY_HPP

#include <regex>

#include "backend/protobuf/common_objects/account.hpp"
#include "backend/protobuf/common_objects/account_asset.hpp"
#include "backend/protobuf/common_objects/asset.hpp"
#include "backend/protobuf/common_objects/domain.hpp"
#include "backend/protobuf/common_objects/peer.hpp"
#include "backend/protobuf/common_objects/signature.hpp"
#include "common/result.hpp"
#include "interfaces/common_objects/common_objects_factory.hpp"
#include "primitive.pb.h"
#include "validators/answer.hpp"

namespace shared_model {
namespace proto {

/**
* ProtoCommonObjectsFactory constructs protobuf-based objects.
* It performs stateful validation with provided validator
* @tparam Validator
*/
template <typename Validator>
class ProtoCommonObjectsFactory : public interface::CommonObjectsFactory {
public:
FactoryResult<std::unique_ptr<interface::Peer>> createPeer(
const interface::types::AddressType &address,
const interface::types::PubkeyType &public_key) override {
iroha::protocol::Peer peer;
peer.set_address(address);
peer.set_peer_key(crypto::toBinaryString(public_key));
auto proto_peer = std::make_unique<Peer>(std::move(peer));

auto answer =
validate(*proto_peer, [this](const auto &peer, auto &reasons) {
validator_.validatePeer(reasons, peer);
});

if (answer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look very readable. Maybe, you should rename the variable to errors? It represents something wrong happened only, so I think it's a reasonable name

Copy link
Contributor

Choose a reason for hiding this comment

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

If you choose to follow the advice, change names of all variables in below functions as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Answer can be empty, which indicates lack of errors. But i agree that errors is a better name here

return iroha::expected::makeError(answer.reason());
}

return iroha::expected::makeValue<std::unique_ptr<interface::Peer>>(
std::move(proto_peer));
}

FactoryResult<std::unique_ptr<interface::Account>> createAccount(
const interface::types::AccountIdType &account_id,
const interface::types::DomainIdType &domain_id,
interface::types::QuorumType quorum,
const interface::types::JsonType &jsonData) override {
iroha::protocol::Account account;
account.set_account_id(account_id);
account.set_domain_id(domain_id);
account.set_quorum(quorum);
account.set_json_data(jsonData);

auto proto_account = std::make_unique<Account>(std::move(account));

auto answer = validate(
*proto_account, [this](const auto &account, auto &reasons) {
validator_.validateAccountId(reasons, account.accountId());
validator_.validateDomainId(reasons, account.domainId());
validator_.validateQuorum(reasons, account.quorum());
});

if (answer) {
return iroha::expected::makeError(answer.reason());
}

return iroha::expected::makeValue<std::unique_ptr<interface::Account>>(
std::move(proto_account));
}

FactoryResult<std::unique_ptr<interface::AccountAsset>>
createAccountAsset(const interface::types::AccountIdType &account_id,
const interface::types::AssetIdType &asset_id,
const interface::Amount &balance) override {
iroha::protocol::AccountAsset asset;
asset.set_account_id(account_id);
asset.set_asset_id(asset_id);
auto proto_balance = asset.mutable_balance();
convertToProtoAmount(*proto_balance->mutable_value(),
balance.intValue());
proto_balance->set_precision(balance.precision());

auto proto_asset = std::make_unique<AccountAsset>(std::move(asset));

auto answer =
validate(*proto_asset, [this](const auto &asset, auto &reasons) {
validator_.validateAccountId(reasons, asset.accountId());
validator_.validateAssetId(reasons, asset.assetId());
});

if (answer) {
return iroha::expected::makeError(answer.reason());
}

return iroha::expected::makeValue<
std::unique_ptr<interface::AccountAsset>>(std::move(proto_asset));
}

FactoryResult<std::unique_ptr<interface::Amount>> createAmount(
boost::multiprecision::uint256_t value,
interface::types::PrecisionType precision) override {
iroha::protocol::Amount amount;
amount.set_precision(precision);
convertToProtoAmount(*amount.mutable_value(), value);

auto proto_amount = std::make_unique<Amount>(std::move(amount));

auto answer = validate(*proto_amount, [](const auto &, auto &) {
// no validation needed,
// since any amount is valid in general context
Copy link
Contributor

Choose a reason for hiding this comment

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

There's still validateAmount function, so please use it

});

if (answer) {
return iroha::expected::makeError(answer.reason());
}

return iroha::expected::makeValue<std::unique_ptr<interface::Amount>>(
std::move(proto_amount));
}

FactoryResult<std::unique_ptr<interface::Amount>> createAmount(
std::string amount) override {
// taken from iroha::model::Amount
// check if valid number
const static std::regex e("([0-9]*\\.[0-9]+|[0-9]+)");
if (!std::regex_match(amount, e)) {
return iroha::expected::makeError("number string is invalid");
}

// get precision
auto dot_place = amount.find('.');
interface::types::PrecisionType precision;
if (dot_place > amount.size()) {
precision = 0;
} else {
precision = amount.size() - dot_place - 1;
// erase dot from the string
amount.erase(std::remove(amount.begin(), amount.end(), '.'),
amount.end());
}

auto begin = amount.find_first_not_of('0');

// create uint256 value from obtained string
boost::multiprecision::uint256_t value = 0;
if (begin <= amount.size()) {
value = boost::multiprecision::uint256_t(amount.substr(begin));
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't overflow possible anywhere? Of course, I don't think someone's going to provide more than uint256, but still, for safety reasons, is it possible to handle such kind of error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is taken from lib/amount. It has worked so far, and I do not want to touch it :). It will be changed in the future, then we can replace it here

}

return createAmount(value, precision);
}

FactoryResult<std::unique_ptr<interface::Asset>> createAsset(
const interface::types::AssetIdType &asset_id,
const interface::types::DomainIdType &domain_id,
interface::types::PrecisionType precision) override {
iroha::protocol::Asset asset;
asset.set_asset_id(asset_id);
asset.set_domain_id(domain_id);
asset.set_precision(precision);

auto proto_asset = std::make_unique<Asset>(std::move(asset));

auto answer =
validate(*proto_asset, [this](const auto &asset, auto &reasons) {
validator_.validateAssetId(reasons, asset.assetId());
validator_.validateDomainId(reasons, asset.domainId());
});

if (answer) {
return iroha::expected::makeError(answer.reason());
}

return iroha::expected::makeValue<std::unique_ptr<interface::Asset>>(
std::move(proto_asset));
}

FactoryResult<std::unique_ptr<interface::Domain>> createDomain(
const interface::types::DomainIdType &domain_id,
const interface::types::RoleIdType &default_role) override {
iroha::protocol::Domain domain;
domain.set_domain_id(domain_id);
domain.set_default_role(default_role);

auto proto_domain = std::make_unique<Domain>(std::move(domain));

auto answer =
validate(*proto_domain, [this](const auto &domain, auto &reason) {
validator_.validateDomainId(reason, domain.domainId());
validator_.validateRoleId(reason, domain.defaultRole());
});

if (answer) {
return iroha::expected::makeError(answer.reason());
}

return iroha::expected::makeValue<std::unique_ptr<interface::Domain>>(
std::move(proto_domain));
}

FactoryResult<std::unique_ptr<interface::Signature>> createSignature(
const interface::types::PubkeyType &key,
const interface::Signature::SignedType &signed_data) override {
iroha::protocol::Signature signature;
signature.set_pubkey(crypto::toBinaryString(key));
signature.set_signature(crypto::toBinaryString(signed_data));

auto proto_singature =
std::make_unique<Signature>(std::move(signature));

auto answer = validate(
*proto_singature, [this](const auto &signature, auto &reason) {
validator_.validatePubkey(reason, signature.publicKey());
});

if (answer) {
return iroha::expected::makeError(answer.reason());
}

return iroha::expected::makeValue<
std::unique_ptr<interface::Signature>>(std::move(proto_singature));
}

private:
/**
* Perform validation of a given object
* @param o - object to be validated
* @param f - function which populates reason parameter with errors.
* second parameter (reasons) must be passed by non-const reference
* @return validation result
*/
template <typename T, typename ValidationFunc>
validation::Answer validate(const T &o, ValidationFunc &&f) const {
shared_model::validation::Answer answer;
validation::ReasonsGroupType reasons;
f(o, reasons);
if (not reasons.second.empty()) {
answer.addReason(std::move(reasons));
}
return answer;
}

Validator validator_;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

There is an empty space between curly braces in the beginning of the file, but not here

} // namespace proto
} // namespace shared_model

#endif // IROHA_PROTO_COMMON_OBJECTS_FACTORY_HPP
98 changes: 98 additions & 0 deletions shared_model/interfaces/common_objects/common_objects_factory.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/**
* Copyright Soramitsu Co., Ltd. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef IROHA_COMMON_OBJECTS_FACTORY_HPP
#define IROHA_COMMON_OBJECTS_FACTORY_HPP

#include <memory>

#include "common/result.hpp"
#include "interfaces/common_objects/account.hpp"
#include "interfaces/common_objects/peer.hpp"
#include "interfaces/common_objects/types.hpp"
#include "interfaces/common_objects/domain.hpp"

namespace shared_model {
namespace interface {
/**
* CommonObjectsFactory provides methods to construct simple objects
Copy link
Contributor

Choose a reason for hiding this comment

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

What is "simple object"? Can you, please, define it in more specific way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess "common_objects" is a better name here since they are in the folder with this name

* such as peer, account etc.
*/
class CommonObjectsFactory {
public:
template <typename T>
using FactoryResult = iroha::expected::Result<T, std::string>;

/**
* Create peer instance
*/
virtual FactoryResult<std::unique_ptr<Peer>> createPeer(
const types::AddressType &address,
const types::PubkeyType &public_key) = 0;

/**
* Create account instance
*/
virtual FactoryResult<std::unique_ptr<Account>> createAccount(
const types::AccountIdType &account_id,
const types::DomainIdType &domain_id,
types::QuorumType quorum,
const types::JsonType &jsonData) = 0;

/**
* Create account asset instance
*/
virtual FactoryResult<std::unique_ptr<AccountAsset>> createAccountAsset(
const types::AccountIdType &account_id,
const types::AssetIdType &asset_id,
const Amount &balance) = 0;

/**
* Create amount instance from string
*
* @param value integer will be divided by 10 * precision,
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't this implementation issues, which should not be located in interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the description of the contract which factory provides.
It basically says that if you pass this precision, this is the final result you should get.

* so value 123 with precision 2 will become Amount of 1.23
*/
virtual FactoryResult<std::unique_ptr<Amount>> createAmount(
boost::multiprecision::uint256_t value,
types::PrecisionType precision) = 0;

/**
* Create amount instance from string
*
* @param amount must represent valid number.
* For example: "1.23", "10" etc.
*/
virtual FactoryResult<std::unique_ptr<Amount>> createAmount(
std::string amount) = 0;
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 const reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because we modify this parameter in the code below. This logic was not written by me, and will be thrown out in the future, so it is simpler to just pass by value for now.


/**
* Create asset instance
*/
virtual FactoryResult<std::unique_ptr<Asset>> createAsset(
const types::AssetIdType &asset_id,
const types::DomainIdType &domain_id,
types::PrecisionType precision) = 0;

/**
* Create domain instance
*/
virtual FactoryResult<std::unique_ptr<Domain>> createDomain(
const types::DomainIdType &domain_id,
const types::RoleIdType &default_role) = 0;

/**
* Create signature instance
*/
virtual FactoryResult<std::unique_ptr<Signature>> createSignature(
const interface::types::PubkeyType &key,
const interface::Signature::SignedType &signed_data) = 0;

virtual ~CommonObjectsFactory() = default;
};
} // namespace interface
} // namespace shared_model

#endif // IROHA_COMMONOBJECTSFACTORY_HPP
Copy link
Contributor

Choose a reason for hiding this comment

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

No empty line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is present

8 changes: 8 additions & 0 deletions test/module/shared_model/backend_proto/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,11 @@ target_link_libraries(permissions_test
shared_model_proto_backend
)

addtest(proto_common_objects_factory_test
common_objects/proto_common_objects_factory_test.cpp
)
target_link_libraries(proto_common_objects_factory_test
shared_model_cryptography
shared_model_stateless_validation
schema)
Copy link
Contributor

Choose a reason for hiding this comment

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

Bracket on a new line?


Loading