Implement decorator-based notifications for users

Refactors the keystone.notifications module to use decorators, and
applies those new decorators to user CRUD operations at the manager
layer.

blueprint notifications

Change-Id: Ic7937657196a0ad19923153a5135b1451a533e81

keystone/identity/core.py

@@ -26,6 +26,7 @@ from keystone.common import dependency
 from keystone.common import manager
 from keystone import config
 from keystone import exception
+from keystone import notifications
 from keystone.openstack.common import importutils
 from keystone.openstack.common import log as logging
 
@@ -260,6 +261,7 @@ class Manager(manager.Manager):
             ref = self._set_domain_id(ref, domain_id)
         return ref
 
+    @notifications.created('user')
     @domains_configured
     def create_user(self, user_id, user_ref):
         user = user_ref.copy()
@@ -301,6 +303,7 @@ class Manager(manager.Manager):
             user_list = self._set_domain_id(user_list, domain_id)
         return user_list
 
+    @notifications.updated('user')
     @domains_configured
     def update_user(self, user_id, user_ref, domain_scope=None):
         user = user_ref.copy()
@@ -317,6 +320,7 @@ class Manager(manager.Manager):
             ref = self._set_domain_id(ref, domain_id)
         return ref
 
+    @notifications.deleted('user')
     @domains_configured
     def delete_user(self, user_id, domain_scope=None):
         domain_id, driver = self._get_domain_id_and_driver(domain_scope)

keystone/notifications.py

@@ -23,62 +23,75 @@ from keystone.openstack.common.notifier import api as notifier_api
 LOG = log.getLogger(__name__)
 
 
-def notify_created(resource_id, resource_type, host=None):
+class ManagerNotificationWrapper(object):
-    """Send resource creation notification.
+    """Send event notifications for ``Manager`` methods.
 
-    :param resource_id: ID of the resource being created
+    Sends a notification if the wrapped Manager method does not raise an
-    :param resource_type: type of resource being created
+    ``Exception`` (such as ``keystone.exception.NotFound``).
-    :param host: host of the resource
+
+    :param resource_type: type of resource being affected
+    :param host: host of the resource (optional)
     """
+    def __init__(self, operation, resource_type, host=None):
+        self.operation = operation
+        self.resource_type = resource_type
+        self.host = host
 
-    _send_notification(resource_id, resource_type, 'created', host=host)
+    def __call__(self, f):
+        def wrapper(*args, **kwargs):
+            """Send a notification if the wrapped callable is successful."""
+            try:
+                result = f(*args, **kwargs)
+            except Exception:
+                raise
+            else:
+                _send_notification(
+                    self.operation,
+                    self.resource_type,
+                    args[1],  # f(self, resource_id, ...)
+                    self.host)
+            return result
+
+        return wrapper
 
 
-def notify_updated(resource_id, resource_type, host=None):
+def created(*args, **kwargs):
-    """Send resource update notification.
+    """Decorator to send notifications for ``Manager.create_*`` methods."""
-
+    return ManagerNotificationWrapper('created', *args, **kwargs)
-    :param resource_id: ID of the resource being updated
-    :param resource_type: type of resource being updated
-    :param host: host of the resource
-    """
-
-    _send_notification(resource_id, resource_type, 'updated', host=host)
 
 
-def notify_deleted(resource_id, resource_type, host=None):
+def updated(*args, **kwargs):
-    """Send resource deletion notification.
+    """Decorator to send notifications for ``Manager.update_*`` methods."""
-
+    return ManagerNotificationWrapper('updated', *args, **kwargs)
-    :param resource_id: ID of the resource being deleted
-    :param resource_type: type of resource being deleted
-    :param host: host of the resource
-    """
-
-    _send_notification(resource_id, resource_type, 'deleted', host=host)
 
 
-def _send_notification(resource_id, resource_type, operation, host=None):
+def deleted(*args, **kwargs):
-    """Send resource update notification to inform observers about resource
+    """Decorator to send notifications for ``Manager.delete_*`` methods."""
-       changes. This method doesn't raise an exception when sending the
+    return ManagerNotificationWrapper('deleted', *args, **kwargs)
-       notification fails.
 
