launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-config-signal-bug-952491/+merge/97158

This branch adds a utility method on the Config objects' manager (Config.object) to track changes made to individual config items.

Drive-by fix: run the tests without the verbose flag!
-- 
https://code.launchpad.net/~rvb/maas/maas-config-signal-bug-952491/+merge/97158
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-config-signal-bug-952491 into lp:maas.

=== modified file 'Makefile'
--- Makefile	2012-03-13 03:59:49 +0000
+++ Makefile	2012-03-13 08:53:18 +0000
@@ -44,8 +44,8 @@
 	utilities/maasdb start ./db/ disposable
 
 test: bin/test.maas bin/test.pserv
-	bin/test.maas -- -v
-	bin/test.pserv -v
+	bin/test.maas
+	bin/test.pserv
 
 lint: sources = setup.py src templates utilities
 lint: bin/flake8

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-03-12 07:25:31 +0000
+++ src/maasserver/models.py	2012-03-13 08:53:18 +0000
@@ -757,6 +757,10 @@
 
     """
 
+    def __init__(self):
+        super(ConfigManager, self).__init__()
+        self._config_changed_connections = {}
+
     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
@@ -801,6 +805,34 @@
         except Config.DoesNotExist:
             self.create(name=name, value=value)
 
+    def config_changed_connect(self, config_name, method):
+        """Connect a method to Django's 'update' signal for given config name.
+
+        :param config_name: The name of the config item to track.
+        :type config_name: basestring
+        :param method: The method to be called.
+        :type method: callable
+
+        The provided callabe should follow Django's convention.  E.g:
+
+        >>> def callable(sender, instance, created, **kwargs):
+        >>>     pass
+        >>>
+        >>> Config.objects.config_changed_connect('config_name', callable)
+        """
+        connections = self._config_changed_connections.setdefault(
+            config_name, [])
+        connections.append(method)
+        self._config_changed_connections[config_name] = connections
+
+    def _config_changed(self, sender, instance, created, **kwargs):
+        if instance.name in self._config_changed_connections:
+            for connection in self._config_changed_connections[instance.name]:
+                connection(sender, instance, created, **kwargs)
+
+
+config_manager = ConfigManager()
+
 
 class Config(models.Model):
     """Configuration settings.
@@ -814,12 +846,16 @@
     name = models.CharField(max_length=255, unique=False)
     value = JSONObjectField(null=True)
 
-    objects = ConfigManager()
+    objects = config_manager
 
     def __unicode__(self):
         return "%s: %s" % (self.name, self.value)
 
 
+# Connect config_manager._config_changed the post save signal of Config.
+post_save.connect(config_manager._config_changed, sender=Config)
+
+
 # Register the models in the admin site.
 admin.site.register(Consumer)
 admin.site.register(Config)

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-03-13 05:34:38 +0000
+++ src/maasserver/tests/test_models.py	2012-03-13 08:53:18 +0000
@@ -679,8 +679,19 @@
             "%s's" % name.capitalize(), default_config['maas_name'])
 
 
+# A utility class which tracks the calls to its 'call' method and
+# stores the arguments given to 'call' in 'self.calls'.
+class Listener:
+
+    def __init__(self):
+        self.calls = []
+
+    def call(self, *args, **kwargs):
+        self.calls.append([args, kwargs])
+
+
 class ConfigTest(TestCase):
-    """Testing of the :class:`Config` model."""
+    """Testing of the :class:`Config` model and its related manager class."""
 
     def test_manager_get_config_found(self):
         Config.objects.create(name='name', value='config')
@@ -722,3 +733,36 @@
         self.assertSequenceEqual(
             ['config2'],
             [config.value for config in Config.objects.filter(name='name')])
+
+    def test_manager_config_changed_connect_connects(self):
+        listener = Listener()
+        name = factory.getRandomString()
+        value = factory.getRandomString()
+        Config.objects.config_changed_connect(name, listener.call)
+        Config.objects.set_config(name, value)
+        config = Config.objects.get(name=name)
+
+        self.assertEqual(1, len(listener.calls))
+        self.assertEqual((Config, config, True), listener.calls[0][0])
+
+    def test_manager_config_changed_connect_connects_multiple(self):
+        listener = Listener()
+        listener2 = Listener()
+        name = factory.getRandomString()
+        value = factory.getRandomString()
+        Config.objects.config_changed_connect(name, listener.call)
+        Config.objects.config_changed_connect(name, listener2.call)
+        Config.objects.set_config(name, value)
+
+        self.assertEqual(1, len(listener.calls))
+        self.assertEqual(1, len(listener2.calls))
+
+    def test_manager_config_changed_connect_connects_by_config_name(self):
+        listener = Listener()
+        name = factory.getRandomString()
+        value = factory.getRandomString()
+        Config.objects.config_changed_connect(name, listener.call)
+        another_name = factory.getRandomString()
+        Config.objects.set_config(another_name, value)
+
+        self.assertEqual(0, len(listener.calls))