From 6cf8dc6e569842afa5f3d4a7236727f844f7ba63 Mon Sep 17 00:00:00 2001 From: Ethan Lynn Date: Wed, 17 Feb 2016 16:32:51 +0800 Subject: [PATCH] Fix problems in unicode template support If we use unicode as resource name or parameter, we are likely to encounter unicode problems. This patch aims to fix unicode problems, and there are several things to be clear: 1. For the class wrap with six.python_2_unicode_compatible, in python2 their original __str__ function will map to __unicode__, and new __str__ function comes from __unicode__.encode. So we should always return unicode in __str__() if wrap with python_2_unicode_compatible. 2. python_2_unicode_compatible will not handle __repr__, __repr__ should return str in all versions of python. Co-Authored-By: Ethan Lynn Closes-Bug: #1524194 Change-Id: Ib4af43d153e0edf9d66308bf8e7b93a3be501d2e --- heat/common/i18n.py | 19 +++ heat/engine/attributes.py | 2 + heat/engine/dependencies.py | 8 +- heat/engine/rsrc_defn.py | 2 + heat/engine/scheduler.py | 9 +- heat/engine/update.py | 2 + heat/tests/engine/test_scheduler.py | 22 ++++ .../functional/test_unicode_template.py | 113 ++++++++++++++++++ 8 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 heat_integrationtests/functional/test_unicode_template.py diff --git a/heat/common/i18n.py b/heat/common/i18n.py index 784dffb25f..e2d72811be 100644 --- a/heat/common/i18n.py +++ b/heat/common/i18n.py @@ -16,7 +16,10 @@ # It's based on oslo.i18n usage in OpenStack Keystone project and # recommendations from http://docs.openstack.org/developer/oslo.i18n/usage.html +import six + import oslo_i18n as i18n +from oslo_utils import encodeutils _translators = i18n.TranslatorFactory(domain='heat') @@ -33,3 +36,19 @@ _LI = _translators.log_info _LW = _translators.log_warning _LE = _translators.log_error _LC = _translators.log_critical + + +def repr_wraper(klass): + """A decorator that defines __repr__ method under Python 2. + + Under Python 2 it will encode repr return value to str type. + Under Python 3 it does nothing. + """ + if six.PY2: + if '__repr__' not in klass.__dict__: + raise ValueError("@repr_wraper cannot be applied " + "to %s because it doesn't define __repr__()." % + klass.__name__) + klass._repr = klass.__repr__ + klass.__repr__ = lambda self: encodeutils.safe_encode(self._repr()) + return klass diff --git a/heat/engine/attributes.py b/heat/engine/attributes.py index 4d21759640..9f768bf193 100644 --- a/heat/engine/attributes.py +++ b/heat/engine/attributes.py @@ -18,6 +18,7 @@ import six from heat.common.i18n import _ from heat.common.i18n import _LW +from heat.common.i18n import repr_wraper from heat.engine import constraints as constr from heat.engine import support @@ -123,6 +124,7 @@ class Attribute(object): } +@repr_wraper class Attributes(collections.Mapping): """Models a collection of Resource Attributes.""" diff --git a/heat/engine/dependencies.py b/heat/engine/dependencies.py index 2d00e15376..80cce8b26e 100644 --- a/heat/engine/dependencies.py +++ b/heat/engine/dependencies.py @@ -18,12 +18,14 @@ import six from heat.common import exception from heat.common.i18n import _ +from heat.common.i18n import repr_wraper class CircularDependencyException(exception.HeatException): msg_fmt = _("Circular Dependency Found: %(cycle)s") +@repr_wraper @six.python_2_unicode_compatible class Node(object): """A node in a dependency graph.""" @@ -88,7 +90,7 @@ class Node(object): def __str__(self): """Return a human-readable string representation of the node.""" - text = '{%s}' % ', '.join(str(n) for n in self) + text = '{%s}' % ', '.join(six.text_type(n) for n in self) return six.text_type(text) def __repr__(self): @@ -143,7 +145,8 @@ class Graph(collections.defaultdict): def __str__(self): """Convert the graph to a human-readable string.""" - pairs = ('%s: %s' % (str(k), str(v)) for k, v in six.iteritems(self)) + pairs = ('%s: %s' % (six.text_type(k), six.text_type(v)) + for k, v in six.iteritems(self)) text = '{%s}' % ', '.join(pairs) return six.text_type(text) @@ -165,6 +168,7 @@ class Graph(collections.defaultdict): raise CircularDependencyException(cycle=six.text_type(graph)) +@repr_wraper @six.python_2_unicode_compatible class Dependencies(object): """Helper class for calculating a dependency graph.""" diff --git a/heat/engine/rsrc_defn.py b/heat/engine/rsrc_defn.py index 70af81c06e..933234c9d1 100644 --- a/heat/engine/rsrc_defn.py +++ b/heat/engine/rsrc_defn.py @@ -19,6 +19,7 @@ import six from heat.common import exception from heat.common.i18n import _LW +from heat.common.i18n import repr_wraper from heat.engine import function from heat.engine import properties @@ -27,6 +28,7 @@ LOG = log.getLogger(__name__) __all__ = ['ResourceDefinition'] +@repr_wraper class ResourceDefinitionCore(object): """A definition of a resource, independent of any template format.""" diff --git a/heat/engine/scheduler.py b/heat/engine/scheduler.py index 24de978a33..44b20c0a6d 100644 --- a/heat/engine/scheduler.py +++ b/heat/engine/scheduler.py @@ -16,12 +16,14 @@ import types import eventlet from oslo_log import log as logging +from oslo_utils import encodeutils from oslo_utils import excutils import six from six import reraise as raise_ from heat.common.i18n import _ from heat.common.i18n import _LI +from heat.common.i18n import repr_wraper from heat.common import timeutils LOG = logging.getLogger(__name__) @@ -40,10 +42,10 @@ def task_description(task): if name is not None and isinstance(task, (types.MethodType, types.FunctionType)): if getattr(task, '__self__', None) is not None: - return '%s from %s' % (name, task.__self__) + return '%s from %s' % (six.text_type(name), task.__self__) else: return six.text_type(name) - return repr(task) + return encodeutils.safe_decode(repr(task)) class Timeout(BaseException): @@ -331,6 +333,7 @@ def wrappertask(task): return wrapper +@repr_wraper class DependencyTaskGroup(object): """Task which manages group of subtasks that have ordering dependencies.""" @@ -369,7 +372,7 @@ class DependencyTaskGroup(object): def __repr__(self): """Return a string representation of the task.""" text = '%s(%s)' % (type(self).__name__, self.name) - return six.text_type(text) + return text def __call__(self): """Return a co-routine which runs the task group.""" diff --git a/heat/engine/update.py b/heat/engine/update.py index e528aab4a9..b233ee9da7 100644 --- a/heat/engine/update.py +++ b/heat/engine/update.py @@ -16,6 +16,7 @@ import six from heat.common import exception from heat.common.i18n import _LI +from heat.common.i18n import repr_wraper from heat.engine import dependencies from heat.engine import scheduler from heat.objects import resource as resource_objects @@ -23,6 +24,7 @@ from heat.objects import resource as resource_objects LOG = logging.getLogger(__name__) +@repr_wraper class StackUpdate(object): """A Task to perform the update of an existing stack to a new template.""" diff --git a/heat/tests/engine/test_scheduler.py b/heat/tests/engine/test_scheduler.py index af77b9afe5..f56d61c9af 100644 --- a/heat/tests/engine/test_scheduler.py +++ b/heat/tests/engine/test_scheduler.py @@ -16,6 +16,7 @@ import contextlib import eventlet import six +from heat.common.i18n import repr_wraper from heat.common import timeutils from heat.engine import dependencies from heat.engine import scheduler @@ -823,6 +824,27 @@ class DescriptionTest(common.HeatTestCase): self.assertEqual('o', scheduler.task_description(C())) + def test_unicode(self): + @repr_wraper + @six.python_2_unicode_compatible + class C(object): + def __str__(self): + return u'C "\u2665"' + + def __repr__(self): + return u'\u2665' + + def __call__(self): + pass + + def m(self): + pass + + self.assertEqual(u'm from C "\u2665"', + scheduler.task_description(C().m)) + self.assertEqual(u'\u2665', + scheduler.task_description(C())) + class WrapperTaskTest(common.HeatTestCase): diff --git a/heat_integrationtests/functional/test_unicode_template.py b/heat_integrationtests/functional/test_unicode_template.py new file mode 100644 index 0000000000..924c1100be --- /dev/null +++ b/heat_integrationtests/functional/test_unicode_template.py @@ -0,0 +1,113 @@ +# 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. + +from heat_integrationtests.functional import functional_base + + +class StackUnicodeTemplateTest(functional_base.FunctionalTestsBase): + + random_template = u''' +heat_template_version: 2014-10-16 +description: \u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0 +parameters: + \u53c2\u6570: + type: number + default: 10 + label: \u6807\u7b7e + description: \u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0 +resources: + \u8d44\u6e90: + type: OS::Heat::RandomString + properties: + length: {get_param: \u53c2\u6570} +outputs: + \u8f93\u51fa: + description: \u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0 + value: {get_attr: [\u8d44\u6e90, value]} +''' + + def setUp(self): + super(StackUnicodeTemplateTest, self).setUp() + + def _assert_results(self, result): + self.assertTrue(result['disable_rollback']) + self.assertIsNone(result['parent']) + self.assertEqual(u'\u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0', + result['template_description']) + self.assertEqual(u'10', result['parameters'][u'\u53c2\u6570']) + + def _assert_preview_results(self, result): + self._assert_results(result) + res = result['resources'][0] + self.assertEqual('/resources/%s' % res['resource_name'], + res['resource_identity']['path']) + + def _assert_create_results(self, result): + self._assert_results(result) + output = result['outputs'][0] + self.assertEqual(u'\u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0', + output['description']) + self.assertEqual(u'\u8f93\u51fa', output['output_key']) + self.assertIsNotNone(output['output_value']) + + def _assert_resource_results(self, result): + self.assertEqual(u'\u8d44\u6e90', result['resource_name']) + self.assertEqual('OS::Heat::RandomString', + result['resource_type']) + + def test_template_validate_basic(self): + ret = self.client.stacks.validate(template=self.random_template) + expected = { + 'Description': u'\u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0', + 'Parameters': { + u'\u53c2\u6570': { + 'Default': 10, + 'Description': u'\u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0', + 'Label': u'\u6807\u7b7e', + 'NoEcho': 'false', + 'Type': 'Number'} + } + } + self.assertEqual(expected, ret) + + def test_template_validate_override_default(self): + env = {'parameters': {u'\u53c2\u6570': 5}} + ret = self.client.stacks.validate(template=self.random_template, + environment=env) + expected = { + 'Description': u'\u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0', + 'Parameters': { + u'\u53c2\u6570': { + 'Default': 10, + 'Value': 5, + 'Description': u'\u8fd9\u662f\u4e00\u4e2a\u63cf\u8ff0', + 'Label': u'\u6807\u7b7e', + 'NoEcho': 'false', + 'Type': 'Number'} + } + } + self.assertEqual(expected, ret) + + def test_stack_preview(self): + result = self.client.stacks.preview( + template=self.random_template, + stack_name=self._stack_rand_name(), + disable_rollback=True).to_dict() + self._assert_preview_results(result) + + def test_create_stack(self): + stack_identifier = self.stack_create(template=self.random_template) + stack = self.client.stacks.get(stack_identifier) + self._assert_create_results(stack.to_dict()) + rl = self.client.resources.list(stack_identifier) + self.assertEqual(1, len(rl)) + self._assert_resource_results(rl[0].to_dict())