Fix xml metadata for volumes api in cinder.

Fixes bug 1040891.

Change-Id: Idf8cc641162617ade2aeb77b666270cf1ce54c54
Signed-off-by: Matthew Treinish <treinish@linux.vnet.ibm.com>

diff --git a/cinder/api/openstack/common.py b/cinder/api/openstack/common.py
index 04ec611faade5a810ff89823c2d020056e6d702d..255a0a743a08bb0bd082668fe789f3bbf42f39d3 100644 (file)
--- a/cinder/api/openstack/common.py
+++ b/cinder/api/openstack/common.py
@@ -22,6 +22,8 @@ import urlparse
 import webob
 
 from cinder import flags
+from cinder.api.openstack import wsgi
+from cinder.api.openstack import xmlutil
 from cinder.openstack.common import log as logging
 
 
@@ -241,3 +243,75 @@ class ViewBuilder(object):
         prefix_parts = list(urlparse.urlsplit(prefix))
         url_parts[0:2] = prefix_parts[0:2]
         return urlparse.urlunsplit(url_parts)
+
+
+class MetadataDeserializer(wsgi.MetadataXMLDeserializer):
+    def deserialize(self, text):
+        dom = minidom.parseString(text)
+        metadata_node = self.find_first_child_named(dom, "metadata")
+        metadata = self.extract_metadata(metadata_node)
+        return {'body': {'metadata': metadata}}
+
+
+class MetaItemDeserializer(wsgi.MetadataXMLDeserializer):
+    def deserialize(self, text):
+        dom = minidom.parseString(text)
+        metadata_item = self.extract_metadata(dom)
+        return {'body': {'meta': metadata_item}}
+
+
+class MetadataXMLDeserializer(wsgi.XMLDeserializer):
+
+    def extract_metadata(self, metadata_node):
+        """Marshal the metadata attribute of a parsed request"""
+        if metadata_node is None:
+            return {}
+        metadata = {}
+        for meta_node in self.find_children_named(metadata_node, "meta"):
+            key = meta_node.getAttribute("key")
+            metadata[key] = self.extract_text(meta_node)
+        return metadata
+
+    def _extract_metadata_container(self, datastring):
+        dom = minidom.parseString(datastring)
+        metadata_node = self.find_first_child_named(dom, "metadata")
+        metadata = self.extract_metadata(metadata_node)
+        return {'body': {'metadata': metadata}}
+
+    def create(self, datastring):
+        return self._extract_metadata_container(datastring)
+
+    def update_all(self, datastring):
+        return self._extract_metadata_container(datastring)
+
+    def update(self, datastring):
+        dom = minidom.parseString(datastring)
+        metadata_item = self.extract_metadata(dom)
+        return {'body': {'meta': metadata_item}}
+
+
+metadata_nsmap = {None: xmlutil.XMLNS_V11}
+
+
+class MetaItemTemplate(xmlutil.TemplateBuilder):
+    def construct(self):
+        sel = xmlutil.Selector('meta', xmlutil.get_items, 0)
+        root = xmlutil.TemplateElement('meta', selector=sel)
+        root.set('key', 0)
+        root.text = 1
+        return xmlutil.MasterTemplate(root, 1, nsmap=metadata_nsmap)
+
+
+class MetadataTemplateElement(xmlutil.TemplateElement):
+    def will_render(self, datum):
+        return True
+
+
+class MetadataTemplate(xmlutil.TemplateBuilder):
+    def construct(self):
+        root = MetadataTemplateElement('metadata', selector='metadata')
+        elem = xmlutil.SubTemplateElement(root, 'meta',
+                                          selector=xmlutil.get_items)
+        elem.set('key', 0)
+        elem.text = 1
+        return xmlutil.MasterTemplate(root, 1, nsmap=metadata_nsmap)

diff --git a/cinder/api/openstack/volume/volumes.py b/cinder/api/openstack/volume/volumes.py
index 0525a4f3540eac704c9d85f5ccb125ee1f6853c9..8388c5f639210d2657f21395eda7acd54ffd1a26 100644 (file)
--- a/cinder/api/openstack/volume/volumes.py
+++ b/cinder/api/openstack/volume/volumes.py
@@ -17,6 +17,7 @@
 
 from webob import exc
 import webob
+from xml.dom import minidom
 
 from cinder.api.openstack import common
 from cinder.api.openstack import wsgi
@@ -104,10 +105,8 @@ def _translate_volume_summary_view(context, vol, image_id=None):
     LOG.audit(_("vol=%s"), vol, context=context)
 
     if vol.get('volume_metadata'):
