Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unit test api_version to enable api_version 2 #4785

Merged
merged 5 commits into from
Nov 2, 2023
Merged

Fix unit test api_version to enable api_version 2 #4785

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
737a89f
Fix unit test api_version to enable api_version 2
pwang200 Oct 25, 2023
c15fd32
move unit test specific to src/test/jtx, default apiVersion to max
pwang200 Oct 26, 2023
2d3d91c
clang-format
intelliot Oct 31, 2023
e1d1c27
make journal the last parameter
pwang200 Oct 31, 2023
3e9b260
Merge branch 'develop' into api_version-fix
intelliot Nov 2, 2023
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
11 changes: 8 additions & 3 deletions src/ripple/net/RPCCall.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,23 @@ fromNetwork(
std::unordered_map<std::string, std::string> headers = {});
} // namespace RPCCall

/** Given a rippled command line, return the corresponding JSON.
*/

intelliot marked this conversation as resolved.
Show resolved Hide resolved
Json::Value
cmdLineToJSONRPC(std::vector<std::string> const& args, beast::Journal j);
rpcCmdToJson(
std::vector<std::string> const& args,
Json::Value& retParams,
beast::Journal j,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Usually we make journal the last parameter.

unsigned int apiVersion);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe not for this PR, but we should consider making apiVersion either an enum type with one enum per API version or maybe a strongly-typed int (int with a tag, like we do with base_uint).


/** Internal invocation of RPC client.
* Used by both rippled command line as well as rippled unit tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this comment ! 👍

*/
std::pair<int, Json::Value>
rpcClient(
std::vector<std::string> const& args,
Config const& config,
Logs& logs,
unsigned int apiVersion,
std::unordered_map<std::string, std::string> const& headers = {});

} // namespace ripple
Expand Down
47 changes: 11 additions & 36 deletions src/ripple/net/impl/RPCCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1472,11 +1472,12 @@ struct RPCCallImp
//------------------------------------------------------------------------------

