diff --git a/src/Makefile.am b/src/Makefile.am --- a/src/Makefile.am +++ b/src/Makefile.am @@ -165,6 +165,7 @@ reverselock.h \ rpc/blockchain.h \ rpc/client.h \ + rpc/command.h \ rpc/jsonrpcrequest.h \ rpc/mining.h \ rpc/misc.h \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -86,6 +86,7 @@ test/rcu_tests.cpp \ test/reverselock_tests.cpp \ test/rpc_tests.cpp \ + test/rpc_server_tests.cpp \ test/rwcollection_tests.cpp \ test/sanity_tests.cpp \ test/scheduler_tests.cpp \ diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -47,8 +47,6 @@ TestingSetup test; - SetRPCWarmupFinished(); - std::string result; std::string result2; std::string filtered; diff --git a/src/rpc/command.h b/src/rpc/command.h new file mode 100644 --- /dev/null +++ b/src/rpc/command.h @@ -0,0 +1,36 @@ +// Copyright (c) 2018-2019 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_RPC_COMMAND_H +#define BITCOIN_RPC_COMMAND_H + +#include + +#include + +/** + * Base class for all RPC commands. + */ +class RPCCommand { +private: + const std::string name; + + /* + * Child classes should define dependencies as private members. + * These dependencies must not be changed, or successive calls to the same + * command will give unexpected behavior. + */ + + // TODO: Parameter definitions (these will be used to generate help + // messages as well) + +public: + RPCCommand(std::string nameIn) : name(nameIn) {} + + virtual UniValue Execute(const UniValue &args) const = 0; + + std::string GetName() const { return name; }; +}; + +#endif // BITCOIN_RPC_COMMAND_H diff --git a/src/rpc/server.h b/src/rpc/server.h --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -1,6 +1,6 @@ // Copyright (c) 2010 Satoshi Nakamoto // Copyright (c) 2009-2016 The Bitcoin Core developers -// Copyright (c) 2017-2018 The Bitcoin developers +// Copyright (c) 2017-2019 The Bitcoin developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -8,6 +8,7 @@ #define BITCOIN_RPC_SERVER_H #include "amount.h" +#include "rpc/command.h" #include "rpc/jsonrpcrequest.h" #include "rpc/protocol.h" #include "uint256.h" @@ -51,6 +52,9 @@ * Class for registering and managing all RPC calls. */ class RPCServer : public boost::noncopyable { +private: + std::map> commands; + public: RPCServer() {} @@ -60,6 +64,17 @@ */ UniValue ExecuteCommand(Config &config, const JSONRPCRequest &request) const; + + /** + * Register an RPC command. + */ + void RegisterCommand(std::unique_ptr command); + + /** + * Return a non-owning pointer to an RPCCommand if one matches the given + * command name. Returns a nullptr RPCCommand* if no command matches. + */ + RPCCommand *MatchCommand(std::string name) const; }; /** diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -1,5 +1,6 @@ // Copyright (c) 2010 Satoshi Nakamoto // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2018-2019 The Bitcoin developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -31,6 +32,7 @@ static bool fRPCInWarmup = true; static std::string rpcWarmupStatus("RPC server started"); static CCriticalSection cs_rpcWarmup; +static CCriticalSection cs_rpcRegister; /* Timer-creating functions */ static RPCTimerInterface *timerInterface = nullptr; /* Map of name to timer. */ @@ -46,13 +48,39 @@ } } - // TODO Only call tableRPC.execute() if no context-sensitive RPC command - // exists + std::string commandName = request.strMethod; + RPCCommand *command = MatchCommand(commandName); + if (command != nullptr) { + try { + return command->Execute(request.params); + } catch (const std::exception &e) { + throw JSONRPCError(RPC_MISC_ERROR, e.what()); + } + } + + // TODO Remove the below call to tableRPC.execute() and only call it for + // context-free RPC commands via an implementation of RPCCommand. // Check if context-free RPC method is valid and execute it return tableRPC.execute(config, request); } +void RPCServer::RegisterCommand(std::unique_ptr command) { + if (command != nullptr) { + LOCK(cs_rpcRegister); + commands[command->GetName()] = std::move(command); + } +} + +RPCCommand *RPCServer::MatchCommand(std::string name) const { + auto iter = commands.find(name); + if (iter == commands.end()) { + return nullptr; + } + + return iter->second.get(); +} + static struct CRPCSignals { boost::signals2::signal Started; boost::signals2::signal Stopped; diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -102,6 +102,7 @@ rcu_tests.cpp reverselock_tests.cpp rpc_tests.cpp + rpc_server_tests.cpp rwcollection_tests.cpp sanity_tests.cpp scheduler_tests.cpp diff --git a/src/test/rpc_server_tests.cpp b/src/test/rpc_server_tests.cpp new file mode 100644 --- /dev/null +++ b/src/test/rpc_server_tests.cpp @@ -0,0 +1,64 @@ +// Copyright (c) 2018-2019 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "chainparams.h" +#include "config.h" +#include "rpc/jsonrpcrequest.h" +#include "rpc/server.h" +#include "util.h" + +#include "test/test_bitcoin.h" + +#include + +#include + +class TestRPCCommand : public RPCCommand { +public: + TestRPCCommand(std::string nameIn) : RPCCommand(nameIn) {} + + UniValue Execute(const UniValue &args) const override { return UniValue(); } +}; + +BOOST_FIXTURE_TEST_SUITE(rpc_server_tests, TestingSetup) + +BOOST_AUTO_TEST_CASE(rpc_server_match_command) { + RPCServer rpcServer; + const std::string commandName = "testcommand"; + rpcServer.RegisterCommand(MakeUnique(commandName)); + + BOOST_CHECK(rpcServer.MatchCommand(commandName)); + + // Checking non-existent command errors as expected + BOOST_CHECK_EQUAL(rpcServer.MatchCommand("this-command-does-not-exist"), + NULLPTR(RPCCommand)); +} + +class ArgsTestRPCCommand : public RPCCommand { +public: + ArgsTestRPCCommand(std::string nameIn) : RPCCommand(nameIn) {} + + UniValue Execute(const UniValue &args) const override { + BOOST_CHECK_EQUAL(args["arg1"].get_str(), "value1"); + return UniValue(); + } +}; + +BOOST_AUTO_TEST_CASE(rpc_server_execute_command) { + DummyConfig config; + RPCServer rpcServer; + const std::string commandName = "testcommand"; + rpcServer.RegisterCommand(MakeUnique(commandName)); + + UniValue args(UniValue::VOBJ); + args.pushKV("arg1", "value1"); + + JSONRPCRequest request; + request.strMethod = commandName; + request.params = args; + + rpcServer.ExecuteCommand(config, request); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -83,6 +83,18 @@ // instead of unit tests, but for now we need these here. RPCServer rpcServer; RegisterAllRPCCommands(config, rpcServer, tableRPC); + + /** + * This fixes a bug which is not apparent in most tests since test coverage + * for RPC code is either poor or non-existent. RPC does not come out of + * warmup state on its own and requires intervention by the testing + * framework if the full bitcoind init path is not taken. + */ + std::string rpcWarmupStatus; + if (RPCIsInWarmup(&rpcWarmupStatus)) { + SetRPCWarmupFinished(); + } + ClearDatadirCache(); pathTemp = fs::temp_directory_path() / strprintf("test_bitcoin_%lu_%i", (unsigned long)GetTime(),