Bug in IfThenElse
Closed this issue · 9 comments
Hi, I found an issue when using IfThenElse. Looks like it's unable to validate dependent properties. I also noticed the Validator Selector does not select the validator governing the required validation because the schema does not have PropertyNames.
Check out my unit tests below:
master...jinhong-:master
I think removing the PropertyNames != null seems to do the trick
By the way i noticed the sub validation error messages from if then else gets swallowed. Would it make sense to return the underlying sub validation error messages instead?
On a separate note, it would be good if there is a way to include the full property path since in theory you can have 2 of the same properties but in different paths eg. A.B, B.B C.B will all print as B is required but i am not sure which B is the correct B
Regarding the error messaging, the exception has a Path property that allows you to see the full path to the offending element.
I'll check into the validation issue. Could you post your schema and test JSON? I pass the official schema test suite. They may be interested in adding your case.
I took it from here:
https://ajv.js.org/keywords.html#ifthenelse
But if you notice i tweaked the schema in my test cases because i think there are still variations in how it is being implemented. Look at this particular case where the "minimum" of the "power" property is 9000.
In their example, an empty object is considered valid. But I personally believe it should not be valid because it'll fall into the "then" validation since the "if" validation is valid. And even if it isn't, it should at least fall into the "else" validation but in the example, they are treating it as valid.
What i noticed is that the validation error comes out as:
Message: Validation of if succeeded, but validation of then failed
PropertyName: then
What I meant to say is instead of showing this, I think it would make sense to show "Required property not found" since that is the validation that was applied
In their example, an empty object is considered valid. But I personally believe it should not be valid because it'll fall into the "then" validation since the "if" validation is valid. And even if it isn't, it should at least fall into the "else" validation but in the example, they are treating it as valid.
@handrews (the primary author of JSON Schema, currently) and I both agree with your assessment of this example. {} should be an invalid instance. I'll make a test to cover this case.
I can also add your test cases.
On the topic of messaging, I'll try to incorporate the error messages from the failed subschema.
I have identified a bug in that required is ignored if there are no properties defined. This is incorrect behavior. I've fixed that, but I'm also working on the error messaging before I push out a new version.
I also still need to pull in your tests. I'm having some issues with running my tests right now. I think it's just a local issue; the server build seems to run fine, but I don't want to rely on it.
@jinhong- you may be interested in #112 and json-schema-org/json-schema-spec#530. I think this will open up the mechanism to customizing the error messages.
I'm closing this issue in favor of that one since the if-then-else bug has been fixed.
@jinhong- you may be interested in the available prerelease of v10. The schema implementation has been completely reworked, including the output format. I think you'll find that the new format is more explicit as to what the actual failure is.