jsonrainbow / json-schema

PHP implementation of JSON schema. Fork of the http://jsonschemaphpv.sourceforge.net/ project
MIT License
3.52k stars 349 forks source link

display violation elements #578

Open thawkins opened 5 years ago

thawkins commented 5 years ago

I would like to propose having the error logging on the constraints checkers optionaly store a value called "element" that shows the actual value that failed the constraint, this would be valuable in understanding the violations. this could be linked to a constraints::[option] to turn it on and off.

for example

changing line 52 in the enumconstraints.php from

$this->addError($path, 'Does not have a value in the enumeration ' . json_encode($schema->enum), 'enum', array('enum' => $schema->enum));

to:

$this->addError($path, 'Does not have a value in the enumeration ' . json_encode($schema->enum), 'enum', array('enum' => $schema->enum, 'element'=>$element)); 

would meet this requirement, i'm sure there are lots of other places in the other constraints checkers where this would make sense.

There is one hazard, if the $element is itself the root of a large subtree it could bloat things on the output, in particular if the violation is detected on the root of the document being validated. requiredConstraint already displays the name of the violating element as does many other constraint checkers.

erayd commented 5 years ago

We already provide an explicit path / pointer to the failing element, so that users may refer directly to the value within the source document. Can you expand on why that isn't sufficient?

thawkins commented 5 years ago

You have to dig into tbe document to find the issue in the structure if you only have the path, particularly with enums its useful to show what the actual value that failed the constraint check is, its make it easier to determine what the failing item is.

I have been working on validating a 2000 line schema against a set of mongodb records, we wrote an etl to work througb all the records in a collection and run the validator against each, storing the violations and the record id in another collection. Having tbe violating element in the output makes interpretting that data much easier.

Its not a big deal, i can hack the package for our use, but it woukd have been easer for me to have it in the distribution.

erayd commented 5 years ago

@thawkins I'm open to having it included, provided it doesn't impact performance and doesn't introduce needless complexity.

@shmax What do you think?

shmax commented 5 years ago

No objections.

erayd commented 5 years ago

@thawkins In light of the discussion above, please feel free to submit a PR that implements this, and we will look to getting it merged in to the v6 branch.