Adding more information to validation errors
All jsonschema validation failures will be printed in one error message. Also this message will contain the name of the parameter which doesn't pass validation. Closes-bug: #1540809 Change-Id: I7fa82d2e873326f8a061ea13394eb0831c638e21
This commit is contained in:
parent
3f33ca2543
commit
11e6439938
@ -15,7 +15,6 @@
|
||||
|
||||
import functools
|
||||
|
||||
import jsonschema
|
||||
from oslo_utils import reflection
|
||||
|
||||
from sahara import exceptions as ex
|
||||
@ -24,6 +23,21 @@ from sahara.utils import api as u
|
||||
from sahara.utils import api_validator
|
||||
|
||||
|
||||
def _get_path(path):
|
||||
if path:
|
||||
path_string = path[0]
|
||||
for x in path[1:]:
|
||||
path_string += '[%s]' % str(x)
|
||||
return path_string + ': '
|
||||
return ''
|
||||
|
||||
|
||||
def _generate_error(errors):
|
||||
message = [_get_path(list(e.path)) + e.message for e in errors]
|
||||
if message:
|
||||
return ex.SaharaException('\n'.join(message), "VALIDATION_ERROR")
|
||||
|
||||
|
||||
def validate(schema, *validators):
|
||||
def decorator(func):
|
||||
@functools.wraps(func)
|
||||
@ -32,13 +46,13 @@ def validate(schema, *validators):
|
||||
try:
|
||||
if schema:
|
||||
validator = api_validator.ApiValidator(schema)
|
||||
validator.validate(request_data)
|
||||
errors = validator.iter_errors(request_data)
|
||||
error = _generate_error(errors)
|
||||
if error:
|
||||
return u.bad_request(error)
|
||||
if validators:
|
||||
for validator in validators:
|
||||
validator(**kwargs)
|
||||
except jsonschema.ValidationError as e:
|
||||
e.code = "VALIDATION_ERROR"
|
||||
return u.bad_request(e)
|
||||
except ex.SaharaException as e:
|
||||
return u.bad_request(e)
|
||||
except Exception as e:
|
||||
|
@ -36,7 +36,7 @@ class TestJobCreateValidation(u.ValidationTestCase):
|
||||
"type": "Jar",
|
||||
},
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
"'Jar' is not one of " + str(edp.JOB_TYPES_ALL)))
|
||||
"type: 'Jar' is not one of " + str(edp.JOB_TYPES_ALL)))
|
||||
|
||||
@mock.patch('sahara.service.edp.api.get_job_binary')
|
||||
def test_check_binaries(self, get_job_binary):
|
||||
|
@ -62,5 +62,5 @@ class TestJobBinaryValidation(u.ValidationTestCase):
|
||||
"url": "internal-db://abacaba",
|
||||
},
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
"'internal-db://abacaba' is not a "
|
||||
"url: 'internal-db://abacaba' is not a "
|
||||
"'valid_job_location'"))
|
||||
|
@ -40,11 +40,11 @@ class TestTagsAddingValidation(u.ValidationTestCase):
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u"'%s' is not a 'valid_tag'" % tag))
|
||||
u"tags[0]: '%s' is not a 'valid_tag'" % tag))
|
||||
for symb in wrong_symbols:
|
||||
tag = "a%sa" % symb
|
||||
data['tags'] = [tag]
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u"'%s' is not a 'valid_tag'" % tag))
|
||||
u"tags[0]: '%s' is not a 'valid_tag'" % tag))
|
||||
|
@ -114,7 +114,7 @@ class TestClusterCreateValidation(u.ValidationTestCase):
|
||||
'hadoop_version': "0.1",
|
||||
'user_keypair_id': '!'},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
"'!' is not a 'valid_keypair_name'")
|
||||
"user_keypair_id: '!' is not a 'valid_keypair_name'")
|
||||
)
|
||||
|
||||
def test_cluster_create_v_image_exists(self):
|
||||
|
@ -207,7 +207,7 @@ class TestScalingValidation(u.ValidationTestCase):
|
||||
self._assert_cluster_scaling_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u"'name' is a required property")
|
||||
u"resize_node_groups[0]: 'name' is a required property")
|
||||
)
|
||||
data = {
|
||||
'resize_node_groups': [
|
||||
@ -219,7 +219,8 @@ class TestScalingValidation(u.ValidationTestCase):
|
||||
self._assert_cluster_scaling_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u"'count' is a required property")
|
||||
u"resize_node_groups[0]: 'count' is a required "
|
||||
u"property")
|
||||
)
|
||||
|
||||
@mock.patch("sahara.service.api.OPS")
|
||||
@ -236,7 +237,7 @@ class TestScalingValidation(u.ValidationTestCase):
|
||||
self._assert_cluster_scaling_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
"{'node_group_template_id': "
|
||||
"add_node_groups[0]: {'node_group_template_id': "
|
||||
"'5185a809-6bf7-44ed-9de3-618270550e2c'} "
|
||||
"is not valid under any of the given schemas")
|
||||
)
|
||||
@ -252,7 +253,7 @@ class TestScalingValidation(u.ValidationTestCase):
|
||||
self._assert_cluster_scaling_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u"{'node_group_template_id': "
|
||||
u"add_node_groups[0]: {'node_group_template_id': "
|
||||
u"'5185a809-6bf7-44ed-9de3-618270550e2c', "
|
||||
u"'name': 'a'} is not valid under any "
|
||||
u"of the given schemas")
|
||||
@ -314,7 +315,7 @@ class TestScalingValidation(u.ValidationTestCase):
|
||||
self._assert_cluster_scaling_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u"{} is not of type 'array'")
|
||||
u"resize_node_groups: {} is not of type 'array'")
|
||||
)
|
||||
data = {
|
||||
'add_node_groups': {}
|
||||
@ -322,7 +323,7 @@ class TestScalingValidation(u.ValidationTestCase):
|
||||
self._assert_cluster_scaling_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u"{} is not of type 'array'")
|
||||
u"add_node_groups: {} is not of type 'array'")
|
||||
)
|
||||
data = {
|
||||
'resize_node_groups': [],
|
||||
@ -330,7 +331,7 @@ class TestScalingValidation(u.ValidationTestCase):
|
||||
self._assert_cluster_scaling_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u'[] is too short')
|
||||
u'resize_node_groups: [] is too short')
|
||||
)
|
||||
|
||||
@mock.patch("sahara.service.api.OPS")
|
||||
|
@ -44,7 +44,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase):
|
||||
]
|
||||
},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
"{'name': 'a'} is not valid under "
|
||||
"node_groups[0]: {'name': 'a'} is not valid under "
|
||||
"any of the given schemas")
|
||||
)
|
||||
self._assert_create_object_validation(
|
||||
@ -58,7 +58,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase):
|
||||
]
|
||||
},
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
"{'name': 'a', 'flavor_id': '42'} "
|
||||
"node_groups[0]: {'name': 'a', 'flavor_id': '42'} "
|
||||
"is not valid under any of the given schemas")
|
||||
|
||||
)
|
||||
@ -74,7 +74,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase):
|
||||
]
|
||||
},
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
"{'node_processes': ['namenode'], "
|
||||
"node_groups[0]: {'node_processes': ['namenode'], "
|
||||
"'name': 'a', "
|
||||
"'flavor_id': '42'} "
|
||||
"is not valid under any of the given schemas")
|
||||
@ -117,7 +117,8 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase):
|
||||
]
|
||||
},
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
"{'node_group_template_id': '', 'name': 'test'} "
|
||||
"node_groups[0]: {'node_group_template_id': '', "
|
||||
"'name': 'test'} "
|
||||
"is not valid under any of the given schemas")
|
||||
|
||||
)
|
||||
@ -135,7 +136,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase):
|
||||
]
|
||||
},
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
"{'count': 3, "
|
||||
"node_groups[0]: {'count': 3, "
|
||||
"'node_group_template_id': 'test', "
|
||||
"'name': 'test'} "
|
||||
"is not valid under any of the given schemas")
|
||||
|
@ -70,7 +70,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
|
||||
'node_processes': []
|
||||
},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u'\[\] is too short')
|
||||
u'node_processes: \[\] is too short')
|
||||
)
|
||||
|
||||
def test_ng_template_create_v_names(self):
|
||||
@ -186,7 +186,8 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
|
||||
'volumes_per_node': -1
|
||||
},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u'-1(.0)? is less than the minimum of 0')
|
||||
u'volumes_per_node: -1(.0)? is less than the minimum '
|
||||
u'of 0')
|
||||
)
|
||||
self._assert_create_object_validation(
|
||||
data={
|
||||
@ -198,7 +199,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
|
||||
'volumes_size': 0
|
||||
},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u'0(.0)? is less than the minimum of 1')
|
||||
u'volumes_size: 0(.0)? is less than the minimum of 1')
|
||||
)
|
||||
|
||||
def test_ng_template_create_v_types(self):
|
||||
@ -247,7 +248,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
|
||||
'image_id': '12345'
|
||||
},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
"'12345' is not a 'uuid'")
|
||||
"image_id: '12345' is not a 'uuid'")
|
||||
)
|
||||
self._assert_create_object_validation(
|
||||
data={
|
||||
@ -335,7 +336,8 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
|
||||
'volumes_per_node': -1
|
||||
},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u'-1(.0)? is less than the minimum of 0')
|
||||
u'volumes_per_node: -1(.0)? is less than the minimum '
|
||||
u'of 0')
|
||||
)
|
||||
self._assert_create_object_validation(
|
||||
data={
|
||||
@ -347,7 +349,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
|
||||
'volumes_size': 0
|
||||
},
|
||||
bad_req_i=(1, 'VALIDATION_ERROR',
|
||||
u'0(.0)? is less than the minimum of 1')
|
||||
u'volumes_size: 0(.0)? is less than the minimum of 1')
|
||||
)
|
||||
self._assert_create_object_validation(
|
||||
data={
|
||||
@ -373,7 +375,8 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
|
||||
}
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR', "'qwerty' is not a 'posix_path'")
|
||||
bad_req_i=(1, 'VALIDATION_ERROR', "volume_mount_prefix: 'qwerty' "
|
||||
"is not a 'posix_path'")
|
||||
)
|
||||
|
||||
def test_wrong_floating_ip_pool(self):
|
||||
|
49
sahara/tests/unit/service/validation/test_validation.py
Normal file
49
sahara/tests/unit/service/validation/test_validation.py
Normal file
@ -0,0 +1,49 @@
|
||||
# Copyright (c) 2016 Mirantis Inc.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
|
||||
# implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import mock
|
||||
|
||||
from sahara import exceptions
|
||||
from sahara.service import validation
|
||||
from sahara.tests.unit import base
|
||||
|
||||
|
||||
class TestValidation(base.SaharaTestCase):
|
||||
def test_get_path(self):
|
||||
self.assertEqual('', validation._get_path([]))
|
||||
self.assertEqual('param: ', validation._get_path(['param']))
|
||||
self.assertEqual(
|
||||
'param[0][id]: ', validation._get_path(['param', 0, 'id']))
|
||||
|
||||
def test_generate_error(self):
|
||||
# no errors occurred during validation
|
||||
self.assertIsNone(validation._generate_error([]))
|
||||
|
||||
# one error occurred during validation
|
||||
err = mock.Mock(message='some error', path=['param'])
|
||||
generated_err = validation._generate_error([err])
|
||||
self.assertIsInstance(generated_err, exceptions.SaharaException)
|
||||
self.assertEqual('VALIDATION_ERROR', generated_err.code)
|
||||
self.assertIn('param: some error', generated_err.message)
|
||||
|
||||
# several errors occurred during validation
|
||||
errors = [mock.Mock(message='first error', path=['param1']),
|
||||
mock.Mock(
|
||||
message='second error', path=['param2', 0, 'subparam'])]
|
||||
generated_err = validation._generate_error(errors)
|
||||
self.assertIsInstance(generated_err, exceptions.SaharaException)
|
||||
self.assertEqual('VALIDATION_ERROR', generated_err.code)
|
||||
self.assertIn('param1: first error\nparam2[0][subparam]: second error',
|
||||
generated_err.message)
|
@ -317,25 +317,25 @@ class ValidationTestCase(base.SaharaTestCase):
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
u"None is not of type 'string'")
|
||||
u"name: None is not of type 'string'")
|
||||
)
|
||||
data.update({'name': ""})
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
u"'' is too short")
|
||||
u"name: '' is too short")
|
||||
)
|
||||
data.update({'name': ('a' * 51)})
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
u"'%s' is too long" % ('a' * 51))
|
||||
u"name: '%s' is too long" % ('a' * 51))
|
||||
)
|
||||
data.update({'name': 'a-!'})
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, "VALIDATION_ERROR",
|
||||
u"'a-!' is not a 'valid_name_hostname'")
|
||||
u"name: 'a-!' is not a 'valid_name_hostname'")
|
||||
)
|
||||
|
||||
def _prop_types_str(self, prop_types):
|
||||
@ -356,12 +356,13 @@ class ValidationTestCase(base.SaharaTestCase):
|
||||
if isinstance(value, str):
|
||||
value_str = "'%s'" % value_str
|
||||
data.update({p_name: value})
|
||||
message = ("%s is not of type %s" %
|
||||
(value_str,
|
||||
message = ("%s: %s is not of type %s" %
|
||||
(p_name, value_str,
|
||||
self._prop_types_str(prop_types)))
|
||||
if "enum" in prop:
|
||||
message = [message, "%s is not one of %s" %
|
||||
(value_str, prop["enum"])]
|
||||
message = [message, "%s: %s is not one of %s" %
|
||||
(p_name, value_str,
|
||||
prop["enum"])]
|
||||
self._assert_create_object_validation(
|
||||
data=data,
|
||||
bad_req_i=(1, 'VALIDATION_ERROR', message)
|
||||
|
Loading…
Reference in New Issue
Block a user