From f75a9e41eab3c695c6115f22c594ee97343849ee Mon Sep 17 00:00:00 2001
From: Zane Bitter <zbitter@redhat.com>
Date: Tue, 28 Jul 2015 20:23:43 -0400
Subject: [PATCH] ReST API: Refactor error handler as a context manager

The format parse method was making nasty use of magic strings that were
intended only as error messages to the user in order to switch between two
different behaviours that are each exercised in exactly one place. Since it
is basically just a method for consistent error handling, refactor it as a
context manager.

Change-Id: Ibc33cc5186601a2669652484b2360c8123d263cd
---
 heat/api/openstack/v1/stacks.py               | 21 +++++-------
 .../api/openstack_v1/test_api_openstack_v1.py | 34 +++++++++----------
 2 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/heat/api/openstack/v1/stacks.py b/heat/api/openstack/v1/stacks.py
index f149e720bc..b051d60b9a 100644
--- a/heat/api/openstack/v1/stacks.py
+++ b/heat/api/openstack/v1/stacks.py
@@ -15,6 +15,7 @@
 Stack endpoint for Heat v1 ReST API.
 """
 
+import contextlib
 from oslo_log import log as logging
 import six
 from six.moves.urllib import parse
@@ -69,17 +70,10 @@ class InstantiationData(object):
             self.data[rpc_api.PARAM_EXISTING] = True
 
     @staticmethod
-    def format_parse(data, data_type):
-        """
-        Parse the supplied data as JSON or YAML, raising the appropriate
-        exception if it is in the wrong format.
-        """
-
+    @contextlib.contextmanager
+    def parse_error_check(data_type):
         try:
-            if data_type == 'Environment':
-                return environment_format.parse(data)
-            else:
-                return template_format.parse(data)
+            yield
         except ValueError as parse_ex:
             mdict = {'type': data_type, 'error': six.text_type(parse_ex)}
             msg = _("%(type)s not in valid format: %(error)s") % mdict
@@ -121,7 +115,8 @@ class InstantiationData(object):
         else:
             raise exc.HTTPBadRequest(_("No template specified"))
 
-        return self.format_parse(template_data, 'Template')
+        with self.parse_error_check('Template'):
+            return template_format.parse(template_data)
 
     def environment(self):
         """
@@ -135,8 +130,8 @@ class InstantiationData(object):
             if isinstance(env_data, dict):
                 env = env_data
             else:
-                env = self.format_parse(env_data,
-                                        'Environment')
+                with self.parse_error_check('Environment'):
+                    env = environment_format.parse(env_data)
 
         environment_format.default_for_missing(env)
         parameters = self.data.get(self.PARAM_USER_PARAMS, {})
diff --git a/heat/tests/api/openstack_v1/test_api_openstack_v1.py b/heat/tests/api/openstack_v1/test_api_openstack_v1.py
index 3f2064e226..400d2d1473 100644
--- a/heat/tests/api/openstack_v1/test_api_openstack_v1.py
+++ b/heat/tests/api/openstack_v1/test_api_openstack_v1.py
@@ -34,6 +34,7 @@ import heat.api.openstack.v1.stacks as stacks
 from heat.common import exception as heat_exc
 from heat.common import identifier
 from heat.common import policy
+from heat.common import template_format
 from heat.common import urlfetch
 from heat.common import wsgi
 from heat.rpc import api as rpc_api
@@ -64,22 +65,18 @@ def to_remote_error(error):
 
 class InstantiationDataTest(common.HeatTestCase):
 
-    def test_format_parse(self):
-        data = {"AWSTemplateFormatVersion": "2010-09-09",
-                "key1": ["val1[0]", "val1[1]"],
-                "key2": "val2"}
-        json_repr = ('{"AWSTemplateFormatVersion" : "2010-09-09",'
-                     '"key1": [ "val1[0]", "val1[1]" ], '
-                     '"key2": "val2" }')
-        parsed = stacks.InstantiationData.format_parse(json_repr, 'foo')
-        self.assertEqual(data, parsed)
+    def test_parse_error_success(self):
+        with stacks.InstantiationData.parse_error_check('Garbage'):
+            pass
 
-    def test_format_parse_invalid(self):
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          stacks.InstantiationData.format_parse,
-                          '!@#$%^&not json', 'Garbage')
+    def test_parse_error(self):
+        def generate_error():
+            with stacks.InstantiationData.parse_error_check('Garbage'):
+                raise ValueError
 
-    def test_format_parse_invalid_message(self):
+        self.assertRaises(webob.exc.HTTPBadRequest, generate_error)
+
+    def test_parse_error_message(self):
         # make sure the parser error gets through to the caller.
         bad_temp = '''
 heat_template_version: '2013-05-23'
@@ -89,10 +86,13 @@ parameters:
     description: bla
         '''
 
-        parse_ex = self.assertRaises(webob.exc.HTTPBadRequest,
-                                     stacks.InstantiationData.format_parse,
-                                     bad_temp, 'foo')
+        def generate_error():
+            with stacks.InstantiationData.parse_error_check('foo'):
+                template_format.parse(bad_temp)
+
+        parse_ex = self.assertRaises(webob.exc.HTTPBadRequest, generate_error)
         self.assertIn('line 4, column 3', six.text_type(parse_ex))
+        self.assertIn('foo', six.text_type(parse_ex))
 
     def test_stack_name(self):
         body = {'stack_name': 'wibble'}