Skip to content

Commit

Permalink
Don't let best_match traverse into applicators with equally bad sub-e…
Browse files Browse the repository at this point in the history
…rrors.

I.e., if an anyOf or oneOf has branches where all errors seem equally bad,
return the anyOf/oneOf itself, rather than traversing into its context.

Closes: #646

Which should now produce a nicer error.
  • Loading branch information
Julian committed Jul 26, 2022
1 parent 8e090fb commit bd5ea73
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 4 deletions.
8 changes: 7 additions & 1 deletion jsonschema/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from collections import defaultdict, deque
from pprint import pformat
from textwrap import dedent, indent
import heapq
import itertools

import attr
Expand Down Expand Up @@ -383,5 +384,10 @@ def best_match(errors, key=relevance):
best = max(itertools.chain([best], errors), key=key)

while best.context:
best = min(best.context, key=key)
# Calculate the minimum via nsmallest, because we don't recurse if
# all nested errors have the same relevance (i.e. if min == max == all)
smallest = heapq.nsmallest(2, best.context, key=key)
if len(smallest) == 2 and key(smallest[0]) == key(smallest[1]):
return best
best = smallest[0]
return best
65 changes: 65 additions & 0 deletions jsonschema/tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,36 @@ def test_if_the_most_relevant_error_is_anyOf_it_is_traversed(self):
best = self.best_match_of(instance={"foo": {"bar": 12}}, schema=schema)
self.assertEqual(best.validator_value, "array")

def test_no_anyOf_traversal_for_equally_relevant_errors(self):
"""
We don't traverse into an anyOf (as above) if all of its context errors
seem to be equally "wrong" against the instance.
"""

schema = {
"anyOf": [
{"type": "string"},
{"type": "integer"},
{"type": "object"},
],
}
best = self.best_match_of(instance=[], schema=schema)
self.assertEqual(best.validator, "anyOf")

def test_anyOf_traversal_for_single_equally_relevant_error(self):
"""
We *do* traverse anyOf with a single nested error, even though it is
vacuously equally relevant to itself.
"""

schema = {
"anyOf": [
{"type": "string"},
],
}
best = self.best_match_of(instance=[], schema=schema)
self.assertEqual(best.validator, "type")

def test_if_the_most_relevant_error_is_oneOf_it_is_traversed(self):
"""
If the most relevant error is an oneOf, then we traverse its context
Expand All @@ -89,6 +119,36 @@ def test_if_the_most_relevant_error_is_oneOf_it_is_traversed(self):
best = self.best_match_of(instance={"foo": {"bar": 12}}, schema=schema)
self.assertEqual(best.validator_value, "array")

def test_no_oneOf_traversal_for_equally_relevant_errors(self):
"""
We don't traverse into an oneOf (as above) if all of its context errors
seem to be equally "wrong" against the instance.
"""

schema = {
"oneOf": [
{"type": "string"},
{"type": "integer"},
{"type": "object"},
],
}
best = self.best_match_of(instance=[], schema=schema)
self.assertEqual(best.validator, "oneOf")

def test_oneOf_traversal_for_single_equally_relevant_error(self):
"""
We *do* traverse oneOf with a single nested error, even though it is
vacuously equally relevant to itself.
"""

schema = {
"oneOf": [
{"type": "string"},
],
}
best = self.best_match_of(instance=[], schema=schema)
self.assertEqual(best.validator, "type")

def test_if_the_most_relevant_error_is_allOf_it_is_traversed(self):
"""
Now, if the error is allOf, we traverse but select the *most* relevant
Expand All @@ -109,6 +169,11 @@ def test_if_the_most_relevant_error_is_allOf_it_is_traversed(self):
self.assertEqual(best.validator_value, "string")

def test_nested_context_for_oneOf(self):
"""
We traverse into nested contexts (a oneOf containing an error in a
nested oneOf here).
"""

schema = {
"properties": {
"foo": {
Expand Down
10 changes: 7 additions & 3 deletions jsonschema/tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -1959,11 +1959,15 @@ def test_schema_error_message(self):
)

def test_it_uses_best_match(self):
# This is a schema that best_match will recurse into
schema = {"oneOf": [{"type": "string"}, {"type": "array"}]}
schema = {
"oneOf": [
{"type": "number", "minimum": 20},
{"type": "array"},
],
}
with self.assertRaises(exceptions.ValidationError) as e:
validators.validate(12, schema)
self.assertIn("12 is not of type", str(e.exception))
self.assertIn("12 is less than the minimum of 20", str(e.exception))


class TestRefResolver(TestCase):
Expand Down

0 comments on commit bd5ea73

Please sign in to comment.