On Aug 11, 2009, at 06:15 PM, Leonard Richardson wrote:

>Leonard Richardson has proposed merging lp:~leonardr/lazr.restful/simple-root into lp:lazr.restful.
>
>Requested reviews:
>    LAZR Developers (lazr-developers)
>
>The default ServiceRootResource has a very complicated implementation of getTopLevelPublications(). It goes through all the registered adapters and sees which ones are adapters for utilities, and which ones are adapters for ITopLevelEntryLink. The most obvious effect of this is that you have to register all your collections as Zope utilities.
>
>This is fine for Launchpad, because Launchpad's collections are already registered as Zope utilities. But other web services don't do this. To take an extreme example, the WSGI example web service defines its entire object model within the root resource. So why not 
>
>I created a class called SimpleRootResource which asks its subclasses for two data structures: one for top-level collections and one for top-level entry links. It uses this data structure to drive both getTopLevelPublications() and the get() method used by TraverseWithGet. The data structure is a little bit complicated, but a SimpleRootResource subclass is significantly less complicated than the corresponding ServiceRootResource. The WSGI sample application now has much less code and a little bit less ZCML.
>
>To get this to work without circular references, I had to move some classes around. I ended up moving all the "simple implementation of complex zope/lazr.restful feature x" classes into lazr.restful.simple. It now contains SimplePublicationMixin, SimplePublication, TraverseWithGet, SimpleRequest, and SimpleWebServiceRootResource. Most of the code in this branch is this new file + changes to the import statements in other files.

Hi Leonard,

It's great to see the work you're doing on simplifying lazr.restful for simple
applications.  The more I use lazr.restful the more I like it, and I think
it's really going to help drive up adoption by making it easier to use.  I'm
going to be attending my local Python users group meeting and overwhelmingly,
people want me to do a presentation on lazr.restful.

I've deleted the chunks that look fine to me.  All my other comments are
fairly trivial.  Great branch, r=me.

 review approve
 status approve


=== modified file 'src/lazr/restful/example/wsgi/README.txt'
--- src/lazr/restful/example/wsgi/README.txt	2009-08-03 13:12:20 +0000
+++ src/lazr/restful/example/wsgi/README.txt	2009-08-11 17:47:07 +0000
> @@ -109,12 +109,11 @@
>  Utilities
>  =========
>  
> -The service root, the top-level collection of key-value pairs, and the
> -the web service configuration object are all registered as Zope
> -utilities. A Zope utility is basically a singleton. These three
> -classes are registered as singletons because from time to time,
> -lazr.restful needs access to a canonical instance of one of these
> -classes.
> +The service root and the the web service configuration object are both

s/the the/the/

> +registered as Zope utilities. A Zope utility is basically a
> +singleton. These two classes are registered as singletons because
> +from time to time, lazr.restful needs access to a canonical instance
> +of one of these classes.
>  
>  Utilities are registered in lazr/restful/example/wsgi/configure.zcml,
>  and each one is associated with a Zope interface. So the root resource
> @@ -123,10 +122,6 @@
>  that passes an interface class into getUtility(), that code is getting
>  the singleton utility registered for that interface.
>  
> -All top-level objects published by a lazr.restful web service must be
> -registered as utilities. This is why the set of key-value pairs is
> -registered as a utility for IPairSet.
> -
>  Traversal
>  =========
>  

=== modified file 'src/lazr/restful/example/wsgi/root.py'
--- src/lazr/restful/example/wsgi/root.py	2009-07-21 15:21:35 +0000
+++ src/lazr/restful/example/wsgi/root.py	2009-08-11 16:40:43 +0000
> @@ -9,41 +9,24 @@
>  
>  from zope.component import adapts, getUtility
>  from zope.interface import implements
> -from zope.publisher.interfaces.browser import IDefaultBrowserLayer
>  from zope.traversing.browser.interfaces import IAbsoluteURL
>  
> -from lazr.restful import ServiceRootResource
> -from lazr.restful.example.wsgi.resources import IPairSet, KeyValuePair
> -from lazr.restful.interfaces import (
> -    ICollection, IServiceRootResource, IWebServiceConfiguration)
> -from lazr.restful.publisher import TraverseWithGet, WebServiceRequestTraversal
> -
> -
> -class WSGIExampleWebServiceRootResource(ServiceRootResource, TraverseWithGet):
> +from lazr.restful.example.wsgi.resources import (
> +    IKeyValuePair, PairSet, KeyValuePair)
> +from lazr.restful.interfaces import IWebServiceConfiguration
> +from lazr.restful.simple import SimpleServiceRootResource
> +from lazr.restful.publisher import WebServiceRequestTraversal
> +
> +
> +class WSGIExampleWebServiceRootResource(SimpleServiceRootResource):
>      """The root resource for the WSGI example web service."""
> -    implements (IServiceRootResource)
> -
> -    @property
> -    def top_level_names(self):
> -        return self.top_level_objects.keys()
> -
> -    def get(self, name):
> -        """Traverse to a top-level object."""
> -        return self.top_level_objects.get(name)
> -
> -    @property
> -    def top_level_objects(self):
> -        """Access or create the list of top-level objects."""
> -
> -        if getattr(self, '_top_level_objects', None) is None:
> -            pairs = [KeyValuePair(self, "foo", "bar"),
> -                     KeyValuePair(self, "1", "2")]
> -            pairset = getUtility(IPairSet)
> -            pairset.pairs = pairs
> -            self._top_level_objects = {
> -                'pairs': pairset,
> -            }
> -        return self._top_level_objects
> +
> +    def _build_top_level_objects(self):
> +        pairset = PairSet()
> +        pairset.pairs = [KeyValuePair(self, "foo", "bar"),
> +                         KeyValuePair(self, "1", "2")]
> +        collections = {'pairs': (IKeyValuePair, pairset)}
> +        return collections, {}

