From 9111ad1a9dc37d49d085aa317712625e635197c0 Mon Sep 17 00:00:00 2001 From: Tom Ritchford Date: Thu, 18 Jun 2015 15:37:29 -0400 Subject: [PATCH] Handle empty Json values: * Replace Json::Value::isNull() and Json::Value::empty with operator bool() --- .../app/misc/tests/AccountTxPaging.test.cpp | 12 +++---- src/ripple/app/paths/PathRequest.cpp | 2 +- src/ripple/json/impl/Object.cpp | 2 +- src/ripple/json/impl/json_value.cpp | 20 ++++------- src/ripple/json/json_value.h | 12 +++---- src/ripple/json/tests/json_value.test.cpp | 33 +++++++++---------- src/ripple/net/impl/RPCCall.cpp | 2 +- src/ripple/protocol/impl/STParsedJSON.cpp | 6 ++-- src/ripple/protocol/tests/STObject.test.cpp | 6 ++-- src/ripple/rpc/handlers/AccountCurrencies.cpp | 2 +- src/ripple/rpc/handlers/AccountInfo.cpp | 2 +- src/ripple/rpc/handlers/AccountLines.cpp | 4 +-- src/ripple/rpc/handlers/AccountObjects.cpp | 2 +- src/ripple/rpc/handlers/AccountOffers.cpp | 2 +- src/ripple/rpc/handlers/AccountTx.cpp | 2 +- src/ripple/rpc/handlers/GatewayBalances.cpp | 2 +- src/ripple/rpc/handlers/NoRippleCheck.cpp | 2 +- src/ripple/rpc/handlers/OwnerInfo.cpp | 5 ++- src/ripple/rpc/impl/LookupLedger.cpp | 4 +-- src/ripple/rpc/tests/Status.test.cpp | 14 ++++---- src/ripple/server/impl/ServerHandlerImp.cpp | 6 ++-- src/ripple/websocket/Handler.h | 2 +- 22 files changed, 67 insertions(+), 77 deletions(-) diff --git a/src/ripple/app/misc/tests/AccountTxPaging.test.cpp b/src/ripple/app/misc/tests/AccountTxPaging.test.cpp index 6fb5b6721..7bf7069db 100644 --- a/src/ripple/app/misc/tests/AccountTxPaging.test.cpp +++ b/src/ripple/app/misc/tests/AccountTxPaging.test.cpp @@ -151,8 +151,8 @@ struct AccountTxPaging_test : beast::unit_test::suite expect (next(limit, forward, token, min_ledger, max_ledger) == 1); checkTransaction (txs_[0], 5, 7); - expect(token["ledger"].isNull()); - expect(token["seq"].isNull()); + expect(! token["ledger"]); + expect(! token["seq"]); } token = Json::nullValue; @@ -200,8 +200,8 @@ struct AccountTxPaging_test : beast::unit_test::suite expect(next(limit, forward, token, min_ledger, max_ledger) == 1); checkTransaction (txs_[0], 6, 11); - expect(token["ledger"].isNull()); - expect(token["seq"].isNull()); + expect(! token["ledger"]); + expect(! token["seq"]); } token = Json::nullValue; @@ -241,8 +241,8 @@ struct AccountTxPaging_test : beast::unit_test::suite checkTransaction (txs_[2], 3, 5); } - expect (token["ledger"].isNull()); - expect (token["seq"].isNull()); + expect (! token["ledger"]); + expect (! token["seq"]); } }; diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index 3c40ece6f..4b6ff9723 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -452,7 +452,7 @@ Json::Value PathRequest::doUpdate (RippleLineCache::ref cache, bool fast) jvStatus[jss::destination_account] = getApp().accountIDCache().toBase58(*raDstAccount); jvStatus[jss::destination_amount] = saDstAmount.getJson (0); - if (!jvId.isNull ()) + if (jvId) jvStatus["id"] = jvId; Json::Value jvArray = Json::arrayValue; diff --git a/src/ripple/json/impl/Object.cpp b/src/ripple/json/impl/Object.cpp index d7f30dbf6..ebd081906 100644 --- a/src/ripple/json/impl/Object.cpp +++ b/src/ripple/json/impl/Object.cpp @@ -204,7 +204,7 @@ void doCopyFrom (Object& to, Json::Value const& from) void copyFrom (Json::Value& to, Json::Value const& from) { - if (to.empty()) // Short circuit this very common case. + if (!to) // Short circuit this very common case. to = from; else doCopyFrom (to, from); diff --git a/src/ripple/json/impl/json_value.cpp b/src/ripple/json/impl/json_value.cpp index cb22da2ac..c58083829 100644 --- a/src/ripple/json/impl/json_value.cpp +++ b/src/ripple/json/impl/json_value.cpp @@ -822,28 +822,20 @@ Value::size () const } -bool -Value::empty () const +Value::operator bool () const { if (isNull ()) - return true; + return false; - if (isString ()) { + if (isString ()) + { auto s = asCString(); - return !(s && strlen(s)); + return s && strlen(s); } - return (isArray () || isObject ()) && ! size (); + return ! (isArray () || isObject ()) || size (); } - -bool -Value::operator! () const -{ - return isNull (); -} - - void Value::clear () { diff --git a/src/ripple/json/json_value.h b/src/ripple/json/json_value.h index d3420bdb0..da1fdf895 100644 --- a/src/ripple/json/json_value.h +++ b/src/ripple/json/json_value.h @@ -253,6 +253,8 @@ public: double asDouble () const; bool asBool () const; + /** isNull() tests to see if this field is null. Don't use this method to + test for emptiness: use empty(). */ bool isNull () const; bool isBool () const; bool isInt () const; @@ -269,12 +271,10 @@ public: /// Number of values in array or object UInt size () const; - /// \brief Return true if empty array, empty object, or null; - /// otherwise, false. - bool empty () const; - - /// Return isNull() - bool operator! () const; + /** Returns false if this is an empty array, empty object, empty string, + or null. */ + explicit + operator bool() const; /// Remove all object members and array elements. /// \pre type() is arrayValue, objectValue, or nullValue diff --git a/src/ripple/json/tests/json_value.test.cpp b/src/ripple/json/tests/json_value.test.cpp index 83c7cdece..9f55aa2d0 100644 --- a/src/ripple/json/tests/json_value.test.cpp +++ b/src/ripple/json/tests/json_value.test.cpp @@ -25,30 +25,29 @@ namespace ripple { -class json_value_test : public beast::unit_test::suite +struct json_value_test : beast::unit_test::suite { -public: - void test_empty() + void test_bool() { - expect (Json::Value().empty()); + expect (! Json::Value()); - expect (Json::Value("").empty()); + expect (! Json::Value("")); - expect (! Json::Value("empty").empty()); - expect (! Json::Value(false).empty()); - expect (! Json::Value(true).empty()); - expect (! Json::Value(0).empty()); - expect (! Json::Value(1).empty()); + expect (bool (Json::Value("empty"))); + expect (bool (Json::Value(false))); + expect (bool (Json::Value(true))); + expect (bool (Json::Value(0))); + expect (bool (Json::Value(1))); Json::Value array (Json::arrayValue); - expect (array.empty()); + expect (! array); array.append(0); - expect (!array.empty()); + expect (bool (array)); Json::Value object (Json::objectValue); - expect (object.empty()); + expect (! object); object[""] = false; - expect (!object.empty()); + expect (bool (object)); } void test_bad_json () @@ -148,7 +147,7 @@ public: expect (v1.asDouble () == 2.5); Json::Value v2 = std::move(v1); - expect (v1.isNull ()); + expect (!v1); expect (v2.isDouble ()); expect (v2.asDouble () == 2.5); expect (v1 != v2); @@ -156,7 +155,7 @@ public: v1 = std::move(v2); expect (v1.isDouble ()); expect (v1.asDouble () == 2.5); - expect (v2.isNull ()); + expect (! v2); expect (v1 != v2); pass (); @@ -220,7 +219,7 @@ public: void run () { - test_empty (); + test_bool (); test_bad_json (); test_edge_cases (); test_copy (); diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index ebde04099..444cb74ad 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -1047,7 +1047,7 @@ struct RPCCallImp if (!reader.parse (strData, jvReply)) throw std::runtime_error ("couldn't parse reply from server"); - if (jvReply.isNull ()) + if (!jvReply) throw std::runtime_error ("expected reply to have result, error and id properties"); Json::Value jvResult (Json::objectValue); diff --git a/src/ripple/protocol/impl/STParsedJSON.cpp b/src/ripple/protocol/impl/STParsedJSON.cpp index 85ddd0a65..475644a54 100644 --- a/src/ripple/protocol/impl/STParsedJSON.cpp +++ b/src/ripple/protocol/impl/STParsedJSON.cpp @@ -532,7 +532,7 @@ static boost::optional parseLeaf ( AccountID uAccount, uIssuer; Currency uCurrency; - if (! account.isNull ()) + if (account) { // human account id if (! account.isString ()) @@ -556,7 +556,7 @@ static boost::optional parseLeaf ( } } - if (!currency.isNull ()) + if (currency) { // human currency if (!currency.isString ()) @@ -577,7 +577,7 @@ static boost::optional parseLeaf ( } } - if (!issuer.isNull ()) + if (issuer) { // human account id if (!issuer.isString ()) diff --git a/src/ripple/protocol/tests/STObject.test.cpp b/src/ripple/protocol/tests/STObject.test.cpp index 505764ada..bc53d02ae 100644 --- a/src/ripple/protocol/tests/STObject.test.cpp +++ b/src/ripple/protocol/tests/STObject.test.cpp @@ -44,9 +44,9 @@ public: bool parseJSONString (std::string const& json, Json::Value& to) { Json::Reader reader; - return (reader.parse(json, to) && - !to.isNull() && - to.isObject()); + return reader.parse(json, to) && + bool (to) && + to.isObject(); } void testParseJSONArrayWithInvalidChildrenObjects () diff --git a/src/ripple/rpc/handlers/AccountCurrencies.cpp b/src/ripple/rpc/handlers/AccountCurrencies.cpp index d355a6977..c49ed19b1 100644 --- a/src/ripple/rpc/handlers/AccountCurrencies.cpp +++ b/src/ripple/rpc/handlers/AccountCurrencies.cpp @@ -61,7 +61,7 @@ Json::Value doAccountCurrencies (RPC::Context& context) Json::Value jvAccepted ( RPC::accountFromString (accountID, bIndex, strIdent, iIndex, bStrict)); - if (!jvAccepted.empty ()) + if (jvAccepted) return jvAccepted; std::set send, receive; diff --git a/src/ripple/rpc/handlers/AccountInfo.cpp b/src/ripple/rpc/handlers/AccountInfo.cpp index 16fd4294c..3f2bdd421 100644 --- a/src/ripple/rpc/handlers/AccountInfo.cpp +++ b/src/ripple/rpc/handlers/AccountInfo.cpp @@ -60,7 +60,7 @@ Json::Value doAccountInfo (RPC::Context& context) auto jvAccepted = RPC::accountFromString ( accountID, bIndex, strIdent, iIndex, bStrict); - if (!jvAccepted.empty ()) + if (jvAccepted) return jvAccepted; auto const sleAccepted = cachedRead(*ledger, diff --git a/src/ripple/rpc/handlers/AccountLines.cpp b/src/ripple/rpc/handlers/AccountLines.cpp index 5a8bc8bdd..61265d42f 100644 --- a/src/ripple/rpc/handlers/AccountLines.cpp +++ b/src/ripple/rpc/handlers/AccountLines.cpp @@ -95,7 +95,7 @@ Json::Value doAccountLines (RPC::Context& context) auto jv = RPC::accountFromString ( accountID, bIndex, strIdent, iIndex, false); - if (! jv.empty ()) + if (jv) { for (auto it = jv.begin (); it != jv.end (); ++it) result[it.memberName ()] = it.key (); @@ -125,7 +125,7 @@ Json::Value doAccountLines (RPC::Context& context) result = RPC::accountFromString ( raPeerAccount, bPeerIndex, strPeer, iPeerIndex, false); - if (! result.empty ()) + if (result) return result; } diff --git a/src/ripple/rpc/handlers/AccountObjects.cpp b/src/ripple/rpc/handlers/AccountObjects.cpp index ff0f1ada7..4e6ad6836 100644 --- a/src/ripple/rpc/handlers/AccountObjects.cpp +++ b/src/ripple/rpc/handlers/AccountObjects.cpp @@ -60,7 +60,7 @@ Json::Value doAccountObjects (RPC::Context& context) ? context.params[jss::account_index].asUInt () : 0; auto jv = RPC::accountFromString ( accountID, bIndex, strIdent, iIndex, false); - if (! jv.empty ()) + if (jv) { for (auto it = jv.begin (); it != jv.end (); ++it) result[it.memberName ()] = it.key (); diff --git a/src/ripple/rpc/handlers/AccountOffers.cpp b/src/ripple/rpc/handlers/AccountOffers.cpp index cc4f215fc..886a15310 100644 --- a/src/ripple/rpc/handlers/AccountOffers.cpp +++ b/src/ripple/rpc/handlers/AccountOffers.cpp @@ -50,7 +50,7 @@ Json::Value doAccountOffers (RPC::Context& context) Json::Value const jv = RPC::accountFromString ( accountID, bIndex, strIdent, iIndex, false); - if (! jv.empty ()) + if (jv) { for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it) result[it.memberName ()] = it.key (); diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 4312ac1a7..ebacc77a8 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -162,7 +162,7 @@ Json::Value doAccountTx (RPC::Context& context) ret[jss::ledger_index_max] = uLedgerMax; if (params.isMember (jss::limit)) ret[jss::limit] = limit; - if (!resumeToken.isNull()) + if (resumeToken) ret[jss::marker] = resumeToken; return ret; diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 7097d8cf4..9763d3a77 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -82,7 +82,7 @@ Json::Value doGatewayBalances (RPC::Context& context) Json::Value jvAccepted = RPC::accountFromString ( accountID, bIndex, strIdent, iIndex, bStrict); - if (!jvAccepted.empty ()) + if (jvAccepted) return jvAccepted; context.loadType = Resource::feeHighBurdenRPC; diff --git a/src/ripple/rpc/handlers/NoRippleCheck.cpp b/src/ripple/rpc/handlers/NoRippleCheck.cpp index 670d1e598..967e9031c 100644 --- a/src/ripple/rpc/handlers/NoRippleCheck.cpp +++ b/src/ripple/rpc/handlers/NoRippleCheck.cpp @@ -100,7 +100,7 @@ Json::Value doNoRippleCheck (RPC::Context& context) Json::Value const jv = RPC::accountFromString ( accountID, bIndex, strIdent, iIndex, false); - if (! jv.empty ()) + if (jv) { for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it) result[it.memberName ()] = it.key (); diff --git a/src/ripple/rpc/handlers/OwnerInfo.cpp b/src/ripple/rpc/handlers/OwnerInfo.cpp index d05c6addb..654961a9d 100644 --- a/src/ripple/rpc/handlers/OwnerInfo.cpp +++ b/src/ripple/rpc/handlers/OwnerInfo.cpp @@ -52,7 +52,7 @@ Json::Value doOwnerInfo (RPC::Context& context) iIndex, false); - ret[jss::accepted] = jAccepted.empty () ? + ret[jss::accepted] = ! jAccepted ? context.netOps.getOwnerInfo (closedLedger, accountID) : jAccepted; auto const& currentLedger = context.netOps.getCurrentLedger (); @@ -63,9 +63,8 @@ Json::Value doOwnerInfo (RPC::Context& context) iIndex, false); - ret[jss::current] = jCurrent.empty () ? + ret[jss::current] = ! jCurrent ? context.netOps.getOwnerInfo (currentLedger, accountID) : jCurrent; - return ret; } diff --git a/src/ripple/rpc/impl/LookupLedger.cpp b/src/ripple/rpc/impl/LookupLedger.cpp index cf7fd717a..82922654b 100644 --- a/src/ripple/rpc/impl/LookupLedger.cpp +++ b/src/ripple/rpc/impl/LookupLedger.cpp @@ -48,7 +48,7 @@ Status ledgerFromRequest ( // We need to support the legacy "ledger" field. auto& legacyLedger = params[jss::ledger]; - if (!legacyLedger.empty()) + if (legacyLedger) { if (legacyLedger.asString().size () > 12) hashValue = legacyLedger; @@ -56,7 +56,7 @@ Status ledgerFromRequest ( indexValue = legacyLedger; } - if (!hashValue.empty()) + if (hashValue) { if (! hashValue.isString ()) return {rpcINVALID_PARAMS, "ledgerHashNotString"}; diff --git a/src/ripple/rpc/tests/Status.test.cpp b/src/ripple/rpc/tests/Status.test.cpp index ea2d3c1d8..afb0f5ffb 100644 --- a/src/ripple/rpc/tests/Status.test.cpp +++ b/src/ripple/rpc/tests/Status.test.cpp @@ -107,19 +107,19 @@ private: { testcase ("OK"); fillJson (Status ()); - expect (value_.empty(), "Value for empty status"); + expect (! value_, "Value for empty status"); fillJson (0); - expect (value_.empty(), "Value for 0 status"); + expect (! value_, "Value for 0 status"); fillJson (Status::OK); - expect (value_.empty(), "Value for OK status"); + expect (! value_, "Value for OK status"); fillJson (tesSUCCESS); - expect (value_.empty(), "Value for tesSUCCESS"); + expect (! value_, "Value for tesSUCCESS"); fillJson (rpcSUCCESS); - expect (value_.empty(), "Value for rpcSUCCESS"); + expect (! value_, "Value for rpcSUCCESS"); } template @@ -133,10 +133,10 @@ private: fillJson (Status (status, messages)); auto prefix = label + ": "; - expect (!value_.empty(), prefix + "No value"); + expect (bool (value_), prefix + "No value"); auto error = value_[jss::error]; - expect (!error.empty(), prefix + "No error."); + expect (bool (error), prefix + "No error."); auto code = error[jss::code].asInt(); expect (status == code, prefix + "Wrong status " + diff --git a/src/ripple/server/impl/ServerHandlerImp.cpp b/src/ripple/server/impl/ServerHandlerImp.cpp index 5674ea5c2..2fb98077a 100644 --- a/src/ripple/server/impl/ServerHandlerImp.cpp +++ b/src/ripple/server/impl/ServerHandlerImp.cpp @@ -242,7 +242,7 @@ ServerHandlerImp::processRequest ( Json::Reader reader; if ((request.size () > 1000000) || ! reader.parse (request, jsonRPC) || - jsonRPC.isNull () || + ! jsonRPC || ! jsonRPC.isObject ()) { HTTPReply (400, "Unable to parse request", output); @@ -258,7 +258,7 @@ ServerHandlerImp::processRequest ( Json::Value const& method = jsonRPC ["method"]; - if (method.isNull ()) { + if (! method) { HTTPReply (400, "Null method", output); return; } @@ -313,7 +313,7 @@ ServerHandlerImp::processRequest ( // and we take that first entry and validate that it's an object. Json::Value params = jsonRPC [jss::params]; - if (params.isNull () || params.empty()) + if (! params) params = Json::Value (Json::objectValue); else if (!params.isArray () || params.size() != 1) diff --git a/src/ripple/websocket/Handler.h b/src/ripple/websocket/Handler.h index 9e44ea370..3d995e0d4 100644 --- a/src/ripple/websocket/Handler.h +++ b/src/ripple/websocket/Handler.h @@ -403,7 +403,7 @@ public: send (cpClient, jvResult, false); } else if (!jrReader.parse (mpMessage->get_payload (), jvRequest) || - jvRequest.isNull () || !jvRequest.isObject ()) + ! jvRequest || !jvRequest.isObject ()) { Json::Value jvResult (Json::objectValue);