From 3dbb3275b5a160c6e6276e1abedd8fc29f22e2c6 Mon Sep 17 00:00:00 2001 From: Devwrat Joshi Date: Tue, 14 Jun 2022 14:25:58 +0900 Subject: [PATCH 1/5] Add macro to check for required json key --- include/mujincontrollerclient/mujinjson.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/include/mujincontrollerclient/mujinjson.h b/include/mujincontrollerclient/mujinjson.h index b9ea3ec8..9ce498d7 100644 --- a/include/mujincontrollerclient/mujinjson.h +++ b/include/mujincontrollerclient/mujinjson.h @@ -37,6 +37,15 @@ #include #include +#ifndef MUJINCLIENTJSON_LOAD_REQUIRED_JSON_VALUE_BY_KEY +#define MUJINCLIENTJSON_LOAD_REQUIRED_JSON_VALUE_BY_KEY(rValue, key, param) \ + { \ + if (!(mujinclient::mujinjson_external::LoadJsonValueByKey(rValue, key, param))) \ + { \ + throw mujinclient::mujinjson_external::MujinJSONException(boost::str(boost::format(("[%s, %u] assert(mujinclient::mujinjson_external::LoadJsonValueByKey(%s, %s, %s))"))%__FILE__%__LINE__%#rValue%key%#param), mujinclient::mujinjson_external::MJE_Failed); \ + } \ + } +#endif // MUJINCLIENTJSON_LOAD_REQUIRED_JSON_VALUE_BY_KEY namespace mujinclient { namespace mujinjson_external { @@ -567,13 +576,15 @@ template inline void SaveJsonValue(rapidjson::Document& v, const T& t) template inline void SetJsonValueByKey(rapidjson::Value& v, const U& key, const T& t, rapidjson::Document::AllocatorType& alloc); //get one json value by key, and store it in local data structures -template void inline LoadJsonValueByKey(const rapidjson::Value& v, const char* key, T& t) { +template bool inline LoadJsonValueByKey(const rapidjson::Value& v, const char* key, T& t) { if (!v.IsObject()) { throw MujinJSONException("Cannot load value of non-object.", MJE_Failed); } - if (v.HasMember(key)) { + if (v.HasMember(key) && !v.FindMember(key)->value.IsNull()) { LoadJsonValue(v[key], t); + return true; } + return false; } template inline void LoadJsonValueByKey(const rapidjson::Value& v, const char* key, T& t, const U& d) { if (!v.IsObject()) { From eab857fe3bb3062f18b798816d15224eeabfd2db Mon Sep 17 00:00:00 2001 From: Devwrat Joshi Date: Tue, 14 Jun 2022 14:29:59 +0900 Subject: [PATCH 2/5] Add test scripts for MUJINCLIENTJSON_LOAD_REQUIRED_JSON_VALUE_BY_KEY macro --- test/CMakeLists.txt | 1 + test/testjsonutils.cpp | 64 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 test/testjsonutils.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 9e599c24..f8d72cfe 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -29,3 +29,4 @@ build_test(settaskparameters) build_test(showresults) build_test(uploadregister) build_test(uploadregistercec) +build_test(testjsonutils) diff --git a/test/testjsonutils.cpp b/test/testjsonutils.cpp new file mode 100644 index 00000000..9fe7fa48 --- /dev/null +++ b/test/testjsonutils.cpp @@ -0,0 +1,64 @@ +#include +#include +#include + +// The tests will expect json documents containing these keys +using namespace mujinclient::mujinjson_external; +const std::string expectedJsonKey = "key1"; + +void TestExistingKey() +{ + rapidjson::Document document; + document.SetObject(); + int expectedResult = 1; + // Expected key and an extra key are in the document + SetJsonValueByKey(document, expectedJsonKey.c_str(), expectedResult, document.GetAllocator()); + SetJsonValueByKey(document, "extraKey", expectedResult, document.GetAllocator()); + int result = -1; + MUJINCLIENTJSON_LOAD_REQUIRED_JSON_VALUE_BY_KEY(document, expectedJsonKey.c_str(), result); + if(result == expectedResult) { + std::cout << "OK: For existing key test, we got result = " << result; + } + else { + std::cout << "FAILED: For existing key test, we got result = " << result; + } + std::cout << " when expected result was " << expectedResult << "\n"; +} + +void TestNonExistentKey() +{ + rapidjson::Document document; + document.SetObject(); + // Expected key is not in the document + SetJsonValueByKey(document, "extraKey", 0, document.GetAllocator()); + int result = -1; + int exceptionLine = -1; + try + { + exceptionLine = __LINE__ + 1; + MUJINCLIENTJSON_LOAD_REQUIRED_JSON_VALUE_BY_KEY(document, expectedJsonKey.c_str(), result); + } + catch(const MujinJSONException& e) + { + std::stringstream ss; + ss << "mujin (): [" << __FILE__<<", "<< exceptionLine <<"] assert(mujinclient::mujinjson_external::LoadJsonValueByKey(document, key1, result))"; + if(ss.str() == std::string(e.what())) { + std::cout << "OK: "; + } + else{ + std::cout << "FAILED: \n"; + std::cout << "Expected = " << ss.str() << "\n"; + } + std::cout << "Got exception message = " << e.what() << "\n"; + } + catch(const std::exception& e) + { + std::cout << "FAILED: Caught an exception, but not the right type.\n" << "Exception message = " << e.what() << "\n"; + } +} + +int main() +{ + TestExistingKey(); + TestNonExistentKey(); +} From 92adce784bc9f86a363e7652b089f26bb455047d Mon Sep 17 00:00:00 2001 From: Devwrat Joshi Date: Tue, 14 Jun 2022 14:34:03 +0900 Subject: [PATCH 3/5] Add comment --- include/mujincontrollerclient/mujinjson.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/mujincontrollerclient/mujinjson.h b/include/mujincontrollerclient/mujinjson.h index 9ce498d7..141d9e9c 100644 --- a/include/mujincontrollerclient/mujinjson.h +++ b/include/mujincontrollerclient/mujinjson.h @@ -576,6 +576,7 @@ template inline void SaveJsonValue(rapidjson::Document& v, const T& t) template inline void SetJsonValueByKey(rapidjson::Value& v, const U& key, const T& t, rapidjson::Document::AllocatorType& alloc); //get one json value by key, and store it in local data structures +//returns false if the key does not exist or is null, else returns true template bool inline LoadJsonValueByKey(const rapidjson::Value& v, const char* key, T& t) { if (!v.IsObject()) { throw MujinJSONException("Cannot load value of non-object.", MJE_Failed); From c4bed9173d6a25a2876e402ec6278bf328fdbdeb Mon Sep 17 00:00:00 2001 From: Devwrat Joshi Date: Tue, 14 Jun 2022 15:36:20 +0900 Subject: [PATCH 4/5] Return true as long as key exists Should not return false if the value is null, as long as the key exists --- include/mujincontrollerclient/mujinjson.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mujincontrollerclient/mujinjson.h b/include/mujincontrollerclient/mujinjson.h index 141d9e9c..6d050a68 100644 --- a/include/mujincontrollerclient/mujinjson.h +++ b/include/mujincontrollerclient/mujinjson.h @@ -581,7 +581,7 @@ template bool inline LoadJsonValueByKey(const rapidjson::Value& v, cons if (!v.IsObject()) { throw MujinJSONException("Cannot load value of non-object.", MJE_Failed); } - if (v.HasMember(key) && !v.FindMember(key)->value.IsNull()) { + if (v.HasMember(key)) { LoadJsonValue(v[key], t); return true; } From e58649b81c7d611a3dd1438774b0fcba5b30419d Mon Sep 17 00:00:00 2001 From: Devwrat Joshi Date: Tue, 14 Jun 2022 17:57:34 +0900 Subject: [PATCH 5/5] Add printing additional info in case of json parsing error --- include/mujincontrollerclient/mujinjson.h | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/include/mujincontrollerclient/mujinjson.h b/include/mujincontrollerclient/mujinjson.h index 6d050a68..bc8a66b4 100644 --- a/include/mujincontrollerclient/mujinjson.h +++ b/include/mujincontrollerclient/mujinjson.h @@ -581,9 +581,18 @@ template bool inline LoadJsonValueByKey(const rapidjson::Value& v, cons if (!v.IsObject()) { throw MujinJSONException("Cannot load value of non-object.", MJE_Failed); } - if (v.HasMember(key)) { - LoadJsonValue(v[key], t); - return true; + rapidjson::Value::ConstMemberIterator itMember = v.FindMember(key); + if( itMember != v.MemberEnd() ) { + const rapidjson::Value& rMember = itMember->value; + if( !rMember.IsNull() ) { + try { + LoadJsonValue(rMember, t); + return true; + } + catch (const MujinJSONException& ex) { + throw MujinJSONException("Got \"" + ex.GetSubMessage() + "\" while parsing the value of \"" + key + "\"", MJE_Failed); + } + } } return false; }