From b4f098bdccb25b6d124050e4ecda48371c7ccc4a Mon Sep 17 00:00:00 2001 From: Julian Berman Date: Sun, 27 Oct 2013 20:38:49 -0400 Subject: Different strategy that's a lot more robust. --- jsonschema/exceptions.py | 40 ++++----- jsonschema/tests/test_exceptions.py | 157 ++++++++++++++++++++---------------- 2 files changed, 106 insertions(+), 91 deletions(-) diff --git a/jsonschema/exceptions.py b/jsonschema/exceptions.py index 8df987d..04a3c0b 100644 --- a/jsonschema/exceptions.py +++ b/jsonschema/exceptions.py @@ -7,6 +7,9 @@ from jsonschema import _utils from jsonschema.compat import PY3, iteritems +WEAK_MATCHES = frozenset(["anyOf", "oneOf"]) +STRONG_MATCHES = frozenset() + _unset = _utils.Unset() @@ -30,24 +33,6 @@ class _Error(Exception): return NotImplemented return self._contents() == other._contents() - def __lt__(self, other): - if not isinstance(other, self.__class__): - # On Py2 Python will "helpfully" make this succeed. So be more - # forceful, because we really don't want this to work, it probably - # means a ValidationError and a SchemaError are being compared - # accidentally. - if not PY3: - message = "unorderable types: %s() < %s()" % ( - self.__class__.__name__, other.__class__.__name__, - ) - raise TypeError(message) - return NotImplemented - - is_deeper = len(self.path) > len(other.path) - is_weak_matcher = self.validator in ("anyOf", "oneOf") - other_is_weak_matcher = other.validator in ("anyOf", "oneOf") - return is_deeper or is_weak_matcher > other_is_weak_matcher - def __ne__(self, other): return not self == other @@ -159,11 +144,20 @@ class FormatError(Exception): __str__ = __unicode__ -def best_match(errors): - first = next(iter(errors), None) - if first is None: +def by_relevance(weak=WEAK_MATCHES, strong=STRONG_MATCHES): + def relevance(error): + validator = error.validator + return -len(error.path), validator not in weak, validator in strong + return relevance + + +def best_match(errors, key=by_relevance()): + errors = iter(errors) + best = next(errors, None) + if best is None: return - best = max(itertools.chain([first], errors)) + best = max(itertools.chain([best], errors), key=key) + while best.context: - best = min(best.context) + best = min(best.context, key=key) return best diff --git a/jsonschema/tests/test_exceptions.py b/jsonschema/tests/test_exceptions.py index 92976b3..d6c7490 100644 --- a/jsonschema/tests/test_exceptions.py +++ b/jsonschema/tests/test_exceptions.py @@ -2,7 +2,19 @@ from jsonschema import Draft4Validator, exceptions from jsonschema.tests.compat import mock, unittest -class TestValidationErrorSorting(unittest.TestCase): +class TestBestMatch(unittest.TestCase): + def best_match(self, errors): + errors = list(errors) + best = exceptions.best_match(errors) + reversed_best = exceptions.best_match(reversed(errors)) + self.assertEqual( + best, + reversed_best, + msg="Didn't return a consistent best match!\n" + "Got: {0}\n\nThen: {1}".format(best, reversed_best), + ) + return best + def test_shallower_errors_are_better_matches(self): validator = Draft4Validator( { @@ -14,24 +26,8 @@ class TestValidationErrorSorting(unittest.TestCase): } } ) - errors = sorted(validator.iter_errors({"foo" : {"bar" : []}})) - self.assertEqual( - [list(error.path) for error in errors], - [["foo", "bar"], ["foo"]], - ) - - def test_global_errors_are_even_better_matches(self): - validator = Draft4Validator( - { - "minProperties" : 2, - "properties" : {"foo" : {"type" : "array"}}, - } - ) - errors = sorted(validator.iter_errors({"foo" : {"bar" : []}})) - self.assertEqual( - [list(error.path) for error in errors], - [["foo"], []], - ) + best = self.best_match(validator.iter_errors({"foo" : {"bar" : []}})) + self.assertEqual(best.validator, "minProperties") def test_oneOf_and_anyOf_are_weak_matches(self): """ @@ -43,47 +39,21 @@ class TestValidationErrorSorting(unittest.TestCase): validator = Draft4Validator( { "minProperties" : 2, + "anyOf" : [{"type" : "string"}, {"type" : "number"}], "oneOf" : [{"type" : "string"}, {"type" : "number"}], } ) - errors = sorted(validator.iter_errors({})) - self.assertEqual( - [error.validator for error in errors], ["oneOf", "minProperties"], - ) - - def test_cannot_sort_errors_of_mixed_types(self): - with self.assertRaises(TypeError): - v = exceptions.ValidationError("Oh", instance=3) - s = exceptions.SchemaError("No!", instance=3) - v < s - - -class TestBestMatch(unittest.TestCase): - def test_for_errors_without_context_it_returns_the_max(self): - """ - The ``max`` will be the error which is most "shallow" in the instance. - - """ - - validator = Draft4Validator( - { - "properties" : { - "foo" : { - "minProperties" : 2, - "properties" : {"bar" : {"type" : "object"}}, - }, - }, - } - ) - errors = sorted(validator.iter_errors({"foo" : {"bar" : []}})) - self.assertIs(exceptions.best_match(errors), errors[-1]) + best = self.best_match(validator.iter_errors({})) + self.assertEqual(best.validator, "minProperties") - def test_context_for_anyOf(self): + def test_if_the_most_relevant_error_is_anyOf_it_is_traversed(self): """ - For the anyOf validator, we use the min, to assume the least. + If the most relevant error is an anyOf, then we traverse its context + and select the otherwise *least* relevant error, since in this case + that means the most specific, deep, error inside the instance. - Other errors are not necessarily relevant, since only one needs to - match. + I.e. since only one of the schemas must match, we look for the most + relevant one. """ @@ -99,16 +69,17 @@ class TestBestMatch(unittest.TestCase): }, }, ) - errors = validator.iter_errors({"foo" : {"bar" : 12}}) - best = exceptions.best_match(errors) + best = self.best_match(validator.iter_errors({"foo" : {"bar" : 12}})) self.assertEqual(best.validator_value, "array") - def test_context_for_oneOf(self): + def test_if_the_most_relevant_error_is_oneOf_it_is_traversed(self): """ - For the oneOf validator, we use the min, to assume the least. + If the most relevant error is an oneOf, then we traverse its context + and select the otherwise *least* relevant error, since in this case + that means the most specific, deep, error inside the instance. - Other errors are not necessarily relevant, since only one needs to - match. + I.e. since only one of the schemas must match, we look for the most + relevant one. """ @@ -124,13 +95,13 @@ class TestBestMatch(unittest.TestCase): }, }, ) - errors = validator.iter_errors({"foo" : {"bar" : 12}}) - best = exceptions.best_match(errors) + best = self.best_match(validator.iter_errors({"foo" : {"bar" : 12}})) self.assertEqual(best.validator_value, "array") - def test_context_for_allOf(self): + def test_if_the_most_relevant_error_is_allOf_it_is_traversed(self): """ - allOf just yields all the errors globally, so each should be considered + Now, if the error is allOf, we traverse but select the *most* relevant + error from the context, because all schemas here must match anyways. """ @@ -146,8 +117,7 @@ class TestBestMatch(unittest.TestCase): }, }, ) - errors = validator.iter_errors({"foo" : {"bar" : 12}}) - best = exceptions.best_match(errors) + best = self.best_match(validator.iter_errors({"foo" : {"bar" : 12}})) self.assertEqual(best.validator_value, "string") def test_nested_context_for_oneOf(self): @@ -172,8 +142,7 @@ class TestBestMatch(unittest.TestCase): }, }, ) - errors = validator.iter_errors({"foo" : {"bar" : 12}}) - best = exceptions.best_match(errors) + best = self.best_match(validator.iter_errors({"foo" : {"bar" : 12}})) self.assertEqual(best.validator_value, "array") def test_one_error(self): @@ -186,3 +155,55 @@ class TestBestMatch(unittest.TestCase): def test_no_errors(self): validator = Draft4Validator({}) self.assertIsNone(exceptions.best_match(validator.iter_errors({}))) + + +class TestByRelevance(unittest.TestCase): + def test_short_paths_are_better_matches(self): + shallow = exceptions.ValidationError("Oh no!", path=["baz"]) + deep = exceptions.ValidationError("Oh yes!", path=["foo", "bar"]) + match = max([shallow, deep], key=exceptions.by_relevance()) + self.assertIs(match, shallow) + + match = max([deep, shallow], key=exceptions.by_relevance()) + self.assertIs(match, shallow) + + def test_global_errors_are_even_better_matches(self): + shallow = exceptions.ValidationError("Oh no!", path=[]) + deep = exceptions.ValidationError("Oh yes!", path=["foo"]) + + errors = sorted([shallow, deep], key=exceptions.by_relevance()) + self.assertEqual( + [list(error.path) for error in errors], + [["foo"], []], + ) + + errors = sorted([deep, shallow], key=exceptions.by_relevance()) + self.assertEqual( + [list(error.path) for error in errors], + [["foo"], []], + ) + + def test_weak_validators_are_lower_priority(self): + weak = exceptions.ValidationError("Oh no!", path=[], validator="a") + normal = exceptions.ValidationError("Oh yes!", path=[], validator="b") + + best_match = exceptions.by_relevance(weak="a") + + match = max([weak, normal], key=best_match) + self.assertIs(match, normal) + + match = max([normal, weak], key=best_match) + self.assertIs(match, normal) + + def test_strong_validators_are_higher_priority(self): + weak = exceptions.ValidationError("Oh no!", path=[], validator="a") + normal = exceptions.ValidationError("Oh yes!", path=[], validator="b") + strong = exceptions.ValidationError("Oh fine!", path=[], validator="c") + + best_match = exceptions.by_relevance(weak="a", strong="c") + + match = max([weak, normal, strong], key=best_match) + self.assertIs(match, strong) + + match = max([strong, normal, weak], key=best_match) + self.assertIs(match, strong) -- cgit v1.2.1