Adds Model.on_change(callback) to watch changes. Like signals but better.

apps/amo/models.py

@@ -1,3 +1,4 @@
+from collections import defaultdict
 import contextlib
 import threading
 
@@ -157,6 +158,113 @@ class ManagerBase(caching.base.CachingManager, UncachedManagerBase):
                                   using=self._db, *args, **kwargs)
 
 
+class _NoChangeInstance(object):
+    """A proxy for object instances to make safe operations within an
+    OnChangeMixin.on_change() callback.
+    """
+
+    def __init__(self, instance):
+        self.__instance = instance
+
+    def __repr__(self):
+        return u'<%s for %r>' % (self.__class__.__name__, self.__instance)
+
+    def __getattr__(self, attr):
+        return getattr(self.__instance, attr)
+
+    def __setattr__(self, attr, val):
+        if attr.endswith('__instance'):
+            # _NoChangeInstance__instance
+            self.__dict__[attr] = val
+        else:
+            setattr(self.__instance, attr, val)
+
+    def save(self, *args, **kw):
+        kw['_signal'] = False
+        return self.__instance.save(*args, **kw)
+
+    def update(self, *args, **kw):
+        kw['_signal'] = False
+        return self.__instance.update(*args, **kw)
+
+
+_on_change_callbacks = defaultdict(list)
+
+
+# @TODO(Kumar) liberate: move OnChangeMixin Model mixin to nuggets
+class OnChangeMixin(object):
+    """Mixin for a Model that allows you to observe attribute changes.
+
+    Register change observers with::
+
+        class YourModel(amo.models.OnChangeMixin,
+                        amo.models.ModelBase):
+            # ...
+            pass
+
+        YourModel.on_change(callback)
+
+    """
+
+    def __init__(self, *args, **kw):
+        super(OnChangeMixin, self).__init__(*args, **kw)
+        self._initial_attr = dict(self.__dict__)
+
+    @classmethod
+    def on_change(cls, callback):
+        """Register a function to call on save or update to respond to changes.
+
+        For example::
+
+            def watch_status(old_attr={}, new_attr={},
+                             instance=None, sender=None, **kw):
+                if old_attr.get('status') != new_attr.get('status'):
+                    # ...
+                    new_instance.save(_signal=False)
+            TheModel.on_change(watch_status)
+
+        .. note::
+
+            Any call to instance.save() or instance.update() within a callback
+            will not trigger any change handlers.
+
+        """
+        _on_change_callbacks[cls].append(callback)
+
+    def _send_changes(self, old_attr, new_attr_kw):
+        new_attr = old_attr.copy()
+        new_attr.update(new_attr_kw)
+        for cb in _on_change_callbacks[self.__class__]:
+            cb(old_attr=old_attr, new_attr=new_attr,
+               instance=_NoChangeInstance(self), sender=self.__class__)
+
+    def save(self, *args, **kw):
+        """
+        Save changes to the model instance.
+
+        If _signal=False is in ``kw`` the on_change() callbacks won't be called.
+        """
+        signal = kw.pop('_signal', True)
+        result = super(OnChangeMixin, self).save(*args, **kw)
+        if signal:
+            self._send_changes(self._initial_attr, dict(self.__dict__))
+        return result
+
+    def update(self, **kw):
+        """
+        Shortcut for doing an UPDATE on this object.
+
+        If _signal=False is in ``kw`` the post_save signal won't be sent.
+        """
+        cls = self.__class__
+        signal = kw.pop('_signal', True)
+        old_attr = dict(self.__dict__)
+        result = cls.objects.filter(pk=self.pk).update(**kw)
+        if signal:
+            self._send_changes(old_attr, kw)
+        return result
+
+
 class ModelBase(caching.base.CachingMixin, models.Model):
     """
     Base class for AMO models to abstract some common features.

apps/amo/tests/test_models.py

@@ -1,7 +1,9 @@
 from test_utils import TestCase
 
+from mock import Mock
 from nose.tools import eq_
 
+import amo.models
 from amo.models import manual_order
 from amo import models as context
 from addons.models import Addon
@@ -39,3 +41,68 @@ def test_use_master():
             eq_(local.pinned, True)
         eq_(local.pinned, True)
     eq_(local.pinned, False)
+
+
+class TestModelBase(TestCase):
+    fixtures = ['base/addon_3615']
+
+    def setUp(self):
+        self.saved_cb = amo.models._on_change_callbacks.copy()
+        amo.models._on_change_callbacks.clear()
+        self.cb = Mock()
+        Addon.on_change(self.cb)
+
+    def tearDown(self):
+         amo.models._on_change_callbacks = self.saved_cb
+
+    def test_change_called_on_new_instance_save(self):
+        for create_addon in (Addon, Addon.objects.create):
+            addon = create_addon(site_specific=False, type=amo.ADDON_EXTENSION)
+            addon.site_specific = True
+            addon.save()
+            assert self.cb.called
+            kw = self.cb.call_args[1]
+            eq_(kw['old_attr']['site_specific'], False)
+            eq_(kw['new_attr']['site_specific'], True)
+            eq_(kw['instance'].id, addon.id)
+            eq_(kw['sender'], Addon)
+
+    def test_change_called_on_update(self):
+        addon = Addon.objects.get(pk=3615)
+        addon.update(site_specific=False)
+        assert self.cb.called
+        kw = self.cb.call_args[1]
+        eq_(kw['old_attr']['site_specific'], True)
+        eq_(kw['new_attr']['site_specific'], False)
+        eq_(kw['instance'].id, addon.id)
+        eq_(kw['sender'], Addon)
+
+    def test_change_called_on_save(self):
+        addon = Addon.objects.get(pk=3615)
+        addon.site_specific = False
+        addon.save()
+        assert self.cb.called
+        kw = self.cb.call_args[1]
+        eq_(kw['old_attr']['site_specific'], True)
+        eq_(kw['new_attr']['site_specific'], False)
+        eq_(kw['instance'].id, addon.id)
+        eq_(kw['sender'], Addon)
+
+    def test_change_is_not_recursive(self):
+
+        class fn:
+            called = False
+
+        def callback(old_attr=None, new_attr=None, instance=None,
+                     sender=None, **kw):
+            fn.called = True
+            # Both save and update should be protected:
+            instance.update(site_specific=False)
+            instance.save()
+
+        Addon.on_change(callback)
+
+        addon = Addon.objects.get(pk=3615)
+        addon.save()
+        assert fn.called
+        # No exception = pass