launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/config-table into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/config-table/+merge/93861

This branch adds a config object that can store any pickleable python object in the database.  We will use it to store the many global config options we will need to store. (the PickleableObjectField is a slightly modified and simplified version of http://djangosnippets.org/snippets/1694/).

Note that I've renamed 'src/maasserver/macaddress.py' => 'src/maasserver/fields.py' where we will put the custom fields.
-- 
https://code.launchpad.net/~rvb/maas/config-table/+merge/93861
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/config-table into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-02-20 13:35:51 +0000
+++ src/maasserver/api.py	2012-02-20 15:35:22 +0000
@@ -41,8 +41,8 @@
     NodesNotAvailable,
     PermissionDenied,
     )
+from maasserver.fields import validate_mac
 from maasserver.forms import NodeWithMACAddressesForm
-from maasserver.macaddress import validate_mac
 from maasserver.models import (
     FileStorage,
     MACAddress,

=== renamed file 'src/maasserver/macaddress.py' => 'src/maasserver/fields.py'
--- src/maasserver/macaddress.py	2012-01-27 14:30:12 +0000
+++ src/maasserver/fields.py	2012-02-20 15:35:22 +0000
@@ -14,11 +14,24 @@
     "MACAddressFormField",
     ]
 
+from base64 import (
+    b64decode,
+    b64encode,
+    )
+from copy import deepcopy
+from cPickle import (
+    dumps,
+    loads,
+    )
 import re
 
 from django.core.validators import RegexValidator
-from django.db.models import Field
+from django.db.models import (
+    Field,
+    SubfieldBase,
+    )
 from django.forms import RegexField
+from django.utils.encoding import force_unicode
 import psycopg2.extensions
 
 
@@ -67,3 +80,47 @@
 
 
 psycopg2.extensions.register_adapter(MACAddressField, MACAddressAdapter)
+
+
+# Fix the protocol used in case the default value changes.
+DEFAULT_PROTOCOL = 2
+
+
+class PickleableObjectField(Field):
+    """A field that will store any pickleable python object."""
+
+    __metaclass__ = SubfieldBase
+
+    def __init__(self, *args, **kwargs):
+        # The field cannot be edited.
+        kwargs.setdefault('editable', False)
+        super(PickleableObjectField, self).__init__(*args, **kwargs)
+
+    def to_python(self, value):
+        """db -> python: b64decode and unpickle the object."""
+        if value is not None:
+            try:
+                return loads(b64decode(value))
+            except:
+                return value
+        else:
+            return None
+
+    def get_db_prep_value(self, value):
+        """python -> db: pickle and b64encode."""
+        if value is not None:
+            # We call force_unicode here explicitly, so that the encoded string
+            # isn't rejected by the postgresql_psycopg2 backend.
+            return force_unicode(
+                b64encode(dumps(deepcopy(value), DEFAULT_PROTOCOL)))
+        else:
+            return None
+
+    def get_internal_type(self):
+        return 'TextField'
+
+    def get_prep_lookup(self, lookup_type, value):
+        if lookup_type not in ['exact', 'isnull']:
+            raise TypeError('Lookup type %s is not supported.' % lookup_type)
+        return super(PickleableObjectField, self).get_prep_lookup(
+            lookup_type, value)

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-02-16 16:29:18 +0000
+++ src/maasserver/forms.py	2012-02-20 15:35:22 +0000
@@ -21,7 +21,7 @@
     )
 from django.contrib.auth.models import User
 from django.forms import ModelForm
