Launchpad itself

Merge lp:~matsubara/launchpad/bug-606184-pageid-for-collections into lp:launchpad

  1. bug-606184-pageid-for-collections
  2. Merge into devel
Proposed by Diogo Matsubara
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 11467
Proposed branch: lp:~matsubara/launchpad/bug-606184-pageid-for-collections
Merge into: lp:launchpad
Diff against target: 93 lines (+50/-2)
2 files modified
lib/canonical/launchpad/webapp/servers.py (+18/-1)
lib/canonical/launchpad/webapp/tests/test_pageid.py (+32/-1)
To merge this branch: bzr merge lp:~matsubara/launchpad/bug-606184-pageid-for-collections
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Review via email: mp+33801@code.launchpad.net

Commit message

Fixes bug 606184 by including the origin page resource in the page id for ICollectionResource views.

Description of the change

== Summary ==

This branch fixes bug 606184 by including the origin page resource in the page
id for ICollectionResource views.

== Proposed fix ==

When the page id is generated for API requests, the code inspects if the view
is a ICollectionResource, then inspects the type_url attribute and add part of
it as the collection identifier to the page id.

== Pre-implementation notes ==

I had a pre-imp with Gary. Initially Gary suggested that this could be
accomplished with adapters and I tried that route but got stuck. Then we
agreed to change the implementation to be simpler so I could move this forward.

== Implementation details ==

The origin page resource comes from ICollectionResource.type_url, so I used
that as the identifier for the given collection and added that as part of the
page id. That way when engineers are analysing such OOPS summaries they'll be
able to easily see if the OOPS is in their domain or not, as requested in
the bug report.

== Tests ==

bin/test -tt test_pageid

== Demo and Q/A ==

Once the code lands on staging, it's enough to check a staging oops summary
and see if the "scopedcollection:collectionresource" oopses contain the
additional information about the origin page resource.

== Lint ==

No lint left over in the two files modified by this branch.

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

merge-conditional

Cool, thank you.

My only comment is that I'd like to see more of a comment explaining what is going on. things to explain: what is a collection_identifier? How do you use it to relate a page id to actual code, if you are trying to debug an OOPS? Extra credit: what code is responsible for generating the collection_identifier?

Please answer at least the first two questions, and also add the information to the page ids section of https://dev.launchpad.net/Foundations/Webservice

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/webapp/servers.py 2010-08-27 16:03:48 +0000
@@ -13,6 +13,7 @@
13import xmlrpclib13import xmlrpclib
1414
15from lazr.restful.interfaces import (15from lazr.restful.interfaces import (
16 ICollectionResource,
16 IWebServiceConfiguration,17 IWebServiceConfiguration,
17 IWebServiceVersion,18 IWebServiceVersion,
18 )19 )
@@ -1151,9 +1152,25 @@
1151 root_object_interface = IWebServiceApplication1152 root_object_interface = IWebServiceApplication
11521153
1153 def constructPageID(self, view, context):1154 def constructPageID(self, view, context):
1154 """Add the web service named operation (if any) to the page ID."""1155 """Add the web service named operation (if any) to the page ID.
1156
1157 See https://dev.launchpad.net/Foundations/Webservice for more
1158 information about WebService page IDs.
1159 """
1155 pageid = super(WebServicePublication, self).constructPageID(1160 pageid = super(WebServicePublication, self).constructPageID(
1156 view, context)1161 view, context)
1162 if ICollectionResource.providedBy(view):
1163 # collection_identifier is a way to differentiate between
1164 # CollectionResource objects. CollectionResource objects are
1165 # objects that serve a list of Entry resources through the
1166 # WebService, so by querying the CollectionResource.type_url
1167 # attribute we're able to find out the resource type the
1168 # collection holds. See lazr.restful._resource.py to see how
1169 # the type_url is constructed.
1170 # We don't need the full URL, just the type of the resource.
1171 collection_identifier = view.type_url.split('/')[-1]
1172 if collection_identifier:
1173 pageid += ':' + collection_identifier
1157 op = (view.request.get('ws.op')1174 op = (view.request.get('ws.op')
1158 or view.request.query_string_params.get('ws.op'))1175 or view.request.query_string_params.get('ws.op'))
1159 if op:1176 if op:
11601177
=== modified file 'lib/canonical/launchpad/webapp/tests/test_pageid.py'
--- lib/canonical/launchpad/webapp/tests/test_pageid.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/webapp/tests/test_pageid.py 2010-08-27 16:03:48 +0000
@@ -5,7 +5,8 @@
5__metaclass__ = type5__metaclass__ = type
66
77
8import unittest8from zope.interface import implements
9from lazr.restful.interfaces import ICollectionResource
910
10from canonical.launchpad.webapp.servers import WebServicePublication11from canonical.launchpad.webapp.servers import WebServicePublication
11from lp.testing import TestCase12from lp.testing import TestCase
@@ -34,6 +35,16 @@
34 self.request = FakeRequest()35 self.request = FakeRequest()
3536
3637
38class FakeCollectionResourceView(FakeView):
39 """A view object that provides ICollectionResource."""
40 implements(ICollectionResource)
41
42 def __init__(self):
43 super(FakeCollectionResourceView, self).__init__()
44 self.type_url = (
45 u'https://launchpad.dev/api/devel/#milestone-page-resource')
46
47
37class TestWebServicePageIDs(TestCase):48class TestWebServicePageIDs(TestCase):
38 """Ensure that the web service enhances the page ID correctly."""49 """Ensure that the web service enhances the page ID correctly."""
3950
@@ -68,3 +79,23 @@
68 self.view.request.query_string_params['ws.op'] = 'operation-name-2'79 self.view.request.query_string_params['ws.op'] = 'operation-name-2'
69 self.assertEqual(80 self.assertEqual(
70 self.makePageID(), 'FakeContext:FakeView:operation-name-2')81 self.makePageID(), 'FakeContext:FakeView:operation-name-2')
82
83
84class TestCollectionResourcePageIDs(TestCase):
85 """Ensure page ids for collections display the origin page resource."""
86
87 def setUp(self):
88 super(TestCollectionResourcePageIDs, self).setUp()
89 self.publication = WebServicePublication(db=None)
90 self.view = FakeCollectionResourceView()
91 self.context = FakeContext()
92
93 def makePageID(self):
94 return self.publication.constructPageID(self.view, self.context)
95
96 def test_origin_pageid_for_collection(self):
97 # When the view provides a ICollectionResource, make sure the origin
98 # page resource is included in the page ID.
99 self.assertEqual(
100 self.makePageID(),
101 'FakeContext:FakeCollectionResourceView:#milestone-page-resource')