> > === modified file 'src/lazr/restful/_resource.py'
> >  class EntryFieldURL(AbsoluteURL):
> >      """An IAbsoluteURL adapter for EntryField objects."""
> >      component.adapts(EntryField, IHTTPRequest)
> > @@ -1243,7 +1246,7 @@
> >      def __init__(self, context, request):
> >          """Associate this resource with a specific object and request."""
> >          super(EntryResource, self).__init__(context, request)
> > -        self.entry = IEntry(context)
> > +        self.entry = getMultiAdapter((context, request), IEntry)
> >
> 
> Not that if context is already providing IEntry, you'll get an error. I don't
> know if this is a plausible case, but if it is, you'll want to only query the
> adapter, if context doesn't provide IEntry.

I could go either way on this, but I kind of consider it an error to pass an Entry object directly into EntryResource. I think allowing that would encourage sloppy thinking and cause lazr.restful developers to confuse entries and data model objects. What do you think?
 
> > +        # Create and register marker interfaces for request objects.
> > +        for version in set(
> > +            utility.active_versions + [utility.latest_version_uri_prefix]):
> > +            classname = ("IWebServiceClientRequestVersion" +
> > +                         version.replace('.', '_'))
> 
> There are probably other characters that need to be squashed '-' is likely to
> be used.

I was lazy. I've added a make_identifier_safe function to lazr.restful.utils and a doctest for it.
 
> > === modified file 'src/lazr/restful/docs/multiversion.txt'
> > +URL generation
> > +--------------
> > +
> > +    >>> from zope.component import getSiteManager
> > +    >>> from zope.traversing.browser.interfaces import IAbsoluteURL
> > +    >>> from lazr.restful.interfaces import (
> > +    ...     IRequestAwareLocation, IWebServiceLayer)
> > +    >>> from lazr.restful.simple import AbsoluteURL
> > +    >>> sm = getSiteManager()
> > +    >>> sm.registerAdapter(
> > +    ...     AbsoluteURL, (IRequestAwareLocation, IWebServiceLayer),
> > +    ...     provided=IAbsoluteURL)
> 
> Can you add a paragraph explaining what the above does?

OK.

> >  Here's the interface for the 'set' object that manages the contacts.
> >
> > +    >>> from zope.interface import implements
> >      >>> from lazr.restful.interfaces import ITraverseWithGet
> >      >>> class IContactSet(ITestDataObject, ITraverseWithGet):
> > -    ...     def getAll(self):
> > +    ...     def getAllContacts():
> >      ...         "Get all contacts."
> >      ...
> > -    ...     def get(self, name):
> > +    ...     def get(request, name):
> >      ...         "Retrieve a single contact by name."
> 
> Why is request part of the interface signature?

That method doesn't need to be defined in IContactSet at all, because it's inherited from ITraverseWithGet. ITraverseWithGet.get() needs to take a request as an argument because the traversal rules can change from one version to another. But they don't change in this case. I've removed the redundant definition.

> >  Here's a simple ContactSet with a predefined list of contacts.
> >
> > +    >>> from zope.publisher.interfaces.browser import IBrowserRequest
> > +    >>> from lazr.restful.interfaces import IServiceRootResource
> >      >>> from lazr.restful.simple import TraverseWithGet
> > -    >>> from zope.publisher.interfaces.browser import IBrowserRequest
> >      >>> class ContactSet(TraverseWithGet):
> >      ...     implements(IContactSet)
> > -    ...     path = "contacts"
> >      ...
> >      ...     def __init__(self):
> >      ...         self.contacts = CONTACTS
> >      ...
> > -    ...     def get(self, name):
> > -    ...         contacts = [contact for contacts in self.contacts
> > -    ...                     if pair.name == name]
> > +    ...     def get(self, request, name):
> > +    ...         contacts = [contact for contact in self.contacts
> > +    ...                     if contact.name == name]
> >      ...         if len(contacts) == 1:
> >      ...             return contacts[0]
> >      ...         return None
> 
> From the implementation, I don't see why request is required?

As before, it's inherited from ITraverseWithGet.
 
> >  Once we register ContactCollection as the ICollection implementation,
> >  we can adapt the ContactSet object to a web service ICollection.
> >
> > +<<<<<<< TREE
> >      >>> for version in ['beta', 'dev', '1.0']:
> >      ...     sm.registerAdapter(
> >      ...         ContactCollection, provided=ICollection, name=version)
> 
> Conflict marker.

