From c85ec934fb54a5c6b7fd8fecfe41f498e4cfb31a Mon Sep 17 00:00:00 2001 From: artyom-yurin Date: Wed, 24 Jul 2019 11:07:43 +0300 Subject: [PATCH] CompareAndSetAccountDetail command Signed-off-by: artyom-yurin --- docs/source/api/commands.rst | 60 +++++ irohad/ametsuchi/command_executor.hpp | 5 + .../impl/postgres_command_executor.cpp | 187 ++++++++++++++++ .../impl/postgres_command_executor.hpp | 5 + shared_model/backend/protobuf/CMakeLists.txt | 1 + .../protobuf/commands/impl/proto_command.cpp | 4 +- .../proto_compare_and_set_account_detail.cpp | 42 ++++ .../proto_compare_and_set_account_detail.hpp | 37 +++ shared_model/interfaces/CMakeLists.txt | 1 + shared_model/interfaces/commands/command.hpp | 4 +- .../interfaces/commands/command_variant.hpp | 4 +- .../compare_and_set_account_detail.hpp | 52 +++++ .../impl/compare_and_set_account_detail.cpp | 27 +++ shared_model/schema/commands.proto | 10 + shared_model/validators/field_validator.cpp | 9 + shared_model/validators/field_validator.hpp | 5 + .../validators/transaction_validator.hpp | 18 ++ .../ametsuchi/postgres_executor_test.cpp | 210 ++++++++++++++++++ test/module/shared_model/command_mocks.hpp | 11 + .../mock_command_factory.cpp | 23 ++ .../mock_command_factory.hpp | 15 ++ .../validators/field_validator_test.cpp | 19 ++ .../validators/validators_fixture.hpp | 7 + 23 files changed, 753 insertions(+), 3 deletions(-) create mode 100644 shared_model/backend/protobuf/commands/impl/proto_compare_and_set_account_detail.cpp create mode 100644 shared_model/backend/protobuf/commands/proto_compare_and_set_account_detail.hpp create mode 100644 shared_model/interfaces/commands/compare_and_set_account_detail.hpp create mode 100644 shared_model/interfaces/commands/impl/compare_and_set_account_detail.cpp diff --git a/docs/source/api/commands.rst b/docs/source/api/commands.rst index bf1caeaaf0f..4f7fb253d6e 100644 --- a/docs/source/api/commands.rst +++ b/docs/source/api/commands.rst @@ -834,3 +834,63 @@ Possible Stateful Validation Errors .. [#f1] https://www.ietf.org/rfc/rfc1035.txt .. [#f2] https://www.ietf.org/rfc/rfc1123.txt + +Compare and Set Account Detail +------------------------------ + +Purpose +^^^^^^^ + +Purpose of compare and set account detail command is to set key-value information for a given account if the old value matches the value passed. + +Schema +^^^^^^ + +.. code-block:: proto + + message CompareAndSetAccountDetail{ + string account_id = 1; + string key = 2; + string value = 3; + oneof opt_old_value { + string old_value = 4; + } + } + +.. note:: + Pay attention, that old_value field is optional. + This is due to the fact that the key-value pair might not exist. + +Structure +^^^^^^^^^ + +.. csv-table:: + :header: "Field", "Description", "Constraint", "Example" + :widths: 15, 30, 20, 15 + + "Account ID", "id of the account to which the key-value information was set. If key-value pair doesnot exist , then it will be created", "an existing account", "artyom@soramitsu" + "Key", "key of information being set", "`[A-Za-z0-9_]{1,64}`", "Name" + "Value", "new value for the corresponding key", "length of value ≤ 4096", "Artyom" + "Old value", "current value for the corresponding key", "length of value ≤ 4096", "Artem" + +Validation +^^^^^^^^^^ + +Three cases: + + Case 1. When transaction creator wants to set account detail to his/her account and he or she has permission GetMyAccountDetail / GetAllAccountsDetail / GetDomainAccountDetail + + Case 2. When transaction creator wants to set account detail to another account and he or she has permissions SetAccountDetail and GetAllAccountsDetail / GetDomainAccountDetail + + Case 3. SetAccountDetail permission was granted to transaction creator and he or she has permission GetAllAccountsDetail / GetDomainAccountDetail + +Possible Stateful Validation Errors +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. csv-table:: + :header: "Code", "Error Name", "Description", "How to solve" + + "1", "Could not compare and set account detail", "Internal error happened", "Try again or contact developers" + "2", "No such permissions", "Command's creator does not have permission to set and read account detail for this account", "Grant the necessary permission" + "3", "No such account", "Cannot find account to set account detail to", "Make sure account id is correct" + "4", "No match values", "Old values do not match", "Make sure old value is correct" diff --git a/irohad/ametsuchi/command_executor.hpp b/irohad/ametsuchi/command_executor.hpp index 17fa1930cd1..42d2f882088 100644 --- a/irohad/ametsuchi/command_executor.hpp +++ b/irohad/ametsuchi/command_executor.hpp @@ -29,6 +29,7 @@ namespace shared_model { class SetQuorum; class SubtractAssetQuantity; class TransferAsset; + class CompareAndSetAccountDetail; } // namespace interface } // namespace shared_model @@ -119,6 +120,10 @@ namespace iroha { virtual CommandResult operator()( const shared_model::interface::TransferAsset &command) = 0; + + virtual CommandResult operator()( + const shared_model::interface::CompareAndSetAccountDetail + &command) = 0; }; } // namespace ametsuchi } // namespace iroha diff --git a/irohad/ametsuchi/impl/postgres_command_executor.cpp b/irohad/ametsuchi/impl/postgres_command_executor.cpp index bd6f532527e..2e5854a676e 100644 --- a/irohad/ametsuchi/impl/postgres_command_executor.cpp +++ b/irohad/ametsuchi/impl/postgres_command_executor.cpp @@ -14,6 +14,7 @@ #include "interfaces/commands/add_peer.hpp" #include "interfaces/commands/add_signatory.hpp" #include "interfaces/commands/append_role.hpp" +#include "interfaces/commands/compare_and_set_account_detail.hpp" #include "interfaces/commands/create_account.hpp" #include "interfaces/commands/create_asset.hpp" #include "interfaces/commands/create_domain.hpp" @@ -266,6 +267,71 @@ namespace { return query; } + shared_model::interface::types::DomainIdType getDomainFromName( + const shared_model::interface::types::AccountIdType &account_id) { + // TODO 03.10.18 andrei: IR-1728 Move getDomainFromName to shared_model + std::vector res; + boost::split(res, account_id, boost::is_any_of("@")); + return res.at(1); + } + + /** + * Generate an SQL subquery which checks if creator has corresponding + * permissions for target account + * It verifies individual, domain, and global permissions, and returns true if + * any of listed permissions is present + */ + auto hasQueryPermission( + const shared_model::interface::types::AccountIdType &creator, + const shared_model::interface::types::AccountIdType &target_account, + shared_model::interface::permissions::Role indiv_permission_id, + shared_model::interface::permissions::Role all_permission_id, + shared_model::interface::permissions::Role domain_permission_id, + const shared_model::interface::types::DomainIdType &creator_domain, + const shared_model::interface::types::DomainIdType + &target_account_domain) { + const auto bits = shared_model::interface::RolePermissionSet::size(); + const auto perm_str = + shared_model::interface::RolePermissionSet({indiv_permission_id}) + .toBitstring(); + const auto all_perm_str = + shared_model::interface::RolePermissionSet({all_permission_id}) + .toBitstring(); + const auto domain_perm_str = + shared_model::interface::RolePermissionSet({domain_permission_id}) + .toBitstring(); + + boost::format cmd(R"( + has_indiv_perm AS ( + SELECT (COALESCE(bit_or(rp.permission), '0'::bit(%1%)) + & '%3%') = '%3%' FROM role_has_permissions AS rp + JOIN account_has_roles AS ar on ar.role_id = rp.role_id + WHERE ar.account_id = %2% + ), + has_all_perm AS ( + SELECT (COALESCE(bit_or(rp.permission), '0'::bit(%1%)) + & '%4%') = '%4%' FROM role_has_permissions AS rp + JOIN account_has_roles AS ar on ar.role_id = rp.role_id + WHERE ar.account_id = %2% + ), + has_domain_perm AS ( + SELECT (COALESCE(bit_or(rp.permission), '0'::bit(%1%)) + & '%5%') = '%5%' FROM role_has_permissions AS rp + JOIN account_has_roles AS ar on ar.role_id = rp.role_id + WHERE ar.account_id = %2% + ), + has_query_perm AS ( + SELECT (%2% = %6% AND (SELECT * FROM has_indiv_perm)) + OR (SELECT * FROM has_all_perm) + OR (%7% = %8% AND (SELECT * FROM has_domain_perm)) AS perm + ) + )"); + + return (cmd % bits % creator % perm_str % all_perm_str % domain_perm_str + % target_account % creator_domain % target_account_domain) + .str(); + } + std::string checkAccountDomainRoleOrGlobalRolePermission( shared_model::interface::permissions::Role global_permission, shared_model::interface::permissions::Role domain_permission, @@ -797,6 +863,51 @@ namespace iroha { ELSE 1 END AS result)"; + const std::string PostgresCommandExecutor::compareAndSetAccountDetailBase = + R"(PREPARE %s (text, text, text, text, text, text, text) AS + WITH %s + old_value AS + ( + SELECT * + FROM account + WHERE + account_id = $2 + AND CASE + WHEN data ? $1 AND data->$1 ?$3 + THEN + CASE + WHEN $5 IS NOT NULL THEN data->$1->$3 = $5::jsonb + ELSE FALSE + END + ELSE TRUE + END + ), + inserted AS + ( + UPDATE account + SET data = jsonb_set( + CASE + WHEN data ? $1 THEN data + ELSE jsonb_set(data, array[$1], '{}') + END, + array[$1, $3], $4::jsonb + ) + WHERE + EXISTS (SELECT * FROM old_value) + AND account_id = $2 + %s + RETURNING (1) + ) + SELECT CASE + WHEN EXISTS (SELECT * FROM inserted) THEN 0 + WHEN NOT EXISTS + (SELECT * FROM account WHERE account_id=$2) THEN 3 + WHEN NOT EXISTS (SELECT * FROM old_value) THEN 4 + %s + ELSE 1 + END + AS result)"; + std::string CommandError::toString() const { return (boost::format("%s: %d with extra info '%s'") % command_name % error_code % error_extra) @@ -1228,6 +1339,43 @@ namespace iroha { sql_, cmd.str(), "TransferAsset", std::move(str_args)); } + CommandResult PostgresCommandExecutor::operator()( + const shared_model::interface::CompareAndSetAccountDetail &command) { + auto &account_id = command.accountId(); + auto &key = command.key(); + auto &value = command.value(); + auto &old_value = command.oldValue(); + + auto cmd = boost::format( + "EXECUTE %1% ('%2%', '%3%', '%4%', '%5%', %6%, '%7%', '%8%')"); + + appendCommandName("compareAndSetAccountDetail", cmd, do_validation_); + + std::string new_json_value = "\"" + value + "\""; + std::string expected_json_value = "NULL"; + + if (old_value) { + expected_json_value = "'\"" + old_value.get() + "\"'"; + } + + cmd = (cmd % creator_account_id_ % account_id % key % new_json_value + % expected_json_value % getDomainFromName(creator_account_id_) + % getDomainFromName(account_id)); + + auto str_args = [&account_id, &key, &new_json_value, &old_value] { + return getQueryArgsStringBuilder() + .append("account_id", account_id) + .append("key", key) + .append("value", new_json_value) + .append("old_value", + old_value ? "\"" + old_value.get() + "\"" : "NULL") + .finalize(); + }; + + return executeQuery( + sql_, cmd.str(), "compareAndSetAccountDetail", std::move(str_args)); + } + void PostgresCommandExecutor::prepareStatements(soci::session &sql) { std::vector statements; @@ -1598,6 +1746,45 @@ namespace iroha { R"( AND (SELECT * FROM has_perm))", R"( WHEN NOT (SELECT * FROM has_perm) THEN 2 )"}}); + statements.push_back( + {"compareAndSetAccountDetail", + compareAndSetAccountDetailBase, + {(boost::format(R"( + has_role_perm AS (%s), + has_grantable_perm AS (%s), + %s, + has_perm AS (SELECT CASE + WHEN (SELECT * FROM has_query_perm) THEN + CASE + WHEN (SELECT * FROM has_grantable_perm) + THEN true + WHEN ($1 = $2) THEN true + WHEN (SELECT * FROM has_role_perm) + THEN true + ELSE false END + ELSE false END + ), + )") + % checkAccountRolePermission( + shared_model::interface::permissions::Role::kSetDetail, "$1") + % checkAccountGrantablePermission( + shared_model::interface::permissions::Grantable:: + kSetMyAccountDetail, + "$1", + "$2") + % hasQueryPermission( + "$1", + "$2", + shared_model::interface::permissions::Role::kGetMyAccDetail, + shared_model::interface::permissions::Role::kGetAllAccDetail, + shared_model::interface::permissions::Role:: + kGetDomainAccDetail, + "$6", + "$7")) + .str(), + R"( AND (SELECT * FROM has_perm))", + R"( WHEN NOT (SELECT * FROM has_perm) THEN 2 )"}}); + for (const auto &st : statements) { prepareStatement(sql, st); } diff --git a/irohad/ametsuchi/impl/postgres_command_executor.hpp b/irohad/ametsuchi/impl/postgres_command_executor.hpp index a782ab869e1..607fa159860 100644 --- a/irohad/ametsuchi/impl/postgres_command_executor.hpp +++ b/irohad/ametsuchi/impl/postgres_command_executor.hpp @@ -83,6 +83,10 @@ namespace iroha { CommandResult operator()( const shared_model::interface::TransferAsset &command) override; + CommandResult operator()( + const shared_model::interface::CompareAndSetAccountDetail &command) + override; + static void prepareStatements(soci::session &sql); private: @@ -111,6 +115,7 @@ namespace iroha { static const std::string setQuorumBase; static const std::string subtractAssetQuantityBase; static const std::string transferAssetBase; + static const std::string compareAndSetAccountDetailBase; }; } // namespace ametsuchi } // namespace iroha diff --git a/shared_model/backend/protobuf/CMakeLists.txt b/shared_model/backend/protobuf/CMakeLists.txt index b5a1ef409cc..e8a210fd1f4 100644 --- a/shared_model/backend/protobuf/CMakeLists.txt +++ b/shared_model/backend/protobuf/CMakeLists.txt @@ -31,6 +31,7 @@ add_library(shared_model_proto_backend commands/impl/proto_set_quorum.cpp commands/impl/proto_subtract_asset_quantity.cpp commands/impl/proto_transfer_asset.cpp + commands/impl/proto_compare_and_set_account_detail.cpp queries/impl/proto_query.cpp queries/impl/proto_get_account.cpp queries/impl/proto_get_account_asset_transactions.cpp diff --git a/shared_model/backend/protobuf/commands/impl/proto_command.cpp b/shared_model/backend/protobuf/commands/impl/proto_command.cpp index b4941e7b471..286dc75e13b 100644 --- a/shared_model/backend/protobuf/commands/impl/proto_command.cpp +++ b/shared_model/backend/protobuf/commands/impl/proto_command.cpp @@ -9,6 +9,7 @@ #include "backend/protobuf/commands/proto_add_peer.hpp" #include "backend/protobuf/commands/proto_add_signatory.hpp" #include "backend/protobuf/commands/proto_append_role.hpp" +#include "backend/protobuf/commands/proto_compare_and_set_account_detail.hpp" #include "backend/protobuf/commands/proto_create_account.hpp" #include "backend/protobuf/commands/proto_create_asset.hpp" #include "backend/protobuf/commands/proto_create_domain.hpp" @@ -43,7 +44,8 @@ namespace { shared_model::proto::SetQuorum, shared_model::proto::SubtractAssetQuantity, shared_model::proto::TransferAsset, - shared_model::proto::RemovePeer>; + shared_model::proto::RemovePeer, + shared_model::proto::CompareAndSetAccountDetail>; /// list of types in proto variant using ProtoCommandListType = ProtoCommandVariantType::types; diff --git a/shared_model/backend/protobuf/commands/impl/proto_compare_and_set_account_detail.cpp b/shared_model/backend/protobuf/commands/impl/proto_compare_and_set_account_detail.cpp new file mode 100644 index 00000000000..2224e5b1564 --- /dev/null +++ b/shared_model/backend/protobuf/commands/impl/proto_compare_and_set_account_detail.cpp @@ -0,0 +1,42 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "backend/protobuf/commands/proto_compare_and_set_account_detail.hpp" + +namespace shared_model { + namespace proto { + + CompareAndSetAccountDetail::CompareAndSetAccountDetail( + iroha::protocol::Command &command) + : compare_and_set_account_detail_{ + command.compare_and_set_account_detail()} {} + + const interface::types::AccountIdType & + CompareAndSetAccountDetail::accountId() const { + return compare_and_set_account_detail_.account_id(); + } + + const interface::types::AccountDetailKeyType & + CompareAndSetAccountDetail::key() const { + return compare_and_set_account_detail_.key(); + } + + const interface::types::AccountDetailValueType & + CompareAndSetAccountDetail::value() const { + return compare_and_set_account_detail_.value(); + } + + const boost::optional + CompareAndSetAccountDetail::oldValue() const { + if (compare_and_set_account_detail_.opt_old_value_case() + == iroha::protocol::CompareAndSetAccountDetail:: + OPT_OLD_VALUE_NOT_SET) { + return boost::none; + } + return compare_and_set_account_detail_.old_value(); + } + + } // namespace proto +} // namespace shared_model diff --git a/shared_model/backend/protobuf/commands/proto_compare_and_set_account_detail.hpp b/shared_model/backend/protobuf/commands/proto_compare_and_set_account_detail.hpp new file mode 100644 index 00000000000..684311e71ca --- /dev/null +++ b/shared_model/backend/protobuf/commands/proto_compare_and_set_account_detail.hpp @@ -0,0 +1,37 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef IROHA_PROTO_COMPARE_AND_SET_ACCOUNT_DETAIL_HPP +#define IROHA_PROTO_COMPARE_AND_SET_ACCOUNT_DETAIL_HPP + +#include "interfaces/commands/compare_and_set_account_detail.hpp" + +#include "commands.pb.h" + +namespace shared_model { + namespace proto { + class CompareAndSetAccountDetail final + : public interface::CompareAndSetAccountDetail { + public: + explicit CompareAndSetAccountDetail(iroha::protocol::Command &command); + + const interface::types::AccountIdType &accountId() const override; + + const interface::types::AccountDetailKeyType &key() const override; + + const interface::types::AccountDetailValueType &value() const override; + + const boost::optional oldValue() + const override; + + private: + const iroha::protocol::CompareAndSetAccountDetail + &compare_and_set_account_detail_; + }; + + } // namespace proto +} // namespace shared_model + +#endif // IROHA_PROTO_COMPARE_AND_SET_ACCOUNT_DETAIL_HPP diff --git a/shared_model/interfaces/CMakeLists.txt b/shared_model/interfaces/CMakeLists.txt index 74b4050cd74..39eda940af8 100644 --- a/shared_model/interfaces/CMakeLists.txt +++ b/shared_model/interfaces/CMakeLists.txt @@ -24,6 +24,7 @@ add_library(shared_model_interfaces commands/impl/set_quorum.cpp commands/impl/subtract_asset_quantity.cpp commands/impl/transfer_asset.cpp + commands/impl/compare_and_set_account_detail.cpp queries/impl/query.cpp queries/impl/get_account.cpp queries/impl/get_account_asset_transactions.cpp diff --git a/shared_model/interfaces/commands/command.hpp b/shared_model/interfaces/commands/command.hpp index 362f72a71e9..b5aa32a91f6 100644 --- a/shared_model/interfaces/commands/command.hpp +++ b/shared_model/interfaces/commands/command.hpp @@ -30,6 +30,7 @@ namespace shared_model { class SetQuorum; class SubtractAssetQuantity; class TransferAsset; + class CompareAndSetAccountDetail; /** * Class provides commands container for all commands in system. @@ -59,7 +60,8 @@ namespace shared_model { SetQuorum, SubtractAssetQuantity, TransferAsset, - RemovePeer>; + RemovePeer, + CompareAndSetAccountDetail>; /** * @return reference to const variant with concrete command diff --git a/shared_model/interfaces/commands/command_variant.hpp b/shared_model/interfaces/commands/command_variant.hpp index 41f96bc0de8..ccfa46eac8b 100644 --- a/shared_model/interfaces/commands/command_variant.hpp +++ b/shared_model/interfaces/commands/command_variant.hpp @@ -6,6 +6,7 @@ #ifndef IROHA_SHARED_MODEL_COMMAND_VARIANT_HPP #define IROHA_SHARED_MODEL_COMMAND_VARIANT_HPP +#include "compare_and_set_account_detail.hpp" #include "interfaces/commands/command.hpp" #include @@ -27,7 +28,8 @@ namespace boost { const shared_model::interface::SetAccountDetail &, const shared_model::interface::SetQuorum &, const shared_model::interface::SubtractAssetQuantity &, - const shared_model::interface::TransferAsset &>; + const shared_model::interface::TransferAsset &, + const shared_model::interface::CompareAndSetAccountDetail &>; } // namespace boost #endif // IROHA_SHARED_MODEL_COMMAND_VARIANT_HPP diff --git a/shared_model/interfaces/commands/compare_and_set_account_detail.hpp b/shared_model/interfaces/commands/compare_and_set_account_detail.hpp new file mode 100644 index 00000000000..bda14f54cf3 --- /dev/null +++ b/shared_model/interfaces/commands/compare_and_set_account_detail.hpp @@ -0,0 +1,52 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef IROHA_SHARED_MODEL_COMPARE_AND_SET_ACCOUNT_DETAIL_HPP +#define IROHA_SHARED_MODEL_COMPARE_AND_SET_ACCOUNT_DETAIL_HPP + +#include +#include "interfaces/base/noncopyable_model_primitive.hpp" + +#include "interfaces/common_objects/types.hpp" + +namespace shared_model { + namespace interface { + + /** + * Set key-value pair of given account if the current value matches provided + * expectation + */ + class CompareAndSetAccountDetail + : public NonCopyableModelPrimitive { + public: + /** + * @return Identity of user to set account detail to + */ + virtual const types::AccountIdType &accountId() const = 0; + + /** + * @return key of data to store in the account + */ + virtual const types::AccountDetailKeyType &key() const = 0; + + /** + * @return detail value to store by given key + */ + virtual const types::AccountDetailValueType &value() const = 0; + + /** + * @return the value expected before the change + */ + virtual const boost::optional oldValue() + const = 0; + + std::string toString() const override; + + bool operator==(const ModelType &rhs) const override; + }; + } // namespace interface +} // namespace shared_model + +#endif // IROHA_SHARED_MODEL_COMPARE_AND_SET_ACCOUNT_DETAIL_HPP diff --git a/shared_model/interfaces/commands/impl/compare_and_set_account_detail.cpp b/shared_model/interfaces/commands/impl/compare_and_set_account_detail.cpp new file mode 100644 index 00000000000..32cb4ac997d --- /dev/null +++ b/shared_model/interfaces/commands/impl/compare_and_set_account_detail.cpp @@ -0,0 +1,27 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "interfaces/commands/compare_and_set_account_detail.hpp" + +namespace shared_model { + namespace interface { + + std::string CompareAndSetAccountDetail::toString() const { + return detail::PrettyStringBuilder() + .init("CompareAndSetAccountDetail") + .append("account_id", accountId()) + .append("key", key()) + .append("value", value()) + .append("old_value", oldValue().value_or("(none)")) + .finalize(); + } + + bool CompareAndSetAccountDetail::operator==(const ModelType &rhs) const { + return accountId() == rhs.accountId() and key() == rhs.key() + and value() == rhs.value() and oldValue() == rhs.oldValue(); + } + + } // namespace interface +} // namespace shared_model diff --git a/shared_model/schema/commands.proto b/shared_model/schema/commands.proto index d280afe3ebc..d930e782a51 100644 --- a/shared_model/schema/commands.proto +++ b/shared_model/schema/commands.proto @@ -96,6 +96,15 @@ message SubtractAssetQuantity { string amount = 2; } +message CompareAndSetAccountDetail{ + string account_id = 1; + string key = 2; + string value = 3; + oneof opt_old_value { + string old_value = 4; + } +} + message Command { oneof command { AddAssetQuantity add_asset_quantity = 1; @@ -115,5 +124,6 @@ message Command { SubtractAssetQuantity subtract_asset_quantity = 15; TransferAsset transfer_asset = 16; RemovePeer remove_peer = 17; + CompareAndSetAccountDetail compare_and_set_account_detail = 18; } } diff --git a/shared_model/validators/field_validator.cpp b/shared_model/validators/field_validator.cpp index 817d267865e..1e10a366f2b 100644 --- a/shared_model/validators/field_validator.cpp +++ b/shared_model/validators/field_validator.cpp @@ -212,6 +212,15 @@ namespace shared_model { } } + void FieldValidator::validateOldAccountDetailValue( + ReasonsGroupType &reason, + const boost::optional + &old_value) const { + if (old_value) { + validateAccountDetailValue(reason, old_value.value()); + } + } + void FieldValidator::validatePrecision( ReasonsGroupType &reason, const interface::types::PrecisionType &precision) const { diff --git a/shared_model/validators/field_validator.hpp b/shared_model/validators/field_validator.hpp index 5020b60ad61..caa283d60d9 100644 --- a/shared_model/validators/field_validator.hpp +++ b/shared_model/validators/field_validator.hpp @@ -110,6 +110,11 @@ namespace shared_model { ReasonsGroupType &reason, const interface::types::AccountDetailValueType &value) const; + void validateOldAccountDetailValue( + ReasonsGroupType &reason, + const boost::optional + &old_value) const; + void validatePrecision( ReasonsGroupType &reason, const interface::types::PrecisionType &precision) const; diff --git a/shared_model/validators/transaction_validator.hpp b/shared_model/validators/transaction_validator.hpp index df90ba80398..d584c23d1aa 100644 --- a/shared_model/validators/transaction_validator.hpp +++ b/shared_model/validators/transaction_validator.hpp @@ -14,6 +14,7 @@ #include "interfaces/commands/add_signatory.hpp" #include "interfaces/commands/append_role.hpp" #include "interfaces/commands/command.hpp" +#include "interfaces/commands/compare_and_set_account_detail.hpp" #include "interfaces/commands/create_account.hpp" #include "interfaces/commands/create_asset.hpp" #include "interfaces/commands/create_domain.hpp" @@ -237,6 +238,23 @@ namespace shared_model { return reason; } + ReasonsGroupType operator()( + const interface::CompareAndSetAccountDetail &casad) const { + ReasonsGroupType reason; + addInvalidCommand(reason, "CompareAndSetAccountDetail"); + + using iroha::operator|; + + validator_.validateAccountId(reason, casad.accountId()); + validator_.validateAccountDetailKey(reason, casad.key()); + validator_.validateAccountDetailValue(reason, casad.value()); + casad.oldValue() | [&reason, this](const auto &oldValue) { + this->validator_.validateOldAccountDetailValue(reason, oldValue); + }; + + return reason; + } + private: FieldValidator validator_; mutable int command_counter{0}; diff --git a/test/module/irohad/ametsuchi/postgres_executor_test.cpp b/test/module/irohad/ametsuchi/postgres_executor_test.cpp index 6fa7be08558..09d394d5c21 100644 --- a/test/module/irohad/ametsuchi/postgres_executor_test.cpp +++ b/test/module/irohad/ametsuchi/postgres_executor_test.cpp @@ -2093,5 +2093,215 @@ namespace iroha { CHECK_ERROR_CODE_AND_MESSAGE(cmd_result, 7, query_args); } + class CompareAndSetAccountDetail : public CommandExecutorTest { + public: + void SetUp() override { + CommandExecutorTest::SetUp(); + createDefaultRole(); + createDefaultDomain(); + createDefaultAccount(); + account2_id = "id2@" + domain_id; + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCreateAccount( + "id2", + domain_id, + shared_model::interface::types::PubkeyType( + std::string('2', 32))), + true)); + } + shared_model::interface::types::AccountIdType account2_id; + }; + + /** + * @given command + * @when trying to set kv + * @then kv is set + */ + TEST_F(CompareAndSetAccountDetail, Valid) { + addOnePerm(shared_model::interface::permissions::Role::kGetMyAccDetail); + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, "key", "value", boost::none))); + auto kv = sql_query->getAccountDetail(account_id); + ASSERT_TRUE(kv); + ASSERT_EQ(kv.get(), R"({"id@domain": {"key": "value"}})"); + } + + /** + * @given command + * @when trying to set kv when has grantable permission + * @then kv is set + */ + TEST_F(CompareAndSetAccountDetail, ValidGrantablePerm) { + addOnePerm( + shared_model::interface::permissions::Role::kGetDomainAccDetail); + auto perm = + shared_model::interface::permissions::Grantable::kSetMyAccountDetail; + CHECK_SUCCESSFUL_RESULT(execute( + *mock_command_factory->constructGrantPermission(account_id, perm), + true, + account2_id)); + + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account2_id, "key", "value", boost::none), + false, + account_id)); + auto kv = sql_query->getAccountDetail(account2_id); + ASSERT_TRUE(kv); + ASSERT_EQ(kv.get(), R"({"id@domain": {"key": "value"}})"); + } + + /** + * @given command + * @when trying to set kv when has role permission + * @then kv is set + */ + TEST_F(CompareAndSetAccountDetail, ValidRolePerm) { + addAllPerms(); + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account2_id, "key", "value", boost::none), + false, + account_id)); + auto kv = sql_query->getAccountDetail(account2_id); + ASSERT_TRUE(kv); + ASSERT_EQ(kv.get(), R"({"id@domain": {"key": "value"}})"); + } + + /** + * @given command + * @when trying to set kv while having no permissions + * @then corresponding error code is returned + */ + TEST_F(CompareAndSetAccountDetail, NoPerms) { + auto cmd_result = + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account2_id, "key", "value", boost::none), + false, + account_id); + + std::vector query_args{account2_id, "key", "value"}; + CHECK_ERROR_CODE_AND_MESSAGE(cmd_result, 2, query_args); + + auto kv = sql_query->getAccountDetail(account2_id); + ASSERT_TRUE(kv); + ASSERT_EQ(kv.get(), "{}"); + } + + /** + * @given command + * @when trying to set kv to non-existing account + * @then corresponding error code is returned + */ + TEST_F(CompareAndSetAccountDetail, NoAccount) { + addAllPerms(); + auto cmd_result = + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + "doge@noaccount", "key", "value", boost::none), + false, + account_id); + + std::vector query_args{"doge@noaccount", "key", "value"}; + CHECK_ERROR_CODE_AND_MESSAGE(cmd_result, 3, query_args); + } + + /** + * @given command + * @when trying to set kv and then set kv1 with correct old value + * @then kv1 is set + */ + TEST_F(CompareAndSetAccountDetail, ValidOldValue) { + addOnePerm(shared_model::interface::permissions::Role::kGetMyAccDetail); + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, "key", "value", boost::none))); + + auto kv = sql_query->getAccountDetail(account_id); + ASSERT_TRUE(kv); + ASSERT_EQ(kv.get(), R"({"id@domain": {"key": "value"}})"); + + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, + "key", + "value1", + boost::optional< + shared_model::interface::types::AccountDetailValueType>( + "value")))); + auto kv1 = sql_query->getAccountDetail(account_id); + ASSERT_TRUE(kv1); + ASSERT_EQ(kv1.get(), R"({"id@domain": {"key": "value1"}})"); + } + + /** + * @given command + * @when trying to set kv and then set kv1 with incorrect old value + * @then corresponding error code is returned + */ + TEST_F(CompareAndSetAccountDetail, InvalidOldValue) { + addOnePerm(shared_model::interface::permissions::Role::kGetMyAccDetail); + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, "key", "value", boost::none))); + + auto kv = sql_query->getAccountDetail(account_id); + ASSERT_TRUE(kv); + ASSERT_EQ(kv.get(), R"({"id@domain": {"key": "value"}})"); + + auto cmd_result = + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, + "key", + "value1", + boost::optional< + shared_model::interface::types::AccountDetailValueType>( + "oldValue"))); + + std::vector query_args{ + account_id, "key", "value1", "oldValue"}; + CHECK_ERROR_CODE_AND_MESSAGE(cmd_result, 4, query_args); + } + + /** + * @given Two commands + * @when trying to set kv and then set k1v1 + * @then kv and k1v1 are set + */ + TEST_F(CompareAndSetAccountDetail, DifferentKeys) { + addOnePerm(shared_model::interface::permissions::Role::kGetMyAccDetail); + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, "key", "value", boost::none))); + + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, "key1", "value1", boost::none))); + + auto ad = sql_query->getAccountDetail(account_id); + ASSERT_TRUE(ad); + ASSERT_EQ(ad.get(), + R"({"id@domain": {"key": "value", "key1": "value1"}})"); + } + + /** + * @given commands + * @when trying to set kv without oldValue where v is empty string + * @then corresponding error code is returned + */ + TEST_F(CompareAndSetAccountDetail, EmptyDetail) { + addOnePerm(shared_model::interface::permissions::Role::kGetMyAccDetail); + CHECK_SUCCESSFUL_RESULT( + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, "key", "", boost::none))); + + auto cmd_result = + execute(*mock_command_factory->constructCompareAndSetAccountDetail( + account_id, "key", "value", boost::none)); + + std::vector query_args{account_id, "key", "value"}; + CHECK_ERROR_CODE_AND_MESSAGE(cmd_result, 4, query_args); + } + } // namespace ametsuchi } // namespace iroha diff --git a/test/module/shared_model/command_mocks.hpp b/test/module/shared_model/command_mocks.hpp index 40f9821ec7a..87a34d68dcc 100644 --- a/test/module/shared_model/command_mocks.hpp +++ b/test/module/shared_model/command_mocks.hpp @@ -7,6 +7,7 @@ #define IROHA_COMMAND_MOCKS_HPP #include +#include #include #include "cryptography/public_key.hpp" #include "interfaces/commands/add_asset_quantity.hpp" @@ -14,6 +15,7 @@ #include "interfaces/commands/add_signatory.hpp" #include "interfaces/commands/append_role.hpp" #include "interfaces/commands/command.hpp" +#include "interfaces/commands/compare_and_set_account_detail.hpp" #include "interfaces/commands/create_account.hpp" #include "interfaces/commands/create_asset.hpp" #include "interfaces/commands/create_domain.hpp" @@ -148,6 +150,15 @@ namespace shared_model { MOCK_CONST_METHOD0(amount, const Amount &()); MOCK_CONST_METHOD0(description, const types::DescriptionType &()); }; + + struct MockCompareAndSetAccountDetail + : public shared_model::interface::CompareAndSetAccountDetail { + MOCK_CONST_METHOD0(accountId, const types::AccountIdType &()); + MOCK_CONST_METHOD0(key, const types::AccountDetailKeyType &()); + MOCK_CONST_METHOD0(value, const types::AccountDetailValueType &()); + MOCK_CONST_METHOD0( + oldValue, const boost::optional()); + }; } // namespace interface } // namespace shared_model diff --git a/test/module/shared_model/mock_objects_factories/mock_command_factory.cpp b/test/module/shared_model/mock_objects_factories/mock_command_factory.cpp index 1bd50429d8f..e495549e8dc 100644 --- a/test/module/shared_model/mock_objects_factories/mock_command_factory.cpp +++ b/test/module/shared_model/mock_objects_factories/mock_command_factory.cpp @@ -312,5 +312,28 @@ namespace shared_model { return specific_cmd_mock; }); } + + MockCommandFactory::FactoryResult + MockCommandFactory::constructCompareAndSetAccountDetail( + const shared_model::interface::types::AccountIdType &account_id, + const shared_model::interface::types::AccountDetailKeyType &cmd_key, + const shared_model::interface::types::AccountDetailValueType &cmd_value, + const boost::optional< + shared_model::interface::types::AccountDetailValueType> + cmd_old_value) const { + return createFactoryResult( + [&account_id, &cmd_key, &cmd_value, &cmd_old_value]( + FactoryResult specific_cmd_mock) { + EXPECT_CALL(*specific_cmd_mock, accountId()) + .WillRepeatedly(ReturnRefOfCopy(account_id)); + EXPECT_CALL(*specific_cmd_mock, key()) + .WillRepeatedly(ReturnRefOfCopy(cmd_key)); + EXPECT_CALL(*specific_cmd_mock, value()) + .WillRepeatedly(ReturnRefOfCopy(cmd_value)); + EXPECT_CALL(*specific_cmd_mock, oldValue()) + .WillRepeatedly(Return(cmd_old_value)); + return specific_cmd_mock; + }); + } } // namespace interface } // namespace shared_model diff --git a/test/module/shared_model/mock_objects_factories/mock_command_factory.hpp b/test/module/shared_model/mock_objects_factories/mock_command_factory.hpp index 74fba7b0c54..b62ae036047 100644 --- a/test/module/shared_model/mock_objects_factories/mock_command_factory.hpp +++ b/test/module/shared_model/mock_objects_factories/mock_command_factory.hpp @@ -190,6 +190,21 @@ namespace shared_model { const types::DescriptionType &description, const Amount &amount) const; + /** + * Construct a mocked CompareAndSetAccountDetail + * @param account_id to be in that command + * @param key to be in that command + * @param value to be in that command + * @param old_value to be in that command + * @return pointer to the created command + */ + FactoryResult + constructCompareAndSetAccountDetail( + const types::AccountIdType &account_id, + const types::AccountDetailKeyType &key, + const types::AccountDetailValueType &value, + const boost::optional old_value) const; + private: /** * Actually create a pointer to the mocked command diff --git a/test/module/shared_model/validators/field_validator_test.cpp b/test/module/shared_model/validators/field_validator_test.cpp index 2c91d251dd5..6a98a995e2b 100644 --- a/test/module/shared_model/validators/field_validator_test.cpp +++ b/test/module/shared_model/validators/field_validator_test.cpp @@ -100,6 +100,12 @@ class FieldValidatorTest : public ValidatorsTest { &FieldValidatorTest::detail_value, detail_value_test_cases)); + field_validators.insert( + makeValidator("old_value", + &FieldValidator::validateOldAccountDetailValue, + &FieldValidatorTest::detail_old_value, + detail_old_value_test_cases)); + field_validators.insert(makeValidator("domain_id", &FieldValidator::validateDomainId, &FieldValidatorTest::domain_id, @@ -497,6 +503,19 @@ class FieldValidatorTest : public ValidatorsTest { // 5 Mb, value greater than can put into one setAccountDetail std::string(5 * 1024 * 1024, '0'))}; + std::vector detail_old_value_test_cases{ + makeValidCase(&FieldValidatorTest::detail_old_value, boost::none), + makeValidCase(&FieldValidatorTest::detail_old_value, "valid old value"), + makeValidCase(&FieldValidatorTest::detail_old_value, + std::string(4096, '0')), + makeValidCase(&FieldValidatorTest::detail_old_value, ""), + makeInvalidCase("long_value", + "old_value", + &FieldValidatorTest::detail_old_value, + // 5 Mb, value greater than can put into one + // CompareAndSetAccountDetail + std::string(5 * 1024 * 1024, '0'))}; + std::vector description_test_cases{ makeValidCase(&FieldValidatorTest::description, "valid description"), makeValidCase(&FieldValidatorTest::description, ""), diff --git a/test/module/shared_model/validators/validators_fixture.hpp b/test/module/shared_model/validators/validators_fixture.hpp index 1ced3cdba36..8ed5aba6b70 100644 --- a/test/module/shared_model/validators/validators_fixture.hpp +++ b/test/module/shared_model/validators/validators_fixture.hpp @@ -54,6 +54,8 @@ class ValidatorsTest : public ::testing::Test { {"iroha.protocol.RevokePermission.account_id", setString(account_id)}, {"iroha.protocol.SetAccountDetail.account_id", setString(account_id)}, {"iroha.protocol.SetAccountQuorum.account_id", setString(account_id)}, + {"iroha.protocol.CompareAndSetAccountDetail.account_id", + setString(account_id)}, {"iroha.protocol.AppendRole.role_name", setString(role_name)}, {"iroha.protocol.DetachRole.role_name", setString(role_name)}, {"iroha.protocol.CreateRole.role_name", setString(role_name)}, @@ -82,9 +84,13 @@ class ValidatorsTest : public ::testing::Test { {"iroha.protocol.RevokePermission.permission", setEnum(grantable_permission)}, {"iroha.protocol.SetAccountDetail.key", setString(detail_key)}, + {"iroha.protocol.CompareAndSetAccountDetail.key", + setString(detail_key)}, {"iroha.protocol.GetAccountDetail.key", setString(detail_key)}, {"iroha.protocol.GetAccountDetail.writer", setString(writer)}, {"iroha.protocol.SetAccountDetail.value", setString("")}, + {"iroha.protocol.CompareAndSetAccountDetail.value", setString("")}, + {"iroha.protocol.CompareAndSetAccountDetail.old_value", setString("")}, {"iroha.protocol.GetTransactions.tx_hashes", addString(hash)}, {"iroha.protocol.SetAccountQuorum.quorum", setUInt32(quorum)}, {"iroha.protocol.TransferAsset.description", setString("")}, @@ -259,6 +265,7 @@ class ValidatorsTest : public ::testing::Test { std::string domain_id; std::string detail_key; std::string detail_value; + boost::optional detail_old_value; std::string description; std::string public_key; std::string hash;