-from maasserver.macaddress import MACAddressFormField
+from maasserver.fields import MACAddressFormField
 from maasserver.models import (
     MACAddress,
     Node,

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-02-17 14:20:02 +0000
+++ src/maasserver/models.py	2012-02-20 15:35:22 +0000
@@ -11,6 +11,7 @@
 __metaclass__ = type
 __all__ = [
     "generate_node_system_id",
+    "Config",
     "FileStorage",
     "NODE_STATUS",
     "Node",
@@ -33,7 +34,10 @@
     CannotDeleteUserException,
     PermissionDenied,
     )
-from maasserver.macaddress import MACAddressField
+from maasserver.fields import (
+    MACAddressField,
+    PickleableObjectField,
+    )
 from metadataserver import nodeinituser
 from piston.models import (
     Consumer,
@@ -549,8 +553,74 @@
         self.data.save(filename, content)
 
 
+class ConfigManager(models.Manager):
+    """A utility to manage the configuration settings.
+
+    """
+
+    def get_config(self, name, default=None):
+        """Return the config value corresponding to the given config name.
+        Return None or the provided default if the config value does not
+        exist.
+
+        :param name: The name of the config item.
+        :type name: str
+        :param name: The optional default value to return if no such config
+            item exists.
+        :type name: object
+        :return: A config value.
+        :rtype: object
+        :raises: Config.MultipleObjectsReturned
+        """
+        try:
+            return self.get(name=name).value
+        except Config.DoesNotExist:
+            return default
+
+    def get_config_list(self, name):
+        """Return the config value list corresponding to the given config
+        name.
+
+        :param name: The name of the config items.
+        :type name: str
+        :return: A list of the config values.
+        :rtype: list
+        """
+        return [config.value for config in self.filter(name=name)]
+
+    def set_config(self, name, value):
+        """Return the config value list corresponding to the given config
+        name.
+
+        :param name: The name of the config item to create.
+        :type name: str
+        :param value: The value of the config item to create.
+        :type value: object
+        """
+        self.create(name=name, value=value)
+
+
+class Config(models.Model):
+    """Configuration settings.
+
+    :ivar name: The name of the configuration option.
+    :type name: string
+    :ivar value: The configuration value.
+    :type value: Any pickleable python object.
+    """
+
+    name = models.CharField(max_length=255, unique=False)
+    value = PickleableObjectField(null=True)
+
+    objects = ConfigManager()
+
+    def __unicode__(self):
+        return "%s: %s" % (self.name, self.value)
+
+
 # Register the models in the admin site.
 admin.site.register(Consumer)
+admin.site.register(Config)
 admin.site.register(FileStorage)
 admin.site.register(MACAddress)
 admin.site.register(Node)

=== modified file 'src/maasserver/tests/test_macaddressfield.py'
--- src/maasserver/tests/test_macaddressfield.py	2012-02-16 11:51:13 +0000
+++ src/maasserver/tests/test_macaddressfield.py	2012-02-20 15:35:22 +0000
@@ -12,7 +12,7 @@
 __all__ = []
 
 from django.core.exceptions import ValidationError
-from maasserver.macaddress import validate_mac
+from maasserver.fields import validate_mac
 from maasserver.models import MACAddress
 from maasserver.testing import TestCase
 from maasserver.testing.factory import factory

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-02-17 15:40:01 +0000
+++ src/maasserver/tests/test_models.py	2012-02-20 15:35:22 +0000
@@ -23,12 +23,13 @@
     PermissionDenied,
     )
 from maasserver.models import (
+    Config,
     GENERIC_CONSUMER,
-    SYSTEM_USERS,
     MACAddress,
     Node,
     NODE_STATUS,
     NODE_STATUS_CHOICES_DICT,
+    SYSTEM_USERS,
     UserProfile,
     )
 from maasserver.testing import TestCase
@@ -409,3 +410,69 @@
         # out intact.
         storage = factory.make_file_storage(filename="x", data=binary_data)
         self.assertEqual(binary_data, storage.data.read())
+
+
+class ConfigTest(TestCase):
+    """Testing of the :class:`Config` model."""
+
+    def test_stores_types(self):
+        values = [
+            None,
+            True,
+            False,
+            3.33,
+            "A simple string",
+            [1, 2.43, "3"],
+            {"not": 5, 4: "test"},
+            set([1, "42"])
+            ]
+        for value in values:
+            name = factory.getRandomString()
+            config = Config(name=name, value=value)
+            config.save()
+
+            config = Config.objects.get(name=name)
+            self.assertEqual(value, config.value)
+
+    def test_field_exact_lookup(self):
+        # Value can be query via an 'exact' lookup.
+        Config.objects.create(name='name', value=[4, 6, {}])
+        config = Config.objects.get(value=[4, 6, {}])
+        self.assertEqual('name', config.name)
+
+    def test_field_none_lookup(self):
+        # Value can be queried via a 'isnull' lookup.
+        Config.objects.create(name='name', value=None)
+        config = Config.objects.get(value__isnull=True)
+        self.assertEqual('name', config.name)
+
+    def test_field_another_lookup_fails(self):
+        # Others lookups are not allowed.
+        self.assertRaises(TypeError, Config.objects.get, value__gte=3)
+
+    def test_manager_get_config_found(self):
+        Config.objects.create(name='name', value='config')
+        Config.objects.filter(value__isnull=True)
+        config = Config.objects.get_config('name')
+        self.assertEqual('config', config)
+
+    def test_manager_get_config_not_found(self):
+        config = Config.objects.get_config('name', 'default value')
+        self.assertEqual('default value', config)
+
+    def test_manager_get_config_not_found_none(self):
+        config = Config.objects.get_config('name')
+        self.assertIsNone(config)
+
+    def test_manager_get_config_list_returns_config_list(self):
+        Config.objects.create(name='name', value='config1')
+        Config.objects.create(name='name', value='config2')
+        config_list = Config.objects.get_config_list('name')
+        self.assertSequenceEqual(['config1', 'config2'], config_list)
+
+    def test_manager_set_config_creates_config(self):
+        Config.objects.set_config('name', 'config1')
+        Config.objects.set_config('name', 'config2')
+        self.assertSequenceEqual(
+            ['config1', 'config2'],
+            [config.value for config in Config.objects.filter(name='name')])