From 2e8fa7c48fe4cca5674f895bd3985dc9243a9702 Mon Sep 17 00:00:00 2001
From: Eoghan Glynn <eglynn@redhat.com>
Date: Tue, 8 Apr 2014 10:53:12 +0000
Subject: [PATCH] Allowed nested resource metadata in POST'd samples

Fixes bug 1302664

Previously, posting samples with nested metadata caused the mongo
driver to fail on the embedded period in a metadata key.

Now, we explicitly unwind the flattened resource metadata before
publishing the sample.

Change-Id: Ibb0980afc880218962328a9b7fe792015d58d1d2
---
 ceilometer/api/controllers/v2.py              |  3 +-
 .../api/v2/test_post_samples_scenarios.py     | 30 ++++++++++++++++
 ceilometer/tests/test_utils.py                | 35 +++++++++++++++++++
 ceilometer/utils.py                           | 14 ++++++++
 4 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/ceilometer/api/controllers/v2.py b/ceilometer/api/controllers/v2.py
index 6785309c3..beba57f11 100644
--- a/ceilometer/api/controllers/v2.py
+++ b/ceilometer/api/controllers/v2.py
@@ -870,7 +870,8 @@ class MeterController(rest.RestController):
                 project_id=s.project_id,
                 resource_id=s.resource_id,
                 timestamp=s.timestamp.isoformat(),
-                resource_metadata=s.resource_metadata,
+                resource_metadata=utils.restore_nesting(s.resource_metadata,
+                                                        separator='.'),
                 source=s.source)
             published_samples.append(published_sample)
 
diff --git a/ceilometer/tests/api/v2/test_post_samples_scenarios.py b/ceilometer/tests/api/v2/test_post_samples_scenarios.py
index 27b280034..1236be127 100644
--- a/ceilometer/tests/api/v2/test_post_samples_scenarios.py
+++ b/ceilometer/tests/api/v2/test_post_samples_scenarios.py
@@ -64,6 +64,36 @@ class TestPostSamples(FunctionalTest,
         self.assertEqual(s1, data.json)
         self.assertEqual(s1[0], self.published[0][1]['args']['data'][0])
 
+    def test_nested_metadata(self):
+        s1 = [{'counter_name': 'apples',
+               'counter_type': 'gauge',
+               'counter_unit': 'instance',
+               'counter_volume': 1,
+               'resource_id': 'bd9431c1-8d69-4ad3-803a-8d4a6b89fd36',
+               'project_id': '35b17138-b364-4e6a-a131-8f3099c5be68',
+               'user_id': 'efd87807-12d2-4b38-9c70-5f5c2ac427ff',
+               'resource_metadata': {'nest.name1': 'value1',
+                                     'name2': 'value2',
+                                     'nest.name2': 'value3'}}]
+
+        data = self.post_json('/meters/apples/', s1)
+
+        # timestamp not given so it is generated.
+        s1[0]['timestamp'] = data.json[0]['timestamp']
+        # Ignore message id that is randomly generated
+        s1[0]['message_id'] = data.json[0]['message_id']
+        # source is generated if not provided.
+        s1[0]['source'] = '%s:openstack' % s1[0]['project_id']
+
+        unwound = copy.copy(s1[0])
+        unwound['resource_metadata'] = {'nest': {'name1': 'value1',
+                                                 'name2': 'value3'},
+                                        'name2': 'value2'}
+        # only the published sample should be unwound, not the representation
+        # in the API response
+        self.assertEqual(s1[0], data.json[0])
+        self.assertEqual(unwound, self.published[0][1]['args']['data'][0])
+
     def test_invalid_counter_type(self):
         s1 = [{'counter_name': 'my_counter_name',
                'counter_type': 'INVALID_TYPE',
diff --git a/ceilometer/tests/test_utils.py b/ceilometer/tests/test_utils.py
index 91326d539..d847016bc 100644
--- a/ceilometer/tests/test_utils.py
+++ b/ceilometer/tests/test_utils.py
@@ -78,6 +78,41 @@ class TestUtils(test.BaseTestCase):
             pairs = list(utils.recursive_keypairs(data))
             self.assertEqual(expected, pairs)
 
+    def test_restore_nesting_unested(self):
+        metadata = {'a': 'A', 'b': 'B'}
+        unwound = utils.restore_nesting(metadata)
+        self.assertIs(metadata, unwound)
+
+    def test_restore_nesting(self):
+        metadata = {'a': 'A', 'b': 'B',
+                    'nested:a': 'A',
+                    'nested:b': 'B',
+                    'nested:twice:c': 'C',
+                    'nested:twice:d': 'D',
+                    'embedded:e': 'E'}
+        unwound = utils.restore_nesting(metadata)
+        expected = {'a': 'A', 'b': 'B',
+                    'nested': {'a': 'A', 'b': 'B',
+                               'twice': {'c': 'C', 'd': 'D'}},
+                    'embedded': {'e': 'E'}}
+        self.assertEqual(expected, unwound)
+        self.assertIsNot(metadata, unwound)
+
+    def test_restore_nesting_with_separator(self):
+        metadata = {'a': 'A', 'b': 'B',
+                    'nested.a': 'A',
+                    'nested.b': 'B',
+                    'nested.twice.c': 'C',
+                    'nested.twice.d': 'D',
+                    'embedded.e': 'E'}
+        unwound = utils.restore_nesting(metadata, separator='.')
+        expected = {'a': 'A', 'b': 'B',
+                    'nested': {'a': 'A', 'b': 'B',
+                               'twice': {'c': 'C', 'd': 'D'}},
+                    'embedded': {'e': 'E'}}
+        self.assertEqual(expected, unwound)
+        self.assertIsNot(metadata, unwound)
+
     def test_decimal_to_dt_with_none_parameter(self):
         self.assertIsNone(utils.decimal_to_dt(None))
 
diff --git a/ceilometer/utils.py b/ceilometer/utils.py
index 615764278..23fc7f6af 100644
--- a/ceilometer/utils.py
+++ b/ceilometer/utils.py
@@ -54,6 +54,20 @@ def recursive_keypairs(d, separator=':'):
             yield name, value
 
 
+def restore_nesting(d, separator=':'):
+    """Unwinds a flattened dict to restore nesting.
+    """
+    d = copy.copy(d) if any([separator in k for k in d.keys()]) else d
+    for k, v in d.items():
+        if separator in k:
+            top, rem = k.split(separator, 1)
+            nest = d[top] if isinstance(d.get(top), dict) else {}
+            nest[rem] = v
+            d[top] = restore_nesting(nest, separator)
+            del d[k]
+    return d
+
+
 def dt_to_decimal(utc):
     """Datetime to Decimal.