Refactor serializer import to XmlBodyMiddleware

Previously, the keystone/middleware/core.py module would import
keystone/common/serializer.py regardless of the XmlBodyMiddleware class
being used. This change modifies keystone/common/serializer.py to raise
an error if there is an issue with using lxml at runtime.

Change-Id: I977275d0df8666b49ddbfadd4f5606d7c2292b1f
Closes-Bug: 1351016

keystone/common/serializer.py

@@ -20,14 +20,22 @@ by convention, with a few hardcoded exceptions.
 
 """
 
-from lxml import etree
+try:
+    from lxml import etree
+except ImportError:
+    etree = None
+
 import re
 
 import six
 
+from keystone import exception
 from keystone.i18n import _
+from keystone.openstack.common import log as logging
 
 
+LOG = logging.getLogger(__name__)
+
 DOCTYPE = '<?xml version="1.0" encoding="UTF-8"?>'
 XMLNS = 'http://docs.openstack.org/identity/api/v2.0'
 XMLNS_LIST = [
@@ -40,16 +48,6 @@ XMLNS_LIST = [
     },
 ]
 
-PARSER = etree.XMLParser(
-    resolve_entities=False,
-    remove_comments=True,
-    remove_pis=True)
-
-# NOTE(dolph): lxml.etree.Entity() is just a callable that currently returns an
-# lxml.etree._Entity instance, which doesn't appear to be part of the
-# public API, so we discover the type dynamically to be safe
-ENTITY_TYPE = type(etree.Entity('x'))
-
 
 def from_xml(xml):
     """Deserialize XML to a dictionary."""
@@ -70,9 +68,25 @@ def to_xml(d, xmlns=None):
 
 
 class XmlDeserializer(object):
+
+    def __init__(self):
+        if etree is None:
+            LOG.warning('lxml not installed')
+            raise exception.UnexpectedError()
+
+        self.parser = etree.XMLParser(resolve_entities=False,
+                                      remove_comments=True,
+                                      remove_pis=True)
+
+        # NOTE(dolph): lxml.etree.Entity() is just a callable that currently
+        # returns an lxml.etree._Entity instance, which doesn't appear to be
+        # part of the public API, so we discover the type dynamically to be
+        # safe
+        self.entity_type = type(etree.Entity('x'))
+
     def __call__(self, xml_str):
         """Returns a dictionary populated by decoding the given xml string."""
-        dom = etree.fromstring(xml_str.strip(), PARSER)
+        dom = etree.fromstring(xml_str.strip(), self.parser)
         return self.walk_element(dom, True)
 
     def _deserialize_links(self, links):
@@ -145,7 +159,7 @@ class XmlDeserializer(object):
         links = None
         truncated = False
         for child in [self.walk_element(x) for x in element
-                      if not isinstance(x, ENTITY_TYPE)]:
+                      if not isinstance(x, self.entity_type)]:
             if list_item_tag:
                 # FIXME(gyee): special-case lists for now until we
                 # figure out how to properly handle them.
@@ -178,6 +192,12 @@ class XmlDeserializer(object):
 
 
 class XmlSerializer(object):
+
+    def __init__(self):
+        if etree is None:
+            LOG.warning('lxml not installed')
+            raise exception.UnexpectedError()
+
     def __call__(self, d, xmlns=None):
         """Returns an xml etree populated by the given dictionary.
 

keystone/tests/test_serializer.py

@@ -14,9 +14,12 @@
 
 import copy
 
+from lxml import etree
+import mock
 from testtools import matchers
 
 from keystone.common import serializer
+from keystone import exception
 from keystone import tests
 from keystone.tests import matchers as ksmatchers
 
@@ -314,3 +317,21 @@ identity-service/2.0/identity-dev-guide-2.0.pdf" type="application/pdf"/>
                     OS-KSADM:password="mypass"/>
         """ % dict(xmlns=xmlns)
         self.assertThat(serializer.from_xml(xml), matchers.Equals(expected))
+
+    @mock.patch('keystone.common.serializer.etree', new=etree)
+    def test_XmlDeserializer_with_installed_succeeds(self):
+        serializer.XmlDeserializer()
+
+    @mock.patch('keystone.common.serializer.etree', new=None)
+    def test_XmlDeserializer_without_etree_installed_fails(self):
+        self.assertRaises(exception.UnexpectedError,
+                          serializer.XmlDeserializer)
+
+    @mock.patch('keystone.common.serializer.etree', new=etree)
+    def test_XmlSerializer_with_installed_succeeds(self):
+        serializer.XmlSerializer()
+
+    @mock.patch('keystone.common.serializer.etree', new=None)
+    def test_XmlSerializer_without_etree_installed_fails(self):
+        self.assertRaises(exception.UnexpectedError,
+                          serializer.XmlSerializer)

requirements-py3.txt

@@ -12,7 +12,6 @@ six>=1.7.0
 SQLAlchemy>=0.8.4,<=0.8.99,>=0.9.7,<=0.9.99
 sqlalchemy-migrate>=0.9.1
 passlib
-lxml>=2.3
 iso8601>=0.1.9
 python-keystoneclient>=0.9.0
 keystonemiddleware>=1.0.0

requirements.txt

@@ -11,7 +11,6 @@ six>=1.7.0
 SQLAlchemy>=0.8.4,<=0.8.99,>=0.9.7,<=0.9.99
 sqlalchemy-migrate>=0.9.1
 passlib
-lxml>=2.3
 iso8601>=0.1.9
 python-keystoneclient>=0.9.0
 keystonemiddleware>=1.0.0

test-requirements-py3.txt

@@ -23,6 +23,8 @@ pymongo>=2.5
 coverage>=3.6
 # fixture stubbing
 fixtures>=0.3.14
+# xml parsing
+lxml>=2.3
 # mock object framework
 mock>=1.0
 oslotest

test-requirements.txt

@@ -20,6 +20,8 @@ ldappool>=1.0 # MPL
 coverage>=3.6
 # fixture stubbing
 fixtures>=0.3.14
+# xml parsing
+lxml>=2.3
 # mock object framework
 mock>=1.0
 oslotest