How about coding it like this?

    pairset.pairs = [
        KeyValuePair(self, 'foo', 'bar'),
        KeyValuePair(self, '1', '2'),
        ]
    collections = dict(pairs=(IKeyValuePair, pairset))

>  
>  
>  class WSGIExampleWebServiceRootAbsoluteURL:

=== added file 'src/lazr/restful/simple.py'
--- src/lazr/restful/simple.py	1970-01-01 00:00:00 +0000
+++ src/lazr/restful/simple.py	2009-08-11 17:55:34 +0000
> @@ -0,0 +1,226 @@
> +"""Simple implementations of various Zope and lazr.restful interfaces."""
> +
> +__metaclass__ = type
> +__all__ = [
> +    'SimplePublicationMixin',
> +    'SimplePublication',
> +    'TraverseWithGet',
> +    'SimpleRequest',
> +    'SimpleWebServiceRootResource'

Alphabetize; and please end the last line with a trailing comma.

> +    ]
> +
> +import urllib
> +
> +from zope.component import queryMultiAdapter
> +from zope.interface import implements
> +from zope.publisher.browser import BrowserRequest
> +from zope.publisher.interfaces import IPublication, IPublishTraverse, NotFound
> +from zope.publisher.publish import mapply
> +from zope.security.management import endInteraction, newInteraction
> +
> +from lazr.restful import EntryAdapterUtility, ServiceRootResource
> +from lazr.restful.interfaces import ITraverseWithGet, IWebServiceLayer
> +from lazr.restful.publisher import (
> +    WebServicePublicationMixin, WebServiceRequestTraversal)
> +
> +class SimplePublicationMixin(object):
> +    """A very simple implementation of `IPublication`.
> +
> +    The object passed to the constructor is returned by getApplication().
> +    """
> +    implements(IPublication)
> +
> +    def __init__(self, application):
> +        """Create the test publication.
> +
> +        The object at which traversal should start is passed as parameter.
> +        """
> +        self.application = application
> +
> +    def beforeTraversal(self, request):
> +        """Sets the request as the current interaction.
> +
> +        (It also ends any previous interaction, that's convenient when
> +        tests don't go through the whole request.)
> +        """
> +        endInteraction()
> +        newInteraction(request)
> +
> +    def getApplication(self, request):
> +        """Returns the application passed to the constructor."""
> +        return self.application
> +
> +    def callTraversalHooks(self, request, ob):
> +        """Does nothing."""
> +
> +    def traverseName(self, request, ob, name):
> +        """Traverse by looking for an `IPublishTraverse` adapter."""
> +        # XXX flacoste 2009/03/06 bug=338831. This is copied from
> +        # zope.app.publication.publicationtraverse.PublicationTraverse.
> +        # This should really live in zope.publisher, we are copying because
> +        # we don't want to depend on zope.app stuff.
> +        # Namespace support was dropped.
> +        if name == '.':
> +            return ob
> +
> +        if IPublishTraverse.providedBy(ob):
> +            ob2 = ob.publishTraverse(request, name)
> +        else:
> +            # self is marker.
> +            adapter = queryMultiAdapter(
> +                (ob, request), IPublishTraverse, default=self)
> +            if adapter is not self:
> +                ob2 = adapter.publishTraverse(request, name)
> +            else:
> +                raise NotFound(ob, name, request)
> +
> +        return self.wrapTraversedObject(ob2)
> +
> +    def wrapTraversedObject(self, ob):
> +        """Wrap the traversed object, for instance in a security proxy.
> +
> +        By default, does nothing."""
> +        return ob
> +
> +    def afterTraversal(self, request, ob):
> +        """Does nothing."""
> +
> +    def callObject(self, request, ob):
> +        """Call the object, returning the result."""
> +        return mapply(ob, request.getPositionalArguments(), request)
> +
> +    def afterCall(self, request, ob):
> +        """Does nothing."""
> +
> +    def handleException(self, object, request, exc_info, retry_allowed=1):
> +        """Prints the exception."""
> +        # Reproduce the behavior of ZopePublication by looking up a view
> +        # for this exception.
> +        exception = exc_info[1]
> +        view = queryMultiAdapter((exception, request), name='index.html')
> +        if view is not None:
> +            exc_info = None

