Merge lp:~sinzui/lazr.restful/component-lookup-error-404 into lp:lazr.restful

[Curtis Hovey]

Convert API traversal ComponentLookupErrors to NotFound.

An oops is reported when getPublishedSources distro_series has extra slash.
    ComponentLookupError: (
        (<lazr.restful.declarations.DistroSeriesEntry_1_0Adapter>,
         <canonical.launchpad.webapp.servers.WebServiceClientRequest
            instance URL=http://1.0/ubuntu/natty/>),
        <InterfaceClass lazr.restful.interfaces._rest.IEntry>)

This error happens in WebServicePublicationMixin.getResouce() which
we expect to raise a NotFound error.

RULES

    Pre-implementation: no one
    * This error happens because it getResource assumes anything that
      implements IEntry or IEntryField can be adapted to the underling
      object to make the resource. This is not always true because
      lazr.restful decorators declare parameter types.
    * The method raises NotFound if nothing an be adapted, but since
      it cannot be certain it's checks are correct, it needs to
      watch for ComponentLookupError and change it to NotFound.

QA

    On qastaging after it was updated to use lazr.restful 0.19.10
    * Verify https://api.qastaging.launchpad.net/devel/ubuntu/+archive/primary?distro_series=%2Fubuntu%2Fnatty%2F&exact_match=true&source_name=%22bzr%22&status=Published&ws.op=getPublishedSources&ws.size=1
      does not oops.

LINT

    src/lazr/restful/publisher.py
    src/lazr/restful/tests/test_navigation.py

TEST

    ./bin/test -vc -t NavigationTestCase

IMPLEMENTATION

I wrapped the entire if-elif block in a try-except to convert the error
into a NotFound error.
    src/lazr/restful/publisher.py
    src/lazr/restful/tests/test_navigation.py

[lp:~sinzui/lazr.restful/component-lookup-error-404]

206. By Curtis Hovey on 2012-12-06

Fix comment.

207. By Curtis Hovey on 2012-12-06

Updated version for release.

[j.c.sackett (jcsackett)]

Looks good.

review: Approve

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2012-10-23 04:35:25 +0000
+++ src/lazr/restful/NEWS.txt	2012-12-06 18:54:18 +0000
@@ -2,6 +2,11 @@
 NEWS for lazr.restful
 =====================
 
+0.19.10 (2012-12-06)
+
+Fixed bug 809863: WebServicePublicationMixin.getResouce() converts
+ComponentLookupErrors to NotFound.
+
 0.19.9 (2012-10-23)
 
 Fixed bug 924291: The FixedVocabularyFieldMarshaller will now correctly return

=== modified file 'src/lazr/restful/publisher.py'
--- src/lazr/restful/publisher.py	2011-03-31 01:13:59 +0000
+++ src/lazr/restful/publisher.py	2012-12-06 18:54:18 +0000
@@ -36,8 +36,6 @@
 from zope.schema.interfaces import IBytes
 from zope.security.checker import ProxyFactory
 
-from lazr.uri import URI
-
 from lazr.restful import (
     CollectionResource,
     EntryField,
@@ -163,26 +161,29 @@
         supports the ICollection, or IEntry interface we wrap it into the
         appropriate resource.
         """
-        if (ICollection.providedBy(ob) or
-            queryMultiAdapter((ob, request), ICollection) is not None):
-            # Object supports ICollection protocol.
-            resource = CollectionResource(ob, request)
-        elif (IEntry.providedBy(ob) or
-              queryMultiAdapter((ob, request), IEntry) is not None):
-            # Object supports IEntry protocol.
-            resource = EntryResource(ob, request)
-        elif (IEntryField.providedBy(ob) or
-              queryAdapter(ob, IEntryField) is not None):
-            # Object supports IEntryField protocol.
-            resource = EntryFieldResource(ob, request)
-        elif queryMultiAdapter((ob, request), IHTTPResource) is not None:
-            # Object can be adapted to a resource.
-            resource = queryMultiAdapter((ob, request), IHTTPResource)
-        elif IHTTPResource.providedBy(ob):
-            # A resource knows how to take care of itself.
-            return ob
-        else:
-            # This object should not be published on the web service.
+        try:
+            if (ICollection.providedBy(ob) or
+                queryMultiAdapter((ob, request), ICollection) is not None):
+                # Object supports ICollection protocol.
+                resource = CollectionResource(ob, request)
+            elif (IEntry.providedBy(ob) or
+                  queryMultiAdapter((ob, request), IEntry) is not None):
+                # Object supports IEntry protocol.
+                resource = EntryResource(ob, request)
+            elif (IEntryField.providedBy(ob) or
+                  queryAdapter(ob, IEntryField) is not None):
+                # Object supports IEntryField protocol.
+                resource = EntryFieldResource(ob, request)
+            elif queryMultiAdapter((ob, request), IHTTPResource) is not None:
+                # Object can be adapted to a resource.
+                resource = queryMultiAdapter((ob, request), IHTTPResource)
+            elif IHTTPResource.providedBy(ob):
+                # A resource knows how to take care of itself.
+                return ob
+            else:
+                # This object should not be published on the web service.
+                raise NotFound(ob, '')
+        except ComponentLookupError:
             raise NotFound(ob, '')
 
         # Wrap the resource in a security proxy.
@@ -211,7 +212,6 @@
             WebServicePublicationMixin, self).callObject(request, object)
         self._processNotifications(request)
         if request.response.getStatus() / 100 == 3:
-            vhost = URI(request.getApplicationURL()).host
             if IWebBrowserInitiatedRequest.providedBy(request):
                 # This request was (probably) sent by a web
                 # browser. Because web browsers, content negotiation,
@@ -237,9 +237,9 @@
                     (scheme, netloc, path, query, fragment) = (
                         urlparse.urlsplit(location))
                     if query == '':
-                       query = qs_append
+                        query = qs_append
                     else:
-                       query += '&' + qs_append
+                        query += '&' + qs_append
                     uri = urlparse.urlunsplit(
                         (scheme, netloc, path, query, fragment))
                     request.response.setHeader("Location", str(uri))
@@ -278,7 +278,6 @@
     def _removeVirtualHostTraversals(self):
         """Remove the /[path_override] and /[version] traversal names."""
         names = list()
-        start_stack = list(self.getTraversalStack())
         config = getUtility(IWebServiceConfiguration)
         if config.path_override is not None:
             api = self._popTraversal(config.path_override)

=== modified file 'src/lazr/restful/tests/test_navigation.py'
--- src/lazr/restful/tests/test_navigation.py	2011-01-21 21:12:17 +0000
+++ src/lazr/restful/tests/test_navigation.py	2012-12-06 18:54:18 +0000
@@ -6,8 +6,16 @@
 
 import unittest
 
-from zope.component import getSiteManager
-from zope.interface import Interface, implements
+from zope.component import (
+    ComponentLookupError,
+    getMultiAdapter,
+    getSiteManager,
+    )
+from zope.interface import (
+    alsoProvides,
+    Interface,
+    implements,
+    )
 from zope.publisher.interfaces import NotFound
 from zope.schema import Text
 from zope.testing.cleanup import cleanUp
@@ -43,6 +51,7 @@
 class ChildEntry:
     """Implementation of an entry wrapping a Child."""
     schema = IChild
+
     def __init__(self, context, request):
         self.context = context
 
@@ -106,6 +115,28 @@
             NotFound, publication.traverseName,
             request, parent, 'child')
 
+    def test_getResource_ComponentLookupError_NotFound(self):
+        # Test that ComponentLookupError becomes NotFound.
+        # IEntry objects may not be adaptable to a resource if
+        # the underlying object cannot be found.
+        class IOrphan(Interface):
+            pass
+
+        class OrphanEntry:
+            schema = IOrphan
+
+            def __init__(self):
+                alsoProvides(self, IEntry)
+
+        publication = Publication(None)
+        request = FakeRequestWithEmptyTraversalStack(version='trunk')
+        entry_obj = OrphanEntry()
+        self.assertRaises(
+            ComponentLookupError,
+            getMultiAdapter, (entry_obj, request), IEntry)
+        self.assertRaises(
+            NotFound, publication.getResource, request, entry_obj)
+
 
 def additional_tests():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt	2012-10-23 04:35:25 +0000
+++ src/lazr/restful/version.txt	2012-12-06 18:54:18 +0000
@@ -1,1 +1,1 @@
-0.19.9
+0.19.10