pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
466 stars 134 forks source link

basic_error_handler in v2 #45

Closed mabondarenko closed 5 years ago

mabondarenko commented 5 years ago

When a validation error occurs, the path parameter of error() method in most cases contains an empty string. Is it possible to implement so that I get the full path to the name of the field on which the validation error occurred? Something like this: root_object.nested_object1.nested_object2.field_name

Without this functionality, validation becomes useless. The user needs to get the name of the field of a particular object.

Look at: https://github.com/pboettch/json-schema-validator/issues/44#issue-398665292


class MyErrorHandler : public basic_error_handler
{
public:
  virtual void error(const string& path, const json& instance, const string& message) override
  {
    std::cout << "path='" << path << "': '" << instance.dump() << "'\n" << message;
    basic_error_handler::error(path, instance, message);
  }
};

>>>

path='': '"2018-09-12 19:00:00"'
A format checker was not provided but a format-attribute for this string is present.  cannot be validated for date-time```
pboettch commented 5 years ago

Yes, sorry. The path is not filled in yet. Haven't had time to work it out.

pboettch commented 5 years ago

How should the path be provided? Just as a string with dots, or via nlohmann::json_pointer?

mabondarenko commented 5 years ago

i think the string with dots will be better. cause is human readable and portable format than json_pointer and others...

mabondarenko commented 5 years ago

... What is a validator? In our language - is like the c++ compiler. it must specify the variable name and the cause of the error.

pboettch commented 5 years ago

I totally agree.

Regarding the std::string vs. json_pointer. From the programmer's point of view a json_pointer could be more versatile than a string. That's why I'm unsure.

I'll try to fix the bug tomorrow.

mabondarenko commented 5 years ago

I totally agree.

Regarding the std::string vs. json_pointer. From the programmer's point of view a json_pointer could be more versatile than a string. That's why I'm unsure.

I'll try to fix the bug tomorrow.

I also do not argue with you and fully agree with your point of view (from c++ point of view). Will we be able to recognize the full name of a field using a json-pointer, and can we, for example, use this type in other languages, such as Java or Python? If yes, we can use json-pointer....

pboettch commented 5 years ago

Yes, json_pointer is standard.

Also, in a custom error-handler you can still transform it to a string separated with dots.

I'll go for it.

mabondarenko commented 5 years ago

I understand you... You will need to think about API compatibility. Thanks you. Nice to talk with smart people.

garethsb commented 5 years ago

On trying to upgrade from v1.0.0, I noticed I was getting slightly garbled exception messages starting "At of ". I also found this is because current throwing_error_handler formats the message as "At " + path + " of " +... but as described in this issue, path is always(?) empty.

pboettch commented 5 years ago

This is what this bug is about. I'm suggesting to change the first parameter of the error-handler from std::string to nlohmann::json_pointer.

pboettch commented 5 years ago

This issue is fixed on the json_pointer-push-pop-branch. But this will require a new version of nlohmann::json. Integration and discussions are currently on-going.

garethsb commented 5 years ago

Thanks! I wanted to test this with nlohmann/json v3.5.0, so I just replaced uses of operator+/json_pointer::push_back from https://github.com/nlohmann/json/pull/1454 with a one-line function modelled on previous json_uri::append, and it's looking pretty good. Error messages are nice - I will probably use a custom handler so that I don't ever serialize the entire json (which could be huge) into the message.

Minor comment: I think the error messages in logical_combination could be clearer:

garethsb commented 5 years ago

With v1.0.0, when a logical combination schema fails, the error message included the detail from the sub-schema(s) that failed. That's not currently the case for logical_combination - the subschema basic_error_handler info is discarded. Do you think it would be possible/worthwhile to include that info?

pboettch commented 5 years ago

The master-branch now contains an error-handler receiving json-pointers to show where in the json-instance the error has appeared. This should fix this issue. Please re-open if things are still missing.