-        meta_dict = {}
-        for i in vol['volume_metadata']:
-            meta_dict[i['key']] = i['value']
-        d['metadata'] = meta_dict
+        metadata = vol.get('volume_metadata')
+        d['metadata'] = dict((item['key'], item['value']) for item in metadata)
     else:
         d['metadata'] = {}
 
@@ -137,8 +136,8 @@ def make_volume(elem):
                                             selector='attachments')
     make_attachment(attachment)
 
-    metadata = xmlutil.make_flat_dict('metadata')
-    elem.append(metadata)
+    # Attach metadata node
+    elem.append(common.MetadataTemplate())
 
 
 volume_nsmap = {None: xmlutil.XMLNS_VOLUME_V1, 'atom': xmlutil.XMLNS_ATOM}
@@ -159,6 +158,47 @@ class VolumesTemplate(xmlutil.TemplateBuilder):
         return xmlutil.MasterTemplate(root, 1, nsmap=volume_nsmap)
 
 
+class CommonDeserializer(wsgi.MetadataXMLDeserializer):
+    """Common deserializer to handle xml-formatted volume requests.
+
+       Handles standard volume attributes as well as the optional metadata
+       attribute
+    """
+
+    metadata_deserializer = common.MetadataXMLDeserializer()
+
+    def _extract_volume(self, node):
+        """Marshal the volume attribute of a parsed request."""
+        volume = {}
+        volume_node = self.find_first_child_named(node, 'volume')
+
+        attributes = ['display_name', 'display_description', 'size',
+                      'volume_type', 'availability_zone']
+        for attr in attributes:
+            if volume_node.getAttribute(attr):
+                volume[attr] = volume_node.getAttribute(attr)
+
+        metadata_node = self.find_first_child_named(volume_node, 'metadata')
+        if metadata_node is not None:
+            volume['metadata'] = self.extract_metadata(metadata_node)
+
+        return volume
+
+
+class CreateDeserializer(CommonDeserializer):
+    """Deserializer to handle xml-formatted create volume requests.
+
+       Handles standard volume attributes as well as the optional metadata
+       attribute
+    """
+
+    def default(self, string):
+        """Deserialize an xml-formatted volume create request."""
+        dom = minidom.parseString(string)
+        volume = self._extract_volume(dom)
+        return {'body': {'volume': volume}}
+
+
 class VolumeController(object):
     """The Volumes API controller for the OpenStack API."""
 
@@ -233,6 +273,7 @@ class VolumeController(object):
         return image_uuid
 
     @wsgi.serializers(xml=VolumeTemplate)
+    @wsgi.deserializers(xml=CreateDeserializer)
     def create(self, req, body):
         """Creates a new volume."""
         context = req.environ['cinder.context']

diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py
index f420d41b35a29c81ef8023613f907453c1127898..6f969edb74ff52e7a9dffdeec8c415746f143824 100644 (file)
--- a/cinder/api/openstack/wsgi.py
+++ b/cinder/api/openstack/wsgi.py
@@ -214,6 +214,18 @@ class XMLDeserializer(TextDeserializer):
         return {'body': self._from_xml(datastring)}
 
 
+class MetadataXMLDeserializer(XMLDeserializer):
+
+    def extract_metadata(self, metadata_node):
+        """Marshal the metadata attribute of a parsed request"""
+        metadata = {}
+        if metadata_node is not None:
+            for meta_node in self.find_children_named(metadata_node, "meta"):
+                key = meta_node.getAttribute("key")
+                metadata[key] = self.extract_text(meta_node)
+        return metadata
+
+
 class DictSerializer(ActionDispatcher):
     """Default request body serialization"""
 

diff --git a/cinder/tests/api/openstack/volume/test_volumes.py b/cinder/tests/api/openstack/volume/test_volumes.py
index 2c69ea55384d2c879195423ee6404ab37107f98d..bee962f55306ed5824eb06532a49ae3415c9e15d 100644 (file)
--- a/cinder/tests/api/openstack/volume/test_volumes.py
+++ b/cinder/tests/api/openstack/volume/test_volumes.py
@@ -348,10 +348,10 @@ class VolumeSerializerTest(test.TestCase):
             elif child.tag == 'metadata':
                 not_seen = set(vol['metadata'].keys())
                 for gr_child in child:
