Merge lp:~wallyworld/lazr.restful/propogate-notifications into lp:lazr.restful

[Ian Booth]

When constructing the data object from an EntryResource to return to the client, include any informational, warning, error messages the server side call may have attached to the request.response object.

When constructing the data object from an EntryResource to return to the client, include any informational, warning, error messages the server side call may have attached to the request.response object.

[lp:~wallyworld/lazr.restful/propogate-notifications]

186. By Ian Booth on 2011-03-23

Set notifications to empty list rather than undefined when there are none

[Leonard Richardson (leonardr)]

I have a couple concerns about this:

1. You're assuming that request.response.notifications exists, even though IHTTPRequest doesn't define a 'response' attribute and IHTTPResponse doesn't define a 'notifications' attribute. We already make the assumption that request.response exists, all over the place, and I'm pretty sure that's a Zope convention. So I'm not worried about that.

But where does 'notifications' come from? That's defined by INotificationsResponse in Launchpad. We can't put Launchpad-specific code in lazr.restful, because it's used by other projects. You have a couple options:

1A: You can move INotificationsResponse into lazr.restful, and only handle the notifications if the response can be adapted to INotificationsResponse. You can then write a stub implementation and test the notifications code in lazr.restful. Users who have no need for this can just choose not to register an adapter from IHTTPResponse to INotificationsResponse.

1B. You can move this code into Launchpad, and make the handling of notifications a Launchpad-specific enhancement to the lazr.restful web service. I recommend 1A instead.

2. These notifications aren't part of the description of the object, so they don't belong in its JSON representation. In the motivating case, bug 698138, the notification is "foo did not previously have any assigned bugs in barproject". That has nothing to do with the bug that was assigned. It's a notification about the *act* of setting that bug's assignee.

Just to drive this home a little more: what if a named operation has no return value, but the process of handling the request results in a notification? What if a DELETE request spawns a notification? You've got no place to set that notification. What if a named operation returns a *list* of entries and also sets some notifications? Your branch is going to give every entry a copy of the same notifications.

