From 0825bd70760f8c2fd0df99d0c582932f851db99f Mon Sep 17 00:00:00 2001 From: Tom Ritchford Date: Thu, 29 Jan 2015 13:07:19 -0500 Subject: [PATCH] Cleanups to Json Object code. * Replace Json::JsonException with std::logic_error. * Move two functions definitions to Object.cpp. * Fix include guards. --- src/ripple/basics/ToString.h | 3 + src/ripple/json/Object.h | 78 +++++--------------------- src/ripple/json/Output.h | 3 + src/ripple/json/Writer.h | 42 ++++++-------- src/ripple/json/impl/Object.cpp | 68 +++++++++++++++++++++- src/ripple/json/impl/Writer.cpp | 13 +++-- src/ripple/json/json_writer.h | 2 +- src/ripple/json/tests/Output.test.cpp | 3 +- src/ripple/rpc/impl/RPCHandler.cpp | 2 +- src/ripple/server/impl/JSONRPCUtil.cpp | 3 +- src/ripple/server/impl/JSONRPCUtil.h | 2 +- 11 files changed, 117 insertions(+), 102 deletions(-) diff --git a/src/ripple/basics/ToString.h b/src/ripple/basics/ToString.h index 7a711d94e..ae34d8ece 100644 --- a/src/ripple/basics/ToString.h +++ b/src/ripple/basics/ToString.h @@ -20,6 +20,9 @@ #ifndef RIPPLE_BASICS_TOSTRING_H_INCLUDED #define RIPPLE_BASICS_TOSTRING_H_INCLUDED +#include +#include + namespace ripple { /** to_string() generalizes std::to_string to handle bools, chars, and strings. diff --git a/src/ripple/json/Object.h b/src/ripple/json/Object.h index 5aa9057c6..3adcf4469 100644 --- a/src/ripple/json/Object.h +++ b/src/ripple/json/Object.h @@ -17,8 +17,8 @@ */ //============================================================================== -#ifndef RIPPLE_RPC_JSONOBJECT_H_INCLUDED -#define RIPPLE_RPC_JSONOBJECT_H_INCLUDED +#ifndef RIPPLE_JSON_OBJECT_H_INCLUDED +#define RIPPLE_JSON_OBJECT_H_INCLUDED #include #include @@ -39,14 +39,14 @@ namespace Json { 1. Only one collection can be open for change at any one time. - This condition is enforced automatically and a JsonException thrown if it + This condition is enforced automatically and a std::logic_error thrown if it is violated. 2. A tag may only be used once in an Object. Some objects have many tags, so this condition might be a little expensive. Enforcement of this condition is turned on in debug builds and - a JsonException is thrown when the tag is added for a second time. + a std::logic_error is thrown when the tag is added for a second time. Code samples: @@ -122,7 +122,7 @@ namespace Json { // Outputs {"hands":{"left":false,"right":true}} - Typical ways to make mistakes and get a JsonException: + Typical ways to make mistakes and get a std::logic_error: Writer writer; Object::Root root (writer); @@ -360,6 +360,14 @@ public: void operator= (T const& t) { object_.set (key_, t); + // Note: This function shouldn't return *this, because it's a trap. + // + // In Json::Value, foo[jss::key] returns a reference to a + // mutable Json::Value contained _inside_ foo. But in the case of + // Json::Object, where we write once only, there isn't any such + // reference that can be returned. Returning *this would return an + // object "a level higher" than in Json::Value, leading to obscure bugs, + // particularly in generic code. } }; @@ -373,36 +381,6 @@ void Array::append (Scalar const& value) writer_->append (value); } -inline void Array::append (Json::Value const& v) -{ - auto t = v.type(); - switch (t) - { - case Json::nullValue: return append (nullptr); - case Json::intValue: return append (v.asInt()); - case Json::uintValue: return append (v.asUInt()); - case Json::realValue: return append (v.asDouble()); - case Json::stringValue: return append (v.asString()); - case Json::booleanValue: return append (v.asBool()); - - case Json::objectValue: - { - auto object = appendObject (); - copyFrom (object, v); - return; - } - - case Json::arrayValue: - { - auto array = appendArray (); - for (auto& item: v) - array.append (item); - return; - } - } - assert (false); // Can't get here. -} - template void Object::set (std::string const& key, Scalar const& value) { @@ -411,36 +389,6 @@ void Object::set (std::string const& key, Scalar const& value) writer_->set (key, value); } -inline void Object::set (std::string const& k, Json::Value const& v) -{ - auto t = v.type(); - switch (t) - { - case Json::nullValue: return set (k, nullptr); - case Json::intValue: return set (k, v.asInt()); - case Json::uintValue: return set (k, v.asUInt()); - case Json::realValue: return set (k, v.asDouble()); - case Json::stringValue: return set (k, v.asString()); - case Json::booleanValue: return set (k, v.asBool()); - - case Json::objectValue: - { - auto object = setObject (k); - copyFrom (object, v); - return; - } - - case Json::arrayValue: - { - auto array = setArray (k); - for (auto& item: v) - array.append (item); - return; - } - } - assert (false); // Can't get here. -} - inline Json::Value& setArray (Json::Value& json, Json::StaticString const& key) { diff --git a/src/ripple/json/Output.h b/src/ripple/json/Output.h index aa8c400ef..a7b610736 100644 --- a/src/ripple/json/Output.h +++ b/src/ripple/json/Output.h @@ -21,9 +21,12 @@ #define RIPPLE_JSON_OUTPUT_H_INCLUDED #include +#include namespace Json { +class Value; + using Output = std::function ; inline diff --git a/src/ripple/json/Writer.h b/src/ripple/json/Writer.h index ed66af298..e2e692392 100644 --- a/src/ripple/json/Writer.h +++ b/src/ripple/json/Writer.h @@ -17,13 +17,12 @@ */ //============================================================================== -#ifndef RIPPLE_RPC_JSONWRITER_H_INCLUDED -#define RIPPLE_RPC_JSONWRITER_H_INCLUDED +#ifndef RIPPLE_JSON_WRITER_H_INCLUDED +#define RIPPLE_JSON_WRITER_H_INCLUDED #include #include -#include -#include + #include #include namespace Json { @@ -200,17 +199,23 @@ public: /*** Output a Json::Value. */ void output (Json::Value const&); + /** Output a null. */ + void output (std::nullptr_t); + + /** Output a float. */ + void output (float); + + /** Output a double. */ + void output (double); + + /** Output a bool. */ + void output (bool); + /** Output numbers or booleans. */ template void output (Type t) { - implOutput (ripple::to_string (t)); - } - - /** Output an error code. */ - void output (ripple::error_code_i t) - { - output (int(t)); + implOutput (std::to_string (t)); } void output (Json::StaticString const& t) @@ -225,23 +230,10 @@ private: void implOutput (std::string const&); }; -class JsonException : public std::exception -{ -public: - explicit JsonException (std::string const& name) : name_(name) {} - const char* what() const throw() override - { - return name_.c_str(); - } - -private: - std::string const name_; -}; - inline void check (bool condition, std::string const& message) { if (!condition) - throw JsonException (message); + throw std::logic_error (message); } } // Json diff --git a/src/ripple/json/impl/Object.cpp b/src/ripple/json/impl/Object.cpp index 74040ffec..d7f30dbf6 100644 --- a/src/ripple/json/impl/Object.cpp +++ b/src/ripple/json/impl/Object.cpp @@ -62,9 +62,9 @@ Collection::Collection (Collection&& that) noexcept void Collection::checkWritable (std::string const& label) { if (!enabled_) - throw JsonException (label + ": not enabled"); + throw std::logic_error (label + ": not enabled"); if (!writer_) - throw JsonException (label + ": not writable"); + throw std::logic_error (label + ": not writable"); } //------------------------------------------------------------------------------ @@ -125,6 +125,70 @@ Object::Proxy Object::operator[] (Json::StaticString const& key) return Proxy (*this, std::string (key)); } +//------------------------------------------------------------------------------ + +void Array::append (Json::Value const& v) +{ + auto t = v.type(); + switch (t) + { + case Json::nullValue: return append (nullptr); + case Json::intValue: return append (v.asInt()); + case Json::uintValue: return append (v.asUInt()); + case Json::realValue: return append (v.asDouble()); + case Json::stringValue: return append (v.asString()); + case Json::booleanValue: return append (v.asBool()); + + case Json::objectValue: + { + auto object = appendObject (); + copyFrom (object, v); + return; + } + + case Json::arrayValue: + { + auto array = appendArray (); + for (auto& item: v) + array.append (item); + return; + } + } + assert (false); // Can't get here. +} + +void Object::set (std::string const& k, Json::Value const& v) +{ + auto t = v.type(); + switch (t) + { + case Json::nullValue: return set (k, nullptr); + case Json::intValue: return set (k, v.asInt()); + case Json::uintValue: return set (k, v.asUInt()); + case Json::realValue: return set (k, v.asDouble()); + case Json::stringValue: return set (k, v.asString()); + case Json::booleanValue: return set (k, v.asBool()); + + case Json::objectValue: + { + auto object = setObject (k); + copyFrom (object, v); + return; + } + + case Json::arrayValue: + { + auto array = setArray (k); + for (auto& item: v) + array.append (item); + return; + } + } + assert (false); // Can't get here. +} + +//------------------------------------------------------------------------------ + namespace { template diff --git a/src/ripple/json/impl/Writer.cpp b/src/ripple/json/impl/Writer.cpp index c69374ce0..417f0cf5f 100644 --- a/src/ripple/json/impl/Writer.cpp +++ b/src/ripple/json/impl/Writer.cpp @@ -21,6 +21,7 @@ #include #include #include +#include namespace Json { @@ -146,7 +147,7 @@ public: void writeObjectTag (std::string const& tag) { -#ifdef DEBUG +#ifndef NDEBUG // Make sure we haven't already seen this tag. auto& tags = stack_.top ().tags; check (tags.find (tag) == tags.end (), "Already seen tag " + tag); @@ -194,7 +195,7 @@ private: * If false, we have to emit a , before we write the next entry. */ bool isFirst = true; -#ifdef DEBUG +#ifndef NDEBUG /** What tags have we already seen in this collection? */ std::set tags; #endif @@ -246,26 +247,28 @@ void Writer::output (Json::Value const& value) outputJson (value, impl_->getOutput()); } -template <> void Writer::output (float f) { auto s = ripple::to_string (f); impl_->output ({s.data (), lengthWithoutTrailingZeros (s)}); } -template <> void Writer::output (double f) { auto s = ripple::to_string (f); impl_->output ({s.data (), lengthWithoutTrailingZeros (s)}); } -template <> void Writer::output (std::nullptr_t) { impl_->output ("null"); } +void Writer::output (bool b) +{ + impl_->output (b ? "true" : "false"); +} + void Writer::implOutput (std::string const& s) { impl_->output (s); diff --git a/src/ripple/json/json_writer.h b/src/ripple/json/json_writer.h index e9ff9746f..75ccb3e0f 100644 --- a/src/ripple/json/json_writer.h +++ b/src/ripple/json/json_writer.h @@ -41,7 +41,7 @@ public: /** \brief Outputs a Value in JSON format without formatting (not human friendly). * * The JSON document is written in a single line. It is not intended for 'human' consumption, - * but may be usefull to support feature such as RPC where bandwith is limited. + * but may be useful to support feature such as RPC where bandwith is limited. * \sa Reader, Value */ diff --git a/src/ripple/json/tests/Output.test.cpp b/src/ripple/json/tests/Output.test.cpp index 78883e1bb..7f31f30a4 100644 --- a/src/ripple/json/tests/Output.test.cpp +++ b/src/ripple/json/tests/Output.test.cpp @@ -18,7 +18,8 @@ //============================================================================== #include -#include +#include +#include #include namespace Json { diff --git a/src/ripple/rpc/impl/RPCHandler.cpp b/src/ripple/rpc/impl/RPCHandler.cpp index 0ff773d5b..9416080fe 100644 --- a/src/ripple/rpc/impl/RPCHandler.cpp +++ b/src/ripple/rpc/impl/RPCHandler.cpp @@ -250,7 +250,7 @@ void executeRPC ( { // Can't ever get here. assert (false); - throw Json::JsonException ("RPC handler with no method"); + throw std::logic_error ("RPC handler with no method"); } } diff --git a/src/ripple/server/impl/JSONRPCUtil.cpp b/src/ripple/server/impl/JSONRPCUtil.cpp index 80e395535..b2fabc084 100644 --- a/src/ripple/server/impl/JSONRPCUtil.cpp +++ b/src/ripple/server/impl/JSONRPCUtil.cpp @@ -45,7 +45,8 @@ std::string getHTTPHeaderTimestamp () return std::string (buffer); } -void HTTPReply (int nStatus, std::string const& content, Json::Output output) +void HTTPReply ( + int nStatus, std::string const& content, Json::Output const& output) { if (ShouldLog (lsTRACE, RPC)) { diff --git a/src/ripple/server/impl/JSONRPCUtil.h b/src/ripple/server/impl/JSONRPCUtil.h index c6c9d74f2..d7b0bd520 100644 --- a/src/ripple/server/impl/JSONRPCUtil.h +++ b/src/ripple/server/impl/JSONRPCUtil.h @@ -25,7 +25,7 @@ namespace ripple { -void HTTPReply (int nStatus, std::string const& strMsg, Json::Output); +void HTTPReply (int nStatus, std::string const& strMsg, Json::Output const&); } // ripple