From 150f06f1b1552b76ff959750e29f484689565484 Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Sat, 7 Jun 2014 13:41:34 +0400 Subject: [PATCH] Improve method resolution rules for multiple inheritance With this change MuranoPL can find correct base class method where old implementation would throw AmbiguousMethodName. Also removes possibility to have several methods with the same name but different signature. This feature didn't worked in most cases, never used anc could cause unexpected program behavior Implements: blueprint muranopl-multiple-inheritance-method-resolution Change-Id: I0a3149b993b1b8a9e9166fce13999e7dd7bf48a5 --- murano/common/engine.py | 16 +--------- murano/dsl/executor.py | 58 +++++++++++++++++-------------------- murano/dsl/murano_class.py | 53 +++++++++++++++++++++++++++++---- murano/dsl/murano_method.py | 3 ++ murano/dsl/murano_object.py | 2 +- murano/dsl/object_store.py | 6 ++-- 6 files changed, 81 insertions(+), 57 deletions(-) diff --git a/murano/common/engine.py b/murano/common/engine.py index e44a3e5d..bba134e4 100644 --- a/murano/common/engine.py +++ b/murano/common/engine.py @@ -23,7 +23,6 @@ from murano.common import config from murano.common.helpers import token_sanitizer from murano.common import rpc from murano.dsl import executor -from murano.dsl import murano_method from murano.dsl import results_serializer from murano.engine import environment from murano.engine import package_class_loader @@ -119,6 +118,7 @@ class TaskExecutor(object): if self.action: self._invoke(exc) except Exception as e: + LOG.warn(e, exc_info=1) reporter = status_reporter.StatusReporter() reporter.initialize(obj) reporter.report_error(obj, str(e)) @@ -130,18 +130,4 @@ class TaskExecutor(object): method_name, args = self.action['method'], self.action['args'] if obj is not None: - if self._is_action(obj, method_name) is False: - raise Exception('%s is not an action' % (method_name,)) - obj.type.invoke(method_name, mpl_executor, obj, args) - - @staticmethod - def _is_action(obj, method_name): - implementations = obj.type.find_method(method_name) - if len(implementations) < 1: - raise Exception('Action %s is not found' % (method_name,)) - if len(implementations) > 1: - raise Exception('Action %s name is ambiguous' % (method_name,)) - declaring_class, _ = implementations[0] - method = declaring_class.get_method(method_name) - return method.usage == murano_method.MethodUsages.Action diff --git a/murano/dsl/executor.py b/murano/dsl/executor.py index 3eaf5270..5130993a 100644 --- a/murano/dsl/executor.py +++ b/murano/dsl/executor.py @@ -13,7 +13,6 @@ # under the License. import collections -import functools import inspect import types import uuid @@ -23,9 +22,9 @@ import eventlet.event import yaql.context import murano.dsl.attribute_store as attribute_store -import murano.dsl.exceptions as exceptions import murano.dsl.expressions as expressions import murano.dsl.helpers as helpers +import murano.dsl.murano_method as murano_method import murano.dsl.murano_object as murano_object import murano.dsl.object_store as object_store import murano.dsl.yaql_functions as yaql_functions @@ -71,44 +70,39 @@ class MuranoDslExecutor(object): raise ValueError() def invoke_method(self, name, this, context, murano_class, *args): + external_call = False if context is None: + external_call = True context = self._root_context - implementations = this.type.find_method(name) + method = this.type.find_single_method(name) + + is_special_method = name in ('initialize', 'destroy') + + if external_call and not is_special_method and \ + method.usage != murano_method.MethodUsages.Action: + raise Exception('{0} is not an action'.format(method.name)) + # TODO (slagun): check method accessibility from murano_class + # restore this from upcast object (no change if there was no upcast) this = this.real_this - delegates = [] - for declaring_class, name in implementations: - method = declaring_class.get_method(name) - if not method: - continue - arguments_scheme = method.arguments_scheme - try: - params = self._evaluate_parameters( - arguments_scheme, context, this, *args) - delegates.append(functools.partial( - self._invoke_method_implementation, - method, this, declaring_class, context, params)) - except TypeError: - continue - if len(delegates) < 1: - raise exceptions.NoMethodFound(name) - elif len(delegates) > 1: - raise exceptions.AmbiguousMethodName(name) - else: - return delegates[0]() + arguments_scheme = method.arguments_scheme + params = self._evaluate_parameters( + arguments_scheme, context, this, *args) + return self._invoke_method_implementation( + method, this, context, params) - def _invoke_method_implementation(self, method, this, murano_class, - context, params): + def _invoke_method_implementation(self, method, this, context, params): body = method.body if not body: return None + murano_class = method.murano_class current_thread = eventlet.greenthread.getcurrent() - if not hasattr(current_thread, '_murano_dsl_thread_marker'): - thread_marker = current_thread._murano_dsl_thread_marker = \ + if not hasattr(current_thread, '_muranopl_thread_marker'): + thread_marker = current_thread._muranopl_thread_marker = \ uuid.uuid4().hex else: - thread_marker = current_thread._murano_dsl_thread_marker + thread_marker = current_thread._muranopl_thread_marker method_id = id(body) this_id = this.object_id @@ -145,7 +139,7 @@ class MuranoDslExecutor(object): thread_marker=None): if thread_marker: current_thread = eventlet.greenthread.getcurrent() - current_thread._murano_dsl_thread_marker = thread_marker + current_thread._muranopl_thread_marker = thread_marker if callable(body): if '_context' in inspect.getargspec(body).args: params['_context'] = self._create_context( @@ -253,10 +247,10 @@ class MuranoDslExecutor(object): try: self._object_store = gc_object_store for obj in objects_to_clean: - methods = obj.type.find_method('destroy') - for cls, method in methods: + methods = obj.type.find_all_methods('destroy') + for method in methods: try: - cls.invoke(method, self, obj, {}) + method.invoke(self, obj, {}) except Exception: pass finally: diff --git a/murano/dsl/murano_class.py b/murano/dsl/murano_class.py index 3511c8d3..68ddf9fa 100644 --- a/murano/dsl/murano_class.py +++ b/murano/dsl/murano_class.py @@ -15,6 +15,7 @@ import collections import inspect +import murano.dsl.exceptions as exceptions import murano.dsl.helpers as helpers import murano.dsl.murano_method as murano_method import murano.dsl.murano_object as murano_object @@ -90,15 +91,53 @@ class MuranoClass(object): def get_property(self, name): return self._properties[name] - def find_method(self, name): + def _find_method_chains(self, name): + initial = [self.methods[name]] if name in self.methods else [] + yielded = False + for parent in self.parents: + for seq in parent._find_method_chains(name): + yield initial + list(seq) + yielded = True + if initial and not yielded: + yield initial + + def find_single_method(self, name): + chains = sorted(self._find_method_chains(name), key=lambda t: len(t)) + result = [] + + for i in range(len(chains)): + if chains[i][0] in result: + continue + add = True + for j in range(i + 1, len(chains)): + common = 0 + if not add: + break + for p in range(len(chains[i])): + if chains[i][-p - 1] is chains[j][-p - 1]: + common += 1 + else: + break + if common == len(chains[i]): + add = False + break + if add: + result.append(chains[i][0]) + if len(result) < 1: + raise exceptions.NoMethodFound(name) + elif len(result) > 1: + raise exceptions.AmbiguousMethodName(name) + return result[0] + + def find_all_methods(self, name): #resolved_name = self._namespace_resolver.resolve_name(name, self.name) if name in self._methods: - return [(self, name)] + return [self.methods[name]] if not self._parents: return [] - return list(set(reduce( - lambda x, y: x + y, - [p.find_method(name) for p in self._parents]))) + return list(reduce( + lambda x, y: x + [t for t in y if t not in x], + [p.find_all_methods(name) for p in self._parents])) def find_property(self, name): types = collections.deque([self]) @@ -110,8 +149,10 @@ class MuranoClass(object): return None def invoke(self, name, executor, this, parameters): + if not self.is_compatible(this): + raise Exception("'this' must be of compatible type") args = executor.to_yaql_args(parameters) - return executor.invoke_method(name, this, None, self, *args) + return executor.invoke_method(name, this.cast(self), None, self, *args) def is_compatible(self, obj): if isinstance(obj, murano_object.MuranoObject): diff --git a/murano/dsl/murano_method.py b/murano/dsl/murano_method.py index 6992492a..2efacbd5 100644 --- a/murano/dsl/murano_method.py +++ b/murano/dsl/murano_method.py @@ -110,3 +110,6 @@ class MuranoMethod(object): def __repr__(self): return 'MuranoMethod({0})'.format(self.name) + + def invoke(self, executor, this, parameters): + return self.murano_class.invoke(self.name, executor, this, parameters) diff --git a/murano/dsl/murano_object.py b/murano/dsl/murano_object.py index 0b1a881a..10cceda7 100644 --- a/murano/dsl/murano_object.py +++ b/murano/dsl/murano_object.py @@ -156,7 +156,7 @@ class MuranoObject(object): raise AttributeError(key) def cast(self, type): - if self.type == type: + if self.type is type: return self for parent in self.__parents.values(): try: diff --git a/murano/dsl/object_store.py b/murano/dsl/object_store.py index 64170a30..a28e1c93 100644 --- a/murano/dsl/object_store.py +++ b/murano/dsl/object_store.py @@ -87,9 +87,9 @@ class ObjectStore(object): if not self.initializing: executor = helpers.get_executor(context) - methods = obj.type.find_method('initialize') - for cls, method in methods: - cls.invoke(method, executor, obj, {}) + methods = obj.type.find_all_methods('initialize') + for method in methods: + method.invoke(executor, obj, {}) return obj @staticmethod