I was looking at my code for this and I had to scratch my head at it.  I
actually don't think this line is needed since exc_info is never used in this
branch again.

> +            request.response.reset()
> +            request.response.setResult(view())
> +        else:
> +            traceback.print_exception(*exc_info)
> +
> +    def endRequest(self, request, ob):
> +        """Ends the interaction."""
> +        endInteraction()
> +
> +
> +class SimplePublication(WebServicePublicationMixin, SimplePublicationMixin):
> +    """A simple publication.
> +
> +    Combines the IPublication implementation of SimplePublicationMixin
> +    with the web service implementation of WebServicePublicationMixin,
> +    """
> +    pass
> +
> +
> +class TraverseWithGet(object):
> +    """An implementation of `IPublishTraverse` that uses the get() method.
> +
> +    This is a simple traversal technique that works with any object
> +    that defines a lookup method called get().
> +
> +    This class should not be confused with
> +    WebServiceRequestTraversal. This class (or any other class that
> +    implements IPublishTraverse) controls traversal in the web
> +    application towards an object that implements IEntry. Once an
> +    IEntry has been found, further traversal (eg. to scoped
> +    collections or fields) always happens with
> +    WebServiceRequestTraversal.
> +    """
> +    implements(ITraverseWithGet)
> +
> +    def publishTraverse(self, request, name):
> +        """See `IPublishTraverse`."""
> +        name = urllib.unquote(name)
> +        value = self.get(name)
> +        if value is None:
> +            raise NotFound(self, name)
> +        return value
> +
> +    def get(self, name):
> +        """See `ITraverseWithGet`."""
> +        raise NotImplementedError
> +
> +
> +class SimpleRequest(WebServiceRequestTraversal, BrowserRequest):
> +    """A web service request with no special features."""
> +    implements(IWebServiceLayer)
> +
> +
> +class SimpleServiceRootResource(ServiceRootResource, TraverseWithGet):
> +    """A service root that expects top-level objects to be defined in code.
> +
> +    ServiceRootResource expects top-level objects to be registered as
> +    Zope interfaces.
> +    """
> +
> +    @property
> +    def top_level_names(self):
> +        return self.top_level_objects.keys()
> +
> +    def get(self, name):
> +        """Traverse to a top-level object."""
> +        return self.top_level_objects.get(name)
> +
> +    def getTopLevelPublications(self):
> +        """Return a mapping of top-level link names to published objects."""
> +        top_level_resources = {}
> +        # First collect the top-level collections.
> +        for name, (schema_interface, obj) in (
> +            self.top_level_collections.items()):
> +            adapter = EntryAdapterUtility.forSchemaInterface(schema_interface)
> +            link_name = ("%s_collection_link" % adapter.plural_type)
> +            top_level_resources[link_name] = obj
> +        # Then collect the top-level entries.
> +        for name, entry_link in self.top_level_entry_links.items():
> +            link_name = ("%s_link" % ITopLevelEntryLink(entry_link).link_name)
> +            top_level_resources[link_name] = entry_link
> +        return top_level_resources
> +
> +    @property
> +    def top_level_objects(self):
> +        """Return this web service's top-level objects."""
> +        objects = {}
> +        for name, (schema_interface, obj) in (
> +            self.top_level_collections.items()):
> +            objects[name] = obj
> +        objects.update(self.top_level_entry_links)
> +        return objects
> +
> +    @property
> +    def top_level_collections(self):
> +        """Return this web service's top-level collections.
> +
> +        :return: A hash mapping a name to a 2-tuple (interface, collection).
> +        The interface is the kind of thing contained in the collection.
> +        """
> +        if not hasattr(self, '_top_level_collections'):
> +            self._top_level_collections, self._top_level_entry_links = (
> +                self._build_top_level_objects())
> +        return self._top_level_collections
> +
> +    @property
> +    def top_level_entry_links(self):
> +        """Return this web service's top-level entry links."""
> +        if not hasattr(self, '_top_level_entry_links'):
> +            self._top_level_collections, self._top_level_entry_links = (
> +                self._build_top_level_objects())
> +        return self._top_level_entry_links
> +
> +    def _build_top_level_objects(self):
> +        """Create the list of top-level objects.
> +
> +        :return: A 2-tuple of hashes (collections, entry_links). The
> +        'collections' hash maps a name to a 2-tuple (interface, object).
> +        The interface is the kind of thing contained in the collection.
> +        For instance, 'users': (IUser, UserSet())
> +
> +        The 'entry_links' hash maps a name to an object.
> +        """
> +        return ({}, {})