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

Proposed by Curtis Hovey
Status: Merged
Approved by: j.c.sackett
Approved revision: 207
Merged at revision: 204
Proposed branch: lp:~sinzui/lazr.restful/component-lookup-error-404
Merge into: lp:lazr.restful
Diff against target: 173 lines (+64/-29)
4 files modified
src/lazr/restful/NEWS.txt (+5/-0)
src/lazr/restful/publisher.py (+25/-26)
src/lazr/restful/tests/test_navigation.py (+33/-2)
src/lazr/restful/version.txt (+1/-1)
To merge this branch: bzr merge lp:~sinzui/lazr.restful/component-lookup-error-404
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+138534@code.launchpad.net

Commit message

Convert API traversal ComponentLookupErrors to NotFound.

Description of the change

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

To post a comment you must log in.
206. By Curtis Hovey

Fix comment.

207. By Curtis Hovey

Updated version for release.

Revision history for this message
j.c.sackett (jcsackett) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/NEWS.txt'
2--- src/lazr/restful/NEWS.txt 2012-10-23 04:35:25 +0000
3+++ src/lazr/restful/NEWS.txt 2012-12-06 18:54:18 +0000
4@@ -2,6 +2,11 @@
5 NEWS for lazr.restful
6 =====================
7
8+0.19.10 (2012-12-06)
9+
10+Fixed bug 809863: WebServicePublicationMixin.getResouce() converts
11+ComponentLookupErrors to NotFound.
12+
13 0.19.9 (2012-10-23)
14
15 Fixed bug 924291: The FixedVocabularyFieldMarshaller will now correctly return
16
17=== modified file 'src/lazr/restful/publisher.py'
18--- src/lazr/restful/publisher.py 2011-03-31 01:13:59 +0000
19+++ src/lazr/restful/publisher.py 2012-12-06 18:54:18 +0000
20@@ -36,8 +36,6 @@
21 from zope.schema.interfaces import IBytes
22 from zope.security.checker import ProxyFactory
23
24-from lazr.uri import URI
25-
26 from lazr.restful import (
27 CollectionResource,
28 EntryField,
29@@ -163,26 +161,29 @@
30 supports the ICollection, or IEntry interface we wrap it into the
31 appropriate resource.
32 """
33- if (ICollection.providedBy(ob) or
34- queryMultiAdapter((ob, request), ICollection) is not None):
35- # Object supports ICollection protocol.
36- resource = CollectionResource(ob, request)
37- elif (IEntry.providedBy(ob) or
38- queryMultiAdapter((ob, request), IEntry) is not None):
39- # Object supports IEntry protocol.
40- resource = EntryResource(ob, request)
41- elif (IEntryField.providedBy(ob) or
42- queryAdapter(ob, IEntryField) is not None):
43- # Object supports IEntryField protocol.
44- resource = EntryFieldResource(ob, request)
45- elif queryMultiAdapter((ob, request), IHTTPResource) is not None:
46- # Object can be adapted to a resource.
47- resource = queryMultiAdapter((ob, request), IHTTPResource)
48- elif IHTTPResource.providedBy(ob):
49- # A resource knows how to take care of itself.
50- return ob
51- else:
52- # This object should not be published on the web service.
53+ try:
54+ if (ICollection.providedBy(ob) or
55+ queryMultiAdapter((ob, request), ICollection) is not None):
56+ # Object supports ICollection protocol.
57+ resource = CollectionResource(ob, request)
58+ elif (IEntry.providedBy(ob) or
59+ queryMultiAdapter((ob, request), IEntry) is not None):
60+ # Object supports IEntry protocol.
61+ resource = EntryResource(ob, request)
62+ elif (IEntryField.providedBy(ob) or
63+ queryAdapter(ob, IEntryField) is not None):
64+ # Object supports IEntryField protocol.
65+ resource = EntryFieldResource(ob, request)
66+ elif queryMultiAdapter((ob, request), IHTTPResource) is not None:
67+ # Object can be adapted to a resource.
68+ resource = queryMultiAdapter((ob, request), IHTTPResource)
69+ elif IHTTPResource.providedBy(ob):
70+ # A resource knows how to take care of itself.
71+ return ob
72+ else:
73+ # This object should not be published on the web service.
74+ raise NotFound(ob, '')
75+ except ComponentLookupError:
76 raise NotFound(ob, '')
77
78 # Wrap the resource in a security proxy.
79@@ -211,7 +212,6 @@
80 WebServicePublicationMixin, self).callObject(request, object)
81 self._processNotifications(request)
82 if request.response.getStatus() / 100 == 3:
83- vhost = URI(request.getApplicationURL()).host
84 if IWebBrowserInitiatedRequest.providedBy(request):
85 # This request was (probably) sent by a web
86 # browser. Because web browsers, content negotiation,
87@@ -237,9 +237,9 @@
88 (scheme, netloc, path, query, fragment) = (
89 urlparse.urlsplit(location))
90 if query == '':
91- query = qs_append
92+ query = qs_append
93 else:
94- query += '&' + qs_append
95+ query += '&' + qs_append
96 uri = urlparse.urlunsplit(
97 (scheme, netloc, path, query, fragment))
98 request.response.setHeader("Location", str(uri))
99@@ -278,7 +278,6 @@
100 def _removeVirtualHostTraversals(self):
101 """Remove the /[path_override] and /[version] traversal names."""
102 names = list()
103- start_stack = list(self.getTraversalStack())
104 config = getUtility(IWebServiceConfiguration)
105 if config.path_override is not None:
106 api = self._popTraversal(config.path_override)
107
108=== modified file 'src/lazr/restful/tests/test_navigation.py'
109--- src/lazr/restful/tests/test_navigation.py 2011-01-21 21:12:17 +0000
110+++ src/lazr/restful/tests/test_navigation.py 2012-12-06 18:54:18 +0000
111@@ -6,8 +6,16 @@
112
113 import unittest
114
115-from zope.component import getSiteManager
116-from zope.interface import Interface, implements
117+from zope.component import (
118+ ComponentLookupError,
119+ getMultiAdapter,
120+ getSiteManager,
121+ )
122+from zope.interface import (
123+ alsoProvides,
124+ Interface,
125+ implements,
126+ )
127 from zope.publisher.interfaces import NotFound
128 from zope.schema import Text
129 from zope.testing.cleanup import cleanUp
130@@ -43,6 +51,7 @@
131 class ChildEntry:
132 """Implementation of an entry wrapping a Child."""
133 schema = IChild
134+
135 def __init__(self, context, request):
136 self.context = context
137
138@@ -106,6 +115,28 @@
139 NotFound, publication.traverseName,
140 request, parent, 'child')
141
142+ def test_getResource_ComponentLookupError_NotFound(self):
143+ # Test that ComponentLookupError becomes NotFound.
144+ # IEntry objects may not be adaptable to a resource if
145+ # the underlying object cannot be found.
146+ class IOrphan(Interface):
147+ pass
148+
149+ class OrphanEntry:
150+ schema = IOrphan
151+
152+ def __init__(self):
153+ alsoProvides(self, IEntry)
154+
155+ publication = Publication(None)
156+ request = FakeRequestWithEmptyTraversalStack(version='trunk')
157+ entry_obj = OrphanEntry()
158+ self.assertRaises(
159+ ComponentLookupError,
160+ getMultiAdapter, (entry_obj, request), IEntry)
161+ self.assertRaises(
162+ NotFound, publication.getResource, request, entry_obj)
163+
164
165 def additional_tests():
166 return unittest.TestLoader().loadTestsFromName(__name__)
167
168=== modified file 'src/lazr/restful/version.txt'
169--- src/lazr/restful/version.txt 2012-10-23 04:35:25 +0000
170+++ src/lazr/restful/version.txt 2012-12-06 18:54:18 +0000
171@@ -1,1 +1,1 @@
172-0.19.9
173+0.19.10

Subscribers

People subscribed via source and target branches