From 3198aa7b19172e6376febe80ba901539f6067154 Mon Sep 17 00:00:00 2001
From: Bulat Gaifullin <gaifullinbf@gmail.com>
Date: Fri, 18 Mar 2016 23:27:35 +0300
Subject: [PATCH] Added policy to format text w/o exception for method traverse

The method traverse can be used for formatting task parameters,
but it raises exception in case if format fails. The most shell tasks
contains symbols '{', '}' and python string format tries to format such
string. This change allows to specify the safe_formatter and
leave argument as is if format fails.

This patch also improves error handling for traverse, the default
text formatter added information about text that cannot be formatted.

Change-Id: I2d398d7a27966d842812d20ad1f5e7c5ecbff676
Related-bug: #1614401
---
 nailgun/nailgun/test/unit/test_utils.py | 30 +++++++++++++++++++++
 nailgun/nailgun/utils/__init__.py       | 35 ++++++++++++++++++++-----
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/nailgun/nailgun/test/unit/test_utils.py b/nailgun/nailgun/test/unit/test_utils.py
index d3a47d322e..5bb9d2a92f 100644
--- a/nailgun/nailgun/test/unit/test_utils.py
+++ b/nailgun/nailgun/test/unit/test_utils.py
@@ -26,6 +26,7 @@ from nailgun.utils import dict_merge
 from nailgun.utils import flatten
 from nailgun.utils import grouper
 from nailgun.utils import http_get
+from nailgun.utils import text_format_safe
 from nailgun.utils import traverse
 
 from nailgun.utils.debian import get_apt_preferences_line
@@ -153,6 +154,35 @@ class TestTraverse(base.BaseUnitTest):
                 }
             ]})
 
+    def test_formatter_returns_informative_error(self):
+        with self.assertRaisesRegexp(ValueError, '{a}'):
+            traverse(self.data, self.TestGenerator, {'b': 13})
+
+    def test_w_safe_formatting_context(self):
+        data = self.data.copy()
+        data['bar'] = 'test {b} value'
+        result = traverse(
+            data, self.TestGenerator, {'a': 13},
+            text_format_safe
+        )
+
+        self.assertEqual(result, {
+            'foo': 'testvalue',
+            'bar': 'test {b} value',
+            'baz': 42,
+            'regex': {
+                'source': 'test {a} string',
+                'error': 'an {a} error'
+            },
+            'list': [
+                {
+                    'x': 'a 13 a',
+                },
+                {
+                    'y': 'b 42 b',
+                }
+            ]})
+
 
 class TestGetDebianReleaseFile(base.BaseUnitTest):
 
diff --git a/nailgun/nailgun/utils/__init__.py b/nailgun/nailgun/utils/__init__.py
index cebb891736..75c2a40560 100644
--- a/nailgun/nailgun/utils/__init__.py
+++ b/nailgun/nailgun/utils/__init__.py
@@ -91,15 +91,35 @@ def dict_merge(a, b):
     return result
 
 
-def traverse(data, generator_class, formatter_context=None):
+def text_format(data, context):
+    try:
+        return data.format(**context)
+    except Exception as e:
+        raise ValueError("Cannot format {0}: {1}".format(data, e))
+
+
+def text_format_safe(data, context):
+    try:
+        return data.format(**context)
+    except Exception as e:
+        logger.warning("Cannot format %s: %s. it will be used as is.",
+                       data, six.text_type(e))
+        return data
+
+
+def traverse(data, generator_class, formatter_context=None, formatter=None):
     """Traverse data.
 
     :param data: an input data to be traversed
     :param generator_class: a generator class to be used
     :param formatter_context: a dict to be passed into .format() for strings
+    :param formatter: the text formatter, by default text_format will be used
     :returns: a dict with traversed data
     """
 
+    if formatter is None:
+        formatter = text_format
+
     # generate value if generator is specified
     if isinstance(data, collections.Mapping) and 'generator' in data:
         try:
@@ -117,19 +137,22 @@ def traverse(data, generator_class, formatter_context=None):
             # so it fails if we try to format them. as a workaround, we
             # can skip them and do copy as is.
             if key != 'regex':
-                rv[key] = traverse(value, generator_class, formatter_context)
+                rv[key] = traverse(
+                    value, generator_class, formatter_context, formatter
+                )
             else:
                 rv[key] = value
         return rv
 
     # format all strings with "formatter_context"
     elif isinstance(data, six.string_types) and formatter_context:
-        return data.format(**formatter_context)
-
+        return formatter(data, formatter_context)
     # we want to traverse all sequences also (lists, tuples, etc)
-    elif isinstance(data, (list, tuple)):
+    elif isinstance(data, (list, tuple, set)):
         return type(data)(
-            (traverse(i, generator_class, formatter_context) for i in data))
+            traverse(i, generator_class, formatter_context, formatter)
+            for i in data
+        )
 
     # just return value as is for all other cases
     return data