Refactor Properties._get_property_value()

Properties._get_property_value() is hard to follow and contains
duplicated logic. It also duplicates some logic with get_user_value().

This patch refactors the shared code into a new _resolve_user_value()
method, and replaces the previous elif tree with more sequential ifs.
This allows us to make a decision in _resolve_user_value() about whether
there is a user-supplied value we should use, rather than the decision
being made based on simple inspection of the data dict at the start.

Change-Id: Id8a5f4c7a6103d6cdb2e46bb3b489909e71e85c2
Story: 2007388

heat/engine/properties.py

@@ -453,56 +453,66 @@ class Properties(collections.Mapping):
         if any(res.action == res.INIT for res in deps):
             return True
 
-    def get_user_value(self, key, validate=False):
+    def get_user_value(self, key):
         if key not in self:
             raise KeyError(_('Invalid Property %s') % key)
 
         prop = self.props[key]
+        value, found = self._resolve_user_value(key, prop, validate=False)
+        return value
+
+    def _resolve_user_value(self, key, prop, validate):
+        """Return the user-supplied value (or None), and whether it was found.
+
+        """
+        if key not in self.data:
+            return None, False
+
         if (self.translation.is_deleted(prop.path) or
                 self.translation.is_replaced(prop.path)):
-            return
-        if key in self.data:
-            try:
-                unresolved_value = self.data[key]
-                if validate:
-                    if self._find_deps_any_in_init(unresolved_value):
-                        validate = False
+            return None, False
 
-                value = self.resolve(unresolved_value)
+        try:
+            unresolved_value = self.data[key]
+            if validate:
+                if self._find_deps_any_in_init(unresolved_value):
+                    validate = False
 
-                if self.translation.has_translation(prop.path):
-                    value = self.translation.translate(prop.path,
-                                                       value,
-                                                       self.data)
+            value = self.resolve(unresolved_value)
 
-                return prop.get_value(value, validate,
-                                      translation=self.translation)
-            # Children can raise StackValidationFailed with unique path which
-            # is necessary for further use in StackValidationFailed exception.
-            # So we need to handle this exception in this method.
-            except exception.StackValidationFailed as e:
-                raise exception.StackValidationFailed(path=e.path,
-                                                      message=e.error_message)
-            # the resolver function could raise any number of exceptions,
-            # so handle this generically
-            except Exception as e:
-                raise ValueError(str(e))
+            if self.translation.has_translation(prop.path):
+                value = self.translation.translate(prop.path,
+                                                   value,
+                                                   self.data)
+
+            return prop.get_value(value, validate,
+                                  translation=self.translation), True
+        # Children can raise StackValidationFailed with unique path which
+        # is necessary for further use in StackValidationFailed exception.
+        # So we need to handle this exception in this method.
+        except exception.StackValidationFailed as e:
+            raise exception.StackValidationFailed(path=e.path,
+                                                  message=e.error_message)
+        # the resolver function could raise any number of exceptions,
+        # so handle this generically
+        except Exception as e:
+            raise ValueError(str(e))
 
     def _get_property_value(self, key, validate=False):
         if key not in self:
             raise KeyError(_('Invalid Property %s') % key)
 
         prop = self.props[key]
-        if not self.translation.is_deleted(prop.path) and key in self.data:
-            return self.get_user_value(key, validate)
-        elif self.translation.has_translation(prop.path):
+        value, found = self._resolve_user_value(key, prop, validate)
+        if found:
+            return value
+        if self.translation.has_translation(prop.path):
             value = self.translation.translate(prop.path, prop_data=self.data,
                                                validate=validate)
             if value is not None or prop.has_default():
                 return prop.get_value(value)
-            elif prop.required():
-                raise ValueError(_('Property %s not assigned') % key)
-        elif prop.has_default():
+
+        if prop.has_default():
             return prop.get_value(None, validate,
                                   translation=self.translation)
         elif prop.required():

heat/engine/resources/openstack/heat/value.py

@@ -103,7 +103,8 @@ class Value(resource.Resource):
                 _('The expression to generate the "value" attribute.'),
                 required=True,
                 update_allowed=True,
-            ))
+            ),
+            self.VALUE)
 
 
 def resource_mapping():