-    :param resource_id: ID of resource in notification
+
-    :param resource_type: type of resource being created, updated,
+def _send_notification(operation, resource_type, resource_id, host=None):
-                       or deleted
+    """Send notification to inform observers about the affected resource.
-    :param operation: operation being performed (created, updated,
+
-                       or deleted)
+    This method doesn't raise an exception when sending the notification fails.
+
+    :param operation: operation being performed (created, updated, or deleted)
+    :param resource_type: type of resource being operated on
+    :param resource_id: ID of resource being operated on
     :param host: resource host
     """
     context = {}
     payload = {'resource_info': resource_id}
     service = 'identity'
     publisher_id = notifier_api.publisher_id(service, host=host)
-    event_type = ('%(service)s.%(resource_type)s.%(operation)s' %
+    event_type = '%(service)s.%(resource_type)s.%(operation)s' % {
-                  {'service': service, 'resource_type': resource_type,
+        'service': service,
-                   'operation': operation})
+        'resource_type': resource_type,
+        'operation': operation}
 
     try:
-        notifier_api.notify(context, publisher_id, event_type,
+        notifier_api.notify(
-                            notifier_api.INFO, payload)
+            context, publisher_id, event_type, notifier_api.INFO, payload)
     except Exception:
         msg = (_('Failed to send %(res_id)s %(event_type)s notification') %
                {'res_id': resource_id, 'event_type': event_type})

keystone/tests/test_notifications.py

@@ -14,86 +14,116 @@
 #   License for the specific language governing permissions and limitations
 #   under the License.
 
+import uuid
+
 from keystone import notifications
 from keystone.openstack.common.notifier import api as notifier_api
 from keystone.tests import core
 
 
+EXP_RESOURCE_TYPE = uuid.uuid4().hex
+
+
+class ArbitraryException(Exception):
+    pass
+
+
+class NotificationsWrapperTestCase(core.TestCase):
+    def setUp(self):
+        super(NotificationsWrapperTestCase, self).setUp()
+
+        self.exp_resource_id = None
+        self.exp_operation = None
+        self.exp_host = None
+        self.send_notification_called = False
+
+        def fake_notify(operation, resource_type, resource_id, host=None):
+            self.assertEqual(self.exp_operation, operation)
+            self.assertEqual(EXP_RESOURCE_TYPE, resource_type)
+            self.assertEqual(self.exp_resource_id, resource_id)
+            self.assertEqual(self.exp_host, host)
+            self.send_notification_called = True
+
+        self.stubs.Set(notifications, '_send_notification', fake_notify)
+
+    @notifications.created(EXP_RESOURCE_TYPE)
+    def create_resource(self, resource_id, data):
+        return data
+
+    def test_resource_created_notification(self):
+        self.exp_operation = 'created'
+        self.exp_resource_id = uuid.uuid4().hex
+        exp_resource_data = {
+            'id': self.exp_resource_id,
+            'key': uuid.uuid4().hex}
+        self.exp_host = None
+
+        self.create_resource(self.exp_resource_id, exp_resource_data)
+        self.assertTrue(self.send_notification_called)
+
+    @notifications.updated(EXP_RESOURCE_TYPE)
+    def update_resource(self, resource_id, data):
+        return data
+
+    def test_resource_updated_notification(self):
+        self.exp_operation = 'updated'
+        self.exp_resource_id = uuid.uuid4().hex
+        exp_resource_data = {
+            'id': self.exp_resource_id,
+            'key': uuid.uuid4().hex}
+        self.exp_host = None
+
+        self.update_resource(self.exp_resource_id, exp_resource_data)
+        self.assertTrue(self.send_notification_called)
+
+    @notifications.deleted(EXP_RESOURCE_TYPE)
+    def delete_resource(self, resource_id):
+        pass
+
+    def test_resource_deleted_notification(self):
+        self.exp_operation = 'deleted'
+        self.exp_resource_id = uuid.uuid4().hex
+        self.exp_host = None
+
+        self.delete_resource(self.exp_resource_id)
+        self.assertTrue(self.send_notification_called)
+
+    @notifications.created(EXP_RESOURCE_TYPE)
+    def create_exception(self, resource_id):
+        raise ArbitraryException()
+
+    def test_create_exception_without_notification(self):
+        self.assertRaises(
+            ArbitraryException, self.create_exception, uuid.uuid4().hex)
+        self.assertFalse(self.send_notification_called)
+
+    @notifications.created(EXP_RESOURCE_TYPE)
+    def update_exception(self, resource_id):
+        raise ArbitraryException()
+
+    def test_update_exception_without_notification(self):
+        self.assertRaises(
+            ArbitraryException, self.update_exception, uuid.uuid4().hex)
+        self.assertFalse(self.send_notification_called)
+
+    @notifications.deleted(EXP_RESOURCE_TYPE)
+    def delete_exception(self, resource_id):
+        raise ArbitraryException()
+
+    def test_delete_exception_without_notification(self):
+        self.assertRaises(
+            ArbitraryException, self.delete_exception, uuid.uuid4().hex)
+        self.assertFalse(self.send_notification_called)
+
+
 class NotificationsTestCase(core.TestCase):