I don't know how this keeps happening. I've never gotten any conflicts on my end.
 
> > +
> > +Using the web service
> > +=====================
> > +
> > +Now that we can create versioned web service requests, let's try out
> > +the different versions of the web service.
> > +
> > +Beta
> > +----
> > +
> > +Here's the service root resource.
> > +
> > +    >>> import simplejson
> > +    >>> request = create_web_service_request('/beta/')
> > +    >>> resource = request.traverse(None)
> > +    >>> body = simplejson.loads(resource())
> > +    >>> print sorted(body.keys())
> > +    ['contacts_collection_link', 'resource_type_link']
> > +
> > +    >>> print body['contacts_collection_link']
> > +    http://api.multiversion.dev/beta/contact_list
> > +
> > +Here's the contact list.
> > +
> > +    >>> request = create_web_service_request('/beta/contact_list')
> > +    >>> resource = request.traverse(None)
> > +    >>> print absoluteURL(contact_set, request)
> > +    http://api.multiversion.dev/beta/contact_list
> > +
> > +    >>> body = simplejson.loads(resource())
> > +    >>> body['total_size']
> > +    2
> > +    >>> for link in sorted(
> > +    ...     [contact['self_link'] for contact in body['entries']]):
> > +    ...     print link
> > +    http://api.multiversion.dev/beta/contact_list/Cleo%20Python
> > +    http://api.multiversion.dev/beta/contact_list/Oliver%20Bluth
> > +
> > +We can traverse through the collection to an entry.
> > +
> > +    >>> request_beta = create_web_service_request(
> > +    ...     '/beta/contact_list/Cleo Python')
> > +    >>> resource = request_beta.traverse(None)
> > +    >>> print absoluteURL(C1, request_beta)
> 
> I don't understand what C1 is here. That's what you presume is the resource
> context? Would be better to use resource.context explicitely, no?
> 
> You'll want to apply any changes also to the other version examples.

I'd like to do that, but resource.context is protected. I've added some explanation to the first couple examples.

> 
> > === modified file 'src/lazr/restful/docs/webservice.txt'
> 
> > +We also need to define a marker interface for each version of the web
> > +service, so that incoming requests can be marked with the appropriate
> > +version string. The configuration above defines two versions, 'beta'
> > +and 'devel'.
> > +
> > +    >>> from lazr.restful.interfaces import (
> > +    ...     IVersionedClientRequestImplementation,
> IWebServiceClientRequest)
> > +    >>> class IWebServiceRequestBeta(IWebServiceClientRequest):
> > +    ...     pass
> > +
> > +    >>> class IWebServiceRequestDevel(IWebServiceClientRequest):
> > +    ...     pass
> > +
> > +    >>> versions = ((IWebServiceRequestBeta, 'beta'),
> > +    ...             (IWebServiceRequestDevel, 'devel'))
> > +
> > +    >>> from zope.interface import alsoProvides
> > +    >>> for cls, version in versions:
> > +    ...     alsoProvides(cls, IVersionedClientRequestImplementation)
> > +    ...     sm.registerUtility(cls, IVersionedClientRequestImplementation,
> > +    ...                        name=version)
> > +
> > +
> 
> Since you do that in quite a few places, you might want to provide a helper in
> lazr.restful.testing to do that kind of registration more expressively?

Good idea. I put it in lazr.restful._resource because it's also used in a non-testing scenario.

> > === modified file 'src/lazr/restful/interfaces/_rest.py'
> 
> > +class IRequestAwareLocation(Interface):
> > +    """An ILocation-like interface for resources that change location.
> > +
> > +    Versioning is the most common reason why a resource would change
> > +    location depending on the request. A resource may have one name or
> > +    parent in version N, and another name or parent in version N+1.
> > +    """
> > +
> > +    def __parent__(request):
> > +        """The parent in the location hierarchy."""
> > +
> > +    def __name__(request):
> > +        """The name within the parent."""
> > +
> > +
> 
> Why is this interface needed? And why keep the attribute names of ILocation
> when you are actually transforming this into methods?
> 
> Why not use simply define an ILocation multi-adapter. getMultiAdapter((ob,
> request), ILocation). And __parent__ and __name__ could still be attributes?

