launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Martin Packman has proposed merging lp:~gz/maas/xmlfield into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~gz/maas/xmlfield/+merge/124723

Adds an XmlField object that exposes the underlying postgres xml type support. We want this in the short term so we have the lshw output in a form we can process to generate tags. When we get that far, the data wants to live at the cluster rather than region level, so the django parts can remain ignorant of the details. This gives us a route forwards on generating tags for now however.

There are a few wrinkles in the implementation, as django doesn't make it easy to use arbitrary functions in SQL. Most notably, this means what the database will accept will depend on the 'xmloption' session configuration parameter, which defaults to allowing any fragment. Trying to insert invalid data raises DatabaseError, which is presumably acceptable. Finally, using the handy processing functions means dropping down to Mangager.raw and writing out the query. 


-- 
https://code.launchpad.net/~gz/maas/xmlfield/+merge/124723
Your team MAAS Maintainers is requested to review the proposed merge of lp:~gz/maas/xmlfield into lp:maas.

=== modified file 'src/maasserver/fields.py'
--- src/maasserver/fields.py	2012-06-28 07:05:11 +0000
+++ src/maasserver/fields.py	2012-09-17 15:50:33 +0000
@@ -41,7 +41,7 @@
 validate_mac = RegexValidator(regex=mac_re, message=mac_error_msg)
 
 
-# The MACAddressField and the JSONObjectField don't introduce any new
+# The MACAddressField, JSONObjectField and XmlField don't introduce any new
 # parameters compared to their parent's constructors so South will handle
 # them just fine.
 # See http://south.aeracode.org/docs/customfields.html#extending-introspection
@@ -50,6 +50,7 @@
     [], [
         "^maasserver\.fields\.MACAddressField",
         "^maasserver\.fields\.JSONObjectField",
+        "^maasserver\.fields\.XmlField",
     ])
 
 
@@ -123,3 +124,29 @@
             raise TypeError("Lookup type %s is not supported." % lookup_type)
         return super(JSONObjectField, self).get_prep_lookup(
             lookup_type, value)
+
+
+class XmlField(Field):
+    """A field for storing xml natively.
+
+    This is not like the removed Django XMLField which just added basic python
+    level checking on top of a text column.
+
+    Really inserts should be wrapped like `XMLPARSE(DOCUMENT value)` but it's
+    hard to do from django so rely on postgres supporting casting from char.
+    """
+
+    description = "XML document or fragment"
+
+    def db_type(self, connection):
+        return "xml"
+
+    def get_db_prep_lookup(self, lookup_type, value):
+        """Limit lookup types to those that work on xml.
+
+        Unlike character fields the xml type is non-comparible, see:
+        <http://www.postgresql.org/docs/devel/static/datatype-xml.html>
+        """
+        if lookup_type != 'isnull':
+            raise TypeError("Lookup type %s is not supported." % lookup_type)
+        return super(XmlField, self).get_db_prep_lookup(lookup_type, value)

=== modified file 'src/maasserver/tests/models.py'
--- src/maasserver/tests/models.py	2012-08-02 15:47:39 +0000
+++ src/maasserver/tests/models.py	2012-09-17 15:50:33 +0000
@@ -19,7 +19,10 @@
     CharField,
     Model,
     )
-from maasserver.fields import JSONObjectField
+from maasserver.fields import (
+    JSONObjectField,
+    XmlField,
+    )
 from maasserver.models.timestampedmodel import TimestampedModel
 
 
@@ -28,6 +31,15 @@
     value = JSONObjectField(null=True)
 
 
+class XmlFieldModel(Model):
+
+    class Meta:
+        db_table = "docs"
+
+    name = CharField(max_length=255, unique=False)
+    value = XmlField(null=True)
+
+
 class MessagesTestModel(Model):
     name = CharField(max_length=255, unique=False)
 

=== modified file 'src/maasserver/tests/test_fields.py'
--- src/maasserver/tests/test_fields.py	2012-04-16 10:00:51 +0000
+++ src/maasserver/tests/test_fields.py	2012-09-17 15:50:33 +0000
@@ -12,6 +12,7 @@
 __metaclass__ = type
 __all__ = []
 
+from django.db import DatabaseError
 from django.core.exceptions import ValidationError
 from maasserver.fields import validate_mac
 from maasserver.models import MACAddress
@@ -20,7 +21,10 @@
     TestCase,
     TestModelTestCase,
     )
-from maasserver.tests.models import JSONFieldModel
+from maasserver.tests.models import (
+    JSONFieldModel,
+    XmlFieldModel,
+    )
 
 
 class TestMACAddressField(TestCase):
@@ -103,3 +107,45 @@
     def test_field_another_lookup_fails(self):
         # Others lookups are not allowed.
         self.assertRaises(TypeError, JSONFieldModel.objects.get, value__gte=3)
+
+
+class TestXmlField(TestModelTestCase):
+
+    app = 'maasserver.tests'
+
+    def test_loads_string(self):
+        name = factory.getRandomString()
+        value = "<test/>"
+        XmlFieldModel.objects.create(name=name, value=value)
+        instance = XmlFieldModel.objects.get(name=name)
+        self.assertEqual(value, instance.value)
+
+    def test_lookup_xpath_exists_result(self):
+        name = factory.getRandomString()
+        XmlFieldModel.objects.create(name=name, value="<test/>")
+        result = XmlFieldModel.objects.raw(
+            "SELECT * FROM docs WHERE xpath_exists(%s, value)", ["//test"])
+        self.assertEqual(name, result[0].name)
+
+    def test_lookup_xpath_exists_no_result(self):
+        name = factory.getRandomString()
+        XmlFieldModel.objects.create(name=name, value="<test/>")
+        result = XmlFieldModel.objects.raw(
+            "SELECT * FROM docs WHERE xpath_exists(%s, value)", ["//miss"])
+        self.assertEqual([], list(result))
+
+    def test_save_empty_rejected(self):
+        self.assertRaises(DatabaseError, XmlFieldModel.objects.create,
+            value="")
+
+    def test_save_non_wellformed_rejected(self):
+        self.assertRaises(DatabaseError, XmlFieldModel.objects.create,
+            value="<bad>")
+
+    def test_lookup_none(self):
+        XmlFieldModel.objects.create(value=None)
+        test_instance = XmlFieldModel.objects.get(value__isnull=True)
+        self.assertIsNone(test_instance.value)
+
+    def test_lookup_exact_unsupported(self):
+        self.assertRaises(TypeError, XmlFieldModel.objects.get, value="")