Handle empty Json values:

* Replace Json::Value::isNull() and Json::Value::empty with operator bool()
This commit is contained in:
Tom Ritchford
2015-06-18 15:37:29 -04:00
committed by Nik Bougalis
parent a5a9242f4e
commit 9111ad1a9d
22 changed files with 67 additions and 77 deletions

View File

@@ -151,8 +151,8 @@ struct AccountTxPaging_test : beast::unit_test::suite
expect (next(limit, forward, token, min_ledger, max_ledger) == 1); expect (next(limit, forward, token, min_ledger, max_ledger) == 1);
checkTransaction (txs_[0], 5, 7); checkTransaction (txs_[0], 5, 7);
expect(token["ledger"].isNull()); expect(! token["ledger"]);
expect(token["seq"].isNull()); expect(! token["seq"]);
} }
token = Json::nullValue; token = Json::nullValue;
@@ -200,8 +200,8 @@ struct AccountTxPaging_test : beast::unit_test::suite
expect(next(limit, forward, token, min_ledger, max_ledger) == 1); expect(next(limit, forward, token, min_ledger, max_ledger) == 1);
checkTransaction (txs_[0], 6, 11); checkTransaction (txs_[0], 6, 11);
expect(token["ledger"].isNull()); expect(! token["ledger"]);
expect(token["seq"].isNull()); expect(! token["seq"]);
} }
token = Json::nullValue; token = Json::nullValue;
@@ -241,8 +241,8 @@ struct AccountTxPaging_test : beast::unit_test::suite
checkTransaction (txs_[2], 3, 5); checkTransaction (txs_[2], 3, 5);
} }
expect (token["ledger"].isNull()); expect (! token["ledger"]);
expect (token["seq"].isNull()); expect (! token["seq"]);
} }
}; };

View File

@@ -452,7 +452,7 @@ Json::Value PathRequest::doUpdate (RippleLineCache::ref cache, bool fast)
jvStatus[jss::destination_account] = getApp().accountIDCache().toBase58(*raDstAccount); jvStatus[jss::destination_account] = getApp().accountIDCache().toBase58(*raDstAccount);
jvStatus[jss::destination_amount] = saDstAmount.getJson (0); jvStatus[jss::destination_amount] = saDstAmount.getJson (0);
if (!jvId.isNull ()) if (jvId)
jvStatus["id"] = jvId; jvStatus["id"] = jvId;
Json::Value jvArray = Json::arrayValue; Json::Value jvArray = Json::arrayValue;

View File

