diff --git a/voluptuous/schema_builder.py b/voluptuous/schema_builder.py index 3673e86..a368dc2 100644 --- a/voluptuous/schema_builder.py +++ b/voluptuous/schema_builder.py @@ -343,6 +343,14 @@ def validate_mapping(path, iterable, out): except er.Invalid as e: if len(e.path) > len(key_path): raise + # This candidate key schema/validator rejected the + # provided key outright -- the data's key has + # nothing to do with this candidate, as opposed to + # a key that matched but whose value was wrong. + # Tag it so callers such as _compile_sequence's + # list-of-dict-alternatives heuristic can tell the + # two situations apart. + e._key_shape_mismatch = True if not error or len(e.path) > len(error.path): error = e continue @@ -395,7 +403,9 @@ def validate_mapping(path, iterable, out): elif error: errors.append(error) else: - errors.append(er.Invalid('extra keys not allowed', key_path)) + no_candidate_error = er.Invalid('extra keys not allowed', key_path) + no_candidate_error._key_shape_mismatch = True + errors.append(no_candidate_error) # for any required keys left that weren't found and don't have defaults: for key in required_keys: @@ -404,7 +414,9 @@ def validate_mapping(path, iterable, out): if hasattr(key, 'msg') and key.msg else 'required key not provided' ) - errors.append(er.RequiredFieldInvalid(msg, path + [key])) + missing_key_error = er.RequiredFieldInvalid(msg, path + [key]) + missing_key_error._key_shape_mismatch = True + errors.append(missing_key_error) if errors: raise er.MultipleInvalid(errors) @@ -619,7 +631,9 @@ def validate_sequence(path, data): out.append(cval) break except er.Invalid as e: - if len(e.path) > len(index_path): + if len(e.path) > len(index_path) and not _is_key_shape_mismatch( + e, index_path + ): raise invalid = e else: @@ -806,6 +820,49 @@ def _normalize_schema_extension( return result +def _is_key_shape_mismatch(invalid, index_path): + """Whether `invalid` reflects a dict-schema alternative whose overall + *shape* doesn't match the data at all, as opposed to one whose shape + matched but a nested value was specifically wrong. + + ``validate_sequence`` tries each schema alternative for a given + sequence item and, by default, stops and re-raises as soon as an + alternative's error is reported deeper than ``index_path`` -- the + assumption being that a deeper path means the alternative's top-level + type matched and a nested detail is what's actually wrong, so + surfacing that specific error immediately is more helpful than + silently falling through to the remaining alternatives. + + A dict/mapping validator breaks that assumption: it always reports + key-related errors ("extra keys not allowed", "required key not + provided", or a rejection from a key validator/schema such as + ``In(...)`` or ``Match(...)``) one level deeper than the path it was + given (it names the offending key), even when none of the + alternative's keys have anything to do with the data's keys, i.e. the + alternative doesn't match the data's shape at all. + + ``validate_mapping`` tags every error of that kind with + ``_key_shape_mismatch`` at the point it's raised/constructed (rather + than this function trying to reverse-engineer it from the error's + class or message, which can't distinguish a literal-key "extra keys + not allowed" from a key-validator's own -- differently classed and + worded -- rejection). Recognize exactly those tagged errors -- and + only when they occur at that one-level-deeper depth -- as shape + mismatches so the caller can keep trying the remaining alternatives + instead of giving up immediately. Any other error at that depth (e.g. + a wrong value for a key that *was* matched) is left alone and still + aborts the search, since that heuristic is correct in that case. + """ + sub_errors = ( + invalid.errors if isinstance(invalid, er.MultipleInvalid) else [invalid] + ) + expected_depth = len(index_path) + 1 + return all( + len(sub.path) == expected_depth and getattr(sub, '_key_shape_mismatch', False) + for sub in sub_errors + ) + + def _compile_scalar(schema): """A scalar value. diff --git a/voluptuous/tests/tests.py b/voluptuous/tests/tests.py index 0608548..827837a 100644 --- a/voluptuous/tests/tests.py +++ b/voluptuous/tests/tests.py @@ -768,6 +768,60 @@ def test_fix_157(): s(['four']) +def test_fix_142_list_of_dict_alternatives(): + """A list of dict-schema alternatives should try every alternative + against each item, not give up after the first alternative whose + keys don't match the item's shape. + + https://github.com/alecthomas/voluptuous/issues/142 + """ + # Case 0: neither alternative matches every item on its own, but each + # item matches a different alternative -- this must validate. + schema = Schema([{1: str}, {2: str}]) + assert schema([{1: 'one'}, {2: 'two'}]) == [{1: 'one'}, {2: 'two'}] + + # Case 1: the first item *does* match the first alternative's key, + # but its value is the wrong type. That's a real, specific error and + # must be reported as such -- not swallowed in favor of trying the + # second alternative against the first item too. + schema = Schema([{1: bool}, {2: str}]) + with pytest.raises( + MultipleInvalid, match=r"expected bool for dictionary value @ data\[0\]\[1\]" + ): + schema([{1: 'one'}, {2: 'two'}]) + + # 3+ alternatives: the fix must not be narrowly special-cased to + # exactly two alternatives. + schema = Schema([{1: str}, {2: str}, {3: str}]) + data = [{1: 'a'}, {2: 'b'}, {3: 'c'}] + assert schema(data) == data + + # A required key missing from an item is also a shape mismatch and + # should fall through to the next alternative. + schema = Schema([{Required(1): str}, {Required(2): str}]) + assert schema([{2: 'two'}]) == [{2: 'two'}] + + # Dict alternatives keyed by a *key validator* (In, Match, etc.), + # rather than a literal key, must also fall through correctly: a + # rejection from the key validator itself is still a shape mismatch, + # not a real value error. + schema = Schema([{In(['a', 'b']): str}, {In(['c', 'd']): str}]) + data = [{'a': 'x'}, {'c': 'y'}] + assert schema(data) == data + + schema = Schema([{Match('^a'): str}, {Match('^c'): str}]) + data = [{'aa': 'x'}, {'cc': 'y'}] + assert schema(data) == data + + # And the same key-validator case still reports a real value error + # correctly instead of swallowing it. + schema = Schema([{In(['a', 'b']): int}, {In(['c', 'd']): str}]) + with pytest.raises( + MultipleInvalid, match=r"expected int for dictionary value @ data\[0\]\['a'\]" + ): + schema([{'a': 'not an int'}, {'c': 'y'}]) + + def test_range_inside(): s = Schema(Range(min=0, max=10)) assert 5 == s(5)