This could work. I'm going to investigate and follow up on this separately.

> 
> > === modified file 'src/lazr/restful/publisher.py'
> > --- src/lazr/restful/publisher.py     2010-01-06 21:52:12 +0000
> > +++ src/lazr/restful/publisher.py     2010-01-06 21:52:12 +0000
> > @@ -62,8 +62,8 @@
> >          # handle traversing to the scoped collection itself.
> >          if len(request.getTraversalStack()) == 0:
> >              try:
> > -                entry = IEntry(ob)
> > -            except TypeError:
> > +                entry = getMultiAdapter((ob, request), IEntry)
> > +            except ComponentLookupError:
> >                  pass
> 
> Do you need to check that ob provides IEntry?

I'd say I do check. If ob doesn't provide IEntry, getMultiAdapter will raise a ComponentLookupError, which I suppress, which gives the superclass a chance to handle the traversal. I've added a comment to this effect.

> >  =======
> >          # Find the version-specific interface this request should
> >          # provide, and provide it.
> > -        try:
> > -            to_provide = getUtility(IVersionedClientRequestImplementation,
> > -                                    name=version)
> > -            alsoProvides(self, to_provide)
> > -        except ComponentLookupError:
> > -            # XXX leonardr 2009-11-23 This stops single-version tests
> > -            # from breaking. I'll probably remove it once the
> > -            # necessary version-specific interfaces are automatically
> > -            # generated.
> > -            pass
> > +        to_provide = getUtility(IVersionedClientRequestImplementation,
> > +                                name=version)
> > +        alsoProvides(self, to_provide)
> >          self.version = version
> 
> With my previous name suggestion, this would become:
> 
>     version_interface = getUtility(IWebClientRequest, name=version)
>     alsoProvides(self, version_interface)
> 
> > === modified file 'src/lazr/restful/simple.py'
> 
> > +class AbsoluteURL(ZopeAbsoluteURL):
> > +    """An IAbsoluteURL implementation for IRequestAwareLocation objects.
> > +
> > +    This code is a copy of Zope's AbsoluteURL implementation, except
> > +    that it deals with IRequestAwareLocation objects rather than
> > +    ILocation objects. In IRequestAwareLocation objects, __name__ and
> > +    __parent__ are not attributes. They're methods that take the
> > +    request object as an argument.
> > +    """
> > +    def __str__(self):
> > +        context = self.context
> > +        request = self.request
> > +
> > +        # The application URL contains all the namespaces that are at the
> > +        # beginning of the URL, such as skins, virtual host specifications
> and
> > +        # so on.
> > +        if (context is None
> > +            or sameProxiedObjects(context, request.getVirtualHostRoot())):
> > +            return request.getApplicationURL()
> > +
> > +        context = IRequestAwareLocation(context)
> > +        container = context.__parent__(request)
> > +        if container is None:
> > +            raise TypeError(_insufficientContext)
> > +
> > +        url = str(getMultiAdapter((container, request), IAbsoluteURL))
> > +        name = context.__name__(request)
> > +        if name is None:
> > +            raise TypeError(_insufficientContext)
> > +
> > +        if name:
> > +            url += '/' + urllib.quote(name.encode('utf-8'), _safe)
> > +
> > +        return url
> > +
> > +    __call__ = __str__
> > +
> 
> I don't understand the need for this class?

I should be able to get rid of it if I can also get rid of IRequestAwareLocation. I needed it because the Zope implementation assumed __parent__ and __name__ were properties.
 
> 
> > === modified file 'src/lazr/restful/tests/test_navigation.py'
> >  class NavigationTestCase(unittest.TestCase):
> >
> > +    def setUp(self):
> > +        # Register ChildEntry as the IEntry implementation for IChild.
> > +        sm = getSiteManager()
> > +        sm.registerAdapter(
> > +            ChildEntry, [IChild, IWebServiceClientRequest],
> provided=IEntry)
> > +
> > +        # Register ParentEntry as the IEntry implementation for IParent.
> > +        sm.registerAdapter(
> > +            ParentEntry, [IParent, IWebServiceClientRequest],
> provided=IEntry)
> > +
> 
> You probably want to add a tearDown calling zope.testing.cleanup.cleanUp to
> make sure these registrations are undone.

Done.