From b603470a1207fcbc9ee30f3a3b4b457a9f83a03a Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 28 Jul 2020 12:18:43 -0400 Subject: [PATCH] Lazily cache parsed value of list/json parameters Parsing the value of JSON parameters at the time we create them (or update the default value) results in exceptions occurring while creating the Stack object, that are then not caught and transformed. The result is an HTTP 500 Internal Error reported by the API, with an unhelpful error message. By not parsing the JSON until it is needed, we ensure that any errors will occur during validation, where errors are appropriately transformed (to StackValidationFailed) and annotated with the necessary information to help the user determine the source. Depends-On: https://review.opendev.org/#/c/748359/ Change-Id: I70e341c344d6254173ad4519276626230087263a Story: 2007957 Task: 40443 --- heat/engine/parameters.py | 37 +++++++++++++++++------------------ heat/tests/test_parameters.py | 4 ++-- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/heat/engine/parameters.py b/heat/engine/parameters.py index 0447738157..4aced043bb 100644 --- a/heat/engine/parameters.py +++ b/heat/engine/parameters.py @@ -369,32 +369,32 @@ class StringParam(Parameter): class ParsedParameter(Parameter): """A template parameter with cached parsed value.""" - __slots__ = ('parsed',) + __slots__ = ('_parsed',) def __init__(self, name, schema, value=None): super(ParsedParameter, self).__init__(name, schema, value) - self._update_parsed() + self._parsed = None - def set_default(self, value): - super(ParsedParameter, self).set_default(value) - self._update_parsed() - - def _update_parsed(self): - if self.has_value(): - if self.user_value is not None: - self.parsed = self.parse(self.user_value) + @property + def parsed(self): + if self._parsed is None: + if self.has_value(): + if self.user_value is not None: + self._parsed = self.parse(self.user_value) + else: + self._parsed = self.parse(self.default()) else: - self.parsed = self.parse(self.default()) + self._parsed = self.default_parsed() + return self._parsed class CommaDelimitedListParam(ParsedParameter, collections.Sequence): """A template parameter of type "CommaDelimitedList".""" - __slots__ = ('parsed',) + __slots__ = tuple() - def __init__(self, name, schema, value=None): - self.parsed = [] - super(CommaDelimitedListParam, self).__init__(name, schema, value) + def default_parsed(self): + return [] def parse(self, value): # only parse when value is not already a list @@ -436,11 +436,10 @@ class CommaDelimitedListParam(ParsedParameter, collections.Sequence): class JsonParam(ParsedParameter): """A template parameter who's value is map or list.""" - __slots__ = ('parsed',) + __slots__ = tuple() - def __init__(self, name, schema, value=None): - self.parsed = {} - super(JsonParam, self).__init__(name, schema, value) + def default_parsed(self): + return {} def parse(self, value): try: diff --git a/heat/tests/test_parameters.py b/heat/tests/test_parameters.py index 66218cf6a7..4779ea4383 100644 --- a/heat/tests/test_parameters.py +++ b/heat/tests/test_parameters.py @@ -379,7 +379,7 @@ class ParameterTestSpecific(common.HeatTestCase): schema = {'Type': 'Json', 'ConstraintDescription': 'wibble'} val = {"foo": "bar", "not_json": len} - err = self.assertRaises(ValueError, + err = self.assertRaises(exception.StackValidationFailed, new_parameter, 'p', schema, val) self.assertIn('Value must be valid JSON', str(err)) @@ -397,7 +397,7 @@ class ParameterTestSpecific(common.HeatTestCase): schema = {'Type': 'Json', 'ConstraintDescription': 'wibble'} val = "I am not a map" - err = self.assertRaises(ValueError, + err = self.assertRaises(exception.StackValidationFailed, new_parameter, 'p', schema, val) self.assertIn('Value must be valid JSON', str(err))