-                    self.assertTrue(gr_child.tag in not_seen)
-                    self.assertEqual(str(vol['metadata'][gr_child.tag]),
+                    self.assertTrue(gr_child.get("key") in not_seen)
+                    self.assertEqual(str(vol['metadata'][gr_child.get("key")]),
                                      gr_child.text)
-                    not_seen.remove(gr_child.tag)
+                    not_seen.remove(gr_child.get('key'))
                 self.assertEqual(0, len(not_seen))
 
     def test_volume_show_create_serializer(self):
@@ -434,3 +434,133 @@ class VolumeSerializerTest(test.TestCase):
         self.assertEqual(len(raw_volumes), len(tree))
         for idx, child in enumerate(tree):
             self._verify_volume(raw_volumes[idx], child)
+
+
+class TestVolumeCreateRequestXMLDeserializer(test.TestCase):
+
+    def setUp(self):
+        super(TestVolumeCreateRequestXMLDeserializer, self).setUp()
+        self.deserializer = volumes.CreateDeserializer()
+
+    def test_minimal_volume(self):
+        self_request = """
+<volume xmlns="http://docs.openstack.org/compute/api/v1.1"
+        size="1"></volume>"""
+        request = self.deserializer.deserialize(self_request)
+        expected = {
+            "volume": {
+                    "size": "1",
+            },
+        }
+        self.assertEquals(request['body'], expected)
+
+    def test_display_name(self):
+        self_request = """
+<volume xmlns="http://docs.openstack.org/compute/api/v1.1"
+        size="1"
+        display_name="Volume-xml"></volume>"""
+        request = self.deserializer.deserialize(self_request)
+        expected = {
+            "volume": {
+                "size": "1",
+                "display_name": "Volume-xml",
+            },
+        }
+        self.assertEquals(request['body'], expected)
+
+    def test_display_description(self):
+        self_request = """
+<volume xmlns="http://docs.openstack.org/compute/api/v1.1"
+        size="1"
+        display_name="Volume-xml"
+        display_description="description"></volume>"""
+        request = self.deserializer.deserialize(self_request)
+        expected = {
+            "volume": {
+                "size": "1",
+                "display_name": "Volume-xml",
+                "display_description": "description",
+            },
+        }
+        self.assertEquals(request['body'], expected)
+
+    def test_volume_type(self):
+        self_request = """
+<volume xmlns="http://docs.openstack.org/compute/api/v1.1"
+        size="1"
+        display_name="Volume-xml"
+        display_description="description"
+        volume_type="289da7f8-6440-407c-9fb4-7db01ec49164"></volume>"""
+        request = self.deserializer.deserialize(self_request)
+        expected = {
+            "volume": {
+                "display_name": "Volume-xml",
+                "size": "1",
+                "display_name": "Volume-xml",
+                "display_description": "description",
+                "volume_type": "289da7f8-6440-407c-9fb4-7db01ec49164",
+            },
+        }
+        self.assertEquals(request['body'], expected)
+
+    def test_availability_zone(self):
+        self_request = """
+<volume xmlns="http://docs.openstack.org/compute/api/v1.1"
+        size="1"
+        display_name="Volume-xml"
+        display_description="description"
+        volume_type="289da7f8-6440-407c-9fb4-7db01ec49164"
+        availability_zone="us-east1"></volume>"""
+        request = self.deserializer.deserialize(self_request)
+        expected = {
+            "volume": {
+                "size": "1",
+                "display_name": "Volume-xml",
+                "display_description": "description",
+                "volume_type": "289da7f8-6440-407c-9fb4-7db01ec49164",
+                "availability_zone": "us-east1",
+            },
+        }
+        self.assertEquals(request['body'], expected)
+
+    def test_metadata(self):
+        self_request = """
+<volume xmlns="http://docs.openstack.org/compute/api/v1.1"
+        display_name="Volume-xml"
+        size="1">
+        <metadata><meta key="Type">work</meta></metadata></volume>"""
+        request = self.deserializer.deserialize(self_request)
+        expected = {
+            "volume": {
+                "display_name": "Volume-xml",
+                "size": "1",
+                "metadata": {
+                    "Type": "work",
+                },
+            },
+        }
+        self.assertEquals(request['body'], expected)
+
+    def test_full_volume(self):
+        self_request = """
+<volume xmlns="http://docs.openstack.org/compute/api/v1.1"
+        size="1"
+        display_name="Volume-xml"
+        display_description="description"
+        volume_type="289da7f8-6440-407c-9fb4-7db01ec49164"
+        availability_zone="us-east1">
+        <metadata><meta key="Type">work</meta></metadata></volume>"""
+        request = self.deserializer.deserialize(self_request)
+        expected = {
+            "volume": {
+                "size": "1",
+                "display_name": "Volume-xml",
+                "display_description": "description",
+                "volume_type": "289da7f8-6440-407c-9fb4-7db01ec49164",
+                "availability_zone": "us-east1",
+                "metadata": {
+                    "Type": "work",
+                },
+            },
+        }
+        self.assertEquals(request['body'], expected)