From ebe7688ccba93cc5ca07e60353b0e6e18ffb49a9 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Thu, 28 Sep 2023 12:56:38 +0100 Subject: [PATCH] Api v1 bool support (#877) * Allow not bool for signer_lists * Allow transactions to be not bool for v1 * Add tests for JsonBool --- CMakeLists.txt | 1 + src/rpc/handlers/AccountInfo.cpp | 2 +- src/rpc/handlers/AccountInfo.h | 12 +-- src/rpc/handlers/NoRippleCheck.cpp | 2 +- src/rpc/handlers/NoRippleCheck.h | 17 ++-- unittests/rpc/JsonBoolTests.cpp | 81 +++++++++++++++++++ unittests/rpc/handlers/AccountInfoTests.cpp | 21 ++++- unittests/rpc/handlers/NoRippleCheckTests.cpp | 25 +++++- 8 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 unittests/rpc/JsonBoolTests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d3234a985..0b5434a06 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -175,6 +175,7 @@ if (tests) unittests/rpc/ForwardingProxyTests.cpp unittests/rpc/WorkQueueTests.cpp unittests/rpc/AmendmentsTests.cpp + unittests/rpc/JsonBoolTests.cpp ## RPC handlers unittests/rpc/handlers/DefaultProcessorTests.cpp unittests/rpc/handlers/TestHandlerTests.cpp diff --git a/src/rpc/handlers/AccountInfo.cpp b/src/rpc/handlers/AccountInfo.cpp index 2a41d7178..64498d2bf 100644 --- a/src/rpc/handlers/AccountInfo.cpp +++ b/src/rpc/handlers/AccountInfo.cpp @@ -176,7 +176,7 @@ tag_invoke(boost::json::value_to_tag, boost::json::va } if (jsonObject.contains(JS(signer_lists))) - input.signerLists = jsonObject.at(JS(signer_lists)).as_bool(); + input.signerLists = boost::json::value_to(jsonObject.at(JS(signer_lists))); return input; } diff --git a/src/rpc/handlers/AccountInfo.h b/src/rpc/handlers/AccountInfo.h index 6268787c3..e7961ad0b 100644 --- a/src/rpc/handlers/AccountInfo.h +++ b/src/rpc/handlers/AccountInfo.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -76,7 +77,7 @@ class AccountInfoHandler std::optional ident; std::optional ledgerHash; std::optional ledgerIndex; - bool signerLists = false; + JsonBool signerLists{false}; }; using Result = HandlerReturnType; @@ -88,14 +89,15 @@ class AccountInfoHandler RpcSpecConstRef spec([[maybe_unused]] uint32_t apiVersion) const { - static auto const rpcSpec = RpcSpec{ + static auto const rpcSpecV1 = RpcSpec{ {JS(account), validation::AccountValidator}, {JS(ident), validation::AccountValidator}, {JS(ledger_hash), validation::Uint256HexStringValidator}, - {JS(ledger_index), validation::LedgerIndexValidator}, - {JS(signer_lists), validation::Type{}}}; + {JS(ledger_index), validation::LedgerIndexValidator}}; - return rpcSpec; + static auto const rpcSpec = RpcSpec{rpcSpecV1, {{JS(signer_lists), validation::Type{}}}}; + + return apiVersion == 1 ? rpcSpecV1 : rpcSpec; } Result diff --git a/src/rpc/handlers/NoRippleCheck.cpp b/src/rpc/handlers/NoRippleCheck.cpp index 8d8146d64..1e155875a 100644 --- a/src/rpc/handlers/NoRippleCheck.cpp +++ b/src/rpc/handlers/NoRippleCheck.cpp @@ -167,7 +167,7 @@ tag_invoke(boost::json::value_to_tag, boost::json:: input.limit = jsonObject.at(JS(limit)).as_int64(); if (jsonObject.contains(JS(transactions))) - input.transactions = jsonObject.at(JS(transactions)).as_bool(); + input.transactions = boost::json::value_to(jsonObject.at(JS(transactions))); if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = jsonObject.at(JS(ledger_hash)).as_string().c_str(); diff --git a/src/rpc/handlers/NoRippleCheck.h b/src/rpc/handlers/NoRippleCheck.h index 16cc6b0c6..820b6f54d 100644 --- a/src/rpc/handlers/NoRippleCheck.h +++ b/src/rpc/handlers/NoRippleCheck.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -62,7 +63,7 @@ class NoRippleCheckHandler std::optional ledgerHash; std::optional ledgerIndex; uint32_t limit = LIMIT_DEFAULT; - bool transactions = false; + JsonBool transactions{false}; }; using Result = HandlerReturnType; @@ -75,7 +76,7 @@ class NoRippleCheckHandler RpcSpecConstRef spec([[maybe_unused]] uint32_t apiVersion) const { - static auto const rpcSpec = RpcSpec{ + static auto const rpcSpecV1 = RpcSpec{ {JS(account), validation::Required{}, validation::AccountValidator}, {JS(role), validation::Required{}, @@ -87,11 +88,15 @@ class NoRippleCheckHandler {JS(limit), validation::Type(), validation::Min(1u), - modifiers::Clamp{LIMIT_MIN, LIMIT_MAX}}, - {JS(transactions), validation::Type()}, - }; + modifiers::Clamp{LIMIT_MIN, LIMIT_MAX}}}; + + static auto const rpcSpec = RpcSpec{ + rpcSpecV1, + { + {JS(transactions), validation::Type()}, + }}; - return rpcSpec; + return apiVersion == 1 ? rpcSpecV1 : rpcSpec; } Result diff --git a/unittests/rpc/JsonBoolTests.cpp b/unittests/rpc/JsonBoolTests.cpp new file mode 100644 index 000000000..ecec892be --- /dev/null +++ b/unittests/rpc/JsonBoolTests.cpp @@ -0,0 +1,81 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2023, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== +#include + +#include +#include + +using namespace rpc; +namespace json = boost::json; +using namespace testing; + +struct JsonBoolTestsCaseBundle +{ + std::string testName; + std::string json; + bool expectedBool; +}; + +class JsonBoolTests : public TestWithParam +{ +public: + struct NameGenerator + { + template + std::string + operator()(const testing::TestParamInfo& info) const + { + auto bundle = static_cast(info.param); + return bundle.testName; + } + }; + + static auto + generateTestValuesForParametersTest() + { + return std::vector{ + {"NullValue", R"({ "test_bool": null })", false}, + {"BoolTrueValue", R"({ "test_bool": true })", true}, + {"BoolFalseValue", R"({ "test_bool": false })", false}, + {"IntTrueValue", R"({ "test_bool": 1 })", true}, + {"IntFalseValue", R"({ "test_bool": 0 })", false}, + {"DoubleTrueValue", R"({ "test_bool": 0.1 })", true}, + {"DoubleFalseValue", R"({ "test_bool": 0.0 })", false}, + {"StringTrueValue", R"({ "test_bool": "true" })", true}, + {"StringFalseValue", R"({ "test_bool": "false" })", true}, + {"ArrayTrueValue", R"({ "test_bool": [0] })", true}, + {"ArrayFalseValue", R"({ "test_bool": [] })", false}, + {"ObjectTrueValue", R"({ "test_bool": { "key": null } })", true}, + {"ObjectFalseValue", R"({ "test_bool": {} })", false}}; + } +}; + +INSTANTIATE_TEST_CASE_P( + JsonBoolCheckGroup, + JsonBoolTests, + ValuesIn(JsonBoolTests::generateTestValuesForParametersTest()), + JsonBoolTests::NameGenerator{}); + +TEST_P(JsonBoolTests, Parse) +{ + auto const testBundle = GetParam(); + const auto jv = json::parse(testBundle.json).as_object(); + ASSERT_TRUE(jv.contains("test_bool")); + EXPECT_EQ(testBundle.expectedBool, value_to(jv.at("test_bool")).value); +} diff --git a/unittests/rpc/handlers/AccountInfoTests.cpp b/unittests/rpc/handlers/AccountInfoTests.cpp index 09255850a..be700ae67 100644 --- a/unittests/rpc/handlers/AccountInfoTests.cpp +++ b/unittests/rpc/handlers/AccountInfoTests.cpp @@ -107,7 +107,7 @@ TEST_P(AccountInfoParameterTest, InvalidParams) runSpawn([&, this](auto yield) { auto const handler = AnyHandler{AccountInfoHandler{mockBackendPtr}}; auto const req = json::parse(testBundle.testJson); - auto const output = handler.process(req, Context{yield}); + auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 2}); ASSERT_FALSE(output); auto const err = rpc::makeError(output.error()); @@ -116,6 +116,25 @@ TEST_P(AccountInfoParameterTest, InvalidParams) }); } +TEST_F(AccountInfoParameterTest, ApiV1SignerListIsNotBool) +{ + static constexpr auto reqJson = R"( + {"ident":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "signer_lists":1} + )"; + auto* rawBackendPtr = static_cast(mockBackendPtr.get()); + EXPECT_CALL(*rawBackendPtr, fetchLedgerBySequence); + runSpawn([&, this](auto yield) { + auto const handler = AnyHandler{AccountInfoHandler{mockBackendPtr}}; + auto const req = json::parse(reqJson); + auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 1}); + ASSERT_FALSE(output); + + auto const err = rpc::makeError(output.error()); + EXPECT_EQ(err.at("error").as_string(), "lgrNotFound"); + EXPECT_EQ(err.at("error_message").as_string(), "ledgerNotFound"); + }); +} + TEST_F(RPCAccountInfoHandlerTest, LedgerNonExistViaIntSequence) { auto const rawBackendPtr = static_cast(mockBackendPtr.get()); diff --git a/unittests/rpc/handlers/NoRippleCheckTests.cpp b/unittests/rpc/handlers/NoRippleCheckTests.cpp index 743cfd0cf..24ee5e746 100644 --- a/unittests/rpc/handlers/NoRippleCheckTests.cpp +++ b/unittests/rpc/handlers/NoRippleCheckTests.cpp @@ -156,7 +156,7 @@ TEST_P(NoRippleCheckParameterTest, InvalidParams) runSpawn([&, this](auto yield) { auto const handler = AnyHandler{NoRippleCheckHandler{mockBackendPtr}}; auto const req = json::parse(testBundle.testJson); - auto const output = handler.process(req, Context{yield}); + auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 2}); ASSERT_FALSE(output); auto const err = rpc::makeError(output.error()); @@ -165,6 +165,29 @@ TEST_P(NoRippleCheckParameterTest, InvalidParams) }); } +TEST_F(NoRippleCheckParameterTest, V1ApiTransactionsIsNotBool) +{ + static constexpr auto reqJson = R"( + { + "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "role": "gateway", + "transactions": "gg" + } + )"; + auto rawBackendPtr = static_cast(mockBackendPtr.get()); + EXPECT_CALL(*rawBackendPtr, fetchLedgerBySequence); + runSpawn([&, this](auto yield) { + auto const handler = AnyHandler{NoRippleCheckHandler{mockBackendPtr}}; + auto const req = json::parse(reqJson); + auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 1}); + ASSERT_FALSE(output); + + auto const err = rpc::makeError(output.error()); + EXPECT_EQ(err.at("error").as_string(), "lgrNotFound"); + EXPECT_EQ(err.at("error_message").as_string(), "ledgerNotFound"); + }); +} + TEST_F(RPCNoRippleCheckTest, LedgerNotExistViaHash) { auto const rawBackendPtr = static_cast(mockBackendPtr.get());