-
-    def test_send_notification_project_created(self):
-        """Test to ensure resource_type is 'project' and operation is
-           'created'.
-        """
-
-        resource_id = 'created_resource_id'
-        self.send_notification_called = False
-
-        def fake_send_notification(resource, resource_type, operation,
-                                   host=None):
-            exp_resource_type = 'project'
-            exp_operation = 'created'
-            self.assertEqual(exp_resource_type, resource_type)
-            self.assertEqual(exp_operation, operation)
-            self.assertEqual(resource_id, resource)
-            self.send_notification_called = True
-
-        self.stubs.Set(notifications, '_send_notification',
-                       fake_send_notification)
-        notifications.notify_created(resource_id, 'project')
-        self.assertTrue(self.send_notification_called)
-
-    def test_send_notification_project_updated(self):
-        """Test to ensure resource_type is 'project' and operation is
-           'updated'.
-        """
-
-        resource_id = 'updated_resource_id'
-        self.send_notification_called = False
-
-        def fake_send_notification(resource, resource_type, operation,
-                                   host=None):
-            exp_resource_type = 'project'
-            exp_operation = 'updated'
-            self.assertEqual(exp_resource_type, resource_type)
-            self.assertEqual(exp_operation, operation)
-            self.assertEqual(resource_id, resource)
-            self.send_notification_called = True
-
-        self.stubs.Set(notifications, '_send_notification',
-                       fake_send_notification)
-        notifications.notify_updated(resource_id, 'project')
-        self.assertTrue(self.send_notification_called)
-
-    def test_send_notification_project_deleted(self):
-        """Test to ensure resource_type is 'project' and operation is
-           'deleted'.
-        """
-
-        resource_id = 'deleted_resource_id'
-        self.send_notification_called = False
-
-        def fake_send_notification(resource, resource_type, operation,
-                                   host=None):
-            exp_resource_type = 'project'
-            exp_operation = 'deleted'
-            self.assertEqual(exp_resource_type, resource_type)
-            self.assertEqual(exp_operation, operation)
-            self.assertEqual(resource_id, resource)
-            self.send_notification_called = True
-
-        self.stubs.Set(notifications, '_send_notification',
-                       fake_send_notification)
-        notifications.notify_deleted(resource_id, 'project')
-        self.assertTrue(self.send_notification_called)
-
     def test_send_notification(self):
         """Test the private method _send_notification to ensure event_type,
            payload, and context are built and passed properly.
         """
 
-        resource = 'some_resource_id'
+        resource = uuid.uuid4().hex
-        resource_type = 'project'
+        resource_type = EXP_RESOURCE_TYPE
         operation = 'created'
         host = None