Changes the accounts_representatives RPC to return per account response (#3789)

* Changes the accounts_representatives RPC to return per account results
* Adds an unit test for checking per account results

nano/node/json_handler.cpp

@@ -14,6 +14,7 @@
 
 #include <algorithm>
 #include <chrono>
+#include <vector>
 
 namespace
 {
@@ -911,19 +912,22 @@ void nano::json_handler::accounts_balances ()
 void nano::json_handler::accounts_representatives ()
 {
 	boost::property_tree::ptree representatives;
-	for (auto & accounts : request.get_child ("accounts"))
+	auto transaction = node.store.tx_begin_read ();
+	for (auto & account_from_request : request.get_child ("accounts"))
 	{
-		auto account (account_impl (accounts.second.data ()));
+		auto account = account_impl (account_from_request.second.data ());
-		auto transaction (node.store.tx_begin_read ());
-		auto info (account_info_impl (transaction, account));
-
 		if (!ec)
 		{
-			boost::property_tree::ptree entry;
+			auto info = account_info_impl (transaction, account);
-			entry.put ("", info.representative.to_account ());
+			if (!ec)
-			representatives.push_back (std::make_pair (accounts.second.data (), entry));
+			{
+				representatives.put (account_from_request.second.data (), info.representative.to_account ());
+				continue;
 			}
 		}
+		representatives.put (account_from_request.second.data (), boost::format ("error: %1%") % ec.message ());
+		ec = {};
+	}
 	response_l.add_child ("representatives", representatives);
 	response_errors ();
 }

nano/rpc_test/rpc.cpp

@@ -3157,6 +3157,7 @@ TEST (rpc, accounts_balances)
 	}
 }
 
+// Tests the  happy path of retrieving an account's representative
 TEST (rpc, accounts_representatives)
 {
 	nano::system system;
@@ -3166,14 +3167,68 @@ TEST (rpc, accounts_representatives)
 	request.put ("action", "accounts_representatives");
 	boost::property_tree::ptree entry;
 	boost::property_tree::ptree accounts;
+	// Adds a valid account present in the ledger.
 	entry.put ("", nano::dev::genesis_key.pub.to_account ());
 	accounts.push_back (std::make_pair ("", entry));
 	request.add_child ("accounts", accounts);
 	auto response (wait_response (system, rpc_ctx, request));
+	// Ensures the response is correct.
 	auto response_representative (response.get_child ("representatives").get<std::string> (nano::dev::genesis->account ().to_account ()));
 	ASSERT_EQ (response_representative, nano::dev::genesis->account ().to_account ());
 }
 
+// Tests the accounts_representatives RPC for expected per account results with
+// two possible error conditions: bad account number and account not found.
+TEST (rpc, accounts_representatives_per_account_result_with_errors)
+{
+	nano::system system;
+	auto node = add_ipc_enabled_node (system);
+	auto const rpc_ctx = add_rpc (system, node);
+	boost::property_tree::ptree request;
+	request.put ("action", "accounts_representatives");
+	boost::property_tree::ptree entry1, entry2, entry3;
+	boost::property_tree::ptree accounts_l;
+
+	// Adds a valid account present in the ledger.
+	entry1.put ("", nano::dev::genesis_key.pub.to_account ());
+	accounts_l.push_back (std::make_pair ("", entry1));
+
+	// Adds an invalid account, malformed number with a wrong checksum.
+	// Got with this formula: key1.substr(0, 40) + key2.substr(40, key2.size()).
+	auto const invalid_key = "nano_36uccgpjzhjsdbj44wm1y5hyz8gefx3wjpp1jircxt84nopxkxti5bzq1rnz";
+	entry2.put ("", invalid_key);
+	accounts_l.push_back (std::make_pair ("", entry2));
+
+	// Adds a valid key but that isn't on the ledger. It wont'be found.
+	auto const valid_key = "nano_1hrts7hcoozxccnffoq9hqhngnn9jz783usapejm57ejtqcyz9dpso1bibuy";
+	entry3.put ("", valid_key);
+	accounts_l.push_back (std::make_pair ("", entry3));
+
+	// Packs all the account entries.
+	request.add_child ("accounts", accounts_l);
+	auto response (wait_response (system, rpc_ctx, request));
+
+	auto get_error_message = [] (nano::error_common error_common) -> std::string {
+		std::error_code ec = error_common;
+		return boost::str (boost::format ("error: %1%") % ec.message ());
+	};
+
+	std::map<std::string, std::string> reply_map{
+		{ nano::dev::genesis_key.pub.to_account (), nano::dev::genesis->account ().to_account () },
+		{ invalid_key, get_error_message (nano::error_common::bad_account_number) },
+		{ valid_key, get_error_message (nano::error_common::account_not_found) }
+	};
+
+	for (auto & representative : response.get_child ("representatives"))
+	{
+		std::string account_text = representative.first;
+		std::string frontier_text = representative.second.get<std::string> ("");
+		ASSERT_EQ (frontier_text, reply_map[account_text]);
+		reply_map.erase (account_text);
+	}
+	ASSERT_EQ (reply_map.size (), 0);
+}
+
 TEST (rpc, accounts_frontiers)
 {
 	nano::system system;