Merge lp:~leonardr/launchpad/optimized-length-2 into lp:launchpad/db-devel

[Leonard Richardson]

This branch integrates the newest version of lazr.restful, which sends 'total_size' instead of 'total_size_link' if it's easy to figure out the total size of a collection. The changes to datamodel.txt show when this happens.

[Aaron Bentley (abentley)]

review: Approve

[Leonard Richardson (leonardr)]

I need a follow-up review. I fixed a minor test failure and added a workaround for bug 620508. I'm fairly sure it won't screw up anything else, but the tests will say for sure.

[Graham Binns (gmb)]

r=me for r11392.

review: Approve (code)

[Michael Nelson (michael.nelson)]

r=me for the change pasted via irc:

14:51 < noodles775> leonardr: I don't see a one-liner? It's r11442 that needs to be reviewed right? This: http://bazaar.launchpad.net/~leonardr/launchpad/optimized-length-2/revision/11442
14:59 < leonardr> noodles775, 11442 corrects a typo made to the original one-liner. i was hoping to just give you the code so it wouldn't be so complicated
14:59 < leonardr> i'll just do another merge proposal and get in the queue
15:01 < noodles775> leonardr: I'm happy to review just the one-line that's changed, but currently I can only see it in 11442 along with a few other changes. If the rest is already reviewed, just paste a diff and I can review that, otherwise yes, a new MP is probably easier.
15:04 < leonardr> noodles775: i see the problem. i had to create a new branch with the old changes, so all the old changes show up as one revision
15:04 < leonardr> this code is the only code that hasn't been reviewed yet:
15:04 < leonardr> + if hasattr(resultset, '_select'):
15:04 < leonardr> + resultset._select.limit = Undef
15:04 < leonardr> + resultset._select.offset = Undef
15:05 < noodles775> leonardr: r=me (I assume the second two lines were there previously, just not indented). Thanks!
15:05 < leonardr> noodles, yeah

review: Approve (code)

=== modified file 'lib/canonical/launchpad/pagetests/webservice/datamodel.txt'
--- lib/canonical/launchpad/pagetests/webservice/datamodel.txt	2010-08-19 14:38:10 +0000
+++ lib/canonical/launchpad/pagetests/webservice/datamodel.txt	2010-08-26 12:53:02 +0000
@@ -7,29 +7,39 @@
 end-to-end tests of code paths that, on the surface, look like they're
 already tested in lazr.restful.
 
-  >>> def get_collection(version, start=0, size=2):
+  >>> def get_collection(version="devel", start=0, size=2):
   ...     collection = webservice.get(
   ...         ("/people?ws.op=find&text=s&ws.start=%s&ws.size=%s" %
   ...          (start, size)),
   ...         api_version=version)
   ...     return collection.jsonBody()
 
-Even if an entire collection fits on one page (so that the total size
-of the collection is obvious), 'total_size_link' will be served
-instead of 'total_size'. (It would be nice to fix this.)
-
-  >>> collection = get_collection("devel", size=100)
-  >>> print sorted(collection.keys())
-  [u'entries', u'start', u'total_size_link']
-  >>> print webservice.get(collection['total_size_link']).jsonBody()
-  9
-
-Even if the last page of the collection is fetched (so that the total
-size of the collection is semi-obvious), 'total_size_link' will be
-served instead of 'total_size'. (It would be nice to fix this.)
-
-  >>> collection = get_collection("devel", start=8)
-  >>> print sorted(collection.keys())
-  [u'entries', u'prev_collection_link', u'start', u'total_size_link']
-  >>> print webservice.get(collection['total_size_link']).jsonBody()
+
+Normally, the total size of a collection is not served along with the
+collection; it's available by following the total_size_link.
+
+  >>> collection = get_collection()
+  >>> print sorted(collection.keys())
+  [u'entries', u'next_collection_link', u'start', u'total_size_link']
+  >>> print webservice.get(collection['total_size_link']).jsonBody()
+  9
+
+If an entire collection fits on one page (making the size of the
+collection obvious), 'total_size' is served instead of
+'total_size_link'.
+
+  >>> collection = get_collection(size=100)
+  >>> print sorted(collection.keys())
+  [u'entries', u'start', u'total_size']
+  >>> print collection['total_size']
+  9
+
+If the last page of the collection is fetched (making the total size
+of the collection semi-obvious), 'total_size' is served instead of
+'total_size_link'.
+
+  >>> collection = get_collection(start=8)
+  >>> print sorted(collection.keys())
+  [u'entries', u'prev_collection_link', u'start', u'total_size']
+  >>> print collection['total_size']
   9

=== modified file 'lib/canonical/launchpad/webapp/batching.py'
--- lib/canonical/launchpad/webapp/batching.py	2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/webapp/batching.py	2010-08-26 12:53:02 +0000
@@ -4,11 +4,13 @@
 __metaclass__ = type
 
 import lazr.batchnavigator
+from storm import Undef
 # and ISQLObjectResultSet
 from storm.zope.interfaces import IResultSet
 from zope.component import adapts
 from zope.interface import implements
 from zope.interface.common.sequence import IFiniteSequence
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
 from canonical.launchpad.webapp.interfaces import ITableBatchNavigator
@@ -30,6 +32,24 @@
         return iter(self.context)
 
     def __len__(self):
+        # XXX 2010-08-24 leonardr bug=620508
+        #
+        # Slicing a ResultSet object returns a copy with ._limit and
+        # ._offset set appropriately. The original object's ._limit
+        # and ._offset are not affected. However, the original and
+        # the copy share a _select object, which means the original
+        # object's ._select.limit and ._select.offset are shared with
+        # the copy.
+        #
+        # This breaks Storm--count() is not supported on a ResultSet
+        # that has a limit or offset set. This code sets
+        # ._select.limit and ._select.offset to the appropriate values
+        # before running the count() query, just as __getitem__ sets
+        # those values before running its query.
+        resultset = removeSecurityProxy(self.context)
+        if hasattr(resultset, '_select'):
+            resultset._select.limit = Undef
+            resultset._select.offset = Undef
         return self.context.count()
 
 

=== modified file 'versions.cfg'
--- versions.cfg	2010-08-25 18:51:51 +0000
+++ versions.cfg	2010-08-26 12:53:02 +0000
@@ -24,14 +24,14 @@
 grokcore.component = 1.6
 httplib2 = 0.6.0
 ipython = 0.9.1
-launchpadlib = 1.6.4
+launchpadlib = 1.6.5
 lazr.authentication = 0.1.1
 lazr.batchnavigator = 1.2.2
 lazr.config = 1.1.3
 lazr.delegates = 1.2.0
 lazr.enum = 1.1.2
 lazr.lifecycle = 1.1
-lazr.restful = 0.11.1
+lazr.restful = 0.11.3
 lazr.restfulclient = 0.10.0
 lazr.smtptest = 1.1
 lazr.testing = 0.1.1