Skip to content

Improve oneOf schema validation messaging #2014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 15, 2015
Merged

Improve oneOf schema validation messaging #2014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
32cd404
Remove redundant oneOf definitions
mnowster Sep 8, 2015
418ec53
Improved messaging for simple type validator
mnowster Sep 8, 2015
cf7b595
Improve error messages from oneOf schema errors
mnowster Sep 9, 2015
1007ad0
Refactor to simplify _parse_valid_types
mnowster Sep 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 3 additions & 18 deletions compose/config/fields_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,7 @@
]
},
"container_name": {"type": "string"},
"cpu_shares": {
"oneOf": [
{"type": "number"},
{"type": "string"}
]
},
"cpu_shares": {"type": ["number", "string"]},
"cpuset": {"type": "string"},
"devices": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
"dns": {"$ref": "#/definitions/string_or_list"},
Expand Down Expand Up @@ -74,18 +69,8 @@
"log_opt": {"type": "object"},

"mac_address": {"type": "string"},
"mem_limit": {
"oneOf": [
{"type": "number"},
{"type": "string"}
]
},
"memswap_limit": {
"oneOf": [
{"type": "number"},
{"type": "string"}
]
},
"mem_limit": {"type": ["number", "string"]},
"memswap_limit": {"type": ["number", "string"]},
"name": {"type": "string"},
"net": {"type": "string"},
"pid": {"type": ["string", "null"]},
Expand Down
77 changes: 60 additions & 17 deletions compose/config/validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ def get_unsupported_config_msg(service_name, error_key):
return msg


def anglicize_validator(validator):
if validator in ["array", "object"]:
return 'an ' + validator
return 'a ' + validator


def process_errors(errors, service_name=None):
"""
jsonschema gives us an error tree full of information to explain what has
Expand All @@ -107,15 +113,57 @@ def _parse_key_from_error_msg(error):
def _clean_error_message(message):
return message.replace("u'", "'")

def _parse_valid_types_from_schema(schema):
def _parse_valid_types_from_validator(validator):
"""
Our defined types using $ref in the schema require some extra parsing
retrieve a helpful type for error message display.
A validator value can be either an array of valid types or a string of
a valid type. Parse the valid types and prefix with the correct article.
"""
if '$ref' in schema:
return schema['$ref'].replace("#/definitions/", "").replace("_", " ")
if isinstance(validator, list):
if len(validator) >= 2:
first_type = anglicize_validator(validator[0])
last_type = anglicize_validator(validator[-1])
types_from_validator = "{}{}".format(first_type, ", ".join(validator[1:-1]))

msg = "{} or {}".format(
types_from_validator,
last_type
)
else:
msg = "{}".format(anglicize_validator(validator[0]))
else:
return str(schema['type'])
msg = "{}".format(anglicize_validator(validator))

return msg

def _parse_oneof_validator(error):
"""
oneOf has multiple schemas, so we need to reason about which schema, sub
schema or constraint the validation is failing on.
Inspecting the context value of a ValidationError gives us information about
which sub schema failed and which kind of error it is.
"""
constraint = [context for context in error.context if len(context.path) > 0]
if constraint:
valid_types = _parse_valid_types_from_validator(constraint[0].validator_value)
msg = "contains {}, which is an invalid type, it should be {}".format(
constraint[0].instance,
valid_types
)
return msg

uniqueness = [context for context in error.context if context.validator == 'uniqueItems']
if uniqueness:
msg = "contains non unique items, please remove duplicates from {}".format(
uniqueness[0].instance
)
return msg

types = [context.validator_value for context in error.context if context.validator == 'type']
valid_types = _parse_valid_types_from_validator(types)

msg = "contains an invalid type, it should be {}".format(valid_types)

return msg

root_msgs = []
invalid_keys = []
Expand Down Expand Up @@ -168,27 +216,22 @@ def _parse_valid_types_from_schema(schema):
required.append(_clean_error_message(error.message))
elif error.validator == 'oneOf':
config_key = error.path[0]
msg = _parse_oneof_validator(error)

valid_types = [_parse_valid_types_from_schema(schema) for schema in error.schema['oneOf']]
valid_type_msg = " or ".join(valid_types)

type_errors.append("Service '{}' configuration key '{}' contains an invalid type, valid types are {}".format(
service_name, config_key, valid_type_msg)
type_errors.append("Service '{}' configuration key '{}' {}".format(
service_name, config_key, msg)
)
elif error.validator == 'type':
msg = "a"
if error.validator_value == "array":
msg = "an"
msg = _parse_valid_types_from_validator(error.validator_value)

if len(error.path) > 0:
config_key = " ".join(["'%s'" % k for k in error.path])
type_errors.append(
"Service '{}' configuration key {} contains an invalid "
"type, it should be {} {}".format(
"type, it should be {}".format(
service_name,
config_key,
msg,
error.validator_value))
msg))
else:
root_msgs.append(
"Service '{}' doesn\'t have any configuration options. "
Expand Down
20 changes: 18 additions & 2 deletions tests/unit/config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ def test_invalid_config_not_unique_items(self):
)

def test_invalid_list_of_strings_format(self):
expected_error_msg = "'command' contains an invalid type, valid types are string or array"
expected_error_msg = "Service 'web' configuration key 'command' contains 1"
expected_error_msg += ", which is an invalid type, it should be a string"
with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
config.load(
config.ConfigDetails(
Expand Down Expand Up @@ -222,7 +223,7 @@ def test_config_extra_hosts_string_raises_validation_error(self):
)

def test_config_extra_hosts_list_of_dicts_validation_error(self):
expected_error_msg = "Service 'web' configuration key 'extra_hosts' contains an invalid type"
expected_error_msg = "key 'extra_hosts' contains {'somehost': '162.242.195.82'}, which is an invalid type, it should be a string"

with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
config.load(
Expand Down Expand Up @@ -269,6 +270,21 @@ def test_valid_config_oneof_string_or_list(self):
)
self.assertEqual(service[0]['entrypoint'], entrypoint)

def test_validation_message_for_invalid_type_when_multiple_types_allowed(self):
expected_error_msg = "Service 'web' configuration key 'mem_limit' contains an invalid type, it should be a number or a string"

with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
config.load(
config.ConfigDetails(
{'web': {
'image': 'busybox',
'mem_limit': ['incorrect']
}},
'working_dir',
'filename.yml'
)
)


class InterpolationTest(unittest.TestCase):
@mock.patch.dict(os.environ)
Expand Down