These notifications should be set at the _request_ level. I suggest picking an HTTP header name like X-Lazr-Notification and setting a value for that header for each notification. (On the client side, XHR should be able to handle multiple values for the same header, but I'm not sure.)

If you make this a feature of lazr.restful, you should also change lazr.restfulclient to be able to access these notifications. If you make this a feature of the Launchpad web service, you should change launchpadlib. (I'm not sure how to do this since lazr.restfulclient abstracts away the request/response cycle. Maybe notifications should just be stored in an instance-wide queue.)

[Leonard Richardson (leonardr)]

review: Disapprove

[lp:~wallyworld/lazr.restful/propogate-notifications]

187. By Ian Booth on 2011-03-24

Merge from trunk

188. By Ian Booth on 2011-03-24

Add notifications support using adapter

Unmerged revisions

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2011-03-23 16:30:43 +0000
+++ src/lazr/restful/NEWS.txt	2011-03-24 12:11:52 +0000
@@ -2,6 +2,15 @@
 NEWS for lazr.restful
 =====================
 
+0.18.1 (2011-03-24)
+===================
+
+The webservice may define an adapter which is used, after an operation on a
+read-write resource, to provide notifications consisting of (level, message)
+tuples. Any notifications are inserted into the response header using the
+'XHR-Notifications' key. They may then be used by the caller to provide extra
+information to the user about the completed request.
+
 0.18.0 (2011-03-23)
 ===================
 

=== modified file 'src/lazr/restful/_resource.py'
--- src/lazr/restful/_resource.py	2011-03-15 17:13:48 +0000
+++ src/lazr/restful/_resource.py	2011-03-24 12:11:52 +0000
@@ -100,6 +100,7 @@
     IFieldMarshaller,
     IHTTPResource,
     IJSONPublishable,
+    INotificationsProvider,
     IReference,
     IReferenceChoice,
     IRepresentationCache,
@@ -942,6 +943,19 @@
                 # status codes using exceptions). Let the publisher
                 # handle it.
                 raise e
+
+        # If the webservice has defined an INotificationsProvider adaptor, use
+        # it to include with the response the relevant notification messages
+        # and their severity levels.
+        notifications_provider = INotificationsProvider(self.request, None)
+        notifications = []
+        if (notifications_provider is not None
+            and notifications_provider.notifications):
+            notifications = ([(notification.level, notification.message)
+                 for notification in notifications_provider.notifications])
+        json_notifications =simplejson.dumps(notifications)
+        self.request.response.setHeader(
+            'XHR-Notifications', json_notifications)
         return result
 
 

=== modified file 'src/lazr/restful/example/base/configure.zcml'
--- src/lazr/restful/example/base/configure.zcml	2011-02-02 14:15:24 +0000
+++ src/lazr/restful/example/base/configure.zcml	2011-03-24 12:11:52 +0000
@@ -21,6 +21,13 @@
       provides="zope.traversing.browser.interfaces.IAbsoluteURL"
       factory="lazr.restful.testing.webservice.DummyAbsoluteURL" />
 
+  <!--Registering this adaptor will allow server side notifications to be
+      sent to the caller.-->
+    <adapter
+        for="lazr.restful.interfaces.IWebServiceClientRequest"
+        provides="lazr.restful.interfaces.INotificationsProvider"
+        factory="lazr.restful.example.base.traversal.web_service_request_to_notification_request"
+        />
 
   <!--Security configuration-->
 

=== modified file 'src/lazr/restful/example/base/traversal.py'
--- src/lazr/restful/example/base/traversal.py	2011-02-02 14:15:24 +0000
+++ src/lazr/restful/example/base/traversal.py	2011-03-24 12:11:52 +0000
@@ -25,6 +25,7 @@
 from lazr.restful.simple import (
     RootResourceAbsoluteURL,
     SimulatedWebsiteRequest,
+    SimulatedWebsiteRequestWithNotifications,
     )
 from lazr.restful import RedirectResource
 
@@ -36,6 +37,13 @@
     return SimulatedWebsiteRequest(body, environ)
 
 
+def web_service_request_to_notification_request(web_service_request):
+    """Create a simulated website request for providing notifications."""
+    body = web_service_request.bodyStream.getCacheStream()
+    environ = dict(web_service_request.environment)
+    return SimulatedWebsiteRequestWithNotifications(body, environ)
+
+
 class RootAbsoluteURL(RootResourceAbsoluteURL):
     """A technique for generating the service's root URL.
 

=== modified file 'src/lazr/restful/interfaces/_rest.py'
--- src/lazr/restful/interfaces/_rest.py	2011-03-15 14:13:10 +0000
+++ src/lazr/restful/interfaces/_rest.py	2011-03-24 12:11:52 +0000
@@ -32,6 +32,7 @@
     'IFieldMarshaller',
     'IFileLibrarian',
     'IHTTPResource',
+    'INotificationsProvider',
     'IJSONPublishable',
     'IJSONRequestCache',
     'IRepresentationCache',
@@ -276,6 +277,21 @@
     """
 
 
+class INotificationsProvider(Interface):
+    """A response object which contains notifications.
+
+    A web framework may define an adapter for this interface, allowing
+    the web service to provide an implementation having a 'notifications'
+    property, providing a list of (level, message) tuples which can be
+    used by the caller to display extra information about the completed
+    request. 'level' matches the standard logging levels:
+    DEBUG = logging.DEBUG     # A debugging message
+    INFO = logging.INFO       # simple confirmation of a change
+    WARNING = logging.WARNING # action will not be successful unless you ...
+    ERROR = logging.ERROR     # the previous action did not succeed, and why
+    """
+
+
 class IWebServiceClientRequest(IBrowserRequest):
     """Interface for requests to the web service."""
     version = Attribute("The version of the web service that the client "

=== modified file 'src/lazr/restful/simple.py'
--- src/lazr/restful/simple.py	2011-02-02 14:15:24 +0000
+++ src/lazr/restful/simple.py	2011-03-24 12:11:52 +0000
@@ -13,9 +13,13 @@
     'RootResource',
     'RootResourceAbsoluteURL',
     'SimulatedWebsiteRequest',
+    'SimulatedWebsiteRequestWithNotifications',
     'TraverseWithGet',
     ]
 
+import cgi
+import collections
+import logging
 import traceback
 import urllib
 
@@ -37,6 +41,7 @@
 from lazr.restful import (
     EntryAdapterUtility, HTTPResource, ServiceRootResource)
 from lazr.restful.interfaces import (
+    INotificationsProvider,
     IRepresentationCache,
     IServiceRootResource,
     ITopLevelEntryLink,
@@ -189,7 +194,31 @@
     other class that implements IWebBrowserOriginatingRequest), you
     can take advantage of the web_url feature.
     """
-    implements(IWebBrowserOriginatingRequest)
+    implements(IWebBrowserOriginatingRequest, INotificationsProvider)
+
+
+Notification = collections.namedtuple(
+    'Notification', ['level', 'message'])
+
+
+class SimulatedWebsiteRequestWithNotifications(SimulatedWebsiteRequest):
+    """A (simulated) request to a website as opposed to a web service.
+
+    If you can adapt a web service request to this class (or some
+    other class that implements INotificationsProvider), you
+    can send notifications to the caller.
+    """
+    implements(INotificationsProvider)
+
+    _notifications = []
+
+    @property
+    def notifications(self):
+        return self._notifications
+
+    def addNotification(self, msg, level=logging.INFO):
+        self.notifications.append(
+            Notification(level, cgi.escape(msg)))
 
 
 class RootResource(ServiceRootResource, TraverseWithGet):

=== modified file 'src/lazr/restful/tests/test_webservice.py'
--- src/lazr/restful/tests/test_webservice.py	2011-03-07 15:16:57 +0000
+++ src/lazr/restful/tests/test_webservice.py	2011-03-24 12:11:52 +0000
@@ -9,6 +9,7 @@
 from lxml import etree
 from operator import attrgetter
 from textwrap import dedent
+import logging
 import random
 import re
 import simplejson
@@ -39,6 +40,7 @@
     ICollection,
     IEntry,
     IFieldHTMLRenderer,
+    INotificationsProvider,
     IResourceGETOperation,
     IServiceRootResource,
     IWebBrowserOriginatingRequest,
@@ -908,3 +910,55 @@
         return ResourceOperation(None, request)
 
 
+class NotificationsProviderTest(EntryTestCase):
+    """Test that notifcations are included in the response headers."""
+
+    testmodule_objects = [HasTwoFields, IHasTwoFields]
+
+    class DummyWebsiteRequestWithNotifications:
+        """A request to the website, as opposed to the web service."""
+        implements(INotificationsProvider)
+
+        @property
+        def notifications(self):
+            return [(logging.INFO, "Informational"),
+                    (logging.WARNING, "Warning")
+                    ]
+
+    def setUp(self):
+        super(NotificationsProviderTest, self).setUp()
+        self.default_media_type = "application/json;include=lp_html"
+        self._register_url_adapter(IHasTwoFields)
+        self._register_notification_adapter()
+
+    def _register_notification_adapter(self):
+        """Simulates a service where an entry corresponds to a web page."""
+
+        # First, create a converter from web service requests to
+        # web service requests with notifications.
+        def web_service_request_to_notification_request(service_request):
+            """Create a corresponding request to the website."""
+            return self.DummyWebsiteRequestWithNotifications()
+
+        getGlobalSiteManager().registerAdapter(
+            web_service_request_to_notification_request,
+            [IWebServiceClientRequest], INotificationsProvider)
+
+    @contextmanager
+    def resource(self, value_1="value 1", value_2="value 2"):
+        """Simplify the entry_resource call."""
+        with self.entry_resource(
+            IHasTwoFields, HasTwoFields,
+            unicode(value_1), unicode(value_2)) as resource:
+            yield resource
+
+    def test_response_notifications(self):
+        with self.resource() as resource:
+            resource.do_GET()
+            notifications = simplejson.loads(
+                resource.request.response.getHeader("XHR-Notifications"))
+            expected_notifications = [
+                (logging.INFO, "Informational"),
+                (logging.WARNING, "Warning")
+                ]
+            self.assertEquals(notifications, expected_notifications)

=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt	2011-03-21 14:00:50 +0000
+++ src/lazr/restful/version.txt	2011-03-24 12:11:52 +0000
@@ -1,1 +1,1 @@
-0.18.0
+0.18.1