@@ -204,7 +204,7 @@ void doCopyFrom (Object& to, Json::Value const& from)
void copyFrom (Json::Value& 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; to = from;
else else
doCopyFrom (to, from); doCopyFrom (to, from);

View File

@@ -822,28 +822,20 @@ Value::size () const
} }
bool Value::operator bool () const
Value::empty () const
{ {
if (isNull ()) if (isNull ())
return true; return false;
if (isString ()) { if (isString ())
{
auto s = asCString(); 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 void
Value::clear () Value::clear ()
{ {

View File

@@ -253,6 +253,8 @@ public:
double asDouble () const; double asDouble () const;
bool asBool () 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 isNull () const;
bool isBool () const; bool isBool () const;
bool isInt () const; bool isInt () const;
@@ -269,12 +271,10 @@ public:
/// Number of values in array or object /// Number of values in array or object
UInt size () const; UInt size () const;
/// \brief Return true if empty array, empty object, or null; /** Returns false if this is an empty array, empty object, empty string,
/// otherwise, false. or null. */
bool empty () const; explicit
operator bool() const;
/// Return isNull()
bool operator! () const;
/// Remove all object members and array elements. /// Remove all object members and array elements.
/// \pre type() is arrayValue, objectValue, or nullValue /// \pre type() is arrayValue, objectValue, or nullValue

View File

@@ -25,30 +25,29 @@
namespace ripple { namespace ripple {
class json_value_test : public beast::unit_test::suite struct json_value_test : beast::unit_test::suite
{ {
public: void test_bool()
void test_empty()
{ {
expect (Json::Value().empty()); expect (! Json::Value());
expect (Json::Value("").empty()); expect (! Json::Value(""));
expect (! Json::Value("empty").empty()); expect (bool (Json::Value("empty")));
expect (! Json::Value(false).empty()); expect (bool (Json::Value(false)));
expect (! Json::Value(true).empty()); expect (bool (Json::Value(true)));
expect (! Json::Value(0).empty()); expect (bool (Json::Value(0)));
expect (! Json::Value(1).empty()); expect (bool (Json::Value(1)));
Json::Value array (Json::arrayValue); Json::Value array (Json::arrayValue);
expect (array.empty()); expect (! array);
array.append(0); array.append(0);
expect (!array.empty()); expect (bool (array));
Json::Value object (Json::objectValue); Json::Value object (Json::objectValue);
expect (object.empty()); expect (! object);
object[""] = false; object[""] = false;
expect (!object.empty()); expect (bool (object));
} }
void test_bad_json () void test_bad_json ()
@@ -148,7 +147,7 @@ public:
expect (v1.asDouble () == 2.5); expect (v1.asDouble () == 2.5);
Json::Value v2 = std::move(v1); Json::Value v2 = std::move(v1);
expect (v1.isNull ()); expect (!v1);
expect (v2.isDouble ()); expect (v2.isDouble ());
expect (v2.asDouble () == 2.5); expect (v2.asDouble () == 2.5);
expect (v1 != v2); expect (v1 != v2);
@@ -156,7 +155,7 @@ public:
v1 = std::move(v2); v1 = std::move(v2);
expect (v1.isDouble ()); expect (v1.isDouble ());
expect (v1.asDouble () == 2.5); expect (v1.asDouble () == 2.5);
expect (v2.isNull ()); expect (! v2);
expect (v1 != v2); expect (v1 != v2);
pass (); pass ();
@@ -220,7 +219,7 @@ public:
void run () void run ()
{ {
test_empty (); test_bool ();
test_bad_json (); test_bad_json ();
test_edge_cases (); test_edge_cases ();
test_copy (); test_copy ();

View File

@@ -1047,7 +1047,7 @@ struct RPCCallImp
if (!reader.parse (strData, jvReply)) if (!reader.parse (strData, jvReply))
throw std::runtime_error ("couldn't parse reply from server"); 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"); throw std::runtime_error ("expected reply to have result, error and id properties");
Json::Value jvResult (Json::objectValue); Json::Value jvResult (Json::objectValue);

View File

@@ -532,7 +532,7 @@ static boost::optional<detail::STVar> parseLeaf (
AccountID uAccount, uIssuer; AccountID uAccount, uIssuer;
Currency uCurrency; Currency uCurrency;
if (! account.isNull ()) if (account)
{ {
// human account id // human account id
if (! account.isString ()) if (! account.isString ())
@@ -556,7 +556,7 @@ static boost::optional<detail::STVar> parseLeaf (
} }
} }
if (!currency.isNull ()) if (currency)
{ {
// human currency // human currency
if (!currency.isString ()) if (!currency.isString ())
@@ -577,7 +577,7 @@ static boost::optional<detail::STVar> parseLeaf (
} }
} }
if (!issuer.isNull ()) if (issuer)
{ {
// human account id // human account id
if (!issuer.isString ()) if (!issuer.isString ())

View File

@@ -44,9 +44,9 @@ public:
bool parseJSONString (std::string const& json, Json::Value& to) bool parseJSONString (std::string const& json, Json::Value& to)
{ {
Json::Reader reader; Json::Reader reader;
return (reader.parse(json, to) && return reader.parse(json, to) &&
!to.isNull() && bool (to) &&
to.isObject()); to.isObject();
} }
void testParseJSONArrayWithInvalidChildrenObjects () void testParseJSONArrayWithInvalidChildrenObjects ()

View File

@@ -61,7 +61,7 @@ Json::Value doAccountCurrencies (RPC::Context& context)
Json::Value jvAccepted ( Json::Value jvAccepted (
RPC::accountFromString (accountID, bIndex, strIdent, iIndex, bStrict)); RPC::accountFromString (accountID, bIndex, strIdent, iIndex, bStrict));
if (!jvAccepted.empty ()) if (jvAccepted)
return jvAccepted; return jvAccepted;
std::set<Currency> send, receive; std::set<Currency> send, receive;

View File

@@ -60,7 +60,7 @@ Json::Value doAccountInfo (RPC::Context& context)
auto jvAccepted = RPC::accountFromString ( auto jvAccepted = RPC::accountFromString (
accountID, bIndex, strIdent, iIndex, bStrict); accountID, bIndex, strIdent, iIndex, bStrict);
if (!jvAccepted.empty ()) if (jvAccepted)
return jvAccepted; return jvAccepted;
auto const sleAccepted = cachedRead(*ledger, auto const sleAccepted = cachedRead(*ledger,

View File

@@ -95,7 +95,7 @@ Json::Value doAccountLines (RPC::Context& context)
auto jv = RPC::accountFromString ( auto jv = RPC::accountFromString (
accountID, bIndex, strIdent, iIndex, false); accountID, bIndex, strIdent, iIndex, false);
if (! jv.empty ()) if (jv)
{ {
for (auto it = jv.begin (); it != jv.end (); ++it) for (auto it = jv.begin (); it != jv.end (); ++it)
result[it.memberName ()] = it.key (); result[it.memberName ()] = it.key ();
@@ -125,7 +125,7 @@ Json::Value doAccountLines (RPC::Context& context)
result = RPC::accountFromString ( result = RPC::accountFromString (
raPeerAccount, bPeerIndex, strPeer, iPeerIndex, false); raPeerAccount, bPeerIndex, strPeer, iPeerIndex, false);
if (! result.empty ()) if (result)
return result; return result;
} }

View File

@@ -60,7 +60,7 @@ Json::Value doAccountObjects (RPC::Context& context)
? context.params[jss::account_index].asUInt () : 0; ? context.params[jss::account_index].asUInt () : 0;
auto jv = RPC::accountFromString ( auto jv = RPC::accountFromString (
accountID, bIndex, strIdent, iIndex, false); accountID, bIndex, strIdent, iIndex, false);
if (! jv.empty ()) if (jv)
{ {
for (auto it = jv.begin (); it != jv.end (); ++it) for (auto it = jv.begin (); it != jv.end (); ++it)
result[it.memberName ()] = it.key (); result[it.memberName ()] = it.key ();

View File

@@ -50,7 +50,7 @@ Json::Value doAccountOffers (RPC::Context& context)
Json::Value const jv = RPC::accountFromString ( Json::Value const jv = RPC::accountFromString (
accountID, bIndex, strIdent, iIndex, false); accountID, bIndex, strIdent, iIndex, false);
if (! jv.empty ()) if (jv)
{ {
for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it) for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it)
result[it.memberName ()] = it.key (); result[it.memberName ()] = it.key ();

View File

@@ -162,7 +162,7 @@ Json::Value doAccountTx (RPC::Context& context)
ret[jss::ledger_index_max] = uLedgerMax; ret[jss::ledger_index_max] = uLedgerMax;
if (params.isMember (jss::limit)) if (params.isMember (jss::limit))
ret[jss::limit] = limit; ret[jss::limit] = limit;
if (!resumeToken.isNull()) if (resumeToken)
ret[jss::marker] = resumeToken; ret[jss::marker] = resumeToken;
return ret; return ret;

View File

@@ -82,7 +82,7 @@ Json::Value doGatewayBalances (RPC::Context& context)
Json::Value jvAccepted = RPC::accountFromString ( Json::Value jvAccepted = RPC::accountFromString (
accountID, bIndex, strIdent, iIndex, bStrict); accountID, bIndex, strIdent, iIndex, bStrict);
if (!jvAccepted.empty ()) if (jvAccepted)
return jvAccepted; return jvAccepted;
context.loadType = Resource::feeHighBurdenRPC; context.loadType = Resource::feeHighBurdenRPC;

View File

@@ -100,7 +100,7 @@ Json::Value doNoRippleCheck (RPC::Context& context)
Json::Value const jv = RPC::accountFromString ( Json::Value const jv = RPC::accountFromString (
accountID, bIndex, strIdent, iIndex, false); accountID, bIndex, strIdent, iIndex, false);
if (! jv.empty ()) if (jv)
{ {
for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it) for (Json::Value::const_iterator it (jv.begin ()); it != jv.end (); ++it)
result[it.memberName ()] = it.key (); result[it.memberName ()] = it.key ();

View File

@@ -52,7 +52,7 @@ Json::Value doOwnerInfo (RPC::Context& context)
iIndex, iIndex,
false); false);
ret[jss::accepted] = jAccepted.empty () ? ret[jss::accepted] = ! jAccepted ?
context.netOps.getOwnerInfo (closedLedger, accountID) : jAccepted; context.netOps.getOwnerInfo (closedLedger, accountID) : jAccepted;
auto const& currentLedger = context.netOps.getCurrentLedger (); auto const& currentLedger = context.netOps.getCurrentLedger ();
@@ -63,9 +63,8 @@ Json::Value doOwnerInfo (RPC::Context& context)
iIndex, iIndex,
false); false);
ret[jss::current] = jCurrent.empty () ? ret[jss::current] = ! jCurrent ?
context.netOps.getOwnerInfo (currentLedger, accountID) : jCurrent; context.netOps.getOwnerInfo (currentLedger, accountID) : jCurrent;
return ret; return ret;
} }

View File

@@ -48,7 +48,7 @@ Status ledgerFromRequest (
// We need to support the legacy "ledger" field. // We need to support the legacy "ledger" field.
auto& legacyLedger = params[jss::ledger]; auto& legacyLedger = params[jss::ledger];
if (!legacyLedger.empty()) if (legacyLedger)
{ {
if (legacyLedger.asString().size () > 12) if (legacyLedger.asString().size () > 12)
hashValue = legacyLedger; hashValue = legacyLedger;
@@ -56,7 +56,7 @@ Status ledgerFromRequest (
indexValue = legacyLedger; indexValue = legacyLedger;
} }
if (!hashValue.empty()) if (hashValue)
{ {
if (! hashValue.isString ()) if (! hashValue.isString ())
return {rpcINVALID_PARAMS, "ledgerHashNotString"}; return {rpcINVALID_PARAMS, "ledgerHashNotString"};

View File

@@ -107,19 +107,19 @@ private:
{ {
testcase ("OK"); testcase ("OK");
fillJson (Status ()); fillJson (Status ());
expect (value_.empty(), "Value for empty status"); expect (! value_, "Value for empty status");
fillJson (0); fillJson (0);
expect (value_.empty(), "Value for 0 status"); expect (! value_, "Value for 0 status");
fillJson (Status::OK); fillJson (Status::OK);
expect (value_.empty(), "Value for OK status"); expect (! value_, "Value for OK status");
fillJson (tesSUCCESS); fillJson (tesSUCCESS);
expect (value_.empty(), "Value for tesSUCCESS"); expect (! value_, "Value for tesSUCCESS");
fillJson (rpcSUCCESS); fillJson (rpcSUCCESS);
expect (value_.empty(), "Value for rpcSUCCESS"); expect (! value_, "Value for rpcSUCCESS");
} }
template <typename Type> template <typename Type>
@@ -133,10 +133,10 @@ private:
fillJson (Status (status, messages)); fillJson (Status (status, messages));
auto prefix = label + ": "; auto prefix = label + ": ";
expect (!value_.empty(), prefix + "No value"); expect (bool (value_), prefix + "No value");
auto error = value_[jss::error]; auto error = value_[jss::error];
expect (!error.empty(), prefix + "No error."); expect (bool (error), prefix + "No error.");
auto code = error[jss::code].asInt(); auto code = error[jss::code].asInt();
expect (status == code, prefix + "Wrong status " + expect (status == code, prefix + "Wrong status " +

View File

@@ -242,7 +242,7 @@ ServerHandlerImp::processRequest (
Json::Reader reader; Json::Reader reader;
if ((request.size () > 1000000) || if ((request.size () > 1000000) ||
! reader.parse (request, jsonRPC) || ! reader.parse (request, jsonRPC) ||
jsonRPC.isNull () || ! jsonRPC ||
! jsonRPC.isObject ()) ! jsonRPC.isObject ())
{ {
HTTPReply (400, "Unable to parse request", output); HTTPReply (400, "Unable to parse request", output);
@@ -258,7 +258,7 @@ ServerHandlerImp::processRequest (
Json::Value const& method = jsonRPC ["method"]; Json::Value const& method = jsonRPC ["method"];
if (method.isNull ()) { if (! method) {
HTTPReply (400, "Null method", output); HTTPReply (400, "Null method", output);
return; return;
} }
@@ -313,7 +313,7 @@ ServerHandlerImp::processRequest (
// and we take that first entry and validate that it's an object. // and we take that first entry and validate that it's an object.
Json::Value params = jsonRPC [jss::params]; Json::Value params = jsonRPC [jss::params];
if (params.isNull () || params.empty()) if (! params)
params = Json::Value (Json::objectValue); params = Json::Value (Json::objectValue);
else if (!params.isArray () || params.size() != 1) else if (!params.isArray () || params.size() != 1)

View File

@@ -403,7 +403,7 @@ public:
send (cpClient, jvResult, false); send (cpClient, jvResult, false);
} }
else if (!jrReader.parse (mpMessage->get_payload (), jvRequest) || else if (!jrReader.parse (mpMessage->get_payload (), jvRequest) ||
jvRequest.isNull () || !jvRequest.isObject ()) ! jvRequest || !jvRequest.isObject ())
{ {
Json::Value jvResult (Json::objectValue); Json::Value jvResult (Json::objectValue);