From 1d72b9dd6134f020b9a134fd2584631769eefa5c Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Sun, 27 Sep 2015 19:23:18 +0300 Subject: [PATCH] Specification of which property/argument violated contract was added When some property or argument value violated its contract it was impossible to tell which property/argument caused the exception. This change adds prefix to ContractViolationException message that tells not only which property or argument failed to validate but for composite properties also the path within the data. For example if we change ApacheHttpServer name property contract from $.string() to $.int() we will get [io.murano.Environment.applications[0]] [io.murano.apps.apache.ApacheHttpServer.name] Value 'ApacheHttpServer' violates int() contract Also for check() contracts ability to provide custom error message was added Change-Id: I6953ec84140f4bed3d50aa181244f89682b2b2fb Closes-Bug: #1496044 --- murano/dsl/exceptions.py | 12 ++++++- murano/dsl/murano_class.py | 3 +- murano/dsl/murano_method.py | 2 +- murano/dsl/type_scheme.py | 68 ++++++++++++++++++++++++------------- murano/dsl/typespec.py | 29 ++++++++++++++-- 5 files changed, 85 insertions(+), 29 deletions(-) diff --git a/murano/dsl/exceptions.py b/murano/dsl/exceptions.py index 638274eb..c8c4039c 100644 --- a/murano/dsl/exceptions.py +++ b/murano/dsl/exceptions.py @@ -83,7 +83,17 @@ class DslContractSyntaxError(Exception): class ContractViolationException(Exception): - pass + def __init__(self, *args, **kwargs): + super(ContractViolationException, self).__init__(*args, **kwargs) + self._path = '' + + @property + def path(self): + return self._path + + @path.setter + def path(self, value): + self._path = value class ValueIsMissingError(Exception): diff --git a/murano/dsl/murano_class.py b/murano/dsl/murano_class.py index fc88f9e7..031f9afb 100644 --- a/murano/dsl/murano_class.py +++ b/murano/dsl/murano_class.py @@ -67,7 +67,8 @@ class MuranoClass(dsl_types.MuranoClass): properties = data.get('Properties') or {} for property_name, property_spec in properties.iteritems(): - spec = typespec.PropertySpec(property_spec, type_obj) + spec = typespec.PropertySpec( + property_name, property_spec, type_obj) type_obj.add_property(property_name, spec) methods = data.get('Methods') or data.get('Workflow') or {} diff --git a/murano/dsl/murano_method.py b/murano/dsl/murano_method.py index 4f8c1abb..7bde6b41 100644 --- a/murano/dsl/murano_method.py +++ b/murano/dsl/murano_method.py @@ -69,7 +69,7 @@ class MuranoMethod(dsl_types.MuranoMethod): raise ValueError() name = record.keys()[0] self._arguments_scheme[name] = typespec.ArgumentSpec( - record[name], self.murano_class) + self.name, name, record[name], self.murano_class) self._yaql_function_definition = \ yaql_integration.build_wrapper_function_definition(self) diff --git a/murano/dsl/type_scheme.py b/murano/dsl/type_scheme.py index 2ba5bbb0..dbabc0b7 100644 --- a/murano/dsl/type_scheme.py +++ b/murano/dsl/type_scheme.py @@ -47,7 +47,8 @@ class TypeScheme(object): return int(value) except Exception: raise exceptions.ContractViolationException( - 'Value {0} violates int() contract'.format(value)) + 'Value {0} violates int() contract'.format( + format_scalar(value))) @specs.parameter('value', nullable=True) @specs.method @@ -60,7 +61,8 @@ class TypeScheme(object): return unicode(value) except Exception: raise exceptions.ContractViolationException( - 'Value {0} violates string() contract'.format(value)) + 'Value {0} violates string() contract'.format( + format_scalar(value))) @specs.parameter('value', nullable=True) @specs.method @@ -89,14 +91,17 @@ class TypeScheme(object): @specs.parameter('value', nullable=True) @specs.parameter('predicate', yaqltypes.Lambda(with_context=True)) + @specs.parameter('msg', yaqltypes.String(nullable=True)) @specs.method - def check(value, predicate): + def check(value, predicate, msg=None): if isinstance(value, TypeScheme.ObjRef) or predicate( root_context.create_child_context(), value): return value else: - raise exceptions.ContractViolationException( - "Value {0} doesn't match predicate".format(value)) + if not msg: + msg = "Value {0} doesn't match predicate".format( + format_scalar(value)) + raise exceptions.ContractViolationException(msg) @specs.parameter('obj', TypeScheme.ObjRef, nullable=True) @specs.name('owned') @@ -117,8 +122,7 @@ class TypeScheme(object): p = p.owner raise exceptions.ContractViolationException( - 'Object {0} violates owned() contract'.format( - obj.object_id)) + 'Object {0} violates owned() contract'.format(obj)) @specs.parameter('obj', TypeScheme.ObjRef, nullable=True) @specs.name('not_owned') @@ -139,8 +143,7 @@ class TypeScheme(object): return obj else: raise exceptions.ContractViolationException( - 'Object {0} violates notOwned() contract'.format( - obj.object_id)) + 'Object {0} violates notOwned() contract'.format(obj)) @specs.parameter('name', dsl.MuranoTypeName( False, root_context)) @@ -182,7 +185,7 @@ class TypeScheme(object): else: raise exceptions.ContractViolationException( 'Value {0} cannot be represented as class {1}'.format( - value, name)) + format_scalar(value), name)) if not helpers.is_instance_of( obj, murano_class.name, version_spec or helpers.get_type(root_context)): @@ -205,12 +208,13 @@ class TypeScheme(object): context.register_function(not_owned) return context - def _map_dict(self, data, spec, context): + def _map_dict(self, data, spec, context, path): if data is None or data is dsl.NO_VALUE: data = {} if not isinstance(data, utils.MappingType): raise exceptions.ContractViolationException( - 'Supplied is not of a dictionary type') + 'Value {0} is not of a dictionary type'.format( + format_scalar(data))) if not spec: return data result = {} @@ -220,23 +224,27 @@ class TypeScheme(object): if yaql_key is not None: raise exceptions.DslContractSyntaxError( 'Dictionary contract ' - 'cannot have more than one expression keys') + 'cannot have more than one expression key') else: yaql_key = key else: - result[key] = self._map(data.get(key), value, context) + result[key] = self._map( + data.get(key), value, context, '{0}[{1}]'.format( + path, format_scalar(key))) if yaql_key is not None: yaql_value = spec[yaql_key] for key, value in data.iteritems(): if key in result: continue - result[self._map(key, yaql_key, context)] = self._map( - value, yaql_value, context) + key = self._map(key, yaql_key, context, path) + result[key] = self._map( + value, yaql_value, context, '{0}[{1}]'.format( + path, format_scalar(key))) return utils.FrozenDict(result) - def _map_list(self, data, spec, context): + def _map_list(self, data, spec, context, path): if not utils.is_sequence(data): if data is None or data is dsl.NO_VALUE: data = [] @@ -267,26 +275,32 @@ class TypeScheme(object): if index >= len(spec) - shift else spec[index] ) - yield self._map(item, spec_item, context) + yield self._map( + item, spec_item, context, '{0}[{1}]'.format(path, index)) return tuple(map_func()) def _map_scalar(self, data, spec): if data != spec: raise exceptions.ContractViolationException( - 'Value {0} is not equal to {1}'.format(data, spec)) + 'Value {0} is not equal to {1}'.format( + format_scalar(data), spec)) else: return data - def _map(self, data, spec, context): + def _map(self, data, spec, context, path): child_context = context.create_child_context() if isinstance(spec, dsl_types.YaqlExpression): child_context[''] = data - return spec(context=child_context) + try: + return spec(context=child_context) + except exceptions.ContractViolationException as e: + e.path = path + raise elif isinstance(spec, utils.MappingType): - return self._map_dict(data, spec, child_context) + return self._map_dict(data, spec, child_context, path) elif utils.is_sequence(spec): - return self._map_list(data, spec, child_context) + return self._map_list(data, spec, child_context, path) else: return self._map_scalar(data, spec) @@ -299,4 +313,10 @@ class TypeScheme(object): data = helpers.evaluate(default, context) context = self.prepare_context(context, this, owner, default) - return self._map(data, self._spec, context) + return self._map(data, self._spec, context, '') + + +def format_scalar(value): + if isinstance(value, types.StringTypes): + return "'{0}'".format(value) + return unicode(value) diff --git a/murano/dsl/typespec.py b/murano/dsl/typespec.py index b4413f67..264ff5ce 100644 --- a/murano/dsl/typespec.py +++ b/murano/dsl/typespec.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import sys import weakref from murano.dsl import exceptions @@ -63,8 +64,32 @@ class Spec(object): class PropertySpec(Spec): - pass + def __init__(self, property_name, declaration, container_class): + super(PropertySpec, self).__init__(declaration, container_class) + self.property_name = property_name + self.class_name = container_class.name + + def validate(self, *args, **kwargs): + try: + return super(PropertySpec, self).validate(*args, **kwargs) + except exceptions.ContractViolationException as e: + msg = u'[{0}.{1}{2}] {3}'.format( + self.class_name, self.property_name, e.path, unicode(e)) + raise exceptions.ContractViolationException, msg, sys.exc_info()[2] class ArgumentSpec(Spec): - pass + def __init__(self, method_name, arg_name, declaration, container_class): + super(ArgumentSpec, self).__init__(declaration, container_class) + self.method_name = method_name + self.arg_name = arg_name + self.class_name = container_class.name + + def validate(self, *args, **kwargs): + try: + return super(ArgumentSpec, self).validate(*args, **kwargs) + except exceptions.ContractViolationException as e: + msg = u'[{0}::{1}({2}{3})] {4}'.format( + self.class_name, self.method_name, self.arg_name, + e.path, unicode(e)) + raise exceptions.ContractViolationException, msg, sys.exc_info()[2]