From 467368d022da63c916aa6c141971b930b6f83561 Mon Sep 17 00:00:00 2001 From: Tristan Penman Date: Sun, 15 Jun 2014 22:09:38 +1000 Subject: [PATCH] Improve contexts for error reporting, and fix unicode bug. --- examples/external_schema.cpp | 9 ++++- include/valijson/utils/utf8_utils.hpp | 3 +- include/valijson/validation_results.hpp | 7 ++-- include/valijson/validation_visitor.hpp | 44 +++++++++++++----------- include/valijson/validator.hpp | 6 ++-- tests/test_validation_errors.cpp | 28 ++++++++++----- xcode/valijson.xcodeproj/project.pbxproj | 4 +-- 7 files changed, 62 insertions(+), 39 deletions(-) diff --git a/examples/external_schema.cpp b/examples/external_schema.cpp index d1cf9cd..784d906 100644 --- a/examples/external_schema.cpp +++ b/examples/external_schema.cpp @@ -69,8 +69,15 @@ int main(int argc, char *argv[]) ValidationResults::Error error; unsigned int errorNum = 1; while (results.popError(error)) { + + std::string context; + std::vector::iterator itr = error.context.begin(); + for (; itr != error.context.end(); itr++) { + context += *itr; + } + cerr << "Error #" << errorNum << std::endl - << " context: " << error.context << endl + << " context: " << context << endl << " desc: " << error.description << endl; ++errorNum; } diff --git a/include/valijson/utils/utf8_utils.hpp b/include/valijson/utils/utf8_utils.hpp index fadc06a..529d010 100644 --- a/include/valijson/utils/utf8_utils.hpp +++ b/include/valijson/utils/utf8_utils.hpp @@ -41,8 +41,9 @@ inline int u8_strlen(const char *s) int count = 0; int i = 0; - while (u8_nextchar(s, &i) != 0) + while (s[i] != 0 && u8_nextchar(s, &i) != 0) { count++; + } return count; } diff --git a/include/valijson/validation_results.hpp b/include/valijson/validation_results.hpp index 80d272e..dc78a24 100644 --- a/include/valijson/validation_results.hpp +++ b/include/valijson/validation_results.hpp @@ -3,6 +3,7 @@ #include #include +#include namespace valijson { @@ -36,12 +37,12 @@ public: * @param context Context string to use * @param description Description string to use */ - Error(const std::string &context, const std::string &description) + Error(const std::vector &context, const std::string &description) : context(context), description(description) { } /// Path to the node that failed validation. - std::string context; + std::vector context; /// A detailed description of the validation error. std::string description; @@ -72,7 +73,7 @@ public: * @param description Description of the validation error. */ void - pushError(const std::string &context, const std::string &description) + pushError(const std::vector &context, const std::string &description) { errors.push_back(Error(context, description)); } diff --git a/include/valijson/validation_visitor.hpp b/include/valijson/validation_visitor.hpp index 1a3d10e..6ff5e16 100644 --- a/include/valijson/validation_visitor.hpp +++ b/include/valijson/validation_visitor.hpp @@ -38,7 +38,7 @@ public: * stop immediately. */ ValidationVisitor(const AdapterType &target, - const std::string &context, + const std::vector &context, const bool strictTypes, ValidationResults *results) : target(target), @@ -120,7 +120,7 @@ public: if (results) { validated = false; results->pushError(context, - std::string("Failed to validate against child schema at index #") + + std::string("Failed to validate against child schema #") + boost::lexical_cast(index) + " of allOf constraint."); } else { return false; @@ -165,7 +165,7 @@ public: } if (results) { - results->pushError(context, "Failed to validate against any child schemas."); + results->pushError(context, "Failed to validate against any child schemas allowed by anyOf constraint."); } return false; @@ -305,10 +305,10 @@ public: // Validate all items against single schema unsigned int index = 0; BOOST_FOREACH( const AdapterType arrayItem, target.getArray() ) { + std::vector newContext = context; + newContext.push_back("[" + boost::lexical_cast(index) + "]"); ValidationVisitor v(arrayItem, - context + "[" + boost::lexical_cast(index) + "]", - strictTypes, - results); + newContext, strictTypes, results); if (!v.validateSchema(*constraint.itemSchema)) { if (results) { results->pushError(context, "Failed to validate item #" + boost::lexical_cast(index) + " in array."); @@ -338,10 +338,10 @@ public: // additionalItems schema unsigned int index = 0; BOOST_FOREACH( const AdapterType arrayItem, target.getArray() ) { + std::vector newContext = context; + newContext.push_back("[" + boost::lexical_cast(index) + "]"); ValidationVisitor v(arrayItem, - context + "[" + boost::lexical_cast(index) + "]", - strictTypes, - results); + newContext, strictTypes, results); if (index >= constraint.itemSchemas->size()) { if (constraint.additionalItemsSchema) { if (!v.validateSchema(*constraint.additionalItemsSchema)) { @@ -376,10 +376,10 @@ public: // Validate each item against additional items schema unsigned int index = 0; BOOST_FOREACH( const AdapterType arrayItem, target.getArray() ) { + std::vector newContext = context; + newContext.push_back("[" + boost::lexical_cast(index) + "]"); ValidationVisitor v(arrayItem, - context + "[" + boost::lexical_cast(index) + "]", - strictTypes, - results); + newContext, strictTypes, results); if (!v.validateSchema(*constraint.additionalItemsSchema)) { if (results) { results->pushError(context, "Failed to validate item #" + @@ -729,7 +729,11 @@ public: const std::string propertyName = m.first; bool propertyNameMatched = false; - ValidationVisitor v(m.second, context + "." + m.first, strictTypes, results); + std::vector newContext = context; + newContext.push_back("[\"" + m.first + "\"]"); + + ValidationVisitor v(m.second, + newContext, strictTypes, results); // Search for matching property name PropertiesConstraint::PropertySchemaMap::const_iterator itr = @@ -739,8 +743,8 @@ public: if (!v.validateSchema(*itr->second)) { if (results) { results->pushError(context, - "Failed to validate against 'properties' schema associated with property name '" + - propertyName + "'."); + "Failed to validate against schema associated with property name '" + + propertyName + "' in properties constraint."); validated = false; } else { return false; @@ -757,8 +761,8 @@ public: if (!v.validateSchema(*itr->second)) { if (results) { results->pushError(context, - "Failed to validate against 'patternProperties' schema associated with regex '" + - itr->first + "'."); + "Failed to validate against schema associated with regex '" + + itr->first + "' in patternProperties constraint."); validated = false; } else { return false; @@ -782,7 +786,7 @@ public: continue; } else if (results) { results->pushError(context, "Failed to validate property '" + - propertyName + "' against additionalProperties schema."); + propertyName + "' against schema in additionalProperties constraint."); validated = false; } else { return false; @@ -965,8 +969,8 @@ private: /// Reference to the JSON value being validated const AdapterType ⌖ - /// String describing the current object context - const std::string context; + /// Vector of strings describing the current object context + const std::vector context; /// Optional pointer to a ValidationResults object to be populated ValidationResults *results; diff --git a/include/valijson/validator.hpp b/include/valijson/validator.hpp index 48d8c96..aa8497f 100644 --- a/include/valijson/validator.hpp +++ b/include/valijson/validator.hpp @@ -18,8 +18,8 @@ class ValidationResults; * This class wraps a Schema object, and encapsulates the logic required to * validate rapidjson values aginst the schema. */ -class Validator { - +class Validator +{ public: /** @@ -68,7 +68,7 @@ public: bool validate(const AdapterType &target, ValidationResults *results) { // Construct a ValidationVisitor to perform validation at the root level - ValidationVisitor v(target, std::string(), + ValidationVisitor v(target, std::vector(1, ""), strictTypes, results); return v.validateSchema(*schema); diff --git a/tests/test_validation_errors.cpp b/tests/test_validation_errors.cpp index 8bcff99..fba6a26 100644 --- a/tests/test_validation_errors.cpp +++ b/tests/test_validation_errors.cpp @@ -53,37 +53,47 @@ TEST_F(TestValidationErrors, AllOfConstraintFailure) ValidationResults::Error error; EXPECT_TRUE( results.popError(error) ); - EXPECT_EQ( "[0]", error.context ); + EXPECT_EQ( 2, error.context.size() ); + EXPECT_EQ( "", error.context[0] ); + EXPECT_EQ( "[0]", error.context[1] ); EXPECT_EQ( "Value type not permitted by 'type' constraint.", error.description ); EXPECT_TRUE( results.popError(error) ); - EXPECT_EQ( "", error.context ); + EXPECT_EQ( 1, error.context.size() ); + EXPECT_EQ( "", error.context[0] ); EXPECT_EQ( "Failed to validate item #0 in array.", error.description ); EXPECT_TRUE( results.popError(error) ); - EXPECT_EQ( "[1]", error.context ); + EXPECT_EQ( 2, error.context.size() ); + EXPECT_EQ( "", error.context[0] ); + EXPECT_EQ( "[1]", error.context[1] ); EXPECT_EQ( "Value type not permitted by 'type' constraint.", error.description ); EXPECT_TRUE( results.popError(error) ); - EXPECT_EQ( "", error.context ); + EXPECT_EQ( 1, error.context.size() ); + EXPECT_EQ( "", error.context[0] ); EXPECT_EQ( "Failed to validate item #1 in array.", error.description ); EXPECT_TRUE( results.popError(error) ); - EXPECT_EQ( "[2]", error.context ); + EXPECT_EQ( 2, error.context.size() ); + EXPECT_EQ( "", error.context[0] ); + EXPECT_EQ( "[2]", error.context[1] ); EXPECT_EQ( "Value type not permitted by 'type' constraint.", error.description ); EXPECT_TRUE( results.popError(error) ); - EXPECT_EQ( "", error.context ); + EXPECT_EQ( 1, error.context.size() ); + EXPECT_EQ( "", error.context[0] ); EXPECT_EQ( "Failed to validate item #2 in array.", error.description ); EXPECT_TRUE( results.popError(error) ); - EXPECT_EQ( "", error.context ); - EXPECT_EQ( "Failed to validate against child schema at index #0 of allOf constraint.", error.description ); + EXPECT_EQ( 1, error.context.size() ); + EXPECT_EQ( "", error.context[0] ); + EXPECT_EQ( "Failed to validate against child schema #0 of allOf constraint.", error.description ); EXPECT_FALSE( results.popError(error) ); while (results.popError(error)) { - std::cerr << error.context << std::endl; + //std::cerr << error.context << std::endl; std::cerr << error.description << std::endl; } } diff --git a/xcode/valijson.xcodeproj/project.pbxproj b/xcode/valijson.xcodeproj/project.pbxproj index 77fac5d..a32f596 100644 --- a/xcode/valijson.xcodeproj/project.pbxproj +++ b/xcode/valijson.xcodeproj/project.pbxproj @@ -16,7 +16,7 @@ 6A725F4917F6404100D6B2FF /* test_rapidjson_adapter.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6AC18D3917CC874100FE0EC9 /* test_rapidjson_adapter.cpp */; }; 6A725F4A17F6404100D6B2FF /* test_validator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6AC18D3517CC86E000FE0EC9 /* test_validator.cpp */; }; 6A725F4D17F8964B00D6B2FF /* test_uri_resolution.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6A725F4B17F8956A00D6B2FF /* test_uri_resolution.cpp */; }; - 6AA8A5DB17F8BDCA002728A0 /* test_dereference_callback.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6AA8A5DA17F8BDCA002728A0 /* test_dereference_callback.cpp */; }; + 6A9E1856194DC44B003F1C4C /* test_dereference_callback.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6AA8A5DA17F8BDCA002728A0 /* test_dereference_callback.cpp */; }; 6AB8FE8717E6A56F0028E147 /* external_schema.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6AB8FE8617E6A56F0028E147 /* external_schema.cpp */; }; 6AB8FE8D17E6A57E0028E147 /* libboost_regex-mt.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 6A477F8417D6BCBB0013571C /* libboost_regex-mt.dylib */; }; 6AB8FEAD17E6C0D70028E147 /* json_reader.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6AB8FEA717E6C08D0028E147 /* json_reader.cpp */; }; @@ -845,7 +845,6 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - 6AA8A5DB17F8BDCA002728A0 /* test_dereference_callback.cpp in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -855,6 +854,7 @@ files = ( 6AB8FEAD17E6C0D70028E147 /* json_reader.cpp in Sources */, 6AB8FEAE17E6C0D70028E147 /* json_value.cpp in Sources */, + 6A9E1856194DC44B003F1C4C /* test_dereference_callback.cpp in Sources */, 6AB8FEAF17E6C0D70028E147 /* json_writer.cpp in Sources */, 6A725F4517F61D7000D6B2FF /* test_validation_errors.cpp in Sources */, 6AF70BB018FE728800342325 /* gtest-all.cc in Sources */,