Merge lp:~justin-fathomdb/nova/skip-validation-openstack11 into lp:~hudson-openstack/nova/trunk

[justinsb]

While the OS API 1.1 schema is broken, let's have a flag to skip validation of it

[justinsb (justin-fathomdb)]

This branch also has an additional pre-req (though it probably doesn't need it) - the osapi-xml-serialization branch. Hence the diff below is not yet very useful until the pre-reqs merge. It's really just this bit:

 +# NOTE(justinsb): Until the OS 1.1 schema is fixed, don't bother checking it
30 +flags.DEFINE_bool('validate_openstack_11',
31 + False,
32 + 'Should we validate openstack 1.1 schema?')
33

+ if xmlns == common.XML_NS_V11:
41 + if not FLAGS.validate_openstack_11:
42 + return True
43 +
44

[termie (termie)]

Ah, I see here where the flag I was discussing in the other patch is defined.

As a somewhat arbitrary stylistic approach for XML in python, I feel that the current best library is etree rather than minidom, the syntax is much more pythonic (and no mixedCase) and requires less push-ups than DOM. Not gating this patch in any way, but worth looking into.

For example, most of the foo.getAttribute('bar', 'baz') would just be foo.get('bar', 'baz')

review: Approve

[Christopher MacGown (0x44)]

This lgtm.

review: Approve

[OpenStack Infra (hudson-openstack)]

The prerequisite lp:~justin-fathomdb/nova/check-xsd has not yet been merged into lp:nova.

[Dan Prince (dan-prince)]

Hi Justin,

I get a conflict when trying to merge this branch w/ trunk:

Text conflict in nova/wsgi.py

review: Needs Fixing

[OpenStack Infra (hudson-openstack)]

The prerequisite lp:~justin-fathomdb/nova/check-xsd has not yet been merged into lp:nova.

[Brian Waldon (bcwaldon)]

Quite a few conflicts...

Unmerged revisions

856. By justinsb on 2011-04-07

Fix pep8

855. By justinsb on 2011-04-07

Temporary workaround - don't bother validating against the broken OpenStack 1.1 schema

854. By justinsb on 2011-04-07

Better formed output: if an order is specified for elements, follow the order

853. By justinsb on 2011-04-07

Merged with lp:~rackspace-titan/nova/osapi-xml-serialization

852. By justinsb on 2011-04-06

Merged with lp:~justin-fathomdb/nova/bug740576 and thereby with trunk

851. By justinsb on 2011-04-06

For now, tolerate the stuff that's broken and dial back the tests so that we don't have to raise 200 bugs yet

850. By justinsb on 2011-04-06

Imported 1.1 schema from http://bazaar.launchpad.net/~annegentle/openstack-manuals/trunk/files/head:/doc/source/docbkx/openstack-compute-api/

849. By justinsb on 2011-04-06

Added validation code

848. By justinsb on 2011-04-06

Merged with trunk

847. By justinsb on 2011-03-29

Moved CloudServers v1.0 under nova/api/openstack/schemas

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-04-06 21:02:16 +0000
+++ nova/api/openstack/servers.py	2011-04-07 07:07:33 +0000
@@ -55,6 +55,16 @@
                            "imageRef"],
                 "link": ["rel", "type", "href"],
             },
+            "elements": {
+                "server": ["metadata", "addresses", "personality"],
+            },
+            "dict_collections": {
+                "metadata": {"item_name": "meta", "item_key": "key"},
+            },
+            "list_collections": {
+                "public": {"item_name": "ip", "item_key": "addr"},
+                "private": {"item_name": "ip", "item_key": "addr"},
+            },
         },
     }
 

=== modified file 'nova/api/openstack/validation.py'
--- nova/api/openstack/validation.py	2011-04-07 07:07:33 +0000
+++ nova/api/openstack/validation.py	2011-04-07 07:07:33 +0000
@@ -39,6 +39,10 @@
 flags.DEFINE_bool('raise_error_on_validation_warnings',
                   False,
                   'Treat XML validation failures as errors?')
+# NOTE(justinsb): Until the OS 1.1 schema is fixed, don't bother checking it
+flags.DEFINE_bool('validate_openstack_11',
+                  False,
+                  'Should we validate openstack 1.1 schema?')
 
 
 class OutputValidator(object):
@@ -70,6 +74,10 @@
             LOG.warning(_("No XML namespace for XML: %s") % xml_text)
             return False
 
+        if xmlns == common.XML_NS_V11:
+            if not FLAGS.validate_openstack_11:
+                return True
+
         xmlschema = common.load_xml_schema(xmlns)
         if not xmlschema:
             LOG.warning(_("Unknown XML namespace: %s") % xmlns)