// Used internally by rpcClient.
static Json::Value
rpcCmdLineToJson(
Json::Value
rpcCmdToJson(
std::vector<std::string> const& args,
Json::Value& retParams,
beast::Journal j)
beast::Journal j,
unsigned int apiVersion)
{
Json::Value jvRequest(Json::objectValue);

Expand All @@ -1493,11 +1494,11 @@ rpcCmdLineToJson(

jvRequest = rpParser.parseCommand(args[0], jvRpcParams, true);

auto insert_api_version = [](Json::Value& jr) {
auto insert_api_version = [apiVersion](Json::Value& jr) {
if (jr.isObject() && !jr.isMember(jss::error) &&
!jr.isMember(jss::api_version))
{
jr[jss::api_version] = RPC::apiMaximumSupportedVersion;
jr[jss::api_version] = apiVersion;
}
};

Expand All @@ -1510,42 +1511,14 @@ rpcCmdLineToJson(
return jvRequest;
}

Json::Value
cmdLineToJSONRPC(std::vector<std::string> const& args, beast::Journal j)
{
Json::Value jv = Json::Value(Json::objectValue);
auto const paramsObj = rpcCmdLineToJson(args, jv, j);

// Re-use jv to return our formatted result.
jv.clear();

// Allow parser to rewrite method.
jv[jss::method] = paramsObj.isMember(jss::method)
? paramsObj[jss::method].asString()
: args[0];

// If paramsObj is not empty, put it in a [params] array.
if (paramsObj.begin() != paramsObj.end())
{
auto& paramsArray = Json::setArray(jv, jss::params);
paramsArray.append(paramsObj);
}
if (paramsObj.isMember(jss::jsonrpc))
jv[jss::jsonrpc] = paramsObj[jss::jsonrpc];
if (paramsObj.isMember(jss::ripplerpc))
jv[jss::ripplerpc] = paramsObj[jss::ripplerpc];
if (paramsObj.isMember(jss::id))
jv[jss::id] = paramsObj[jss::id];
return jv;
}

//------------------------------------------------------------------------------

std::pair<int, Json::Value>
rpcClient(
std::vector<std::string> const& args,
Config const& config,
Logs& logs,
unsigned int apiVersion,
std::unordered_map<std::string, std::string> const& headers)
{
static_assert(
Expand All @@ -1561,7 +1534,8 @@ rpcClient(
try
{
Json::Value jvRpc = Json::Value(Json::objectValue);
jvRequest = rpcCmdLineToJson(args, jvRpc, logs.journal("RPCParser"));
jvRequest =
rpcCmdToJson(args, jvRpc, logs.journal("RPCParser"), apiVersion);

if (jvRequest.isMember(jss::error))
{
Expand Down Expand Up @@ -1698,7 +1672,8 @@ fromCommandLine(
const std::vector<std::string>& vCmd,
Logs& logs)
{
auto const result = rpcClient(vCmd, config, logs);
auto const result =
rpcClient(vCmd, config, logs, RPC::apiMaximumSupportedVersion);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note, this means that as soon as we bump apiMaximumSupportedVersion, the tx_history and ledger_header commands will stop working (as they are no longer recognized in API version 2). Ideally at this point we should remove them from parseCommand in this file (and also remove function RPCParser::parseTxHistory). This probably needs its own issue.


std::cout << result.second.toStyledString();

Expand Down
8 changes: 7 additions & 1 deletion src/test/jtx/impl/Env.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,13 @@ Env::do_rpc(
std::vector<std::string> const& args,
std::unordered_map<std::string, std::string> const& headers)
{
return rpcClient(args, app().config(), app().logs(), headers).second;
return rpcClient(
args,
app().config(),
app().logs(),
RPC::apiMaximumSupportedVersion,
headers)
.second;
}

void
Expand Down
34 changes: 34 additions & 0 deletions src/test/jtx/impl/utility.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
//==============================================================================

#include <ripple/basics/contract.h>
#include <ripple/json/Object.h>
#include <ripple/net/RPCCall.h>
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/protocol/HashPrefix.h>
#include <ripple/protocol/Indexes.h>
Expand Down Expand Up @@ -73,6 +75,38 @@ fill_seq(Json::Value& jv, ReadView const& view)
jv[jss::Sequence] = ar->getFieldU32(sfSequence);
}

Json::Value
cmdToJSONRPC(
std::vector<std::string> const& args,
beast::Journal j,
unsigned int apiVersion)
{
Json::Value jv = Json::Value(Json::objectValue);
auto const paramsObj = rpcCmdToJson(args, jv, j, apiVersion);

// Re-use jv to return our formatted result.
jv.clear();

// Allow parser to rewrite method.
jv[jss::method] = paramsObj.isMember(jss::method)
? paramsObj[jss::method].asString()
: args[0];

// If paramsObj is not empty, put it in a [params] array.
if (paramsObj.begin() != paramsObj.end())
{
auto& paramsArray = Json::setArray(jv, jss::params);
paramsArray.append(paramsObj);
}
if (paramsObj.isMember(jss::jsonrpc))
jv[jss::jsonrpc] = paramsObj[jss::jsonrpc];
if (paramsObj.isMember(jss::ripplerpc))
jv[jss::ripplerpc] = paramsObj[jss::ripplerpc];
if (paramsObj.isMember(jss::id))
jv[jss::id] = paramsObj[jss::id];
return jv;
}

} // namespace jtx
} // namespace test
} // namespace ripple
8 changes: 8 additions & 0 deletions src/test/jtx/utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <ripple/app/ledger/Ledger.h>
#include <ripple/json/json_value.h>
#include <ripple/protocol/STObject.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <stdexcept>
#include <test/jtx/Account.h>

Expand Down Expand Up @@ -61,6 +62,13 @@ fill_fee(Json::Value& jv, ReadView const& view);
void
fill_seq(Json::Value& jv, ReadView const& view);

/** Given a rippled unit test rpc command, return the corresponding JSON. */
Json::Value
cmdToJSONRPC(
std::vector<std::string> const& args,
beast::Journal j,
unsigned int apiVersion = RPC::apiMaximumSupportedVersion);

} // namespace jtx
} // namespace test
} // namespace ripple
Expand Down
3 changes: 2 additions & 1 deletion src/test/rpc/RPCCall_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <test/jtx.h>
#include <test/jtx/utility.h>

#include <boost/algorithm/string.hpp>
#include <initializer_list>
Expand Down Expand Up @@ -6442,7 +6443,7 @@ class RPCCall_test : public beast::unit_test::suite
Json::Value got;
try
{
got = cmdLineToJSONRPC(args, env.journal);
got = jtx::cmdToJSONRPC(args, env.journal);
}
catch (std::bad_cast const&)
{
Expand Down
Loading