From 3ea05620489a3d3f5e8824da23ea194d59e0ef6e Mon Sep 17 00:00:00 2001
From: Zane Bitter <zbitter@redhat.com>
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.

Change-Id: I70e341c344d6254173ad4519276626230087263a
Story: 2007957
Task: 40443
(cherry picked from commit b603470a1207fcbc9ee30f3a3b4b457a9f83a03a)
(cherry picked from commit 4aafceb3798df94abf236f7f3abb5dcf6c425bb3)
---
 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 168ac3e59c..ffdb1e7c02 100644
--- a/heat/engine/parameters.py
+++ b/heat/engine/parameters.py
@@ -372,32 +372,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
@@ -439,11 +439,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 f438c37fba..9ced8e25eb 100644
--- a/heat/tests/test_parameters.py
+++ b/heat/tests/test_parameters.py
@@ -380,7 +380,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', six.text_type(err))
 
@@ -398,7 +398,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', six.text_type(err))