From 12df20ed38dddc1c84c46edeecaff55e6e13d8c6 Mon Sep 17 00:00:00 2001
From: Petr Blaho <pblaho@redhat.com>
Date: Thu, 23 Oct 2014 17:00:13 +0200
Subject: [PATCH] Handle errors when updating unknown params of plan

Adds PlanParametersNotExist to common/exception.
Raises PlanParametersNotExist from PlansManager#set_parameter_values
if some parameter names are not found in environment.
Raised exception contains list of unknown parameters in message.
Test that exception is raised properly.
Test that exception message contains name of unknown parameters.

Change-Id: Iee998bf28462175680e6853731a8901ac3cd5335
Closes-Bug: #1003373
---
 tuskar/common/exception.py        |  5 ++++
 tuskar/manager/plan.py            | 18 ++++++++++++-
 tuskar/tests/manager/test_plan.py | 43 +++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/tuskar/common/exception.py b/tuskar/common/exception.py
index 18c7ee5c..08144b2e 100644
--- a/tuskar/common/exception.py
+++ b/tuskar/common/exception.py
@@ -201,3 +201,8 @@ class PlanExists(DuplicateEntry):
 
 class PlanAlreadyHasRole(DuplicateEntry):
     message = _("Plan %(plan_uuid)s already has role %(role_uuid)s.")
+
+
+class PlanParametersNotExist(Invalid):
+    message = _("There are no parameters named %(param_names)s"
+                " in plan %(plan_uuid)s.")
diff --git a/tuskar/manager/plan.py b/tuskar/manager/plan.py
index 26e30578..7f229127 100644
--- a/tuskar/manager/plan.py
+++ b/tuskar/manager/plan.py
@@ -12,6 +12,7 @@
 
 import logging
 
+from tuskar.common import exception
 from tuskar.manager import models
 from tuskar.manager import name_utils
 from tuskar.storage.exceptions import UnknownName
@@ -291,9 +292,24 @@ class PlansManager(object):
             db_plan.environment_file.contents
         )
 
+        non_existent_params = []
         for param_value in params:
             p = environment.find_parameter_by_name(param_value.name)
-            p.value = param_value.value
+            if p:
+                p.value = param_value.value
+            else:
+                non_existent_params.append(param_value.name)
+
+        if non_existent_params:
+            param_names = ', '.join(non_existent_params)
+            LOG.error(
+                'There are no parameters named %(param_names)s'
+                ' in plan %(plan_uuid)s.' %
+                {'param_names': param_names, 'plan_uuid': plan_uuid})
+            raise exception.PlanParametersNotExist(
+                plan_uuid=plan_uuid,
+                param_names=param_names
+            )
 
         # Save the updated environment.
         env_contents = composer.compose_environment(environment)
diff --git a/tuskar/tests/manager/test_plan.py b/tuskar/tests/manager/test_plan.py
index ef28105b..1b98dbf6 100644
--- a/tuskar/tests/manager/test_plan.py
+++ b/tuskar/tests/manager/test_plan.py
@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from tuskar.common import exception
 from tuskar.manager.models import DeploymentPlan
 from tuskar.manager.models import ParameterValue
 from tuskar.manager.models import PlanParameter
@@ -319,6 +320,48 @@ class PlansManagerTestCase(TestCase):
         self.assertEqual(found_params[2].value, 'm1.small')
         self.assertEqual(found_params[3].value, 'test-key')
 
+    def test_set_non_existent_parameters(self):
+        # Setup
+        test_role = self._add_test_role()
+        test_plan = self.plans_manager.create_plan('p1', 'd1')
+        self.plans_manager.add_role_to_plan(test_plan.uuid, test_role.uuid)
+
+        # Test
+        ns = name_utils.generate_role_namespace(test_role.name,
+                                                test_role.version)
+        not_present_in_role_1_name = ns_utils.apply_template_namespace(
+            ns, 'not_present_in_role_1')
+        not_present_in_role_2_name = ns_utils.apply_template_namespace(
+            ns, 'not_present_in_role_2')
+        update_us = [
+            ParameterValue(ns_utils.apply_template_namespace(ns, 'key_name'),
+                           'test-key'),
+            ParameterValue(ns_utils.apply_template_namespace(ns, 'image_id'),
+                           'test-image'),
+            ParameterValue(not_present_in_role_1_name,
+                           'not-present-in-role-1-value'),
+            ParameterValue(not_present_in_role_2_name,
+                           'not-present-in-role-2-value'),
+        ]
+
+        # Verify
+        exc = self.assertRaises(exception.PlanParametersNotExist,
+                                self.plans_manager.set_parameter_values,
+                                test_plan.uuid,
+                                update_us)
+        self.assertIn(not_present_in_role_1_name, str(exc))
+        self.assertIn(not_present_in_role_2_name, str(exc))
+
+        # Pull it from the database to make sure it was modified
+        found = self.plans_manager.retrieve_plan(test_plan.uuid)
+        found_params = sorted(found.parameters, key=lambda x: x.name)
+        self.assertEqual(4, len(found_params))  # 3 + 1 for scaling
+        self.assertEqual(found_params[0].value, '1')
+        self.assertEqual(found_params[1].value,
+                         '3e6270da-fbf7-4aef-bc78-6d0cfc3ad11b')
+        self.assertEqual(found_params[2].value, 'm1.small')
+        self.assertEqual(found_params[3].value, '')
+
     def test_package_templates(self):
         # Setup
         test_role = self._add_test_role()