Fix compositions returning wrong results when fast_fail is off - #214
Open
gaoflow wants to merge 1 commit into
Open
Fix compositions returning wrong results when fast_fail is off#214gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
anyOf, oneOf, not, if/then/else, contains and propertyNames only try their subschemas out and decide on the raised JsonSchemaValueException. With fast_fail=False nothing is raised, so the except branch is dead and the errors of a rejected attempt leak into the result - valid data was rejected. Subschemas of these keywords are now generated in raising mode, and the except also catches JsonSchemaValuesException, which is what a subschema behind a $ref raises from its own function. That $ref call is now merged into the caller's errors as well instead of aborting the rest of the validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fast_fail=Falseis documented as a way to collect all the errors, so it must not changewhether data is valid. It does:
Every keyword that only tries out a subschema decides on a raised
JsonSchemaValueException, and in collecting mode nothing raises: theexceptbranch is deadand the errors of a rejected attempt leak into the result. That hits
anyOf,oneOf(everybranch counts as a match, so the count is never 1),
not(theelsealways fires),if(itselseis unreachable),containsandpropertyNames. A subschema behind a$refis a secondcase: it stays its own function and reports failure with
JsonSchemaValuesException, whichexcept JsonSchemaValueExceptiondoes not catch, so it escapes the whole validator.I ran the bundled JSON-Schema-Test-Suite through both modes for draft-04/06/07/2019-09:
171 of 5293 cases got a different verdict with
fast_fail=False, 146 of them where thedefault mode is right. The generated-code path is the same, 110 divergences on the composition
files. After this change both are 0, and 0 across the
fast_failxdetailed_exceptionsxuse_defaultmatrix as well.Two halves, both in code generation: subschemas that are only tried out are generated in
raising mode (
trial_validation()) so thetry/exceptworks as before, and thoseexceptclauses also catch
JsonSchemaValuesException. The second part is needed even inside a trialblock, because the exception type of a
$refed function follows the top-levelfast_fail, notthe local one (
{'anyOf': [{'not': {'$ref': ...}}]}). While there, a plain$refcall incollecting mode now merges the referenced function's errors into the caller's list instead of
aborting the rest of the document -
{'a': {'$ref': ...}, 'b': {'type': 'string'}}used toreport only the errors of
a.Generated code for the default
fast_fail=Trueonly changes in that widerexcepttuple; nomeasurable difference in an
anyOf/nothot loop. Existing suite unchanged (4408 passed / 449xfailed / 936 xpassed), pylint stays at 9.91.
Tests in
tests/test_fast_fail.py: a verdict-parity matrix over the six keywords (valid andinvalid data, nested compositions,
$refed variants), error-list assertions for a rejectedattempt and for a genuinely failing composition, error collection through
$ref, and the samecheck on
compile_to_codeoutput. 16 of them fail on master.