=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2011-04-04 17:56:19 +0000
+++ nova/tests/api/openstack/test_servers.py	2011-04-07 07:07:33 +0000
@@ -192,6 +192,26 @@
         print res_dict['server']
         self.assertEqual(res_dict['server']['links'], expected_links)
 
+    def test_get_server_by_id_with_addresses_xml(self):
+        private = "192.168.0.3"
+        public = ["1.2.3.4"]
+        new_return_server = return_server_with_addresses(private, public)
+        self.stubs.Set(nova.db.api, 'instance_get', new_return_server)
+        req = webob.Request.blank('/v1.0/servers/1')
+        req.headers['Accept'] = 'application/xml'
+        res = req.get_response(fakes.wsgi_app())
+        dom = minidom.parseString(res.body)
+        server = dom.childNodes[0]
+        self.assertEquals(server.nodeName, 'server')
+        self.assertEquals(server.getAttribute('id'), '1')
+        self.assertEquals(server.getAttribute('name'), 'server1')
+        (public,) = server.getElementsByTagName('public')
+        (ip,) = public.getElementsByTagName('ip')
+        self.assertEquals(ip.getAttribute('addr'), '1.2.3.4')
+        (private,) = server.getElementsByTagName('private')
+        (ip,) = private.getElementsByTagName('ip')
+        self.assertEquals(ip.getAttribute('addr'),  '192.168.0.3')
+
     def test_get_server_by_id_with_addresses(self):
         private = "192.168.0.3"
         public = ["1.2.3.4"]
@@ -618,6 +638,22 @@
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(res.status_int, 404)
 
+    def test_get_all_server_details_xml_v1_0(self):
+        req = webob.Request.blank('/v1.0/servers/detail')
+        req.headers['Accept'] = 'application/xml'
+        res = req.get_response(fakes.wsgi_app())
+        print res.body
+        dom = minidom.parseString(res.body)
+        for i, server in enumerate(dom.getElementsByTagName('server')):
+            self.assertEqual(server.getAttribute('id'), str(i))
+            self.assertEqual(server.getAttribute('hostId'), '')
+            self.assertEqual(server.getAttribute('name'), 'server%d' % i)
+            self.assertEqual(server.getAttribute('imageId'), '10')
+            self.assertEqual(server.getAttribute('status'), 'BUILD')
+            (meta,) = server.getElementsByTagName('meta')
+            self.assertEqual(meta.getAttribute('key'), 'seq')
+            self.assertEqual(meta.firstChild.data.strip(), str(i))
+
     def test_get_all_server_details_v1_0(self):
         req = webob.Request.blank('/v1.0/servers/detail')
         res = req.get_response(fakes.wsgi_app())

=== modified file 'nova/wsgi.py'
--- nova/wsgi.py	2011-04-07 07:07:33 +0000
+++ nova/wsgi.py	2011-04-07 07:07:33 +0000
@@ -517,6 +517,14 @@
             result.setAttribute('xmlns', xmlns)
 
         if type(data) is list:
+            collections = metadata.get('list_collections', {})
+            if nodename in collections:
+                metadata = collections[nodename]
+                for item in data:
+                    node = doc.createElement(metadata['item_name'])
+                    node.setAttribute(metadata['item_key'], str(item))
+                    result.appendChild(node)
+                return result
             singular = metadata.get('plurals', {}).get(nodename, None)
             if singular is None:
                 if nodename.endswith('s'):
@@ -527,11 +535,31 @@
                 node = self._to_xml_node(doc, metadata, singular, item)
                 result.appendChild(node)
         elif type(data) is dict:
+            collections = metadata.get('dict_collections', {})
+            if nodename in collections:
+                metadata = collections[nodename]
+                for k, v in data.items():
+                    node = doc.createElement(metadata['item_name'])
+                    node.setAttribute(metadata['item_key'], str(k))
+                    text = doc.createTextNode(str(v))
+                    node.appendChild(text)
+                    result.appendChild(node)
+                return result
+
+            # Output each attribute either as an element or as an attribute.
+            # If the elements are specified, follow that order.
             attrs = metadata.get('attributes', {}).get(nodename, {})
+            found_elements = []
             for k, v in data.items():
                 if k in attrs:
                     result.setAttribute(k, str(v))
                 else:
+                    found_elements.append(k)
+            elements = metadata.get('elements', {}).get(nodename,
+                                                        found_elements)
+            for k in elements:
+                v = data.get(k, None)
+                if v is not None:
                     node = self._to_xml_node(doc, metadata, k, v)
                     result.appendChild(node)
         else: