Refactor out a version of lookupLedger returning Status.

src/ripple/rpc/handlers/Ledger.cpp

@@ -31,57 +31,43 @@ LedgerHandler::LedgerHandler (Context& context) : context_ (context)
 {
 }
 
-Status LedgerHandler::check (Json::Value& error)
+Status LedgerHandler::check ()
 {
-    bool needsLedger = context_.params.isMember (jss::ledger) ||
-            context_.params.isMember (jss::ledger_hash) ||
-            context_.params.isMember (jss::ledger_index);
+    auto const& params = context_.params;
+    bool needsLedger = params.isMember (jss::ledger) ||
+            params.isMember (jss::ledger_hash) ||
+            params.isMember (jss::ledger_index);
     if (!needsLedger)
         return Status::OK;
 
-    lookupResult_ = RPC::lookupLedger (
-        context_.params, ledger_, context_.netOps);
+    if (auto s = RPC::lookupLedger (params, ledger_, context_.netOps, result_))
+        return s;
 
-    if (!ledger_)
-    {
-        error = lookupResult_;
-        auto code = error_code_i (error[jss::error_code].asInt());
-        return {code, {error[jss::error_message].asString()}};
-    }
+    bool bFull = params[jss::full].asBool();
+    bool bTransactions = params[jss::transactions].asBool();
+    bool bAccounts = params[jss::accounts].asBool();
+    bool bExpand = params[jss::expand].asBool();
 
-    bool bFull = context_.params.isMember (jss::full)
-            && context_.params[jss::full].asBool ();
-    bool bTransactions = context_.params.isMember (jss::transactions)
-            && context_.params[jss::transactions].asBool ();
-    bool bAccounts = context_.params.isMember (jss::accounts)
-            && context_.params[jss::accounts].asBool ();
-    bool bExpand = context_.params.isMember (jss::expand)
-            && context_.params[jss::expand].asBool ();
     options_ = (bFull ? LEDGER_JSON_FULL : 0)
             | (bExpand ? LEDGER_JSON_EXPAND : 0)
             | (bTransactions ? LEDGER_JSON_DUMP_TXRP : 0)
             | (bAccounts ? LEDGER_JSON_DUMP_STATE : 0);
 
     if (bFull || bAccounts)
-    {
-        if (context_.role != Role::ADMIN)
     {
         // Until some sane way to get full ledgers has been implemented,
         // disallow retrieving all state nodes.
-            error = rpcError (rpcNO_PERMISSION);
+        if (context_.role != Role::ADMIN)
             return rpcNO_PERMISSION;
-        }
 
         if (getApp().getFeeTrack().isLoadedLocal() &&
             context_.role != Role::ADMIN)
         {
-            error = rpcError(rpcTOO_BUSY);
             return rpcTOO_BUSY;
         }
         context_.loadType = Resource::feeHighBurdenRPC;
     }
 
-    std::cerr << "!!! LedgerHandler::check FIVE - true\n";
     return Status::OK;
 }
 

src/ripple/rpc/handlers/Ledger.h

@@ -40,7 +40,7 @@ class LedgerHandler {
 public:
     explicit LedgerHandler (Context&);
 
-    Status check (Json::Value& error);
+    Status check ();
 
     template <class Object>
     void writeResult (Object&);
@@ -63,7 +63,7 @@ public:
 private:
     Context& context_;
     Ledger::pointer ledger_;
-    Json::Value lookupResult_;
+    Json::Value result_;
     int options_;
 };
 
@@ -77,7 +77,7 @@ void LedgerHandler::writeResult (Object& value)
 {
     if (ledger_)
     {
-        RPC::copyFrom (value, lookupResult_);
+        RPC::copyFrom (value, result_);
         addJson (*ledger_, value, options_, context_.yield);
     }
     else

src/ripple/rpc/impl/Handler.cpp

@@ -35,6 +35,19 @@ Handler::Method<Json::Value> byRef (Function const& f)
     };
 }
 
+template <class Object, class HandlerImpl>
+Status handle (Context& context, Object& object)
+{
+    HandlerImpl handler (context);
+
+    auto status = handler.check ();
+    if (status)
+        status.inject (object);
+    else
+        handler.writeResult (object);
+    return status;
+};
+
 class HandlerTable {
   public:
     HandlerTable (std::vector<Handler> const& entries) {
@@ -56,39 +69,19 @@ class HandlerTable {
   private:
     std::map<std::string, Handler> table_;
 
-    template <class Impl>
+    template <class HandlerImpl>
     void addHandler()
     {
-        assert (table_.find(Impl::name()) == table_.end());
-        auto valueMethod = [] (Context& context, Json::Value& object)
-        {
-            Impl handler (context);
-            auto status = handler.check (object);
-            if (!status)
-                handler.writeResult (object);
-            return status;
-        };
+        assert (table_.find(HandlerImpl::name()) == table_.end());
 
-        auto objectMethod = [] (Context& context, Object& object)
-        {
-            Impl handler (context);
-            Json::Value error;
-            auto status = handler.check (error);
-            if (!status)
-                handler.writeResult (object);
-            else
-                RPC::copyFrom (object, error);
-            return status;
-        };
+        Handler h;
+        h.name_ = HandlerImpl::name(),
+        h.valueMethod_ = &handle<Json::Value, HandlerImpl>,
+        h.role_ = HandlerImpl::role(),
+        h.condition_ = HandlerImpl::condition(),
+        h.objectMethod_ = &handle<Object, HandlerImpl>;
 
-        auto handler = Handler {
-            Impl::name(),
-            valueMethod,
-            Impl::role(),
-            Impl::condition(),
-            objectMethod};
-
-        table_[Impl::name()] = handler;
+        table_[HandlerImpl::name()] = h;
     };
 };
 

src/ripple/rpc/impl/LookupLedger.cpp

@@ -45,10 +45,11 @@ static const int LEDGER_VALIDATED = -3;
 // return value.  Otherwise, the object contains the field "validated" and
 // optionally the fields "ledger_hash", "ledger_index" and
 // "ledger_current_index", if they are defined.
-Json::Value lookupLedger (
+Status lookupLedger (
     Json::Value const& params,
     Ledger::pointer& ledger,
-    NetworkOPs& netOps)
+    NetworkOPs& netOps,
+    Json::Value& jsonResult)
 {
     using RPC::make_error;
     ledger.reset();
@@ -74,7 +75,7 @@ Json::Value lookupLedger (
     uint256 ledgerHash;
 
     if (!jsonHash.isString() || !ledgerHash.SetHex (jsonHash.asString ()))
-        return make_error(rpcINVALID_PARAMS, "ledgerHashMalformed");
+        return {rpcINVALID_PARAMS, "ledgerHashMalformed"};
 
     std::int32_t ledgerIndex = LEDGER_CURRENT;
 
@@ -97,7 +98,7 @@ Json::Value lookupLedger (
             else if (index == "validated")
                 ledgerIndex = LEDGER_VALIDATED;
             else
-                return make_error(rpcINVALID_PARAMS, "ledgerIndexMalformed");
+                return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
         }
     }
     else
@@ -105,7 +106,7 @@ Json::Value lookupLedger (
         ledger = netOps.getLedgerByHash (ledgerHash);
 
         if (!ledger)
-            return make_error(rpcLGR_NOT_FOUND, "ledgerNotFound");
+            return {rpcLGR_NOT_FOUND, "ledgerNotFound"};
 
         ledgerIndex = ledger->getLedgerSeq ();
     }
@@ -128,7 +129,7 @@ Json::Value lookupLedger (
             break;
 
         default:
-            return make_error(rpcINVALID_PARAMS, "ledgerIndexMalformed");
+            return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
         }
 
         assert (ledger->isImmutable());
@@ -142,10 +143,9 @@ Json::Value lookupLedger (
         ledger = netOps.getLedgerBySeq (ledgerIndex);
 
         if (!ledger)
-            return make_error(rpcLGR_NOT_FOUND, "ledgerNotFound");
+            return {rpcLGR_NOT_FOUND, "ledgerNotFound"};
     }
 
-    Json::Value jsonResult;
     if (ledger->isClosed ())
     {
         if (ledgerHash != zero)
@@ -190,7 +190,19 @@ Json::Value lookupLedger (
         }
     }
 
-    return jsonResult;
+    return Status::OK;
+}
+
+Json::Value lookupLedger (
+    Json::Value const& params,
+    Ledger::pointer& ledger,
+    NetworkOPs& netOps)
+{
+    Json::Value value (Json::objectValue);
+    if (auto status = lookupLedger (params, ledger, netOps, value))
+        status.inject (value);
+
+    return value;
 }
 
 } // RPC

src/ripple/rpc/impl/LookupLedger.h

@@ -20,13 +20,29 @@
 #ifndef RIPPLE_RPC_LOOKUPLEDGER_H_INCLUDED
 #define RIPPLE_RPC_LOOKUPLEDGER_H_INCLUDED
 
+#include <ripple/rpc/Status.h>
+
 namespace ripple {
 namespace RPC {
 
+/** Look up a ledger from a request and fill a Json::Result with either
+    an error, or data representing a ledger.
+
+    If there is no error in the return value, then the ledger pointer will have
+    been filled.
+*/
 Json::Value lookupLedger (
-    Json::Value const& jvRequest,
-    Ledger::pointer& lpLedger,
-    NetworkOPs& netOps);
+    Json::Value const& request, Ledger::pointer&, NetworkOPs&);
+
+/** Look up a ledger from a request and fill a Json::Result with the data
+    representing a ledger.
+
+    If the returned Status is OK, the ledger pointer will have been filled. */
+Status lookupLedger (
+    Json::Value const& request,
+    Ledger::pointer&,
+    NetworkOPs&,
+    Json::Value& result);
 
 } // RPC
 } // ripple